-
Notifications
You must be signed in to change notification settings - Fork 69
fix(miner): recompute timestamp after sleep #2052
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev-upgrade
Are you sure you want to change the base?
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Updates mining work creation to recompute the candidate block timestamp after any sleep, aiming to avoid stale timestamps when the parent block changes.
Changes:
- Recalculates
tstart/tstampafter re-checking the parent block (post-sleep) incommitNewWork.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| tstamp = tstart.Unix() | ||
| if parent.Time() >= uint64(tstamp) { | ||
| tstamp = int64(parent.Time() + 1) | ||
| } |
Copilot
AI
Feb 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After re-checking parent post-sleep, tstamp is recomputed but the "Mining too far in the future" guard is not re-applied. If the canonical parent advances to a block with a higher parent.Time() while sleeping, this can set header.Time far ahead of wall clock time without waiting again. Consider re-running the future-time wait logic after this recalculation (or moving the sleep until after the final parent selection).
| } | |
| } | |
| // Ensure we are still not mining too far in the future with the updated parent. | |
| if now := time.Now().Unix(); tstamp > now { | |
| wait := time.Duration(tstamp-now) * time.Second | |
| log.Info("Mining too far in the future", "wait", common.PrettyDuration(wait)) | |
| time.Sleep(wait) | |
| } |
| tstart = time.Now() | ||
| tstamp = tstart.Unix() |
Copilot
AI
Feb 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tstart is later used for commit/finalize/total timing metrics, but it gets reassigned here. This changes the meaning of the logged durations (they will no longer include any pre-lock work/sleep) and is likely unintended given the PR scope. Consider keeping the original tstart for metrics and using a separate variable when recomputing the timestamp.
| tstart = time.Now() | |
| tstamp = tstart.Unix() | |
| now := time.Now() | |
| tstamp = now.Unix() |
3a37f22 to
e2dcaa3
Compare
e2dcaa3 to
191d63a
Compare
Proposed changes
recompute timestamp after sleep
Types of changes
What types of changes does your code introduce to XDC network?
Put an
✅in the boxes that applyImpacted Components
Which part of the codebase this PR will touch base on,
Put an
✅in the boxes that applyChecklist
Put an
✅in the boxes once you have confirmed below actions (or provide reasons on not doing so) that