fix(flatkv): fix state sync panic on nil DB handles during snapshot restore#3003
Merged
blindchaser merged 3 commits intomainfrom Mar 2, 2026
Merged
fix(flatkv): fix state sync panic on nil DB handles during snapshot restore#3003blindchaser merged 3 commits intomainfrom
blindchaser merged 3 commits intomainfrom
Conversation
…estore rootmulti.Restore closes the SC store before calling Importer(), leaving all DB handles nil. The old code created a KVImporter against the closed store, which panicked in commitGlobalMetadata() on the nil metadataDB. A second bug in composite.SnapshotImporter.AddModule() never set currentModule, so the EVM importer received zero nodes — masking the nil-DB crash until Close() reached commitGlobalMetadata(). Fixes: - Reopen the store in Importer() when isClosed() is true - Set currentModule in composite SnapshotImporter.AddModule() - Move flatkv data dir under data/ to align with evm_ss convention - Add isClosed() helper to avoid spreading the nil-check assumption
blindchaser
changed the title
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3003 +/- ##
==========================================
- Coverage 58.30% 58.30% -0.01%
==========================================
Files 2108 2108
Lines 173672 173680 +8
==========================================
Hits 101262 101262
- Misses 63390 63398 +8
Partials 9020 9020
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
cody-littley
approved these changes
Mar 2, 2026
yzang2019
reviewed
Mar 2, 2026
sei-db/state_db/sc/flatkv/store.go
Outdated
|
|
||
| func (s *CommitStore) flatkvDir() string { | ||
| return filepath.Join(s.homeDir, flatkvRootDir) | ||
| return filepath.Join(s.homeDir, "data", flatkvRootDir) |
Contributor
There was a problem hiding this comment.
We should do the appending to the dir when we initialize FlatKV in CompositeSC layer instead of changing the function here. We probably don't wanna change the test as well, just use whatever is passed in should be good ?
| ) | ||
|
|
||
| // isClosed reports whether the store's DB handles have been released. | ||
| func (s *CommitStore) isClosed() bool { |
Contributor
There was a problem hiding this comment.
Shall we add an atomic bool just to make sure this is thread safe in the future?
Contributor
Author
There was a problem hiding this comment.
closed flag alone is probably not sufficient since other store fields are still unsynchronized. I’d avoid that for now and revisit when/if we decide to make CommitStore concurrent-safe.
yzang2019
approved these changes
Mar 2, 2026
yzang2019
pushed a commit
that referenced
this pull request
Mar 19, 2026
…estore (#3003) Issue: rootmulti.Restore closes the SC store before calling Importer(), leaving all DB handles nil. The old code created a KVImporter against the closed store, which panicked in commitGlobalMetadata() on the nil metadataDB. A second bug in composite.SnapshotImporter.AddModule() never set currentModule, so the EVM importer received zero nodes — masking the nil-DB crash until Close() reached commitGlobalMetadata(). Fixes: - Reopen the store in Importer() when isClosed() is true - Set currentModule in composite SnapshotImporter.AddModule() - Move flatkv data dir under data/ to align with evm_ss convention - Add isClosed() helper to avoid spreading the nil-check assumption ## Describe your changes and provide context ## Testing performed to validate your change
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue:
rootmulti.Restore closes the SC store before calling Importer(), leaving all DB handles nil. The old code created a KVImporter against the closed store, which panicked in commitGlobalMetadata() on the nil metadataDB.
A second bug in composite.SnapshotImporter.AddModule() never set currentModule, so the EVM importer received zero nodes — masking the nil-DB crash until Close() reached commitGlobalMetadata().
Fixes:
Describe your changes and provide context
Testing performed to validate your change