Skip to content

Backport release/v6.6: Validate snapshots before publication - #4053

Merged
masih merged 2 commits into
release/v6.6from
backport-4036-to-release/v6.6
Aug 28, 2026
Merged

Backport release/v6.6: Validate snapshots before publication#4053
masih merged 2 commits into
release/v6.6from
backport-4036-to-release/v6.6

Conversation

@seidroid

@seidroid seidroid Bot commented Aug 28, 2026

Copy link
Copy Markdown

Backport of #4036 to release/v6.6.

@seidroid seidroid Bot added the backport label Aug 28, 2026
@seidroid seidroid Bot assigned masih Aug 28, 2026
@seidroid

seidroid Bot commented Aug 28, 2026

Copy link
Copy Markdown
Author

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-4036-to-release/v6.6
git worktree add --checkout .worktree/backport-4036-to-release/v6.6 backport-4036-to-release/v6.6
cd .worktree/backport-4036-to-release/v6.6
git reset --hard HEAD^
git cherry-pick -x f28da4c23213808d751969e99a2ee3ccf41f8473
git push --force-with-lease

@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedAug 28, 2026, 5:49 PM

@masih
masih marked this pull request as ready for review August 28, 2026 17:30
@masih
masih enabled auto-merge (squash) August 28, 2026 17:30
@codecov

codecov Bot commented Aug 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 68.50394% with 40 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.53%. Comparing base (3c36273) to head (a000cda).

Files with missing lines Patch % Lines
sei-db/state_db/sc/memiavl/db.go 70.00% 17 Missing and 13 partials ⚠️
sei-db/state_db/sc/memiavl/multitree.go 61.53% 3 Missing and 2 partials ⚠️
sei-db/state_db/sc/memiavl/snapshot.go 64.28% 5 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@               Coverage Diff                @@
##           release/v6.6    #4053      +/-   ##
================================================
- Coverage         59.08%   58.53%   -0.55%     
================================================
  Files              2231     2175      -56     
  Lines            184478   178868    -5610     
================================================
- Hits             109001   104704    -4297     
+ Misses            65650    64808     -842     
+ Partials           9827     9356     -471     
Flag Coverage Δ
sei-db 69.80% <ø> (ø)
sei-db-state-db ?
sei-db-state-db-pr 77.53% <68.50%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
sei-db/state_db/sc/memiavl/multitree.go 80.76% <61.53%> (+1.37%) ⬆️
sei-db/state_db/sc/memiavl/snapshot.go 67.84% <64.28%> (+2.08%) ⬆️
sei-db/state_db/sc/memiavl/db.go 69.60% <70.00%> (+2.83%) ⬆️

... and 56 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@cursor

cursor Bot commented Aug 28, 2026

Copy link
Copy Markdown

PR Summary

High Risk
Changes core state snapshot persistence, symlink handling, and corruption recovery on the consensus state DB; mistakes could cause data loss, failed restarts, or incorrect self-healing during rewrite.

Overview
MemIAVL snapshot rewrite and publication now validate on-disk data against lastCommitInfo (multi-tree version, store set, per-store versions, and root hashes) before adopting or publishing. Structural problems are tagged with errCorruptedSnapshot so only those cases trigger replacement; cancelled contexts, I/O, or permission errors no longer cause deleting or overwriting snapshots that might still be good.

publishSnapshot validates the temp directory first, then atomically renames or reconciles rename conflicts (reuse a valid existing snapshot-N, or remove and replace a corrupt one only after the temp passes validation). Corrupt targets are kept until a validated replacement exists so current cannot be left dangling after a failed write.

Load and write paths are tightened: metadata and mmap layout checks, snapshot open validation, and 32-byte hash checks on snapshot writers wrap corruption as errCorruptedSnapshot; createDBIfNotExist refuses to create an empty DB when current points at a missing snapshot; updateCurrentSymlink clears stale current-tmp for idempotent recovery; partial LoadMultiTree loads close already-opened trees.

Broad db_rewrite_test coverage exercises adopt/skip/replace, environmental vs corruption boundaries, and dangling-link startup behavior.

Reviewed by Cursor Bugbot for commit a000cda. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit a000cda. Configure here.

}
if renameErr := os.Rename(path, targetPath); renameErr != nil {
return fmt.Errorf("rename snapshot directory to %q: %w", targetPath, renameErr)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unsafe snapshot replacement loses data

High Severity

publishSnapshot removes the existing snapshot directory before the replacement rename finishes. A crash in that window, or a failed second rename, leaves current pointing at a deleted directory. RewriteSnapshot then always runs cleanupFailedSnapshotRewrite on publication errors, which deletes the validated temp — the only remaining copy. On restart, removeTmpDirs also drops leftover -tmp dirs, and createDBIfNotExist now refuses to initialize over a dangling current, so the node cannot recover.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit a000cda. Configure here.

@masih
masih merged commit d2e7ad2 into release/v6.6 Aug 28, 2026
63 of 66 checks passed
@masih
masih deleted the backport-4036-to-release/v6.6 branch August 28, 2026 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants