Repository navigation
Backport release/v6.7: fix(seidb): refuse a corrupted changelog in digest replay instead of repairing it - #4285
Conversation
…repairing it (#3983) ## Summary `seidb evm-logical-digest --memiavl-open-mode replay` opens the changelog through the same opener a writer uses, and that opener repairs a tail ending mid-record by truncating it. `Options.ReadOnly` does not prevent this: it gates the DB API, not the changelog open. On a live node the repair fires on a healthy log. A tail ending mid-record is usually the writer mid-append — `write` is not atomic against a concurrent reader, so a reader sees a page-aligned prefix of a multi-page changeset. Truncating it discards a record `seid` has committed, and the writer's descriptor keeps its old offset, leaving a zero-filled hole the binary decoder reads as valid zero-length records. Nothing fails at the time; it surfaces when the node next reopens and replays. Nothing about the directory distinguishes that from real damage, so this stops trying to. Replay refuses instead, leaving the tail where it found it, and reports that a rerun is the next step. The window is a single write, so a rerun clears it; a tail that survives repeated runs is real damage, and the message says so. - `sei-db/wal/wal.go`, `utils.go`: add `Config.NoRepairOnOpen`, which returns `ErrCorrupt` from `open` instead of truncating, and export `ErrCorrupt` so a caller can name the outcome. Default is unset, so every existing caller — `seid` included — keeps repairing as before. - `sei-db/state_db/sc/memiavl/opts.go`, `db.go`: add `Options.NoChangelogRepair` and pass it through. `ReadOnly` alone still repairs, because a reader that cannot rerun is better served by proceeding. - `sei-db/tools/cmd/seidb/operations/evm_logical_digest.go`: set it for replay mode, and translate `ErrCorrupt` into what the operator should do. The translation also covers a torn record found during the replay rather than at the open: `computeWALIndexDelta` and `Catchup` read the changelog after the open returns, and they surface the same sentinel, where a rerun is the same right answer. ### What this does not cover `wal.Open` also finishes an interrupted truncation before it checks for a torn record, by removing and renaming segments — a `.START` file left by `TruncateFront`, or a `.END` file left by `TruncateBack`. It reports no error for either, so there is nothing to gate on and no rerun to prompt. Finishing one underneath the writer makes the writer's own remove fail, and tidwall sets `l.corrupt` past that point, so every later append fails until `seid` restarts. The operator message therefore promises only the changelog tail, which does survive, rather than the whole directory. Both windows are narrower than the mid-record one, which recurs every block. `TruncateFront` runs after a snapshot rewrite, roughly hourly at the default interval. `TruncateBack` runs only inside a `LoadForOverwriting` open, so reaching that window needs a crash during a rollback and a replay before the node restarts. Both are left open deliberately — closing them needs either a check before the open, which the writer can invalidate in the gap, or the directory's `LOCK`, which would require the node stopped and take away the case replay exists for. Replay stays usable on a live node, which is the point: it is the fallback for a height with no snapshot, and on a live migrating node the two backends rarely retain a common snapshot height. ## Test plan - `sei-db/tools/cmd/seidb/operations/memiavl_open_test.go`: tear the last changelog segment, then require the open to report `ErrCorrupt` with the rerun guidance *and* the segment's bytes to be unchanged, since the refusal is worth nothing if the tail does not survive it. Replaying an intact changelog to the requested height is the positive control. Reverting `NoChangelogRepair` makes the first test fail on a successful open, so it pins the behavior rather than restating it. - `sei-db/wal/wal_test.go`: existing `TestOpenAndCorruptedTail` still passes with repair requested, pinning that the default path is untouched. - `go test ./sei-db/wal/... ./sei-db/tools/cmd/seidb/operations/...`, `scripts/ramtest.sh ./sei-db/state_db/sc/memiavl/...` - `make dblint` --------- Co-authored-by: Cursor <cursoragent@cursor.com> (cherry picked from commit 7273a43)
PR SummaryMedium Risk Overview The WAL layer gains Reviewed by Cursor Bugbot for commit 8218fbb. Bugbot is set up for automated code reviews on this repo. Configure here. |
…ckported test CommitStore.Commit takes a version argument on main and none on this branch.
|
The cherry-pick applied without conflicts, but it does not compile as generated: Why this should land before #4255. Verified locally on this branch: All three packages pass, including the two new tests ( |
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
![seidroid[bot]](https://avatars.githubusercontent.com/in/2745135?s=60&v=4){kind=small}
There was a problem hiding this comment.
Faithful backport of #3983: the changelog open path gains a NoRepairOnOpen gate, memiavl exposes it as Options.NoChangelogRepair, and the EVM logical-digest replay opener sets it so it refuses a torn tail instead of truncating a live node's changelog. The one divergence from upstream (store.Commit() instead of store.Commit(store.Version()+1)) correctly matches the memiavl.CommitStore.Commit() signature on this branch, and all new test helpers (newTestMemiavlStore, noncePair, addrN) exist in the operations package here.
Findings: 0 blocking | 1 non-blocking | 0 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- None at the file/PR level.
- 1 non-blocking pre-existing issue(s) listed below under pre-existing issues.
Pre-existing issues
- [suggestion]
memiavl.OpenDB(sei-db/state_db/sc/memiavl/db.go:213-220) returns on thefailed to open changelog WALerror without closing theMultiTreeloaded just above, leaking the snapshot mmaps/fds for the lifetime of the process. This exists on the base branch, but the PR makes that error path routinely reachable (a torn tail on a live node now fails the open instead of being repaired). Harmless for the one-shot seidb CLI; worth amtree.Close()on the error path for long-lived callers that retryOpenDB.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## release/v6.7 #4285 +/- ##
================================================
- Coverage 61.34% 60.60% -0.74%
================================================
Files 2163 2083 -80
Lines 188785 180185 -8600
================================================
- Hits 115813 109208 -6605
+ Misses 62254 60968 -1286
+ Partials 10718 10009 -709
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Adds the `release/v6.7` entries merged since the rc1 changelog (#4110), in prep to cut **v6.7.0-rc2**: - [#4292](#4292) — Flush MemIAVL changelog before exiting on an upgrade panic - [#4285](#4285) — fix(seidb): refuse a corrupted changelog in digest replay instead of repairing it - [#4255](#4255) — feat(seidb): Add JSON output to evm-logical-digest and inspect a FlatKV migration in flight - [#4116](#4116) — rc1 version bump - [#4113](#4113) — rc1 changelog backport Regenerated with `./scripts/generate-changelog.sh release/v6.6 release/v6.7`; only the `## v6.7` PR list changes, so the `backport release/v6.7` cherry-pick applies cleanly (verified with `git apply --check` against `origin/release/v6.7`). Docs-only; no code change. Made with [Cursor](https://cursor.com) Co-authored-by: Cursor <cursoragent@cursor.com>
## Summary - Bump `version.json` from `v6.7.0-rc1` to `v6.7.0-rc2` to cut the second `v6.7` release candidate. Contents since rc1: #4285, #4255, #4292 (all `sei-db` fixes/tooling), plus the rc2 changelog update (#4294). Merge after #4294 so the tag cut by `uci-release-publish` on this `version.json` change includes the updated changelog. ## Test plan - [x] `git diff --check` Made with [Cursor](https://cursor.com) Co-authored-by: Cursor <cursoragent@cursor.com>
3 participants
Backport of #3983 to
release/v6.7.