-
Notifications
You must be signed in to change notification settings - Fork 69
fix(miner): avoid XDPoS-only paths on non-XDPoS engines #2049
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
Guards XDPoS-specific mining worker logic to avoid unsafe type assertions against w.engine when the consensus engine is not *XDPoS.XDPoS.
Changes:
- Adds an
*XDPoS.XDPoStype assertion guard inworker.update()and reuses the asserted engine for channel access. - Makes
checkPreCommit()resilient to non-XDPoS engines by conditionally using XDPoS-specific parent selection and turn-check logic.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
c6d1f44 to
635b6aa
Compare
635b6aa to
5288c41
Compare
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
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
8f0291b to
8667fb3
Compare
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
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
8667fb3 to
5c5e6bf
Compare
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
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| done := make(chan struct{}) | ||
| go func() { | ||
| worker.update() | ||
| close(done) | ||
| }() | ||
|
|
||
| select { |
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.
This test can pass without actually exercising worker.update if the goroutine doesn’t get scheduled promptly (e.g., under heavy CI load): the first select will hit time.After and succeed even if update never started. Add a small synchronization handshake (e.g., a started channel closed right before calling worker.update(), then wait for it with a timeout) so the test reliably asserts update is running before checking it doesn’t return early.
| done := make(chan struct{}) | |
| go func() { | |
| worker.update() | |
| close(done) | |
| }() | |
| select { | |
| done := make(chan struct{}) | |
| started := make(chan struct{}) | |
| go func() { | |
| close(started) | |
| worker.update() | |
| close(done) | |
| }() | |
| select { | |
| case <-started: | |
| // worker.update has started; proceed with timing checks. | |
| case <-time.After(time.Second): | |
| t.Fatal("worker.update did not start in time") | |
| } | |
| select { |
5c5e6bf to
39f5062
Compare
Proposed changes
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