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

feat: stop waiting for signatures if the chain tip advances #5801

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

obycode
Copy link
Contributor

@obycode obycode commented Feb 4, 2025

This could happen at the beginning of a tenure, if the miner for tenure T+1 proposes its first block before it sees the last block from tenure T, so the block it proposed is reorging that last block. Once it sees this block, it should immediately stop waiting for signatures and instead build a new block off of the new tip.

Fixes: #5751

This could happen at the beginning of a tenure, if the miner for tenure
T+1 proposes its first block before it sees the last block from tenure
T, so the block it proposed is reorging that last block. Once it sees
this block, it should immediately stop waiting for signatures and
instead build a new block off of the new tip.

Fixes: #5751
@obycode obycode requested review from a team as code owners February 4, 2025 20:47
@obycode obycode requested a review from a team as a code owner February 4, 2025 20:49
@obycode obycode requested review from kantai and jferrant February 4, 2025 20:52
@obycode
Copy link
Contributor Author

obycode commented Feb 4, 2025

Looking at integration test failures now...

@kantai
Copy link
Member

kantai commented Feb 4, 2025

Hmm -- I was thinking today about reviving this PR (#5284)

I think these have some overlapping goals. #5284 is a little more preemptive, with the downside that maybe the block its waiting on doesn't get confirmed. But this PR (#5801) seems like it may not have a very large behavioral impact (presumably, its block proposal is going to be getting rejected pretty quickly anyways?)

@obycode
Copy link
Contributor Author

obycode commented Feb 4, 2025

Ah, I forgot about that one. They are tackling similar problems.

This one was initiated due to seeing this problem on mainnet, worsened by the fact that these blocks at the edges of tenures often hit the "split-brain" responses from signers, where depending on the timing of the Bitcoin block for each signer, some will approve the last block from the previous tenure, and some will approve the first block from this tenure. This creates a scenario where we're more likely to get stuck in limbo, where, due to unresponsive signers, one or both of these blocks is unable to reach 30% rejections or 70% approvals. In the case we saw, the last block of tenure N-1 got approved and the first block of tenure N was stuck with <30% rejections and <70% approvals, so we just had a stall for the rest of the tenure.

The solution in #5284 would have worked well for that case, but would still have a problem when the opposite happens -- when the last block of tenure N-1 is stuck in limbo. With #5284 in place, the miner for tenure N would wait unnecessarily for that block to get approved, when it could have successfully mined its own first block, ignoring that proposal.

I think the solution here works well in both cases.

This is needed for the case where the miner is knowingly reorging the
previous tenure, which is the canonical tip, but this reorg is allowed
because of the bad timing of blocks.
@obycode obycode requested a review from kantai February 5, 2025 16:20
Copy link
Member

@kantai kantai left a comment

Choose a reason for hiding this comment

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

Looks good to me, just a couple test comments.

@obycode obycode requested a review from kantai February 6, 2025 01:15
@obycode obycode added this to the 3.1.0.0.6 milestone Feb 6, 2025
@obycode obycode requested review from a team February 6, 2025 16:19
@obycode obycode requested review from hstove and jcnelson February 6, 2025 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants