Skip to content

Commit

Permalink
fix: state listener could observe discarded writes (backport #13459) (#…
Browse files Browse the repository at this point in the history
…13462)

* fix: state listener could observe discarded writes (#13459)

* fix: state listener could observe uncommitted writes

Closes: #13457

don't pass listeners to nested cached store,
only the most inner layer's cache writes should be observed.

* Update CHANGELOG.md

* add unit test

* rename

Co-authored-by: Marko <[email protected]>
(cherry picked from commit bb54c59)

# Conflicts:
#	CHANGELOG.md

* Update CHANGELOG.md

Co-authored-by: yihuang <[email protected]>
Co-authored-by: Julien Robert <[email protected]>
  • Loading branch information
3 people authored Oct 6, 2022
1 parent 982b891 commit cb676e2
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 1 deletion.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ Ref: https://keepachangelog.com/en/1.0.0/

* (auth) [#13460](https://github.com/cosmos/cosmos-sdk/pull/13460) The `q auth address-by-id` CLI command has been renamed to `q auth address-by-acc-num` to be more explicit. However, the old `address-by-id` version is still kept as an alias, for backwards compatibility.

### Bug Fixes

* (store) [#13459](https://github.com/cosmos/cosmos-sdk/pull/13459) Don't let state listener observe the uncommitted writes.

## [v0.46.2](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.46.2) - 2022-10-03

### API Breaking Changes
Expand Down
6 changes: 5 additions & 1 deletion store/cachemulti/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ func NewFromKVStore(
keys map[string]types.StoreKey, traceWriter io.Writer, traceContext types.TraceContext,
listeners map[types.StoreKey][]types.WriteListener,
) Store {
if listeners == nil {
listeners = make(map[types.StoreKey][]types.WriteListener)
}
cms := Store{
db: cachekv.NewStore(store),
stores: make(map[types.StoreKey]types.CacheWrap, len(stores)),
Expand Down Expand Up @@ -86,7 +89,8 @@ func newCacheMultiStoreFromCMS(cms Store) Store {
stores[k] = v
}

return NewFromKVStore(cms.db, stores, nil, cms.traceWriter, cms.traceContext, cms.listeners)
// don't pass listeners to nested cache store.
return NewFromKVStore(cms.db, stores, nil, cms.traceWriter, cms.traceContext, nil)
}

// SetTracer sets the tracer for the MultiStore that the underlying
Expand Down
49 changes: 49 additions & 0 deletions store/rootmulti/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -912,3 +912,52 @@ func hashStores(stores map[types.StoreKey]types.CommitKVStore) []byte {
}
return sdkmaps.HashFromMap(m)
}

type MockListener struct {
stateCache []types.StoreKVPair
}

func (tl *MockListener) OnWrite(storeKey types.StoreKey, key []byte, value []byte, delete bool) error {
tl.stateCache = append(tl.stateCache, types.StoreKVPair{
StoreKey: storeKey.Name(),
Key: key,
Value: value,
Delete: delete,
})
return nil
}

func TestStateListeners(t *testing.T) {
var db dbm.DB = dbm.NewMemDB()
ms := newMultiStoreWithMounts(db, pruningtypes.NewPruningOptions(pruningtypes.PruningNothing))

listener := &MockListener{}
ms.AddListeners(testStoreKey1, []types.WriteListener{listener})

require.NoError(t, ms.LoadLatestVersion())
cacheMulti := ms.CacheMultiStore()

store1 := cacheMulti.GetKVStore(testStoreKey1)
store1.Set([]byte{1}, []byte{1})
require.Empty(t, listener.stateCache)

// writes are observed when cache store commit.
cacheMulti.Write()
require.Equal(t, 1, len(listener.stateCache))

// test nested cache store
listener.stateCache = []types.StoreKVPair{}
nested := cacheMulti.CacheMultiStore()

store1 = nested.GetKVStore(testStoreKey1)
store1.Set([]byte{1}, []byte{1})
require.Empty(t, listener.stateCache)

// writes are not observed when nested cache store commit
nested.Write()
require.Empty(t, listener.stateCache)

// writes are observed when inner cache store commit
cacheMulti.Write()
require.Equal(t, 1, len(listener.stateCache))
}

0 comments on commit cb676e2

Please sign in to comment.