Fix RPC read race

[pdrobnjak]

Describe your changes and provide context

Intro

This is the sequence of events during block commitment.
app.WriteState() writes latest state to memiavl (among other things).
app.cms.Commit(true) updates lastCommitInfo (among other things).

Effect on RPC reads

GetTransactionCount resolves stores to use on the following call path: GetTransactionCount -> RPCContextProvider -> CreateQueryContext -> CacheMultiStoreWithVersion.

Stores can be resolved in two ways dependent on whether blockNumber from the RPC query is equal to rs.lastCommitInfo.Version:

sei-chain/sei-cosmos/storev2/rootmulti/store.go

Line 253 in 06a4e24

return rs.CacheMultiStore(), nil

sei-chain/sei-cosmos/storev2/rootmulti/store.go

Lines 257 to 275 in 06a4e24

	stores := make(map[types.StoreKey]types.CacheWrapper)
	// add the transient/mem stores registered in current app.
	for k, store := range rs.ckvStores {
	if store.GetStoreType() != types.StoreTypeIAVL {
	stores[k] = store
	}
	}
	// TODO: May need to add historical SC store as well for nodes that doesn't enable ss but still need historical queries
	// add SS stores for historical queries
	if rs.ssStore != nil {
	for k, store := range rs.ckvStores {
	if store.GetStoreType() == types.StoreTypeIAVL {
	stores[k] = state.NewStore(rs.ssStore, k, version)
	}
	}
	}
	return cachemulti.NewStore(nil, stores, rs.storeKeys, nil, nil, nil), nil

If it is equal we allow reading from memiavl (which holds only latest data if I understood correctly) and if it is different we allow reading from snapshot stores exclusively.

Because of the sequence of events outlined in the intro the following can happen:

1. State from block X+1 (block that's currently executing) was written into memiavl via app.WriteState().
2. eth_getTransactionCount on block X is querried and during execution evaluates this if as true and serves rs.CacheMultiStore() - since app.cms.Commit(true) has not yet executed and updated lastCommitInfo we think that blockNumber from the RPC query is equal to rs.lastCommitInfo.Version.
3. We read the nonce from memiavl which serves the latest nonce instead of the nonce at block X.

Impact

The corruption window lasts from when the state was written to memiavl via app.WriteState() until app.cms.Commit(true) executes.

Issue affects all RPC data that can be served from memiavl during the window outlined above (e.g. eth_getBalance, eth_getTransactionCount, etc.).

Solution

We should never serve memiavl on the RPC read path (or any non-execution read path) and this block of code should be removed entirely.

Logically it does not make sense to serve memiavl as a store on the RPC read path seeing as it only contains the latest state and the read path does not get a clone/must not lock it meaning it can get modified on the fly which leads to inconsistencies.

Testing performed to validate your change

I had a repro of the issue locally so I validated that the issue goes away when this change is introduced.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 39.61%. Comparing base (d285ea5) to head (2d9c95d).
⚠️ Report is 1 commits behind head on main.

❌ Your project status has failed because the head coverage (47.19%) is below the target coverage (60.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2454   +/-   ##
=======================================
  Coverage   39.61%   39.61%           
=======================================
  Files        1570     1570           
  Lines      160980   160977    -3     
=======================================
+ Hits        63768    63769    +1     
+ Misses      92373    92369    -4     
  Partials     4839     4839           

Flag	Coverage Δ
sei-chain	27.30% <ø> (+<0.01%)	⬆️
sei-cosmos	51.64% <ø> (+<0.01%)	⬆️
sei-wasmd	51.41% <ø> (-0.03%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
sei-cosmos/storev2/rootmulti/store.go	25.60% <ø> (+0.13%)	⬆️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.