-
Notifications
You must be signed in to change notification settings - Fork 37.7k
policy: Treat taproot as always active #23512
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
Conversation
fa4e1d9
to
76a903b
Compare
76a903b
to
fa49d61
Compare
Concept ACK |
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
fa49d61
to
fa3e0da
Compare
@achow101 Sorry, I accidentally deleted your comment. It was a comment on the docstring of
Thanks, I've force pushed a new docstring. |
Code Review ACK fa3e0da |
ACK fa3e0da |
utACK fa3e0da |
ACK fa3e0da |
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.
code review + tACK fa3e0da
Code Review + tACK fa3e0da |
fa3e0da policy: Treat taproot as always active (MarcoFalke) Pull request description: Now that taproot is active, it can be treated as if it was always active for policy for the next major release. This simplifies the code and changes two things: * Importing `tr` descriptors can be done before the chain is fully synced. This is fine, because the wallet will already generate `tr` descriptors by default (regardless of the taproot status) after commit 47fe744. * Valid taproot spends won't be rejected from the mempool before taproot is active. This is strictly speaking a bugfix after commit 47fe744, since the wallet may generate taproot spends before the chain is fully synced. For example, a slow node or a purposefully offline node. Currently, the wallet needs the mempool to account for change. See bitcoin#11887. A similar change was done for segwit v0 in bitcoin#13120 . This effectively reverts commit c5ec036. ACKs for top commit: mjdietzx: Code Review ACK fa3e0da achow101: ACK fa3e0da sipa: utACK fa3e0da gruve-p: ACK bitcoin@fa3e0da gunar: Code Review + tACK fa3e0da rajarshimaitra: code review + tACK bitcoin@fa3e0da Tree-SHA512: c6dc7a4e6c345bdec33f256847dc63906ab1696aa683ab9b32a79e715613950884ac3a1a7a44e95f31bb28e58dd64679a616175f7e152b21f5550f3337c8e622
40e871d [miner] always assume we can create witness blocks (glozow) Pull request description: Given the low possibility of a reorg reverting the segwit soft fork, there is no longer a need to check whether segwit is active to see if it's okay to add to the block template (see also #23512, #21009, etc). `TestBlockValidity()` is also run on the block template at the end of `CreateNewBlock()`, so any invalid block would be caught there. ACKs for top commit: gruve-p: ACK 40e871d jnewbery: utACK 40e871d, although I disagree about changing the test for segwit transaction in mempool before activagtion, instead of just removing it: #24421 (comment). achow101: ACK 40e871d theStack: Code-review ACK 40e871d Tree-SHA512: bf4860bf2bed8339622d05228d11d60286edb0c32a9a3c434b8d154913c07ea56e50649f4af7009c2a1c6a58a81d2299ab43b41a6f16dee7d08cc89cc1603019
Now that taproot is active, it can be treated as if it was always active for policy for the next major release. This simplifies the code and changes two things:
tr
descriptors can be done before the chain is fully synced. This is fine, because the wallet will already generatetr
descriptors by default (regardless of the taproot status) after commit 47fe744.A similar change was done for segwit v0 in #13120 .
This effectively reverts commit c5ec036.