Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(store): handle nil end in cachekv/iterator #12945

Merged
merged 10 commits into from
Aug 17, 2022
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
16 changes: 11 additions & 5 deletions store/cachekv/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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})
Expand All @@ -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)
Expand Down
44 changes: 44 additions & 0 deletions store/cachekv/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down