Merge 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#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@e48826a
  MarcoFalke:
    re-ACK e48826a 🥈

Tree-SHA512: c846ba64436d36f8180046ad551d8b0d9e20509b9bc185aa2639055fc28803dd8ec2d6771ab337e80da0b40009ad959590d5772f84a0bf6199b65190d4155bed

src/Makefile.am

@@ -169,6 +169,8 @@ BITCOIN_CORE_H = \
   cuckoocache.h \
   ctpl_stl.h \
   cxxtimer.hpp \
+  deploymentinfo.h \
+  deploymentstatus.h \
   evo/assetlocktx.h \
   evo/dmn_types.h \
   evo/cbtx.h \
@@ -349,7 +351,6 @@ BITCOIN_CORE_H = \
   validation.h \
   validationinterface.h \
   versionbits.h \
-  versionbitsinfo.h \
   walletinitinterface.h \
   wallet/bdb.h \
   wallet/coincontrol.h \
@@ -403,6 +404,7 @@ libbitcoin_server_a_SOURCES = \
   coinjoin/server.cpp \
   consensus/tx_verify.cpp \
   dbwrapper.cpp \
+  deploymentstatus.cpp \
   dsnotificationinterface.cpp \
   evo/assetlocktx.cpp \
   evo/cbtx.cpp \
@@ -698,6 +700,7 @@ libbitcoin_common_a_SOURCES = \
   compressor.cpp \
   core_read.cpp \
   core_write.cpp \
+  deploymentinfo.cpp \
   key.cpp \
   key_io.cpp \
   merkleblock.cpp \
@@ -717,7 +720,6 @@ libbitcoin_common_a_SOURCES = \
   script/sign.cpp \
   script/signingprovider.cpp \
   script/standard.cpp \
-  versionbitsinfo.cpp \
   warnings.cpp \
   $(BITCOIN_CORE_H)
 

src/chainparams.cpp

@@ -8,12 +8,12 @@
 
 #include <chainparamsseeds.h>
 #include <consensus/merkle.h>
+#include <deploymentinfo.h>
 #include <llmq/params.h>
 #include <util/ranges.h>
 #include <util/system.h>
 #include <util/underlying.h>
 #include <versionbits.h>
-#include <versionbitsinfo.h>
 
 #include <arith_uint256.h>
 

src/consensus/params.h

@@ -14,13 +14,32 @@
 
 namespace Consensus {
 
-enum DeploymentPos {
+enum BuriedDeployment : int16_t
+{
+    DEPLOYMENT_HEIGHTINCB = std::numeric_limits<int16_t>::min(),
+    DEPLOYMENT_DERSIG,
+    DEPLOYMENT_CLTV,
+    DEPLOYMENT_BIP147,
+    DEPLOYMENT_CSV,
+    DEPLOYMENT_DIP0001,
+    DEPLOYMENT_DIP0003,
+    DEPLOYMENT_DIP0008,
+    DEPLOYMENT_DIP0020,
+    DEPLOYMENT_DIP0024,
+    DEPLOYMENT_BRR,
+    DEPLOYMENT_V19,
+};
+constexpr bool ValidDeployment(BuriedDeployment dep) { return DEPLOYMENT_HEIGHTINCB <= dep && dep <= DEPLOYMENT_V19; }
+
+enum DeploymentPos : uint16_t
+{
     DEPLOYMENT_TESTDUMMY,
     DEPLOYMENT_V20,     // Deployment of EHF, LLMQ Randomness Beacon
     DEPLOYMENT_MN_RR,   // Deployment of Masternode Reward Location Reallocation
-    // 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_MN_RR; }
 
 /**
  * Struct for each individual consensus rule change using BIP9.
@@ -144,7 +163,39 @@ struct Params {
     LLMQType llmqTypePlatform{LLMQType::LLMQ_NONE};
     LLMQType llmqTypeMnhf{LLMQType::LLMQ_NONE};
     LLMQType llmqTypeAssetLocks{LLMQType::LLMQ_NONE};
+
+    int DeploymentHeight(BuriedDeployment dep) const
+    {
+        switch (dep) {
+        case DEPLOYMENT_HEIGHTINCB:
+            return BIP34Height;
+        case DEPLOYMENT_DERSIG:
+            return BIP66Height;
+        case DEPLOYMENT_CLTV:
+            return BIP65Height;
+        case DEPLOYMENT_BIP147:
+            return BIP147Height;
+        case DEPLOYMENT_CSV:
+            return CSVHeight;
+        case DEPLOYMENT_DIP0001:
+            return DIP0001Height;
+        case DEPLOYMENT_DIP0003:
+            return DIP0003Height;
+        case DEPLOYMENT_DIP0008:
+            return DIP0008Height;
+        case DEPLOYMENT_DIP0020:
+            return DIP0020Height;
+        case DEPLOYMENT_DIP0024:
+            return DIP0024Height;
+        case DEPLOYMENT_BRR:
+            return BRRHeight;
+        case DEPLOYMENT_V19:
+            return V19Height;
+        } // 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,54 @@
+// 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 =*/"v20",
+        /*.gbt_force =*/true,
+    },
+    {
+        /*.name =*/"mn_rr",
+        /*.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_BIP147:
+        return "bip147";
+    case Consensus::DEPLOYMENT_CSV:
+        return "csv";
+    case Consensus::DEPLOYMENT_DIP0001:
+        return "dip0001";
+    case Consensus::DEPLOYMENT_DIP0003:
+        return "dip0003";
+    case Consensus::DEPLOYMENT_DIP0008:
+        return "dip0008";
+    case Consensus::DEPLOYMENT_DIP0020:
+        return "dip0020";
+    case Consensus::DEPLOYMENT_DIP0024:
+        return "dip0024";
+    case Consensus::DEPLOYMENT_BRR:
+        return "realloc";
+    case Consensus::DEPLOYMENT_V19:
+        return "v19";
+    } // 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/evo/mnhftx.cpp

@@ -3,6 +3,7 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <consensus/validation.h>
+#include <deploymentstatus.h>
 #include <evo/mnhftx.h>
 #include <evo/specialtx.h>
 #include <llmq/commitment.h>
@@ -278,10 +279,12 @@ CMNHFManager::Signals CMNHFManager::GetFromCache(const CBlockIndex* const pindex
             return signals;
         }
     }
-    if (VersionBitsState(pindex->pprev, Params().GetConsensus(), Consensus::DEPLOYMENT_V20, versionbitscache) != ThresholdState::ACTIVE) {
+    {
         LOCK(cs_cache);
-        mnhfCache.insert(blockHash, {});
-        return {};
+        if (ThresholdState::ACTIVE != v20_activation.State(pindex->pprev, Params().GetConsensus(), Consensus::DEPLOYMENT_V20)) {
+            mnhfCache.insert(blockHash, {});
+            return {};
+        }
     }
     bool ok = m_evoDb.Read(std::make_pair(DB_SIGNALS, blockHash), signals);
     assert(ok);
@@ -297,8 +300,10 @@ void CMNHFManager::AddToCache(const Signals& signals, const CBlockIndex* const p
         LOCK(cs_cache);
         mnhfCache.insert(blockHash, signals);
     }
-    if (VersionBitsState(pindex->pprev, Params().GetConsensus(), Consensus::DEPLOYMENT_V20, versionbitscache) != ThresholdState::ACTIVE) {
-        return;
+    assert(pindex != nullptr);
+    {
+        LOCK(cs_cache);
+        if (ThresholdState::ACTIVE != v20_activation.State(pindex->pprev, Params().GetConsensus(), Consensus::DEPLOYMENT_V20)) return;
     }
     m_evoDb.Write(std::make_pair(DB_SIGNALS, blockHash), signals);
 }

src/evo/mnhftx.h

@@ -102,6 +102,8 @@ class CMNHFManager : public AbstractEHFManager
     // versionBit <-> height
     unordered_lru_cache<uint256, Signals, StaticSaltedHasher> mnhfCache GUARDED_BY(cs_cache) {MNHFCacheSize};
 
+    // This cache is used only for v20 activation to avoid double lock throught VersionBitsConditionChecker::SignalHeight
+    VersionBitsCache v20_activation GUARDED_BY(cs_cache);
 public:
     explicit CMNHFManager(CEvoDB& evoDb);
     ~CMNHFManager();

src/governance/classes.cpp

@@ -17,6 +17,7 @@
 #include <util/time.h>
 #include <util/underlying.h>
 #include <validation.h>
+#include <versionbits.h>
 
 #include <univalue.h>
 

src/llmq/utils.cpp

@@ -694,27 +694,27 @@ bool IsV19Active(gsl::not_null<const CBlockIndex*> pindex)
 bool IsV20Active(gsl::not_null<const CBlockIndex*> pindex)
 {
     LOCK(cs_llmq_vbc);
-    return VersionBitsState(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V20, llmq_versionbitscache) == ThresholdState::ACTIVE;
+    return llmq_versionbitscache.State(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V20) == ThresholdState::ACTIVE;
 }
 
 bool IsMNRewardReallocationActive(gsl::not_null<const CBlockIndex*> pindex)
 {
     if (!IsV20Active(pindex)) return false;
 
     LOCK(cs_llmq_vbc);
-    return VersionBitsState(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_MN_RR, llmq_versionbitscache) == ThresholdState::ACTIVE;
+    return llmq_versionbitscache.State(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_MN_RR) == ThresholdState::ACTIVE;
 }
 
 ThresholdState GetV20State(gsl::not_null<const CBlockIndex*> pindex)
 {
     LOCK(cs_llmq_vbc);
-    return VersionBitsState(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V20, llmq_versionbitscache);
+    return llmq_versionbitscache.State(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V20);
 }
 
 int GetV20Since(gsl::not_null<const CBlockIndex*> pindex)
 {
     LOCK(cs_llmq_vbc);
-    return VersionBitsStateSinceHeight(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V20, llmq_versionbitscache);
+    return llmq_versionbitscache.StateSinceHeight(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V20);
 }
 
 uint256 DeterministicOutboundConnection(const uint256& proTxHash1, const uint256& proTxHash2)
@@ -981,7 +981,7 @@ bool IsQuorumTypeEnabledInternal(Consensus::LLMQType llmqType, const CQuorumMana
 
         case Consensus::LLMQType::LLMQ_TEST_V17: {
             LOCK(cs_llmq_vbc);
-            return VersionBitsState(pindex, consensusParams, Consensus::DEPLOYMENT_TESTDUMMY, llmq_versionbitscache) == ThresholdState::ACTIVE;
+            return llmq_versionbitscache.State(pindex, consensusParams, Consensus::DEPLOYMENT_TESTDUMMY) == ThresholdState::ACTIVE;
         }
         case Consensus::LLMQType::LLMQ_100_67:
             return pindex->nHeight + 1 >= consensusParams.DIP0020Height;