-
Notifications
You must be signed in to change notification settings - Fork 677
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: Shadow block recovery mechanism #5296
Conversation
…there can be a test function called shadow_load_tenure_info() for shadow tenures
… via a SIP in order to fix a chain stall. Update the validation and processing logic to handle shadow blocks, since they won't have corresponding sortitions.
…s, and add a "Shadow" obtain method.
…reward (since there won't be any burns for it)
…t fetch them again. This would make it so we'd "discover" shadow tenures
…lly first and advance the tenure downloader state machine from that
…entory bitvec (required for the downloader to work)
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.
Looking good!
N.B. This PR needs to be followed by (or extend with) a PR to re-mine a sequence of blocks as shadow blocks, so we can fix a consensus bug on testnet. We'll need more elaborate chain replay logic in the future to preserve causal relationships between affected transactions if possible. At a minimum today, we can preserve the VRF proofs. |
…t build atop shadow blocks point to the coinbase tx
… assumed to have descended from a shadow block, and thus the current PoX anchor block
…minnig after a shadow tenure (in which case ATC-C forbids us from winning)
…vailable if their shadow tenure isn't mined yet, and validate a block's block-commit against a parent shadow block's sortition (since we can't do that in LeaderBlockCommitOp::check_pox())
… that no blocks ever conflict with a shadow block
…s in ATC-C, and add functionality for mining shadow tenures
Okay, there's definitely some latent bug here in the test harness that's causing a failure. I think it may be the same one discovered by @hugocaillard in #4997. |
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.
Overall LGTM! I have some questions and a few places where I think a small change might be appropriate.
@jcnelson I responded to the one unresolved comment - once that's fixed I think this is good to merge! |
Thanks @hstove! Replied. |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
This introduces the notion of "shadow blocks" and "shadow tenures" as a means of recovering from a chain stall. In the event of an irrecoverable chain stall, we'd pass an emergency SIP to declare that one or more empty sortitions actually commit to a "shadow block" that contains whatever state is needed to get the chain un-stuck. All nodes would ship with copies of these shadow blocks, and would process them the same as any other kind of block (except that they could not be mined, uploaded, or downloaded).
Leaving as a draft for now until I can expand test coverage. This touches a lot of consensus-critical code paths.
Here is a summary of consensus changes for shadow blocks:
version
whose highest bit is set.parent_vtxindex
to 0, which is the burnchain coinbase transaction. We assume that shadow blocks all descend from their respective PoX anchor blocks, so this will not be checked for the first child tenure. Instead, the block's parent sortition will be checked to verify that it is a shadow tenure and that the block-commitparent_vtxindex
is 0 when validating the child block.