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

Pass the correct burnchain tip to run_sign_v0 instead of the block election snapshot #5307

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

Conversation

jferrant
Copy link
Collaborator

@jferrant jferrant commented Oct 12, 2024

Tenure extends were not being properly handled as the burnchain tip was not actually being passed through the run_sign_v0 function. Rather it was using the block election snapshot because block.header.consensus_hash is the consensus hash of the election snapshot.

@jferrant jferrant requested a review from a team as a code owner October 12, 2024 03:28
@hstove hstove requested a review from a team as a code owner October 14, 2024 15:31
@jferrant jferrant changed the title Do not issue a BurnchainTipChanged error unless there is a new sortition Pass the correct burnchain tip to run_sign_v0 instead of the block election snapshot Oct 15, 2024
… into fix/tenure-extend-burnchain-tip-check
@jferrant jferrant requested a review from hstove October 16, 2024 16:23
hstove
hstove previously approved these changes Oct 16, 2024
Copy link
Contributor

@hstove hstove left a comment

Choose a reason for hiding this comment

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

Ah makes sense, LGTM!

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 great! Just noticed one misleading comment.

testnet/stacks-node/src/tests/nakamoto_integrations.rs Outdated Show resolved Hide resolved
Signed-off-by: Jacinta Ferrant <[email protected]>
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.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants