feat(unwind): log reorg paths that bypass the on-disk unwind - #23071
Open
MoonBoi9001 wants to merge 3 commits into
Open
feat(unwind): log reorg paths that bypass the on-disk unwind#23071MoonBoi9001 wants to merge 3 commits into
MoonBoi9001 wants to merge 3 commits into
Conversation
Four branches can switch the canonical chain without running the on-disk unwind, and none of them said so in the log, so a reorg that leaves stale index data behind is invisible in a node's journal. Each branch now logs when it skips, making the silent cases visible and countable.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves observability around fork-choice / unwind scenarios that can change the canonical chain without running the on-disk unwind, by adding targeted log lines (and one execution-stage progress read in fork-choice) so these “silent skips” become visible in node journals.
Changes:
- Log when
Sync.UnwindTodrops an unwind request because the target is behind commitment. - Log when
Sync.unwindStageskips unwinding because a stage is at/below the unwind point (Info when exactly at point; Debug when behind). - In fork-choice, log when the unwind path is skipped even though executed progress is above the reorg point; add a stage-progress read to support that decision.
- Log when
UnwindExecutionStagetakes the RAM-only unwind path (disk state untouched).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| execution/stagedsync/sync.go | Adds logs for dropped unwind requests and per-stage unwind skip conditions. |
| execution/stagedsync/stage_execute.go | Adds an Info log when execution unwind is RAM-only (no disk unwind). |
| execution/execmodule/forkchoice.go | Adds an execution progress read and logs when FCU unwind is skipped despite executed state being above reorg target. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The dropped-request log recorded the unwind reason's error, which is often nil (fork-choice unwinds carry none), hiding why the request was dropped. Record the commitment-check error that actually caused the drop instead.
The other error returns on the unwind path wrap their errors with the function name; the new stage-progress read returned its error bare, making failures harder to attribute.
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.
This PR adds a log line to each of the 4 code paths that can switch the canonical chain without running the on-disk unwind (defect 2 in #23062). Today they skip silently, so a reorg that leaves stale data behind looks identical to a healthy one in a node's journal; the logs make the skips visible and countable while the right behavior for each path is decided. Nothing else changes: log statements plus 1 stage-progress read in fork choice.
Where each line fires: