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 |
There was a problem hiding this comment.
Pull request overview
This PR fixes issue #3900 where the replay function was incorrectly using the live MoveOSStore instead of loading data from the snapshot path. The fix ensures that replay operations now properly load and use the snapshot database, making it possible to perform true replays from snapshots.
Key Changes
- Modified
load_snapshot_storeto load the actual snapshot database fromsnapshot_path/snapshot.dbinstead of returning the live store - Removed the unused
moveos_storefield fromIncrementalReplayerstruct since the snapshot store is now loaded on-demand - Added comprehensive regression tests to verify the fix and validate error handling
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
crates/rooch-pruner/src/state_prune/incremental_replayer.rs |
Fixed load_snapshot_store implementation to load from snapshot path, removed unused struct field, added two regression tests |
crates/rooch-pruner/Cargo.toml |
Moved prometheus from dev-dependencies to dependencies (required for MoveOSStore::new with Registry) |
| impl IncrementalReplayer { | ||
| /// Create new incremental replayer | ||
| pub fn new(config: ReplayConfig, moveos_store: MoveOSStore) -> Result<Self> { | ||
| pub fn new(config: ReplayConfig, _moveos_store: MoveOSStore) -> Result<Self> { |
There was a problem hiding this comment.
The _moveos_store parameter is no longer used after the fix but is still required by the API. This could be misleading for callers who might assume it's being used for replay operations. Consider removing this parameter entirely and updating all callers to pass only the ReplayConfig. This would make the API clearer and prevent confusion about which store is actually used during replay.
Previously, the replay function ignored the snapshot contents and used the live MoveOSStore, causing replay operations to read/write against the live DB instead of the snapshot. Changes: - Fix load_snapshot_store to load from snapshot_path/snapshot.db - Add validation for snapshot database path - Add regression tests to verify snapshot isolation - Move prometheus to dependencies (required for MoveOSStore::new) This ensures replay runs against snapshot content, not current DB. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Overview
This PR fixes issue #3900 where the replay function was ignoring snapshot contents and using the live MoveOSStore instead.
Problem
The
load_snapshot_storefunction inIncrementalReplayerwas ignoring thesnapshot_pathparameter and returning the live MoveOSStore. This caused replay operations to read and write against the live database rather than the snapshot, making it impossible to perform a true replay from a snapshot.Solution
Modified the
load_snapshot_storefunction to:snapshot_path/snapshot.dbChanges
crates/rooch-pruner/src/state_prune/incremental_replayer.rs:load_snapshot_storeto load from the actual snapshot pathmoveos_storefield fromIncrementalReplayerstructtest_load_snapshot_store_uses_snapshot_path_not_live_db: Verifies snapshot is loaded from correct pathtest_load_snapshot_store_validates_path: Tests error handling for invalid pathscrates/rooch-pruner/Cargo.toml:prometheusfromdev-dependenciestodependencies(required forMoveOSStore::new)Acceptance Criteria
✅
load_snapshot_storeuses snapshot data from disk✅ Replay runs against snapshot content, not current DB
✅ Regression tests added that verify the fix
Test Results
rooch-prunerpackage pass🤖 Generated with Claude Code