fix: don't mark tipsets with temporal errors as permanently bad#6613
fix: don't mark tipsets with temporal errors as permanently bad#6613LesnyRumcajs merged 2 commits intomainfrom
Conversation
WalkthroughThis PR fixes a chain sync bug where blocks with timestamps in the future (time-travelling blocks) cause the sync process to get stuck. The fix updates the validation logic to skip such blocks rather than permanently marking them as bad, allowing future re-validation and preventing sync stalls. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
LesnyRumcajs
force-pushed
the
dont-mark-time-travelling-tipsets-as-bad
branch
from
e7ed52d to
b7a27e3
Compare
LesnyRumcajs
requested review from
akaladarshi and
sudo-shashank
and removed request for
a team
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/chain_sync/chain_follower.rs (1)
846-858:⚠️ Potential issue | 🔴 CriticalFix the TimeTravellingBlock match pattern—it uses struct syntax for a tuple variant.
The enum at
tipset_syncer.rs:48definesTimeTravellingBlock(u64, u64)as a tuple variant. The pattern at line 855 incorrectly uses{ .. }(struct syntax). Change it to(..)to match the tuple variant shape. The existing code attipset_syncer.rs:131uses the correct(_, _)pattern.✅ Proposed fix
- Err(e) if matches!(e, TipsetSyncerError::TimeTravellingBlock { .. }) => { + Err(e) if matches!(e, TipsetSyncerError::TimeTravellingBlock(..)) => { warn!("Time travelling block detected, skipping tipset for now: {e}"); None }
🤖 Fix all issues with AI agents
In `@CHANGELOG.md`:
- Line 38: The changelog entry currently references the PR number [`#6613`] —
update the entry in CHANGELOG.md to reference the corresponding issue instead
(use the format [`#ISSUE_NO`](link-to-issue): Fixed chain sync getting stuck when
encountering time-travelling blocks by not marking the corresponding tipsets as
permanently bad) if an issue exists; replace the PR link/text with the issue
link and number, keeping the description exact, and only fall back to the PR
number if no issue exists.
Codecov Report❌ Patch coverage is
Additional details and impacted files
... and 19 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
github-merge-queue
Bot
removed this pull request from the merge queue due to no response for status checks
3 participants
Summary of changes
Forest marks blocks as bad to avoid dealing with them later on. This is good. Temporal errors are kind of special case, and we might receive them eventually with correct timestamp. We have logic to ignore them, so that the node doesn't get stuck if such a block becomes part of the canonical chain. Unfortunately, we also have similar logic for entire tipsets, and so we didn't ban the block... we banned the entire tipset. 🙂 This made the node get stuck, as no valid chain could now be constructed.
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes
Other information and links
Change checklist
Outside contributions
Summary by CodeRabbit