diff --git a/sei-db/state_db/sc/memiavl/db.go b/sei-db/state_db/sc/memiavl/db.go index b99bb58ec4..6e9339c799 100644 --- a/sei-db/state_db/sc/memiavl/db.go +++ b/sei-db/state_db/sc/memiavl/db.go @@ -1054,8 +1054,15 @@ func (db *DB) Close() error { return errorutils.Join(errs...) } -// TreeByName wraps MultiTree.TreeByName to add a lock. +// TreeByName wraps MultiTree.TreeByName to add a lock. Safe to call on a nil +// receiver: returns nil so callers can treat an un-opened DB as "store +// missing" rather than panicking. The nil case is exercised when a higher +// layer holds a *CommitStore whose underlying *DB has not yet been opened +// (for example, during state-sync before the snapshot finishes applying). func (db *DB) TreeByName(name string) *Tree { + if db == nil { + return nil + } db.mtx.Lock() defer db.mtx.Unlock() diff --git a/sei-db/state_db/sc/memiavl/store.go b/sei-db/state_db/sc/memiavl/store.go index dc9be9e2d3..a96f5745e5 100644 --- a/sei-db/state_db/sc/memiavl/store.go +++ b/sei-db/state_db/sc/memiavl/store.go @@ -170,6 +170,13 @@ func (cs *CommitStore) LastCommitInfo() *proto.CommitInfo { } func (cs *CommitStore) GetChildStoreByName(name string) types.CommitKVStore { + // The underlying DB is opened lazily via LoadVersion / Rollback. Reads can + // arrive before that happens (for example, the mempool reactor invokes + // CheckTx during state-sync while the snapshot is still being applied), + // so a typed nil return must be safe. + if cs == nil || cs.db == nil { + return nil + } tree := cs.db.TreeByName(name) if tree == nil { // Return an explicitly nil interface (not a typed-nil *Tree wrapped in an @@ -179,6 +186,14 @@ func (cs *CommitStore) GetChildStoreByName(name string) types.CommitKVStore { return tree } +// IsLoaded reports whether the underlying memiavl DB has been opened. It is +// safe to call on a nil receiver. Callers built on top of CommitStore use this +// to distinguish "store has no committed data yet" (during state-sync, before +// LoadVersion) from "store name is misregistered" (a real config error). +func (cs *CommitStore) IsLoaded() bool { + return cs != nil && cs.db != nil +} + func (cs *CommitStore) Exporter(version int64) (types.Exporter, error) { if version < 0 || version > math.MaxUint32 { return nil, fmt.Errorf("version %d out of range", version) diff --git a/sei-db/state_db/sc/memiavl/unloaded_store_test.go b/sei-db/state_db/sc/memiavl/unloaded_store_test.go new file mode 100644 index 0000000000..82d07a954d --- /dev/null +++ b/sei-db/state_db/sc/memiavl/unloaded_store_test.go @@ -0,0 +1,60 @@ +package memiavl + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +// TestCommitStore_IsLoaded covers the pre-load window that arises during +// state-sync: NewCommitStore returns a *CommitStore whose underlying *DB is +// nil until LoadVersion (or Rollback) opens it. Callers downstream of the +// migration router rely on IsLoaded to distinguish that transient state from +// a missing store name. +func TestCommitStore_IsLoaded(t *testing.T) { + t.Run("nil receiver is not loaded", func(t *testing.T) { + var cs *CommitStore + require.False(t, cs.IsLoaded()) + }) + + t.Run("freshly constructed store is not loaded", func(t *testing.T) { + cs := NewCommitStore(t.TempDir(), DefaultConfig()) + require.False(t, cs.IsLoaded()) + }) + + t.Run("LoadVersion marks the store loaded", func(t *testing.T) { + cs := NewCommitStore(t.TempDir(), DefaultConfig()) + require.NoError(t, cs.Initialize([]string{"params"})) + _, err := cs.LoadVersion(0, false) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, cs.Close()) }) + + require.True(t, cs.IsLoaded()) + }) +} + +// TestCommitStore_GetChildStoreByName_BeforeLoad asserts that reads issued +// against an un-opened CommitStore return a nil interface rather than +// panicking on cs.db.TreeByName. The mempool reactor exercises this code +// path while state-sync is still applying chunks (see the panic stack in +// sei-tendermint/internal/mempool/reactor.go:139). +func TestCommitStore_GetChildStoreByName_BeforeLoad(t *testing.T) { + t.Run("nil receiver", func(t *testing.T) { + var cs *CommitStore + require.Nil(t, cs.GetChildStoreByName("params")) + }) + + t.Run("constructed but not loaded", func(t *testing.T) { + cs := NewCommitStore(t.TempDir(), DefaultConfig()) + require.Nil(t, cs.GetChildStoreByName("params")) + }) +} + +// TestDB_TreeByName_NilReceiver guards the lowest layer: even if a caller +// somehow hands TreeByName a nil *DB (e.g. via a typed-nil field that +// escaped initialisation), the call must return nil instead of nil-derefing +// db.mtx. +func TestDB_TreeByName_NilReceiver(t *testing.T) { + var db *DB + require.Nil(t, db.TreeByName("params")) +} diff --git a/sei-db/state_db/sc/migration/router_builder.go b/sei-db/state_db/sc/migration/router_builder.go index 557eda4f98..2878864fdc 100644 --- a/sei-db/state_db/sc/migration/router_builder.go +++ b/sei-db/state_db/sc/migration/router_builder.go @@ -556,8 +556,17 @@ func buildTestOnlyDualWriteRouter( } // Build a function capable of reading data from memiavl. +// +// During state-sync the underlying memiavl DB may not yet be open: the +// snapshot is still being applied while the mempool reactor is already +// dispatching CheckTx calls. Treat that pre-load window as "no committed +// state" by reporting key-not-found rather than erroring; once LoadVersion +// opens the DB the original "store not found" config-error path resumes. func buildMemIAVLReader(memIAVL *memiavl.CommitStore) DBReader { return func(store string, key []byte) ([]byte, bool, error) { + if !memIAVL.IsLoaded() { + return nil, false, nil + } childStore := memIAVL.GetChildStoreByName(store) if childStore == nil { return nil, false, fmt.Errorf("store not found: %s", store) @@ -568,8 +577,16 @@ func buildMemIAVLReader(memIAVL *memiavl.CommitStore) DBReader { } // Build a function capable of writing data to memiavl. +// +// Writes should never reach this closure before memiavl is loaded: the +// commit pipeline only runs after the snapshot has been applied. Return a +// loud error if it ever happens so the bug surfaces instead of corrupting +// silently. func buildMemIAVLWriter(memIAVL *memiavl.CommitStore) DBWriter { return func(changesets []*proto.NamedChangeSet, _ bool) error { + if !memIAVL.IsLoaded() { + return fmt.Errorf("memiavl commit store not loaded yet; refusing to apply %d changeset(s)", len(changesets)) + } err := memIAVL.ApplyChangeSets(changesets) if err != nil { return fmt.Errorf("ApplyChangeSets: %w", err) @@ -581,6 +598,9 @@ func buildMemIAVLWriter(memIAVL *memiavl.CommitStore) DBWriter { // Build a function capable of getting an iterator over a range of keys in a memiavl store. func buildMemIAVLIteratorBuilder(memIAVL *memiavl.CommitStore) DBIteratorBuilder { return func(store string, start []byte, end []byte, ascending bool) (dbm.Iterator, error) { + if !memIAVL.IsLoaded() { + return nil, fmt.Errorf("memiavl commit store not loaded yet; cannot iterate store %q", store) + } childStore := memIAVL.GetChildStoreByName(store) if childStore == nil { return nil, fmt.Errorf("store not found: %s", store) @@ -589,9 +609,14 @@ func buildMemIAVLIteratorBuilder(memIAVL *memiavl.CommitStore) DBIteratorBuilder } } -// Build a function capable of building a proof of the value for a key in a memiavl store. +// Build a function capable of building a proof of the value for a key in a +// memiavl store. Proof generation requires a committed tree, so a pre-load +// call must fail rather than synthesise an empty proof. func buildMemIAVLProofBuilder(memIAVL *memiavl.CommitStore) DBProofBuilder { return func(store string, key []byte) (*ics23.CommitmentProof, error) { + if !memIAVL.IsLoaded() { + return nil, fmt.Errorf("memiavl commit store not loaded yet; cannot build proof for store %q", store) + } childStore := memIAVL.GetChildStoreByName(store) if childStore == nil { return nil, fmt.Errorf("store not found: %s", store) diff --git a/sei-db/state_db/sc/migration/router_builder_unloaded_test.go b/sei-db/state_db/sc/migration/router_builder_unloaded_test.go new file mode 100644 index 0000000000..9962037e35 --- /dev/null +++ b/sei-db/state_db/sc/migration/router_builder_unloaded_test.go @@ -0,0 +1,79 @@ +package migration + +import ( + "testing" + + "github.com/sei-protocol/sei-chain/sei-db/proto" + "github.com/sei-protocol/sei-chain/sei-db/state_db/sc/memiavl" + "github.com/stretchr/testify/require" +) + +// These tests pin down the contract the migration router exposes to higher +// layers while a memiavl-backed CommitStore is still in its pre-load state +// (i.e. after memiavl.NewCommitStore but before LoadVersion). The mempool +// reactor reaches this code path during state-sync via +// App.CheckTx -> BaseApp.GetConsensusParams -> RouterCommitKVStore.Has, and +// previously panicked on a nil *memiavl.DB dereference. + +func newUnloadedMemIAVLForTest(t *testing.T) *memiavl.CommitStore { + t.Helper() + cs := memiavl.NewCommitStore(t.TempDir(), memiavl.DefaultConfig()) + require.False(t, cs.IsLoaded(), "precondition: store must not be loaded") + return cs +} + +func TestBuildMemIAVLReader_BeforeLoad_ReportsNotFoundWithoutError(t *testing.T) { + read := buildMemIAVLReader(newUnloadedMemIAVLForTest(t)) + + value, found, err := read("params", []byte("Block")) + require.NoError(t, err, "reader must not error during the state-sync pre-load window") + require.False(t, found) + require.Nil(t, value) +} + +func TestBuildMemIAVLIteratorBuilder_BeforeLoad_RefusesIteration(t *testing.T) { + iter := buildMemIAVLIteratorBuilder(newUnloadedMemIAVLForTest(t)) + + it, err := iter("params", nil, nil, true) + require.Error(t, err, "iteration before LoadVersion must surface explicitly") + require.Nil(t, it) +} + +func TestBuildMemIAVLWriter_BeforeLoad_RefusesWritesLoudly(t *testing.T) { + write := buildMemIAVLWriter(newUnloadedMemIAVLForTest(t)) + + err := write([]*proto.NamedChangeSet{{ + Name: "params", + Changeset: proto.ChangeSet{Pairs: []*proto.KVPair{ + {Key: []byte("k"), Value: []byte("v")}, + }}, + }}, false) + + require.Error(t, err, "writes before LoadVersion must surface, not corrupt silently") +} + +func TestBuildMemIAVLProofBuilder_BeforeLoad_RefusesProof(t *testing.T) { + proof := buildMemIAVLProofBuilder(newUnloadedMemIAVLForTest(t)) + + _, err := proof("params", []byte("k")) + require.Error(t, err, "proofs require a committed tree and must fail explicitly") +} + +// TestModuleRouter_Read_BeforeLoad_DoesNotPanic exercises the exact code path +// that previously panicked at sei-db/state_db/sc/memiavl/db.go:1059 during +// state-sync: RouterCommitKVStore.Has -> ModuleRouter.Read -> reader closure +// -> memiavl.CommitStore.GetChildStoreByName -> memiavl.DB.TreeByName. +func TestModuleRouter_Read_BeforeLoad_DoesNotPanic(t *testing.T) { + cs := newUnloadedMemIAVLForTest(t) + + route, err := routeToMemIAVL(cs, "params") + require.NoError(t, err) + + router, err := NewModuleRouter(route) + require.NoError(t, err) + + value, found, err := router.Read("params", []byte("Block")) + require.NoError(t, err) + require.False(t, found) + require.Nil(t, value) +}