Skip to content
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

Fix: better win detection on restart #5752

Merged
merged 7 commits into from
Jan 30, 2025
Merged

Conversation

kantai
Copy link
Member

@kantai kantai commented Jan 27, 2025

Description

This improves how miners detect whether or not they won a sortition. This mostly impacts miners on restarts, because previously they used in-memory state to detect a winning sortition (#5750)

@kantai kantai requested review from a team as code owners January 27, 2025 19:55
obycode
obycode previously approved these changes Jan 27, 2025
Copy link
Contributor

@obycode obycode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good 👍. On a similar note, we may also want to open another issue to resume mining if the miner restarted in the middle of a tenure that it is winning. I don't think that is currently in place either.

@kantai
Copy link
Member Author

kantai commented Jan 27, 2025

This looks good 👍. On a similar note, we may also want to open another issue to resume mining if the miner restarted in the middle of a tenure that it is winning. I don't think that is currently in place either.

Well, let me see if this PR addresses that too.

@kantai
Copy link
Member Author

kantai commented Jan 27, 2025

Well, let me see if this PR addresses that too.

It doesn't! But now I have a test for it. Opened issue #5754

@kantai kantai requested a review from a team as a code owner January 27, 2025 22:02
obycode
obycode previously approved these changes Jan 27, 2025
@aldur aldur added this to the 3.1.0.0.5 milestone Jan 28, 2025
@hstove hstove self-requested a review January 28, 2025 15:34
obycode
obycode previously approved these changes Jan 28, 2025
@aldur aldur requested review from rdeioris and jbencin January 29, 2025 15:43
rdeioris
rdeioris previously approved these changes Jan 29, 2025
Copy link
Contributor

@rdeioris rdeioris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

jferrant
jferrant previously approved these changes Jan 29, 2025
Copy link
Collaborator

@jferrant jferrant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@kantai kantai added this pull request to the merge queue Jan 29, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Jan 29, 2025
@hstove hstove dismissed stale reviews from jferrant, rdeioris, and obycode via 8bf3cd2 January 29, 2025 21:02
@hstove
Copy link
Contributor

hstove commented Jan 29, 2025

I went ahead an fixed the conflict (it was just the Changelog)

hstove
hstove previously approved these changes Jan 29, 2025
obycode
obycode previously approved these changes Jan 29, 2025
@hstove hstove enabled auto-merge January 29, 2025 21:11
@jferrant
Copy link
Collaborator

Sorry you gotta update the test to use Secp256k1PrivateKey::random() instead of Secp256k1PrivateKey::new()

@kantai kantai dismissed stale reviews from obycode and hstove via a1e5dee January 29, 2025 22:02
@hstove hstove added this pull request to the merge queue Jan 30, 2025
Merged via the queue into develop with commit 59c7e7c Jan 30, 2025
183 checks passed
@hstove hstove deleted the feat/better-win-detection branch January 30, 2025 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

6 participants