Merge bitcoin/bitcoin#19438: Introduce deploymentstatus

e48826a tests: remove ComputeBlockVersion shortcut from versionbits tests (Anthony Towns)
c5f3672 [refactor] Move ComputeBlockVersion into VersionBitsCache (Anthony Towns)
4a69b4d [move-only] Move ComputeBlockVersion from validation to versionbits (Anthony Towns)
0cfd6c6 [refactor] versionbits: make VersionBitsCache a full class (Anthony Towns)
8ee3e0b [refactor] rpc/blockchain.cpp: SoftForkPushBack (Anthony Towns)
92f48f3 deploymentinfo: Add DeploymentName() (Anthony Towns)
ea68b3a [move-only] Rename versionbitsinfo to deploymentinfo (Anthony Towns)
c64b2c6 scripted-diff: rename versionbitscache (Anthony Towns)
de55304 [refactor] Add versionbits deployments to deploymentstatus.h (Anthony Towns)
2b0d291 [refactor] Add deploymentstatus.h (Anthony Towns)
eccd736 versionbits: Use dedicated lock instead of cs_main (Anthony Towns)
36a4ba0 versionbits: correct doxygen comments (Anthony Towns)

Pull request description:

  Introduces helper functions to make it easy to bury future deployments, along the lines of the suggestion from [11398](bitcoin/bitcoin#11398 (comment)) "I would prefer it if a buried deployment wouldn't require all code paths that check the BIP9 status to require changing".

  This provides three functions: `DeploymentEnabled()` which tests if a deployment can ever be active, `DeploymentActiveAt()` which checks if a deployment should be enforced in the given block, and `DeploymentActiveAfter()` which checks if a deployment should be enforced in the block following the given block, and overloads all three to work both with buried deployments and versionbits deployments.

  This adds a dedicated lock for the versionbits cache, which is acquired internally by the versionbits functions, rather than relying on `cs_main`. It also moves moves versionbitscache into deploymentstatus to avoid a circular dependency with validation.

ACKs for top commit:
  jnewbery:
    ACK e48826a
  gruve-p:
    ACK bitcoin/bitcoin@e48826a
  MarcoFalke:
    re-ACK e48826a 🥈

Tree-SHA512: c846ba64436d36f8180046ad551d8b0d9e20509b9bc185aa2639055fc28803dd8ec2d6771ab337e80da0b40009ad959590d5772f84a0bf6199b65190d4155bed

Author: MarcoFalke

src/Makefile.am

@@ -145,6 +145,8 @@ BITCOIN_CORE_H = \
   core_memusage.h \
   cuckoocache.h \
   dbwrapper.h \
+  deploymentinfo.h \
+  deploymentstatus.h \
   external_signer.h \
   flatfile.h \
   fs.h \
@@ -272,7 +274,6 @@ BITCOIN_CORE_H = \
   validation.h \
   validationinterface.h \
   versionbits.h \
-  versionbitsinfo.h \
   wallet/bdb.h \
   wallet/coincontrol.h \
   wallet/coinselection.h \
@@ -328,6 +329,7 @@ libbitcoin_server_a_SOURCES = \
   chain.cpp \
   consensus/tx_verify.cpp \
   dbwrapper.cpp \
+  deploymentstatus.cpp \
   flatfile.cpp \
   httprpc.cpp \
   httpserver.cpp \
@@ -540,6 +542,7 @@ libbitcoin_common_a_SOURCES = \
   compressor.cpp \
   core_read.cpp \
   core_write.cpp \
+  deploymentinfo.cpp \
   external_signer.cpp \
   init/common.cpp \
   key.cpp \
@@ -561,7 +564,6 @@ libbitcoin_common_a_SOURCES = \
   script/sign.cpp \
   script/signingprovider.cpp \
   script/standard.cpp \
-  versionbitsinfo.cpp \
   warnings.cpp \
   $(BITCOIN_CORE_H)
 

src/chainparams.cpp

@@ -7,9 +7,9 @@
 
 #include <chainparamsseeds.h>
 #include <consensus/merkle.h>
+#include <deploymentinfo.h>
 #include <hash.h> // for signet block challenge hash
 #include <util/system.h>
-#include <versionbitsinfo.h>
 
 #include <assert.h>
 

src/consensus/params.h

@@ -11,13 +11,25 @@
 
 namespace Consensus {
 
-enum DeploymentPos
+enum BuriedDeployment : int16_t
+{
+    // buried deployments get negative values to avoid overlap with DeploymentPos
+    DEPLOYMENT_HEIGHTINCB = std::numeric_limits<int16_t>::min(),
+    DEPLOYMENT_CLTV,
+    DEPLOYMENT_DERSIG,
+    DEPLOYMENT_CSV,
+    DEPLOYMENT_SEGWIT,
+};
+constexpr bool ValidDeployment(BuriedDeployment dep) { return DEPLOYMENT_HEIGHTINCB <= dep && dep <= DEPLOYMENT_SEGWIT; }
+
+enum DeploymentPos : uint16_t
 {
     DEPLOYMENT_TESTDUMMY,
     DEPLOYMENT_TAPROOT, // Deployment of Schnorr/Taproot (BIPs 340-342)
-    // NOTE: Also add new deployments to VersionBitsDeploymentInfo in versionbits.cpp
+    // NOTE: Also add new deployments to VersionBitsDeploymentInfo in deploymentinfo.cpp
     MAX_VERSION_BITS_DEPLOYMENTS
 };
+constexpr bool ValidDeployment(DeploymentPos dep) { return DEPLOYMENT_TESTDUMMY <= dep && dep <= DEPLOYMENT_TAPROOT; }
 
 /**
  * Struct for each individual consensus rule change using BIP9.
@@ -100,7 +112,25 @@ struct Params {
      */
     bool signet_blocks{false};
     std::vector<uint8_t> signet_challenge;
+
+    int DeploymentHeight(BuriedDeployment dep) const
+    {
+        switch (dep) {
+        case DEPLOYMENT_HEIGHTINCB:
+            return BIP34Height;
+        case DEPLOYMENT_CLTV:
+            return BIP65Height;
+        case DEPLOYMENT_DERSIG:
+            return BIP66Height;
+        case DEPLOYMENT_CSV:
+            return CSVHeight;
+        case DEPLOYMENT_SEGWIT:
+            return SegwitHeight;
+        } // no default case, so the compiler can warn about missing cases
+        return std::numeric_limits<int>::max();
+    }
 };
+
 } // namespace Consensus
 
 #endif // BITCOIN_CONSENSUS_PARAMS_H

src/deploymentinfo.cpp

@@ -0,0 +1,36 @@
+// Copyright (c) 2016-2020 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <deploymentinfo.h>
+
+#include <consensus/params.h>
+
+const struct VBDeploymentInfo VersionBitsDeploymentInfo[Consensus::MAX_VERSION_BITS_DEPLOYMENTS] = {
+    {
+        /*.name =*/ "testdummy",
+        /*.gbt_force =*/ true,
+    },
+    {
+        /*.name =*/ "taproot",
+        /*.gbt_force =*/ true,
+    },
+};
+
+std::string DeploymentName(Consensus::BuriedDeployment dep)
+{
+    assert(ValidDeployment(dep));
+    switch (dep) {
+    case Consensus::DEPLOYMENT_HEIGHTINCB:
+        return "bip34";
+    case Consensus::DEPLOYMENT_CLTV:
+        return "bip65";
+    case Consensus::DEPLOYMENT_DERSIG:
+        return "bip66";
+    case Consensus::DEPLOYMENT_CSV:
+        return "csv";
+    case Consensus::DEPLOYMENT_SEGWIT:
+        return "segwit";
+    } // no default case, so the compiler can warn about missing cases
+    return "";
+}

src/deploymentinfo.h

@@ -0,0 +1,29 @@
+// Copyright (c) 2016-2018 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_DEPLOYMENTINFO_H
+#define BITCOIN_DEPLOYMENTINFO_H
+
+#include <consensus/params.h>
+
+#include <string>
+
+struct VBDeploymentInfo {
+    /** Deployment name */
+    const char *name;
+    /** Whether GBT clients can safely ignore this rule in simplified usage */
+    bool gbt_force;
+};
+
+extern const VBDeploymentInfo VersionBitsDeploymentInfo[Consensus::MAX_VERSION_BITS_DEPLOYMENTS];
+
+std::string DeploymentName(Consensus::BuriedDeployment dep);
+
+inline std::string DeploymentName(Consensus::DeploymentPos pos)
+{
+    assert(Consensus::ValidDeployment(pos));
+    return VersionBitsDeploymentInfo[pos].name;
+}
+
+#endif // BITCOIN_DEPLOYMENTINFO_H

src/deploymentstatus.cpp

@@ -0,0 +1,17 @@
+// Copyright (c) 2020 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <deploymentstatus.h>
+
+#include <consensus/params.h>
+#include <versionbits.h>
+
+VersionBitsCache g_versionbitscache;
+
+/* Basic sanity checking for BuriedDeployment/DeploymentPos enums and
+ * ValidDeployment check */
+
+static_assert(ValidDeployment(Consensus::DEPLOYMENT_TESTDUMMY), "sanity check of DeploymentPos failed (TESTDUMMY not valid)");
+static_assert(!ValidDeployment(Consensus::MAX_VERSION_BITS_DEPLOYMENTS), "sanity check of DeploymentPos failed (MAX value considered valid)");
+static_assert(!ValidDeployment(static_cast<Consensus::BuriedDeployment>(Consensus::DEPLOYMENT_TESTDUMMY)), "sanity check of BuriedDeployment failed (overlaps with DeploymentPos)");

src/deploymentstatus.h

@@ -0,0 +1,55 @@
+// Copyright (c) 2020 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_DEPLOYMENTSTATUS_H
+#define BITCOIN_DEPLOYMENTSTATUS_H
+
+#include <chain.h>
+#include <versionbits.h>
+
+#include <limits>
+
+/** Global cache for versionbits deployment status */
+extern VersionBitsCache g_versionbitscache;
+
+/** Determine if a deployment is active for the next block */
+inline bool DeploymentActiveAfter(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::BuriedDeployment dep)
+{
+    assert(Consensus::ValidDeployment(dep));
+    return (pindexPrev == nullptr ? 0 : pindexPrev->nHeight + 1) >= params.DeploymentHeight(dep);
+}
+
+inline bool DeploymentActiveAfter(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos dep)
+{
+    assert(Consensus::ValidDeployment(dep));
+    return ThresholdState::ACTIVE == g_versionbitscache.State(pindexPrev, params, dep);
+}
+
+/** Determine if a deployment is active for this block */
+inline bool DeploymentActiveAt(const CBlockIndex& index, const Consensus::Params& params, Consensus::BuriedDeployment dep)
+{
+    assert(Consensus::ValidDeployment(dep));
+    return index.nHeight >= params.DeploymentHeight(dep);
+}
+
+inline bool DeploymentActiveAt(const CBlockIndex& index, const Consensus::Params& params, Consensus::DeploymentPos dep)
+{
+    assert(Consensus::ValidDeployment(dep));
+    return DeploymentActiveAfter(index.pprev, params, dep);
+}
+
+/** Determine if a deployment is enabled (can ever be active) */
+inline bool DeploymentEnabled(const Consensus::Params& params, Consensus::BuriedDeployment dep)
+{
+    assert(Consensus::ValidDeployment(dep));
+    return params.DeploymentHeight(dep) != std::numeric_limits<int>::max();
+}
+
+inline bool DeploymentEnabled(const Consensus::Params& params, Consensus::DeploymentPos dep)
+{
+    assert(Consensus::ValidDeployment(dep));
+    return params.vDeployments[dep].nTimeout != 0;
+}
+
+#endif // BITCOIN_DEPLOYMENTSTATUS_H

src/init.cpp

@@ -16,6 +16,7 @@
 #include <chain.h>
 #include <chainparams.h>
 #include <compat/sanity.h>
+#include <deploymentstatus.h>
 #include <fs.h>
 #include <hash.h>
 #include <httprpc.h>
@@ -1587,7 +1588,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
         }
     }
 
-    if (chainparams.GetConsensus().SegwitHeight != std::numeric_limits<int>::max()) {
+    if (DeploymentEnabled(chainparams.GetConsensus(), Consensus::DEPLOYMENT_SEGWIT)) {
         // Advertise witness capabilities.
         // The option to not set NODE_WITNESS is only used in the tests and should be removed.
         nLocalServices = ServiceFlags(nLocalServices | NODE_WITNESS);

src/miner.cpp

@@ -13,6 +13,7 @@
 #include <consensus/merkle.h>
 #include <consensus/tx_verify.h>
 #include <consensus/validation.h>
+#include <deploymentstatus.h>
 #include <policy/feerate.h>
 #include <policy/policy.h>
 #include <pow.h>
@@ -120,7 +121,7 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
     assert(pindexPrev != nullptr);
     nHeight = pindexPrev->nHeight + 1;
 
-    pblock->nVersion = ComputeBlockVersion(pindexPrev, chainparams.GetConsensus());
+    pblock->nVersion = g_versionbitscache.ComputeBlockVersion(pindexPrev, chainparams.GetConsensus());
     // -regtest only: allow overriding block.nVersion with
     // -blockversion=N to test forking scenarios
     if (chainparams.MineBlocksOnDemand())
@@ -137,12 +138,12 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
     // This is only needed in case the witness softfork activation is reverted
     // (which would require a very deep reorganization).
     // Note that the mempool would accept transactions with witness data before
-    // IsWitnessEnabled, but we would only ever mine blocks after IsWitnessEnabled
+    // the deployment is active, but we would only ever mine blocks after activation
     // unless there is a massive block reorganization with the witness softfork
     // not activated.
     // TODO: replace this with a call to main to assess validity of a mempool
     // transaction (which in most cases can be a no-op).
-    fIncludeWitness = IsWitnessEnabled(pindexPrev, chainparams.GetConsensus());
+    fIncludeWitness = DeploymentActiveAfter(pindexPrev, chainparams.GetConsensus(), Consensus::DEPLOYMENT_SEGWIT);
 
     int nPackagesSelected = 0;
     int nDescendantsUpdated = 0;

src/net_processing.cpp

@@ -11,6 +11,7 @@
 #include <blockfilter.h>
 #include <chainparams.h>
 #include <consensus/validation.h>
+#include <deploymentstatus.h>
 #include <hash.h>
 #include <index/blockfilterindex.h>
 #include <merkleblock.h>
@@ -997,7 +998,7 @@ void PeerManagerImpl::FindNextBlocksToDownload(NodeId nodeid, unsigned int count
                 // We consider the chain that this peer is on invalid.
                 return;
             }
-            if (!State(nodeid)->fHaveWitness && IsWitnessEnabled(pindex->pprev, consensusParams)) {
+            if (!State(nodeid)->fHaveWitness && DeploymentActiveAt(*pindex, consensusParams, Consensus::DEPLOYMENT_SEGWIT)) {
                 // We wouldn't download this block or its descendants from this peer.
                 return;
             }
@@ -1467,7 +1468,7 @@ void PeerManagerImpl::NewPoWValidBlock(const CBlockIndex *pindex, const std::sha
         return;
     nHighestFastAnnounce = pindex->nHeight;
 
-    bool fWitnessEnabled = IsWitnessEnabled(pindex->pprev, m_chainparams.GetConsensus());
+    bool fWitnessEnabled = DeploymentActiveAt(*pindex, m_chainparams.GetConsensus(), Consensus::DEPLOYMENT_SEGWIT);
     uint256 hashBlock(pblock->GetHash());
 
     {
@@ -2082,7 +2083,7 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, const Peer& peer,
             while (pindexWalk && !m_chainman.ActiveChain().Contains(pindexWalk) && vToFetch.size() <= MAX_BLOCKS_IN_TRANSIT_PER_PEER) {
                 if (!(pindexWalk->nStatus & BLOCK_HAVE_DATA) &&
                         !IsBlockRequested(pindexWalk->GetBlockHash()) &&
-                        (!IsWitnessEnabled(pindexWalk->pprev, m_chainparams.GetConsensus()) || State(pfrom.GetId())->fHaveWitness)) {
+                        (!DeploymentActiveAt(*pindexWalk, m_chainparams.GetConsensus(), Consensus::DEPLOYMENT_SEGWIT) || State(pfrom.GetId())->fHaveWitness)) {
                     // We don't have this block, and it's not yet in flight.
                     vToFetch.push_back(pindexWalk);
                 }
@@ -3397,7 +3398,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
             return;
         }
 
-        if (IsWitnessEnabled(pindex->pprev, m_chainparams.GetConsensus()) && !nodestate->fSupportsDesiredCmpctVersion) {
+        if (DeploymentActiveAt(*pindex, m_chainparams.GetConsensus(), Consensus::DEPLOYMENT_SEGWIT) && !nodestate->fSupportsDesiredCmpctVersion) {
             // Don't bother trying to process compact blocks from v1 peers
             // after segwit activates.
             return;

src/node/interfaces.cpp

@@ -6,6 +6,7 @@
 #include <banman.h>
 #include <chain.h>
 #include <chainparams.h>
+#include <deploymentstatus.h>
 #include <external_signer.h>
 #include <init.h>
 #include <interfaces/chain.h>
@@ -692,7 +693,7 @@ class ChainImpl : public Chain
     {
         LOCK(::cs_main);
         const CBlockIndex* tip = Assert(m_node.chainman)->ActiveChain().Tip();
-        return VersionBitsState(tip, Params().GetConsensus(), Consensus::DEPLOYMENT_TAPROOT, versionbitscache) == ThresholdState::ACTIVE;
+        return DeploymentActiveAfter(tip, Params().GetConsensus(), Consensus::DEPLOYMENT_TAPROOT);
     }
     NodeContext& m_node;
 };