Skip to content

statesync: record finalized version only after state root validation - #2455

Open
kkuehlz wants to merge 1 commit into
mainfrom
statesync-validate-before-finalize
Open

statesync: record finalized version only after state root validation#2455
kkuehlz wants to merge 1 commit into
mainfrom
statesync-validate-before-finalize

Conversation

@kkuehlz

@kkuehlz kkuehlz commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

monad_statesync_client_finalize previously marked the target finalized before comparing the canonical trie's state root against the trusted header. A peer supplying syntactically valid but corrupted state thus became the client's restart baseline: reopening rewinds to the recorded finalized version and initializes sync progress from it, so a normal retry never re-requested the corrupted range. Validate the root first and only then update the finalized version, and warn when a target is rejected.

Copilot AI review requested due to automatic review settings July 23, 2026 20:45
@kkuehlz
kkuehlz force-pushed the statesync-validate-before-finalize branch from ba94759 to e6198db Compare July 23, 2026 20:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a statesync correctness issue where monad_statesync_client_finalize could persist a corrupted sync target as the client’s restart baseline by updating the finalized version before validating the canonical trie’s state root against the trusted header.

Changes:

  • Delay update_finalized_version(tgrt.number) until after state_root() matches the trusted target header.
  • Emit a warning when the target is rejected due to a root mismatch.
  • Add a regression test to ensure restart-after-failure does not inherit the corrupted baseline.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
category/statesync/statesync_client.cpp Reorders finalize to validate state root before persisting finalized version; adds warning log on mismatch.
category/statesync/test/test_statesync.cpp Adds regression test covering retry behavior after failed root validation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread category/statesync/statesync_client.cpp Outdated
Comment thread category/statesync/statesync_client.cpp Outdated
@Baltoli
Baltoli requested a review from a team July 24, 2026 10:12
Chen-Yifan
Chen-Yifan previously approved these changes Aug 7, 2026

@Chen-Yifan Chen-Yifan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

approved but need to address copilot comments.

@kkuehlz
kkuehlz force-pushed the statesync-validate-before-finalize branch from e6198db to 3bcced3 Compare August 24, 2026 17:26
monad_statesync_client_finalize previously marked the target finalized
before comparing the canonical trie's state root against the trusted
header. A peer supplying syntactically valid but corrupted state thus
became the client's restart baseline: reopening rewinds to the recorded
finalized version and initializes sync progress from it, so a normal
retry never re-requested the corrupted range. Validate the root first
and only then update the finalized version, and warn when a target is
rejected.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@kkuehlz
kkuehlz force-pushed the statesync-validate-before-finalize branch from 3bcced3 to dd2e635 Compare August 24, 2026 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants