From 4eeda95ef35ec7864330a3a7e8daaed949dd6091 Mon Sep 17 00:00:00 2001 From: Amir Deris Date: Thu, 28 May 2026 12:00:39 -0700 Subject: [PATCH 1/3] Added concurrent safe read for getComitKVStore and added test --- sei-cosmos/storev2/rootmulti/store.go | 2 ++ sei-cosmos/storev2/rootmulti/store_test.go | 32 ++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/sei-cosmos/storev2/rootmulti/store.go b/sei-cosmos/storev2/rootmulti/store.go index 6284d4def7..4f245257fa 100644 --- a/sei-cosmos/storev2/rootmulti/store.go +++ b/sei-cosmos/storev2/rootmulti/store.go @@ -453,6 +453,8 @@ func (rs *Store) GetCommitStore(key types.StoreKey) types.CommitStore { // GetCommitKVStore Implements interface CommitMultiStore func (rs *Store) GetCommitKVStore(key types.StoreKey) types.CommitKVStore { + rs.mtx.RLock() + defer rs.mtx.RUnlock() return rs.ckvStores[key] } diff --git a/sei-cosmos/storev2/rootmulti/store_test.go b/sei-cosmos/storev2/rootmulti/store_test.go index 4d505aa981..4b249ac6c7 100644 --- a/sei-cosmos/storev2/rootmulti/store_test.go +++ b/sei-cosmos/storev2/rootmulti/store_test.go @@ -7,6 +7,7 @@ import ( "testing" "time" + "github.com/sei-protocol/sei-chain/sei-cosmos/store/mem" "github.com/sei-protocol/sei-chain/sei-cosmos/store/types" "github.com/sei-protocol/sei-chain/sei-cosmos/storev2/state" "github.com/sei-protocol/sei-chain/sei-db/config" @@ -15,6 +16,37 @@ import ( "golang.org/x/time/rate" ) +// TestGetCommitKVStoreNoDataRace verifies that a concurrent Commit (which rewrites +// ckvStores entries under the write lock) and a GetStoreByName call (the path taken +// by ABCI query metrics) do not produce a data race on the ckvStores map. +// can Run it with: go test -race ./sei-cosmos/storev2/rootmulti/... +func TestGetCommitKVStoreNoDataRace(t *testing.T) { + store := &Store{ + storeKeys: map[string]types.StoreKey{}, + ckvStores: map[types.StoreKey]types.CommitKVStore{}, + } + key := types.NewKVStoreKey("bank") + store.storeKeys[key.Name()] = key + store.ckvStores[key] = mem.NewStore() + + var wg sync.WaitGroup + wg.Add(2) + + go func() { + defer wg.Done() + store.mtx.Lock() + defer store.mtx.Unlock() + store.ckvStores[key] = mem.NewStore() + }() + + go func() { + defer wg.Done() + store.GetStoreByName(key.Name()) + }() + + wg.Wait() +} + func TestLastCommitID(t *testing.T) { store := NewStore(t.TempDir(), config.DefaultStateCommitConfig(), config.StateStoreConfig{}, []string{}) require.Equal(t, types.CommitID{}, store.LastCommitID()) From e5a3ced3f914b66fa99f74c836a4995a5fcd7a03 Mon Sep 17 00:00:00 2001 From: Amir Deris Date: Thu, 28 May 2026 16:04:25 -0700 Subject: [PATCH 2/3] Updated test without race condition --- sei-cosmos/storev2/rootmulti/store_test.go | 30 ++++++++++------------ 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/sei-cosmos/storev2/rootmulti/store_test.go b/sei-cosmos/storev2/rootmulti/store_test.go index 4b249ac6c7..90b7aa98c9 100644 --- a/sei-cosmos/storev2/rootmulti/store_test.go +++ b/sei-cosmos/storev2/rootmulti/store_test.go @@ -16,11 +16,7 @@ import ( "golang.org/x/time/rate" ) -// TestGetCommitKVStoreNoDataRace verifies that a concurrent Commit (which rewrites -// ckvStores entries under the write lock) and a GetStoreByName call (the path taken -// by ABCI query metrics) do not produce a data race on the ckvStores map. -// can Run it with: go test -race ./sei-cosmos/storev2/rootmulti/... -func TestGetCommitKVStoreNoDataRace(t *testing.T) { +func TestGetCommitKVStore_ReaderRespectsWriteLock(t *testing.T) { store := &Store{ storeKeys: map[string]types.StoreKey{}, ckvStores: map[types.StoreKey]types.CommitKVStore{}, @@ -29,22 +25,24 @@ func TestGetCommitKVStoreNoDataRace(t *testing.T) { store.storeKeys[key.Name()] = key store.ckvStores[key] = mem.NewStore() - var wg sync.WaitGroup - wg.Add(2) + store.mtx.Lock() + readDone := make(chan types.CommitKVStore, 1) go func() { - defer wg.Done() - store.mtx.Lock() - defer store.mtx.Unlock() - store.ckvStores[key] = mem.NewStore() + readDone <- store.GetCommitKVStore(key) //GetCommitKVStore is blocked until store.mtx is unlocked }() - go func() { - defer wg.Done() - store.GetStoreByName(key.Name()) - }() + select { + case <-readDone: + t.Fatal("GetCommitKVStore returned while write lock held — RLock missing") + case <-time.After(50 * time.Millisecond): + } + + newVal := mem.NewStore() + store.ckvStores = map[types.StoreKey]types.CommitKVStore{key: newVal} + store.mtx.Unlock() - wg.Wait() + require.Same(t, newVal, <-readDone) } func TestLastCommitID(t *testing.T) { From e43d925974b89a186f846ad71a34d9c330603424 Mon Sep 17 00:00:00 2001 From: Amir Deris Date: Fri, 29 May 2026 10:53:00 -0700 Subject: [PATCH 3/3] Fixed other methods reading kv store concurrently --- sei-cosmos/storev2/rootmulti/store.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sei-cosmos/storev2/rootmulti/store.go b/sei-cosmos/storev2/rootmulti/store.go index 4f245257fa..ff02872b44 100644 --- a/sei-cosmos/storev2/rootmulti/store.go +++ b/sei-cosmos/storev2/rootmulti/store.go @@ -398,11 +398,15 @@ func (rs *Store) CacheMultiStoreFromCommitter(snap sctypes.Committer) (types.Cac // GetStore Implements interface MultiStore func (rs *Store) GetStore(key types.StoreKey) types.Store { + rs.mtx.RLock() + defer rs.mtx.RUnlock() return rs.ckvStores[key] } // GetKVStore Implements interface MultiStore func (rs *Store) GetKVStore(key types.StoreKey) types.KVStore { + rs.mtx.RLock() + defer rs.mtx.RUnlock() return rs.ckvStores[key] }