Repository navigation
Re-derive the composite write mode on rollback - #4477
Conversation
Rollback rewound both backends and reset the commit-info latches but kept the write mode and router derived at the pre-rollback height. Rolling back across a migration seam therefore reported the wrong commit info and would route later writes as if the seam were still crossed. Rollback now re-derives the mode and rebuilds the router from the rolled-back metadata, the same step LoadLatest runs.
|
I'll fix CI failures and address comments from users with write access that start with 'Devin'.
|
|
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 #4477 +/- ##
=======================================
Coverage 56.90% 56.91%
=======================================
Files 2130 2130
Lines 167552 167555 +3
=======================================
+ Hits 95353 95361 +8
+ Misses 72194 72189 -5
Partials 5 5
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
PR SummaryMedium Risk Overview The resolve-mode-then-build-router sequence used on load is factored into Tests add Reviewed by Cursor Bugbot for commit befd438. 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.
Rollback now re-derives the Auto write mode and rebuilds the router from the rolled-back metadata, using the same adoptPersistedWriteMode step that LoadLatest uses, so a store rolled back in process matches one rebuilt by a restart. Pinned modes still resolve as a plain copy, the latches are still reset before refreshLastCommitInfo, and both new tests replay the blocks they roll back over against recorded commit info, so I found nothing that blocks; the codex reading (no findings) agrees with this and added nothing to check, and the tree was the PR merge ref, but no Go toolchain was available, so the tests were not run.
seidroid review · decision approve · session 0187fc1992ef4863a492b4b7c15b54b5 · turn resp_claude_8f1507289ff538c4fc0fa0578ecdf6e8 · item 38bf6a7eaeda598bb4dc58100b17780d
Findings: 0 blocking | 0 non-blocking | 0 posted inline
|
The second part of improvements to follow in a separate PR; I will link up the PRs on GH for visibility; thanks all for speedy reviews 🍻 |
…osite-rollback-write-mode # Conflicts: # sei-db/state_db/sc/composite/store_migration_test.go
|
Created backport PR for
Please cherry-pick the changes locally and resolve any conflicts. git fetch origin backport-4477-to-release/v6.7
git worktree add --checkout .worktree/backport-4477-to-release/v6.7 backport-4477-to-release/v6.7
cd .worktree/backport-4477-to-release/v6.7
git reset --hard HEAD^
git cherry-pick -x 6278fe9b1295a861c2eb7c7dac683845e762d0a2
git push --force-with-lease |
`CompositeCommitStore.Rollback` rewinds both backends and resets the commit-info latches, but it keeps the write mode and router that were derived at the pre-rollback height. When an Auto store is rolled back across a migration seam, the in-process store therefore disagrees with what a restart derives from the same disk state. Two cases show this: rolling back below bank completion from a store that derived `FlatKVOnly`, and rolling back to the height before activation after the EVM migration finished. The commit info that `seid rollback` reports is wrong in both. In the bank case, blocks committed afterwards in the same process also route writes as if the migration had completed. The on-disk state is correct, and the next start derives the right mode. The resolve-mode-then-build-router step that `LoadLatest` runs is now a named step, `adoptPersistedWriteMode`. `Rollback` calls it after the latch resets, so a rolled-back store matches a restarted one. Pinned modes are unaffected because resolving a pinned mode is a copy. The new tests roll back across both seams and check that the reported commit info is canonical. They also recommit the rolled-back blocks and require the forward-run commit info at every height, in the same process and after a restart. The bank test also fails if only the mode is re-derived and the router is left stale. No change to the AppHash. (cherry picked from commit 6278fe9)
…-protocol#4484) A store pinned to a steady-state write mode (`evm_migrated`, `all_migrated_but_bank` or `flatkv_only`) copies the configured mode without looking at the migration state persisted in flatkv. When that state says a migration is still in flight, for example after `Rollback` to a height below a migration's completion, the pinned router and commit info no longer match the data. A `flatkv_only` pin never opens memiavl, so its commit info drops memiavl's stores. An `evm_migrated` pin stops running the in-flight migration, so the next block diverges. Either way it persists across restarts for as long as the pin stays. `resolveCurrentWriteMode`, which `LoadLatest` and, since sei-protocol#4477, `Rollback` both go through, now calls `requirePinnedModeMatchesMigration`. It derives the persisted mode and refuses the pin unless the two match or the persisted mode is `MemiavlOnly`, which is what a store whose history began in flatkv derives. The error tells the operator to re-enable auto mode, and reopening in Auto reconciles memiavl down to the rolled-back height. This builds on sei-protocol#4477 and is based on its branch, so the diff here shows only this change. It touches the same fields as sei-protocol#4475, so whichever lands second needs a small rebase. There is no AppHash change for supported configurations: the genesis-pinned `flatkv_only` localnode and CI clusters carry no migration metadata and are unaffected. New composite tests cover the rollback refusal and recovery in Auto, the in-flight refusal for each steady-state pin, and a genesis-pinned store, and the composite, migration and rootmulti packages pass under `-race`.
3 participants
CompositeCommitStore.Rollbackrewinds both backends and resets the commit-info latches, but it keeps the write mode and router that were derived at the pre-rollback height. When an Auto store is rolled back across a migration seam, the in-process store therefore disagrees with what a restart derives from the same disk state. Two cases show this: rolling back below bank completion from a store that derivedFlatKVOnly, and rolling back to the height before activation after the EVM migration finished. The commit info thatseid rollbackreports is wrong in both. In the bank case, blocks committed afterwards in the same process also route writes as if the migration had completed. The on-disk state is correct, and the next start derives the right mode.The resolve-mode-then-build-router step that
LoadLatestruns is now a named step,adoptPersistedWriteMode.Rollbackcalls it after the latch resets, so a rolled-back store matches a restarted one. Pinned modes are unaffected because resolving a pinned mode is a copy. The new tests roll back across both seams and check that the reported commit info is canonical. They also recommit the rolled-back blocks and require the forward-run commit info at every height, in the same process and after a restart. The bank test also fails if only the mode is re-derived and the router is left stale.No change to the AppHash.