From a02dbb7e619afe1647bbb89db2f4963cf9f37497 Mon Sep 17 00:00:00 2001 From: Alex | Interchain Labs Date: Tue, 13 May 2025 12:46:47 -0400 Subject: [PATCH] fix: collections test IterateRaw bug (#24737) --- collections/CHANGELOG.md | 6 ++++++ collections/map.go | 12 ++++++++++-- collections/map_test.go | 30 ++++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/collections/CHANGELOG.md b/collections/CHANGELOG.md index 3cd224f69e..49bd67c1be 100644 --- a/collections/CHANGELOG.md +++ b/collections/CHANGELOG.md @@ -31,6 +31,12 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## [Unreleased] +## [v1.2.1](https://github.com/cosmos/cosmos-sdk/releases/tag/collections%2Fv1.2.1) + +## Bug Fixes + +* [#24737](https://github.com/cosmos/cosmos-sdk/pull/24737) Ensure that map memory will never be reused unintentionally. + ## [v1.2.0](https://github.com/cosmos/cosmos-sdk/releases/tag/collections%2Fv1.2.0) ### Improvements diff --git a/collections/map.go b/collections/map.go index 30f28dbd9e..52cb165168 100644 --- a/collections/map.go +++ b/collections/map.go @@ -223,6 +223,14 @@ func deleteDomain(s store.KVStore, start, end []byte) error { return nil } +// concatNew concatenates two byte slices, prefix and suffix, into a new byte slice and returns the result. +func concatNew(prefix, suffix []byte) []byte { + b := make([]byte, len(prefix)+len(suffix)) + copy(b, prefix) + copy(b[len(prefix):], suffix) + return b +} + // IterateRaw iterates over the collection. The iteration range is untyped, it uses raw // bytes. The resulting Iterator is typed. // A nil start iterates from the first key contained in the collection. @@ -230,12 +238,12 @@ func deleteDomain(s store.KVStore, start, end []byte) error { // A nil start and a nil end iterates over every key contained in the collection. // TODO(tip): simplify after https://github.com/cosmos/cosmos-sdk/pull/14310 is merged func (m Map[K, V]) IterateRaw(ctx context.Context, start, end []byte, order Order) (Iterator[K, V], error) { - prefixedStart := append(m.prefix, start...) + prefixedStart := concatNew(m.prefix, start) var prefixedEnd []byte if end == nil { prefixedEnd = nextBytesPrefixKey(m.prefix) } else { - prefixedEnd = append(m.prefix, end...) + prefixedEnd = concatNew(m.prefix, end) } if bytes.Compare(prefixedStart, prefixedEnd) == 1 { diff --git a/collections/map_test.go b/collections/map_test.go index 5f1ae176df..c88ab42435 100644 --- a/collections/map_test.go +++ b/collections/map_test.go @@ -127,3 +127,33 @@ func Test_encodeKey(t *testing.T) { require.NoError(t, err) require.Equal(t, expectedKey, gotKey) } + +func TestMap_IterateRaw_Prefix(t *testing.T) { + sk, ctx := deps() + // safety check to ensure prefix boundaries are not crossed + require.NoError(t, sk.OpenKVStore(ctx).Set([]byte{0x0, 0x0}, []byte("before prefix"))) + require.NoError(t, sk.OpenKVStore(ctx).Set([]byte{0x2, 0x0}, []byte("after prefix"))) + sb := NewSchemaBuilder(sk) + // case 1. this is abnormal but easy to understand the problem + // prefix := make([]byte, 0, 10) + // prefix = append(prefix, 1) + // case 2. this is more common case, some modules can use this way + prefix := []byte{} + prefix = append(append(prefix, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10), 1, 2, 3, 4, 5, 6, 7, 8, 9, 10) + t.Log(cap(prefix)) // cap is 32 + m := NewMap(sb, prefix, "m", Uint64Key, Uint64Value) + require.NoError(t, m.Set(ctx, 0, 0)) + require.NoError(t, m.Set(ctx, 1, 1)) + require.NoError(t, m.Set(ctx, 2, 2)) + // test non nil end in ascending order + oneBigEndian, err := EncodeKeyWithPrefix(nil, Uint64Key, 1) + require.NoError(t, err) + twoBigEndian, err := EncodeKeyWithPrefix(nil, Uint64Key, 2) + require.NoError(t, err) + iter, err := m.IterateRaw(ctx, oneBigEndian, twoBigEndian, OrderAscending) + require.NoError(t, err) + defer iter.Close() + keys, err := iter.Keys() + require.NoError(t, err) + require.Equal(t, []uint64{1}, keys) +}