Repository navigation
Backport release/v6.7: fix(seidb): keep the memIAVL nonce when state sync restores a mid-mig… - #4465
Conversation
#4463) ## Summary State sync from a snapshot taken during the last phase of the EVM migration writes nonce 0 over the real nonce of contracts in the state store (SS). This PR stops the SS import from writing a zero nonce from a FlatKV account row. Only the SS import path changes. State commitment, the snapshot format, and the AppHash are unchanged, so the fix is safe for a v6.7.x patch. The window: the migration moves keys in key order, so all 0x08 (code hash) keys move before any 0x0a (nonce) key. FlatKV stores the nonce and the code hash in one account row. Between the two prefixes, each contract has a FlatKV row with a correct code hash and nonce 0, while its real nonce is still in memIAVL. The snapshot puts the memIAVL sections first and the FlatKV section after them. So on import the SS gets the real nonce first, and then a zero nonce from the FlatKV row that overwrites it. The wrong value stays until a transaction writes that nonce again. Non-pending `eth_getTransactionCount`, `eth_call` and `eth_estimateGas` for contract-creating calls, and `debug_trace*` read it. - `sei-db/state_db/ss/composite/store.go`: `convertFlatKVNodes` now omits a zero nonce from a merged account row, the same way it already omits a zero code hash. A non-zero FlatKV nonce is never stale: `shouldForwardWriteToNewDB` sends each write to the database that holds the key, and a migrated key is deleted from memIAVL, so the two backends never both hold a valid nonce for one address. An absent nonce reads as 0 in `Keeper.GetNonce`, `DBImpl.Exist`, and `DBImpl.Empty`. - After state sync, an account with nonce 0 has no nonce key in the SS, but a node that replays blocks can hold 8 zero bytes. Point reads return the same value. A raw scan such as `IterateAllNonces` skips those accounts. - Backport to `release/v6.7`: on that branch `CompositeCommitStore.Commit` takes no argument, so the two `cs.Commit(cs.Version() + 1)` calls in `import_migration_test.go` become `cs.Commit()`. ## Test plan - `sei-db/state_db/ss/composite/store_test.go` - `TestImport_FlatKVZeroNonceKeepsMemIAVLNonce`: the stream holds a memIAVL nonce (5) and then a FlatKV account row with nonce 0 and a code hash. The SS keeps nonce 5 and stores the code hash, with `EVMSplit` true and false. - `TestImport_FlatKVZeroNonceIsAbsent`: a FlatKV account row with nonce 0 alone leaves no nonce key in the SS. - `sei-db/state_db/ss/composite/import_migration_test.go` - `TestImport_MidMigrationSnapshotRestoresNonce`: builds a real mid-migration commit store (MemiavlOnly commit, then `MigrateEVM` with batch size 1) and exports a snapshot. It asserts that the stream holds both the memIAVL nonce and the FlatKV account row, then imports it into the cosmos, evm-split, and evm-separate-sub-dbs SS layouts and checks the nonce and the code hash. - Without the `nonce != 0` guard, all three tests fail. With it, they pass. - `scripts/ramtest.sh ./sei-db/state_db/ss/... ./sei-cosmos/storev2/... -count=1` passes, and `-race` on `ss/composite` `TestImport*` passes. - `make fmtcheck` and `make dblint` are clean. Co-authored-by: Cursor <cursoragent@cursor.com> (cherry picked from commit 0b93e66)
PR SummaryMedium Risk Overview
Docs and behavior now align with code hash handling: omit zero values so reads treat a missing nonce as zero without clobbering restored state. New unit tests cover the memIAVL+FlatKV ordering conflict and FlatKV-only zero nonce; an integration test builds a real mid-migration export (batch size 1) and restores it across cosmos, EVM-split, and separate EVM sub-DB configs. Reviewed by Cursor Bugbot for commit 72b7a6e. Bugbot is set up for automated code reviews on this repo. Configure here. |
![seidroid[bot]](https://avatars.githubusercontent.com/in/2745135?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is a clean backport of #4463 (already merged) to release/v6.7, and I found nothing to fix. convertFlatKVNodes now skips a zero nonce when it splits a FlatKV account row. A mid-migration account row whose code hash has moved but whose nonce has not therefore no longer overwrites the memIAVL nonce restored earlier in the stream. An absent nonce still reads as zero, and the new unit tests and end-to-end snapshot tests cover the conflict across all three SS layouts. I confirmed that the APIs the tests depend on exist on the branch (SetMigrationBatchSize, MigrateEVM, keys.FlatKVStoreKey, ErrorExportDone), but I could not run the tests here.
Findings: 0 blocking | 0 non-blocking | 0 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- None at the file/PR level.
|
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 @@
## release/v6.7 #4465 +/- ##
================================================
- Coverage 61.70% 60.73% -0.97%
================================================
Files 2171 2074 -97
Lines 189923 178912 -11011
================================================
- Hits 117192 108668 -8524
+ Misses 61855 60242 -1613
+ Partials 10876 10002 -874
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
CompositeCommitStore.Commit takes no argument on this branch. Co-authored-by: Cursor <cursoragent@cursor.com>
Systematic changelog re-generation for the v6.7.1 patch release. Adds the PRs merged to `release/v6.7` since the v6.7.0 changelog (#4443), as reported by `scripts/generate-changelog.sh release/v6.6 release/v6.7`: - #4473 Backport `release/v6.7`: Make the composite store router an atomic pointer - #4465 Backport `release/v6.7`: fix(seidb): keep the memIAVL nonce when state sync restores a mid-mig… - #4445 Backport `release/v6.7`: Update v6.7 changelog in prep to cut v6.7.0 - #4444 Bump version to v6.7.0 in prep for release Only the `## v6.7` PR list changes, so the `backport release/v6.7` cherry-pick applies cleanly (simulated with `git merge-tree` against `origin/release/v6.7`). Not included: #4474 (the `release/v6.7` backport of #4469), which is still open. If it lands before the cut, this list needs regenerating. Co-authored-by: Cursor <cursoragent@cursor.com>
2 participants
Backport of #4463 to
release/v6.7.