fix: memiavl-nil-deref-during-state-sync#3518
Conversation
PR SummaryMedium Risk Overview memiavl now treats an unopened store as safe nil: New tests lock in nil-receiver behavior and the router contract on an unloaded Reviewed by Cursor Bugbot for commit 1c80d55. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3518 +/- ##
==========================================
- Coverage 59.03% 58.65% -0.38%
==========================================
Files 2199 2159 -40
Lines 182207 178092 -4115
==========================================
- Hits 107569 104468 -3101
+ Misses 64975 64315 -660
+ Partials 9663 9309 -354
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Summary
During state-sync the mempool reactor runs
CheckTx(→GetConsensusParams→RouterCommitKVStore.Has→Router.Read) before memiavl'sLoadVersionopens the underlying*DB. Withcs.db == nilthis nil-derefs and crashes the node. Affects any node still backed by memiavl (defaultMemiavlOnlyincluded), not just EVM-migration nodes.This makes the memiavl read path safe during that pre-load window:
memiavl/db.go:TreeByNamereturns nil on a nil receiver instead of lockingdb.mtx.memiavl/store.go:GetChildStoreByNamereturns a nil interface when unloaded; addsIsLoaded().migration/router_builder.go: memiavl closures gate onIsLoaded(). Reads return key-not-found (no error), soHas/Getno longer panic. Writes/iterators/proofs return an explicit error rather than corrupting state or faking a proof.Scope / known limits:
RouterCommitKVStore(now with a clear "not loaded yet" message instead of a nil deref).cs.dbis still accessed without synchronization vs.LoadVersion; this fixes the deterministic nil-deref, not a potential data race.CheckTxduring state-sync.Test plan
memiavl/unloaded_store_test.go:IsLoadedtransitions; nil-safeGetChildStoreByName/TreeByNamebefore load.migration/router_builder_unloaded_test.go: reader returns not-found without error; writer/iterator/proof error before load;TestModuleRouter_Read_BeforeLoad_DoesNotPanicreproduces the original panic stack.