Remove Taproot activation height #26201
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/26201. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. 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. |
|
Concept ~0, but I object the claim in general that commits "sufficiently buried by other commits, and thus less likely to be reverted". Git isn't PoW, and "burying" commits does not make them less likely to be reverted. |
|
@luke-jr changed the wording to "is (probably) included in v24.0". |
|
This should be updating |
b0f4391 to
8f96e52
Compare
|
@ajtowns done |
b8c2cc1 to
20ef70f
Compare
|
Are you still working on this? |
|
I'll rebase soon(tm). |
src/deploymentinfo.cpp
Outdated
| { | ||
| /*.name =*/ "taproot", | ||
| /*.gbt_force =*/ true, | ||
| }, |
There was a problem hiding this comment.
This will need aRules in rpc/mining.cpp updated
20687de to
7fb3287
Compare
src/rpc/mining.cpp
Outdated
| if (!fPreSegWit) aRules.push_back("!segwit"); | ||
| if (!fPreSegWit) { | ||
| aRules.push_back("!segwit"); | ||
| aRules.push_back("!taproot"); |
There was a problem hiding this comment.
@luke-jr like this?
This will need
aRulesinrpc/mining.cppupdated
There was a problem hiding this comment.
Oh wait, without !
I'll push a fix and add a test...
Before and after this change it should be:
bitcoin-cli getblocktemplate '{"rules": ["segwit"]}' | jq '.rules'
[
"csv",
"!segwit",
"taproot"
]
7fb3287 to
b5d8ddb
Compare
|
Rebased after kernel changes. Fixed a regression in |
| aRules.push_back("csv"); | ||
| if (!fPreSegWit) aRules.push_back("!segwit"); | ||
| if (!fPreSegWit) { 8000 |
There was a problem hiding this comment.
In #27433 I propose getting rid of fPreSegWit.
d7d6897 to
9fc563b
Compare
|
#27433 needed the rebase here |
9fc563b to
301e419
Compare
|
Rebased after #31978. |
301e419 to
f88b9cb
Compare
Drop DEPLOYMENT_TAPROOT from consensus.vDeployments.
Bump MinBIP9WarningHeight.
Clarify what is considered a BuriedDeployment and
drop taproot from getdeploymentinfo RPC.
Add a test to getblocktemplate to ensure the taproot
rule is still set.
Co-Authored-By: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
f88b9cb to
1e9c19e
Compare
|
Rebased after #29039. |
Drop
DEPLOYMENT_TAPROOTfromconsensus.vDeployments.Now that the commit (7c08d81) which changes taproot to be enforced for all blocks is included in v24.0, it seems a good time to remove no longer needed non-consensus code.
Clarify what is considered a
BuriedDeployment.We drop taproot from
getdeploymentinfoRPC, rather than mark it asburied, because this is not a buried deployment in the sense of BIP 90. This is because the activation height has been completely removed from the code, rather than hard coded.See discussion in #24737 and #26162.