Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements strict verification of the final state root during replay operations to address issue #3902. Previously, the verification would always pass if startup info existed, without actually comparing expected vs actual state roots.
Key Changes:
- Modified
replay_changesets_batchedto track and return the expected state root from the last changeset applied - Updated
verify_final_state_rootto accept the expected state root and perform strict comparison against the actual state root from startup info - Enhanced error handling to mark metadata as failed when verification fails
Comments suppressed due to low confidence (1)
crates/rooch-pruner/src/state_prune/incremental_replayer.rs:376
- The replay process writes new SMT nodes but never updates the startup info with the new state root. After applying all changesets, the startup info should be updated to reflect the final state root. Without this, the verification will always compare against the stale initial state root instead of the actual final state. Consider updating the startup info after successfully applying all changesets, similar to what's done in import_state.rs (lines 165-174).
/// Apply a batch of changesets
fn apply_changeset_batch(
&self,
batch: &[(u64, StateChangeSetExt)],
snapshot_store: &MoveOSStore,
report: &mut ReplayReport,
) -> Result<()> {
let mut all_nodes = BTreeMap::new();
let mut total_objects_created = 0u64;
let mut total_objects_updated = 0u64;
// Process each changeset in the batch
for (tx_order, changeset_ext) in batch {
let mut changeset = changeset_ext.state_change_set.clone();
// Convert changeset to SMT nodes using existing API
let (nodes, _stale_indices) = snapshot_store
.get_state_store()
.change_set_to_nodes(&mut changeset)
.map_err(|e| {
anyhow::anyhow!("Failed to convert changeset {} to nodes: {}", tx_order, e)
})?;
// Accumulate nodes for batch write
all_nodes.extend(nodes);
// Count object changes
total_objects_created += self.count_objects_created(changeset_ext);
total_objects_updated += self.count_objects_updated(changeset_ext);
}
// Batch write all nodes atomically
let nodes_count = all_nodes.len();
if !all_nodes.is_empty() {
snapshot_store
.get_state_node_store()
.write_nodes(all_nodes)
.map_err(|e| anyhow::anyhow!("Failed to write {} nodes: {}", nodes_count, e))?;
}
// Update report statistics
report.nodes_updated += nodes_count as u64;
info!(
"Applied batch: {} changesets, {} nodes, {} objects created, {} objects updated",
batch.len(),
nodes_count,
total_objects_created,
total_objects_updated
);
Ok(())
}
Comment on lines
+311
to
+314
| let expected_state_root = changesets | ||
| .last() | ||
| .map(|(_, changeset)| changeset.state_change_set.state_root) | ||
| .unwrap_or_else(H256::zero); |
There was a problem hiding this comment.
When the changesets vector is empty, this code returns H256::zero() as the expected state root. However, this may not be the correct behavior. If no changesets were applied, the expected state root should be the initial state root from the snapshot, not zero. Consider handling the empty changeset case explicitly by either returning an error or using the initial state root from the snapshot.
This commit fixes issue #3902 by implementing strict verification of the final state root during replay operations. Changes: - Modified `replay_changesets_batched` to return the expected final state root from the last changeset applied - Updated `verify_final_state_root` to accept and compare the expected state root against the actual state root from startup info - Verification now fails with detailed error message when state roots don't match - The replay report's `verification_passed` flag is set based on actual comparison - Metadata is marked as failed when verification fails in the main replay method - Added comprehensive tests for both matching and mismatching state root cases Acceptance criteria met: - Mismatch causes ReplayReport failure + metadata status Failed - Matching root passes verification - Tests cover both match and mismatch cases All 106 tests pass, linting passes, binary builds successfully. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
This PR fixes issue #3902 by implementing strict verification of the final state root during replay operations.
Problem
Previously, the
verify_final_state_rootmethod would always pass if startup info existed, without actually comparing the expected state root (from the last changeset applied) against the actual state root (from the snapshot store's startup info). This meant that replay operations could report success even when the final state didn't match expectations.Solution
Changes Made
Modified
replay_changesets_batchedto track and return expected state rootUpdated
verify_final_state_rootto accept and compare expected state rootexpected_state_root: H256Errwith detailed error message on mismatchreport.verification_passedbased on actual comparisonreport.final_state_rootUpdated main
replay_changesetsmethodreplay_changesets_batchedverify_final_state_rootAdded comprehensive tests
test_verify_final_state_root_success: Tests matching state roots passtest_verify_final_state_root_failure: Tests mismatching state roots failAcceptance Criteria Met
✅ Mismatch causes ReplayReport failure + metadata status Failed
report.verification_passedis set to falsereport.is_success()returns false✅ Matching root passes
report.verification_passedis set to true✅ Tests cover both match and mismatch cases
Testing
rooch-prunerpassmake lintpasses (cargo fmt, clippy)Files Changed
crates/rooch-pruner/src/state_prune/incremental_replayer.rs🤖 Generated with Claude Code