Remove Taproot activation height #26201

1. 
   Sjors commented at 11:16 am on September 29, 2022: member

   Drop DEPLOYMENT_TAPROOT from consensus.vDeployments.

   Now that the commit (https://github.com/bitcoin/bitcoin/commit/7c08d81e119570792648fe95bbacddbb1d5f9ae2) which changes taproot to be enforced for all blocks ~is sufficiently buried by other commits, and thus less likely to be reverted~ 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 getdeploymentinfo RPC, rather than mark it as buried, 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.

2. 
   DrahtBot commented at 2:34 pm on September 30, 2022: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage

   For detailed information about the code coverage, see the test coverage report.

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #27433 (getblocktemplate improvements for segwit and sigops by Sjors)
   • #27427 (validation: Replace MinBIP9WarningHeight with MinBIP9WarningStartTime by dimitaracev)

   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.

3. 
   luke-jr commented at 2:04 pm on October 2, 2022: member

   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.
4. 
   Sjors commented at 9:22 am on October 3, 2022: member

   @luke-jr changed the wording to “is (probably) included in v24.0”.
5. 
   ajtowns commented at 3:47 am on October 8, 2022: contributor

   This should be updating MinBIP9WarningHeight above the taproot activation height (otherwise you should see "warnings": "Unknown new rules activated (versionbit 2)" due to miner signalling)
6. Sjors force-pushed on Nov 1, 2022
7. 
   Sjors commented at 9:41 am on November 1, 2022: member

   @ajtowns done
8. in src/consensus/params.h:99 in 8f96e52405 outdated

    95@@ -94,7 +96,7 @@ struct Params {
    96      * BIP 16 exception blocks. */
    97     int SegwitHeight;
    98     /** Don't warn about unknown BIP 9 activations below this height.
    99-     * This prevents us from warning about the CSV and segwit activations. */
   100+     * This prevents us from warning about the CSV,segwit and taproot activations. */
   

   ---

   ---

   

   maflcko commented at 9:48 am on November 1, 2022:

   0     * This prevents us from warning about the CSV, segwit and taproot activations. */
   

   nit

9. in doc/release-notes-26201.md:4 in 8f96e52405 outdated

   0@@ -0,0 +1,5 @@
   1+RPC
   2+---
   3+
   4+- 'taproot' has been removed from 'getdeploymentinfo', rather than marked as buried,
   

   ---

   ---

   

   maflcko commented at 9:49 am on November 1, 2022:

   0- 'taproot' has been removed from 'getdeploymentinfo',
   

   nit: I think release notes should describe changes, not provide extended discussion about alternatives

10. Sjors force-pushed on Nov 1, 2022
11. Sjors force-pushed on Nov 1, 2022
12. DrahtBot added the label Needs rebase on Mar 16, 2023
13. 
    achow101 commented at 3:23 pm on April 25, 2023: member

    Are you still working on this?
14. 
    Sjors commented at 2:12 pm on May 8, 2023: member

    I’ll rebase soon(tm).
15. in src/deploymentinfo.cpp:19 in 20ef70f750 outdated

    10@@ -11,10 +11,6 @@ const struct VBDeploymentInfo VersionBitsDeploymentInfo[Consensus::MAX_VERSION_B
    11         /*.name =*/ "testdummy",
    12         /*.gbt_force =*/ true,
    13     },
    14-    {
    15-        /*.name =*/ "taproot",
    16-        /*.gbt_force =*/ true,
    17-    },
    

    ---

    ---

    

    luke-jr commented at 0:54 am on July 27, 2023:

    This will need aRules in rpc/mining.cpp updated
16. luke-jr changes_requested
17. Sjors force-pushed on Aug 18, 2023
18. Sjors force-pushed on Aug 18, 2023
19. in src/rpc/mining.cpp:866 in 7fb3287fd1 outdated

    860@@ -861,7 +861,10 @@ static RPCHelpMan getblocktemplate()
    861 
    862     UniValue aRules(UniValue::VARR);
    863     aRules.push_back("csv");
    864-    if (!fPreSegWit) aRules.push_back("!segwit");
    865+    if (!fPreSegWit) {
    866+        aRules.push_back("!segwit");
    867+        aRules.push_back("!taproot");
    

    ---

    ---

    

    Sjors commented at 10:30 am on August 18, 2023:

    @luke-jr like this?

    This will need aRules in rpc/mining.cpp updated

    ---

    

    Sjors commented at 10:50 am on August 18, 2023:

    Oh wait, without !

    I’ll push a fix and add a test…

    Before and after this change it should be:

    0bitcoin-cli getblocktemplate '{"rules": ["segwit"]}' | jq '.rules'
    1[
    2  "csv",
    3  "!segwit",
    4  "taproot"
    5]
    

20. DrahtBot removed the label Needs rebase on Aug 18, 2023
21. Sjors force-pushed on Aug 18, 2023
22. 
    Sjors commented at 11:12 am on August 18, 2023: member

    Rebased after kernel changes. Fixed a regression in getblocktemplate’s rules result, and added a test for tit.
23. in src/rpc/mining.cpp:889 in b5d8ddbcb9 outdated

    859@@ -860,8 +860,14 @@ static RPCHelpMan getblocktemplate()
    860     result.pushKV("capabilities", aCaps);
    861 
    862     UniValue aRules(UniValue::VARR);
    863+    // See getblocktemplate changes in BIP 9:
    864+    // ! indicates a more subtle change to the block structure or generation transaction
    865+    // Otherwise clients may assume the rule will not impact usage of the template as-is.
    866     aRules.push_back("csv");
    867-    if (!fPreSegWit) aRules.push_back("!segwit");
    868+    if (!fPreSegWit) {
    

    ---

    ---

    

    Sjors commented at 11:16 am on August 18, 2023:

    In #27433 I propose getting rid of fPreSegWit.
24. DrahtBot added the label CI failed on Sep 12, 2023
25. DrahtBot removed the label CI failed on Sep 13, 2023
26. DrahtBot added the label Needs rebase on Oct 9, 2023
27. 
    Sjors commented at 2:41 pm on October 9, 2023: member

    Rebased after #28591.
28. Sjors force-pushed on Oct 9, 2023
29. DrahtBot removed the label Needs rebase on Oct 9, 2023
30. in src/consensus/params.h:34 in db4344fa9c outdated

    32 };
    33 constexpr bool ValidDeployment(BuriedDeployment dep) { return dep <= DEPLOYMENT_SEGWIT; }
    34 
    35 enum DeploymentPos : uint16_t {
    36     DEPLOYMENT_TESTDUMMY,
    37-    DEPLOYMENT_TAPROOT, // Deployment of Schnorr/Taproot (BIPs 340-342)
    

    ---

    ---

    

    maflcko commented at 2:43 pm on October 20, 2023:

    Maybe add a note here that removing an entry may require bumping MinBIP9WarningHeight?

    ---

    

    Sjors commented at 7:38 am on October 23, 2023:

    Added. IIUC the WarningBitsConditionChecker uses ComputeBlockVersion (also used by the miner), which in turn uses VersionBitsConditionChecker, which uses vDeployments to determine if a softfork is STARTED or LOCKED_IN. If so it won’t issue the warning. This logic is rather convoluted…
31. Sjors force-pushed on Oct 23, 2023
32. DrahtBot added the label CI failed on Oct 23, 2023
33. DrahtBot removed the label CI failed on Oct 26, 2023
34. barrystyle referenced this in commit 6eb671c734 on Dec 16, 2023
35. Sjors force-pushed on Jan 9, 2024
36. 
    Sjors commented at 9:06 am on January 9, 2024: member

    Rebased because #27433 needed it.
37. DrahtBot added the label CI failed on Jan 14, 2024
38. Sjors force-pushed on Feb 13, 2024
39. DrahtBot removed the label CI failed on Feb 13, 2024
40. DrahtBot added the label Needs rebase on Mar 5, 2024
41. Sjors force-pushed on Mar 7, 2024
42. 
    Sjors commented at 10:36 am on March 7, 2024: member

    Rebased due to (trivial) merge conflict after #29547.

    I should also point out that Silent Payments makes use of the Taproot activation height (e.g. for a more efficient indexer), but I don’t think that’s a good reason to keep these consensus params around.

43. DrahtBot removed the label Needs rebase on Mar 7, 2024
44. Remove Taproot activation height

    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>

    46544c0539
45. Sjors force-pushed on May 30, 2024
46. DrahtBot added the label CI failed on May 30, 2024
47. DrahtBot removed the label CI failed on May 30, 2024

Contributors
Sjors DrahtBot luke-jr ajtowns maflcko achow101