diff --git a/CHANGELOG.md b/CHANGELOG.md index b18fa3384b9a..43d16411c856 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -122,6 +122,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/group) [#12888](https://github.com/cosmos/cosmos-sdk/pull/12888) Fix event propagation to the current context of `x/group` message execution `[]sdk.Result`. * (sdk/dec_coins) [#12903](https://github.com/cosmos/cosmos-sdk/pull/12903) Fix nil `DecCoin` creation when converting `Coins` to `DecCoins` * (x/upgrade) [#12906](https://github.com/cosmos/cosmos-sdk/pull/12906) Fix upgrade failure by moving downgrade verification logic after store migration. +* (store) [#12945](https://github.com/cosmos/cosmos-sdk/pull/12945) Fix nil end semantics in store/cachekv/iterator when iterating a dirty cache. ### Deprecated diff --git a/store/cachekv/store.go b/store/cachekv/store.go index 1e7b74ff93a7..29e297f4cf67 100644 --- a/store/cachekv/store.go +++ b/store/cachekv/store.go @@ -281,7 +281,7 @@ const ( // Constructs a slice of dirty items, to use w/ memIterator. func (store *Store) dirtyItems(start, end []byte) { startStr, endStr := conv.UnsafeBytesToStr(start), conv.UnsafeBytesToStr(end) - if startStr > endStr { + if end != nil && startStr > endStr { // Nothing to do here. return } @@ -296,6 +296,7 @@ func (store *Store) dirtyItems(start, end []byte) { // than just not having the cache. if n < 1024 { for key := range store.unsortedCache { + // dbm.IsKeyInDomain is nil safe and returns true iff key is greater than start if dbm.IsKeyInDomain(conv.UnsafeStrToBytes(key), start, end) { cacheValue := store.cache[key] unsorted = append(unsorted, &kv.Pair{Key: []byte(key), Value: cacheValue.value}) @@ -316,13 +317,18 @@ func (store *Store) dirtyItems(start, end []byte) { // Now find the values within the domain // [start, end) startIndex := findStartIndex(strL, startStr) - endIndex := findEndIndex(strL, endStr) + if startIndex < 0 { + startIndex = 0 + } - if endIndex < 0 { + var endIndex int + if end == nil { endIndex = len(strL) - 1 + } else { + endIndex = findEndIndex(strL, endStr) } - if startIndex < 0 { - startIndex = 0 + if endIndex < 0 { + endIndex = len(strL) - 1 } kvL := make([]*kv.Pair, 0) diff --git a/store/cachekv/store_test.go b/store/cachekv/store_test.go index d589932d30fc..aeb4194b34a9 100644 --- a/store/cachekv/store_test.go +++ b/store/cachekv/store_test.go @@ -306,6 +306,50 @@ func TestCacheKVMergeIteratorRandom(t *testing.T) { } } +func TestNilEndIterator(t *testing.T) { + const SIZE = 3000 + + tests := []struct { + name string + write bool + startIndex int + end []byte + }{ + {name: "write=false, end=nil", write: false, end: nil, startIndex: 1000}, + {name: "write=false, end=nil; full key scan", write: false, end: nil, startIndex: 2000}, + {name: "write=true, end=nil", write: true, end: nil, startIndex: 1000}, + {name: "write=false, end=non-nil", write: false, end: keyFmt(3000), startIndex: 1000}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + st := newCacheKVStore() + + for i := 0; i < SIZE; i++ { + kstr := keyFmt(i) + st.Set(kstr, valFmt(i)) + } + + if tt.write { + st.Write() + } + + itr := st.Iterator(keyFmt(tt.startIndex), tt.end) + i := tt.startIndex + j := 0 + for itr.Valid() { + require.Equal(t, keyFmt(i), itr.Key()) + require.Equal(t, valFmt(i), itr.Value()) + itr.Next() + i++ + j++ + } + + require.Equal(t, SIZE-tt.startIndex, j) + }) + } +} + //------------------------------------------------------------------------------------------- // do some random ops