feat(sae): Log state-sync updates - #5932
Open
StephenButtolph wants to merge 2 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The changes are isolated to logging/plumbing, all in-repo call sites and tests are updated for the new syncer signature, and no functional issues were found in the modified code paths.
Pull request overview
This PR improves observability during SAEVM state sync by adding structured progress logs and by routing EVM state-sync logging through AvalancheGo’s logger instead of libevm’s global logger.
Changes:
- Added high-level state-sync progress logs (block sync, snapshot wipe, EVM state sync, cross-chain state sync) with relevant context fields.
- Refactored
evmstate.NewSyncer/trie sync internals to use an injectedlogging.Loggerand zap fields instead oflibevm/log. - Updated call sites, tests, and build/module dependencies to account for the new logger plumbing.
File summaries
| File | Description |
|---|---|
| vms/saevm/statesync/syncer.go | Adds structured logs for block sync, snapshot wipe, and EVM state sync phases. |
| vms/saevm/cchain/statesync/acceptor.go | Adds structured logs around cross-chain state sync. |
| graft/evm/sync/evmstate/trie_sync_tasks.go | Replaces libevm global logging with injected AvalancheGo logger (zap fields). |
| graft/evm/sync/evmstate/trie_sync_stats.go | Injects logger into trie sync stats and logs ETA/progress via AvalancheGo logger. |
| graft/evm/sync/evmstate/trie_sync_stats_test.go | Updates stats test to provide a no-op logger field. |
| graft/evm/sync/evmstate/trie_segments.go | Routes segment progress debug logs through the injected logger. |
| graft/evm/sync/evmstate/sync_test.go | Updates tests to pass a logger into evmstate.NewSyncer. |
| graft/evm/sync/evmstate/state_syncer.go | Adds logger plumbing to StateSync and improves finalize error context. |
| graft/evm/sync/evmstate/BUILD.bazel | Updates deps to include //utils/logging and zap; removes libevm/log. |
| graft/evm/sync/engine/client.go | Updates syncer construction to pass AvalancheGo logger into evmstate.NewSyncer. |
| graft/evm/go.mod | Promotes zap to a direct dependency for the graft/evm module. |
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
JonathanOppenheimer
approved these changes
Sep 5, 2026
| // The snapshot MUST either be empty or match the requested root. | ||
| // It will be regenerated anyway, so we can always wipe it. | ||
| // TODO(powerslider): Push into EVM syncer. | ||
| s.snowCtx.Log.Info("wiping snapshot before syncing state") |
Contributor
There was a problem hiding this comment.
maybe put this above the comment? so the comment is still placed right above the relevant line?
| } | ||
| s.snowCtx.Log.Info("finished wiping snapshot") | ||
|
|
||
| // TODO(powerslider): Remove dependency on graft. |
Comment on lines
+107
to
+111
| s.snowCtx.Log.Info("syncing blocks", | ||
| zap.Stringer("acceptedHash", summary.AcceptedHash), | ||
| zap.Uint64("acceptedHeight", summary.AcceptedHeight), | ||
| zap.Uint64("numToFetch", numBlocksToFetch), | ||
| ) |
Contributor
There was a problem hiding this comment.
Why not put this log in vms/evm/sync/block? like command click on the blockSyncer.Sync. I feel like all the logs should go as close as possible to the real source just like we do with comments.
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.
Why this should be merged
Currently state-syncing is totally silent.
How this works
How this was tested
Locally.
Need to be documented in RELEASES.md?
No.