Repository navigation
fix(seidb): keep the memIAVL nonce when state sync restores a mid-mig… - #4463
Conversation
…ration snapshot During the EVM migration, keys move to FlatKV in key order, so every 0x08 code-hash key moves before any 0x0a nonce key. FlatKV stores both in one account row, so between the two prefixes every contract has a FlatKV row with nonce 0 while its real nonce is still in memIAVL. A snapshot taken in that window carries the memIAVL nonce first and the FlatKV account row after it (sc/composite/exporter.go). convertFlatKVNodes expanded the row into a nonce node with value 0, which overwrote the real nonce in the state store. The state store only receives application writes after the restore, so the wrong nonce stayed until a transaction wrote that nonce again. eth_getTransactionCount (non-pending), eth_call/eth_estimateGas for contract-creating calls, and debug_trace* read it. convertFlatKVNodes now omits a zero nonce, like it already omits a zero code hash. A non-zero FlatKV nonce is never stale: shouldForwardWriteToNewDB sends a 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. Keeper.GetNonce, DBImpl.Exist, and DBImpl.Empty read an absent nonce as 0. This changes only the state-store import path. State commitment, the snapshot format, and the AppHash are unchanged. Notes for operators and reviewers: - The fix applies to new imports only. A node that already state-synced from a mid-migration snapshot keeps the wrong nonces; run state sync on it again from a snapshot taken after the migration completes. - After state sync, an account with nonce 0 has no nonce key in the state store, where 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. Co-authored-by: Cursor <cursoragent@cursor.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
PR SummaryMedium Risk Overview
Adds regression tests: synthetic import streams ( Reviewed by Cursor Bugbot for commit 530b8b0. 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 #4463 +/- ##
=======================================
Coverage 56.80% 56.80%
=======================================
Files 2127 2127
Lines 167022 167022
=======================================
+ Hits 94872 94876 +4
+ Misses 72145 72141 -4
Partials 5 5
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![seidroid[bot]](https://avatars.githubusercontent.com/in/2745135?s=60&v=4){kind=small}
There was a problem hiding this comment.
This fix stops the state-store snapshot importer from writing a zero nonce from a FlatKV account row, so a memIAVL nonce restored earlier in a mid-migration stream is no longer overwritten. The guard holds against the migration router: shouldForwardWriteToNewDB sends a write only to the backend that already holds the key, so a non-zero FlatKV nonce cannot be stale. Rows that are all zero were already dropped by the old IsDelete check, and the three new tests cover the conflict across all SS layouts. Nothing blocks. The codex reading found nothing, which agrees with this review. No Go toolchain was available in the sandbox, so I read the tests but did not run them.
Non-blocking
- After state sync, an account whose FlatKV row holds nonce 0 and a non-zero code hash has no nonce key in the SS. The SC FlatKV
Getreturns 8 zero bytes for the same account (accountFieldValue), and so does a node that replayed blocks. Point reads agree, but raw scans such asIterateAllNonces(used byx/evm/genesis.goexport) now give different results on state-synced and replayed nodes. The values mean the same thing, but a comment where the nonce is read or exported would stop a later reader from treating the gap as a bug.
seidroid review · decision approve · session 01db11662b88408a878de2c8568f7ba3 · turn resp_claude_95306b90f981353d874f10298b5eda1a · item fe62a946ca075560b365c04f9d507376
Findings: 0 blocking | 1 non-blocking | 0 posted inline
|
Git push to origin failed for release/v6.7 with exitcode 1 |
|
/backport |
|
Successfully created backport PR for |
3 participants
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_callandeth_estimateGasfor contract-creating calls, anddebug_trace*read it.sei-db/state_db/ss/composite/store.go:convertFlatKVNodesnow 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:shouldForwardWriteToNewDBsends 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 inKeeper.GetNonce,DBImpl.Exist, andDBImpl.Empty.IterateAllNoncesskips those accounts.release/v6.7: on that branchCompositeCommitStore.Committakes no argument, so the twocs.Commit(cs.Version() + 1)calls inimport_migration_test.gobecomecs.Commit().Test plan
sei-db/state_db/ss/composite/store_test.goTestImport_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, withEVMSplittrue 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.goTestImport_MidMigrationSnapshotRestoresNonce: builds a real mid-migration commit store (MemiavlOnly commit, thenMigrateEVMwith 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.nonce != 0guard, all three tests fail. With it, they pass.scripts/ramtest.sh ./sei-db/state_db/ss/... ./sei-cosmos/storev2/... -count=1passes, and-raceonss/compositeTestImport*passes.make fmtcheckandmake dblintare clean.