diff --git a/collections/CHANGELOG.md b/collections/CHANGELOG.md index 291ec0b9a478..a4cad80a54b5 100644 --- a/collections/CHANGELOG.md +++ b/collections/CHANGELOG.md @@ -35,6 +35,10 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [#17024](https://github.com/cosmos/cosmos-sdk/pull/17024) - Introduces `Triple`, a composite key with three keys. +### API Breaking + +* [#17290](https://github.com/cosmos/cosmos-sdk/pull/17290) - Collections iteration methods (Iterate, Walk) will not error when the collection is empty. + ### Improvements * [#17021](https://github.com/cosmos/cosmos-sdk/pull/17021) Make collections implement the `appmodule.HasGenesis` interface. diff --git a/collections/genesis.go b/collections/genesis.go index c2db579b1126..0c593e4adfd8 100644 --- a/collections/genesis.go +++ b/collections/genesis.go @@ -3,7 +3,6 @@ package collections import ( "context" "encoding/json" - "errors" "fmt" "io" ) @@ -40,13 +39,6 @@ func (m Map[K, V]) exportGenesis(ctx context.Context, writer io.Writer) error { it, err := m.Iterate(ctx, nil) if err != nil { - // if the iterator is invalid, the state can be empty - // and we can just return an empty array. - if errors.Is(err, ErrInvalidIterator) { - _, err = io.WriteString(writer, "]") - return err - } - return err } defer it.Close() diff --git a/collections/indexes/reverse_pair_test.go b/collections/indexes/reverse_pair_test.go index 564b273fa284..da1c1c961166 100644 --- a/collections/indexes/reverse_pair_test.go +++ b/collections/indexes/reverse_pair_test.go @@ -64,8 +64,12 @@ func TestReversePair(t *testing.T) { // assert if we remove address1 atom balance, we can no longer find it in the index err = indexedMap.Remove(ctx, collections.Join("address1", "atom")) require.NoError(t, err) - _, err = indexedMap.Indexes.Denom.MatchExact(ctx, "atom") - require.ErrorIs(t, collections.ErrInvalidIterator, err) + iter, err = indexedMap.Indexes.Denom.MatchExact(ctx, "atom") + require.NoError(t, err) + defer iter.Close() + pks, err = iter.PrimaryKeys() + require.NoError(t, err) + require.Empty(t, pks) } func TestUncheckedReversePair(t *testing.T) { diff --git a/collections/iter.go b/collections/iter.go index 527e15a92871..06f2a9f96029 100644 --- a/collections/iter.go +++ b/collections/iter.go @@ -1,6 +1,7 @@ package collections import ( + "bytes" "context" "errors" "fmt" @@ -161,6 +162,9 @@ func parseRangeInstruction[K any](prefix []byte, keyCodec codec.KeyCodec[K], r R } else { endBytes = nextBytesPrefixKey(prefix) } + if bytes.Compare(startBytes, endBytes) == 1 { + return nil, nil, 0, ErrInvalidIterator + } return startBytes, endBytes, order, nil } @@ -191,9 +195,6 @@ func newIterator[K, V any](ctx context.Context, start, end []byte, order Order, if err != nil { return Iterator[K, V]{}, err } - if !iter.Valid() { - return Iterator[K, V]{}, ErrInvalidIterator - } return Iterator[K, V]{ kc: m.kc, diff --git a/collections/map.go b/collections/map.go index 2a7bb36a0f3a..810b5a06f809 100644 --- a/collections/map.go +++ b/collections/map.go @@ -1,6 +1,7 @@ package collections import ( + "bytes" "context" "fmt" @@ -149,8 +150,7 @@ func (m Map[K, V]) Walk(ctx context.Context, ranger Ranger[K], walkFunc func(key } // Clear clears the collection contained within the provided key range. -// A nil ranger equals to clearing the whole collection. In case the collection -// is empty no error will be returned. +// A nil ranger equals to clearing the whole collection. // NOTE: this API needs to be used with care, considering that as of today // cosmos-sdk stores the deletion records to be committed in a memory cache, // clearing a lot of data might make the node go OOM. @@ -216,6 +216,10 @@ func (m Map[K, V]) IterateRaw(ctx context.Context, start, end []byte, order Orde prefixedEnd = append(m.prefix, end...) } + if bytes.Compare(prefixedStart, prefixedEnd) == 1 { + return Iterator[K, V]{}, ErrInvalidIterator + } + s := m.sa(ctx) var ( storeIter store.Iterator @@ -233,9 +237,6 @@ func (m Map[K, V]) IterateRaw(ctx context.Context, start, end []byte, order Orde return Iterator[K, V]{}, err } - if !storeIter.Valid() { - return Iterator[K, V]{}, ErrInvalidIterator - } return Iterator[K, V]{ kc: m.kc, vc: m.vc, diff --git a/collections/map_test.go b/collections/map_test.go index a011083a42ed..f95935c5720e 100644 --- a/collections/map_test.go +++ b/collections/map_test.go @@ -2,6 +2,7 @@ package collections import ( "context" + "fmt" "testing" "github.com/stretchr/testify/require" @@ -51,8 +52,10 @@ func TestMap_Clear(t *testing.T) { ctx, m := makeTest() err := m.Clear(ctx, nil) require.NoError(t, err) - _, err = m.Iterate(ctx, nil) - require.ErrorIs(t, err, ErrInvalidIterator) + err = m.Walk(ctx, nil, func(key, value uint64) (bool, error) { + return false, fmt.Errorf("should never be called") + }) + require.NoError(t, err) }) t.Run("custom ranger", func(t *testing.T) { @@ -104,6 +107,15 @@ func TestMap_IterateRaw(t *testing.T) { keys, err = iter.Keys() require.NoError(t, err) require.Equal(t, []uint64{2, 1, 0}, keys) + + // test invalid iter + _, err = m.IterateRaw(ctx, []byte{0x2, 0x0}, []byte{0x0, 0x0}, OrderAscending) + require.ErrorIs(t, err, ErrInvalidIterator) + + // test on empty collection iterating does not error + require.NoError(t, m.Clear(ctx, nil)) + _, err = m.IterateRaw(ctx, nil, nil, OrderAscending) + require.NoError(t, err) } func Test_encodeKey(t *testing.T) {