Merge e3014017ba into c5e44a0435

src/consensus/params.h

@@ -8,6 +8,7 @@
 
 #include <uint256.h>
 
+#include <array>
 #include <chrono>
 #include <limits>
 #include <map>
@@ -52,6 +53,14 @@ struct BIP9Deployment {
      *  boundary.
      */
     int min_activation_height{0};
+    /** Period of blocks to check signalling in (usually retarget period, ie params.DifficultyAdjustmentInterval()) */
+    uint32_t period{2016};
+    /**
+     * Minimum blocks including miner confirmation of the total of 2016 blocks in a retargeting period,
+     * which is also used for BIP9 deployments.
+     * Examples: 1916 for 95%, 1512 for testchains.
+     */
+    uint32_t threshold{1916};
 
     /** Constant for nTimeout very far in the future. */
     static constexpr int64_t NO_TIMEOUT = std::numeric_limits<int64_t>::max();
@@ -97,14 +106,7 @@ struct Params {
     /** Don't warn about unknown BIP 9 activations below this height.
      * This prevents us from warning about the CSV and segwit activations. */
     int MinBIP9WarningHeight;
-    /**
+    std::array<BIP9Deployment,MAX_VERSION_BITS_DEPLOYMENTS> vDeployments;
-     * Minimum blocks including miner confirmation of the total of 2016 blocks in a retargeting period,
-     * (nPowTargetTimespan / nPowTargetSpacing) which is also used for BIP9 deployments.
-     * Examples: 1916 for 95%, 1512 for testchains.
-     */
-    uint32_t nRuleChangeActivationThreshold;
-    uint32_t nMinerConfirmationWindow;
-    BIP9Deployment vDeployments[MAX_VERSION_BITS_DEPLOYMENTS];
     /** Proof of work parameters */
     uint256 powLimit;
     bool fPowAllowMinDifficultyBlocks;

src/deploymentinfo.cpp

@@ -8,14 +8,14 @@
 
 #include <string_view>
 
-const struct VBDeploymentInfo VersionBitsDeploymentInfo[Consensus::MAX_VERSION_BITS_DEPLOYMENTS] = {
+const std::array<VBDeploymentInfo,Consensus::MAX_VERSION_BITS_DEPLOYMENTS> VersionBitsDeploymentInfo{
-    {
+    VBDeploymentInfo{
-        /*.name =*/ "testdummy",
+        .name = "testdummy",
-        /*.gbt_force =*/ true,
+        .gbt_force = true,
     },
-    {
+    VBDeploymentInfo{
-        /*.name =*/ "taproot",
+        .name = "taproot",
-        /*.gbt_force =*/ true,
+        .gbt_force = true,
     },
 };
 

src/deploymentinfo.h

@@ -7,6 +7,7 @@
 
 #include <consensus/params.h>
 
+#include <array>
 #include <optional>
 #include <string>
 
@@ -17,7 +18,7 @@ struct VBDeploymentInfo {
     bool gbt_force;
 };
 
-extern const VBDeploymentInfo VersionBitsDeploymentInfo[Consensus::MAX_VERSION_BITS_DEPLOYMENTS];
+extern const std::array<VBDeploymentInfo,Consensus::MAX_VERSION_BITS_DEPLOYMENTS> VersionBitsDeploymentInfo;
 
 std::string DeploymentName(Consensus::BuriedDeployment dep);
 

src/deploymentstatus.h

@@ -20,7 +20,7 @@ inline bool DeploymentActiveAfter(const CBlockIndex* pindexPrev, const Consensus
 inline bool DeploymentActiveAfter(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos dep, VersionBitsCache& versionbitscache)
 {
     assert(Consensus::ValidDeployment(dep));
-    return ThresholdState::ACTIVE == versionbitscache.State(pindexPrev, params, dep);
+    return versionbitscache.IsActiveAfter(pindexPrev, params, dep);
 }
 
 /** Determine if a deployment is active for this block */

src/kernel/chainparams.cpp

@@ -104,18 +104,20 @@ public:
         consensus.fPowAllowMinDifficultyBlocks = false;
         consensus.enforce_BIP94 = false;
         consensus.fPowNoRetargeting = false;
-        consensus.nRuleChangeActivationThreshold = 1815; // 90% of 2016
-        consensus.nMinerConfirmationWindow = 2016; // nPowTargetTimespan / nPowTargetSpacing
         consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].bit = 28;
         consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nStartTime = Consensus::BIP9Deployment::NEVER_ACTIVE;
         consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT;
         consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].min_activation_height = 0; // No activation delay
+        consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].threshold = 1815; // 90%
+        consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].period = 2016;
 
         // Deployment of Taproot (BIPs 340-342)
         consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].bit = 2;
         consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].nStartTime = 1619222400; // April 24th, 2021
         consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].nTimeout = 1628640000; // August 11th, 2021
         consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].min_activation_height = 709632; // Approximately November 12th, 2021
+        consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].threshold = 1815; // 90%
+        consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].period = 2016;
 
         consensus.nMinimumChainWork = uint256{"0000000000000000000000000000000000000000b1f3b93b65b16d035a82be84"};
         consensus.defaultAssumeValid = uint256{"00000000000000000001b658dd1120e82e66d2790811f89ede9742ada3ed6d77"}; // 886157
@@ -216,18 +218,20 @@ public:
         consensus.fPowAllowMinDifficultyBlocks = true;
         consensus.enforce_BIP94 = false;
         consensus.fPowNoRetargeting = false;
-        consensus.nRuleChangeActivationThreshold = 1512; // 75% for testchains
-        consensus.nMinerConfirmationWindow = 2016; // nPowTargetTimespan / nPowTargetSpacing
         consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].bit = 28;
         consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nStartTime = Consensus::BIP9Deployment::NEVER_ACTIVE;
         consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT;
         consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].min_activation_height = 0; // No activation delay
+        consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].threshold = 1512; // 75%
+        consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].period = 2016;
 
         // Deployment of Taproot (BIPs 340-342)
         consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].bit = 2;
         consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].nStartTime = 1619222400; // April 24th, 2021
         consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].nTimeout = 1628640000; // August 11th, 2021
         consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].min_activation_height = 0; // No activation delay
+        consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].threshold = 1512; // 75%
+        consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].period = 2016;
 
         consensus.nMinimumChainWork = uint256{"0000000000000000000000000000000000000000000015f5e0c9f13455b0eb17"};
         consensus.defaultAssumeValid = uint256{"00000000000003fc7967410ba2d0a8a8d50daedc318d43e8baf1a9782c236a57"}; // 3974606
@@ -309,18 +313,21 @@ public:
         consensus.fPowAllowMinDifficultyBlocks = true;
         consensus.enforce_BIP94 = true;
         consensus.fPowNoRetargeting = false;
-        consensus.nRuleChangeActivationThreshold = 1512; // 75% for testchains
+
-        consensus.nMinerConfirmationWindow = 2016; // nPowTargetTimespan / nPowTargetSpacing
         consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].bit = 28;
         consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nStartTime = Consensus::BIP9Deployment::NEVER_ACTIVE;
         consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT;
         consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].min_activation_height = 0; // No activation delay
+        consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].threshold = 1512; // 75%
+        consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].period = 2016;
 
         // Deployment of Taproot (BIPs 340-342)
         consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].bit = 2;
         consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].nStartTime = Consensus::BIP9Deployment::ALWAYS_ACTIVE;
         consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT;
         consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].min_activation_height = 0; // No activation delay
+        consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].threshold = 1512; // 75%
+        consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].period = 2016;
 
         consensus.nMinimumChainWork = uint256{"0000000000000000000000000000000000000000000001d6dce8651b6094e4c1"};
         consensus.defaultAssumeValid = uint256{"0000000000003ed4f08dbdf6f7d6b271a6bcffce25675cb40aa9fa43179a89f3"}; // 72600
@@ -439,20 +446,22 @@ public:
         consensus.fPowAllowMinDifficultyBlocks = false;
         consensus.enforce_BIP94 = false;
         consensus.fPowNoRetargeting = false;
-        consensus.nRuleChangeActivationThreshold = 1815; // 90% of 2016
-        consensus.nMinerConfirmationWindow = 2016; // nPowTargetTimespan / nPowTargetSpacing
         consensus.MinBIP9WarningHeight = 0;
         consensus.powLimit = uint256{"00000377ae000000000000000000000000000000000000000000000000000000"};
         consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].bit = 28;
         consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nStartTime = Consensus::BIP9Deployment::NEVER_ACTIVE;
         consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT;
         consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].min_activation_height = 0; // No activation delay
+        consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].threshold = 1815; // 90%
+        consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].period = 2016;
 
         // Activation of Taproot (BIPs 340-342)
         consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].bit = 2;
         consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].nStartTime = Consensus::BIP9Deployment::ALWAYS_ACTIVE;
         consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT;
         consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].min_activation_height = 0; // No activation delay
+        consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].threshold = 1815; // 90%
+        consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].period = 2016;
 
         // message start is defined as the first 4 bytes of the sha256d of the block script
         HashWriter h{};
@@ -516,18 +525,20 @@ public:
         consensus.fPowAllowMinDifficultyBlocks = true;
         consensus.enforce_BIP94 = opts.enforce_bip94;
         consensus.fPowNoRetargeting = true;
-        consensus.nRuleChangeActivationThreshold = 108; // 75% for testchains
-        consensus.nMinerConfirmationWindow = 144; // Faster than normal for regtest (144 instead of 2016)
 
         consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].bit = 28;
         consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nStartTime = 0;
         consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT;
         consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].min_activation_height = 0; // No activation delay
+        consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].threshold = 108; // 75%
+        consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].period = 144; // Faster than normal for regtest (144 instead of 2016)
 
         consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].bit = 2;
         consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].nStartTime = Consensus::BIP9Deployment::ALWAYS_ACTIVE;
         consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT;
         consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].min_activation_height = 0; // No activation delay
+        consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].threshold = 108; // 75%
+        consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].period = 144;
 
         consensus.nMinimumChainWork = uint256{};
         consensus.defaultAssumeValid = uint256{};

src/rpc/blockchain.cpp

@@ -1234,58 +1234,41 @@ static void SoftForkDescPushBack(const CBlockIndex* blockindex, UniValue& softfo
 static void SoftForkDescPushBack(const CBlockIndex* blockindex, UniValue& softforks, const ChainstateManager& chainman, Consensus::DeploymentPos id)
 {
     // For BIP9 deployments.
-
     if (!DeploymentEnabled(chainman, id)) return;
     if (blockindex == nullptr) return;
 
-    auto get_state_name = [](const ThresholdState state) -> std::string {
-        switch (state) {
-        case ThresholdState::DEFINED: return "defined";
-        case ThresholdState::STARTED: return "started";
-        case ThresholdState::LOCKED_IN: return "locked_in";
-        case ThresholdState::ACTIVE: return "active";
-        case ThresholdState::FAILED: return "failed";
-        }
-        return "invalid";
-    };
-
     UniValue bip9(UniValue::VOBJ);
-
+    BIP9Info info{chainman.m_versionbitscache.Info(*blockindex, chainman.GetConsensus(), id)};
-    const ThresholdState next_state = chainman.m_versionbitscache.State(blockindex, chainman.GetConsensus(), id);
+    const auto& depparams{chainman.GetConsensus().vDeployments[id]};
-    const ThresholdState current_state = chainman.m_versionbitscache.State(blockindex->pprev, chainman.GetConsensus(), id);
-
-    const bool has_signal = (ThresholdState::STARTED == current_state || ThresholdState::LOCKED_IN == current_state);
 
     // BIP9 parameters
-    if (has_signal) {
+    if (info.stats.has_value()) {
-        bip9.pushKV("bit", chainman.GetConsensus().vDeployments[id].bit);
+        bip9.pushKV("bit", depparams.bit);
     }
-    bip9.pushKV("start_time", chainman.GetConsensus().vDeployments[id].nStartTime);
+    bip9.pushKV("start_time", depparams.nStartTime);
-    bip9.pushKV("timeout", chainman.GetConsensus().vDeployments[id].nTimeout);
+    bip9.pushKV("timeout", depparams.nTimeout);
-    bip9.pushKV("min_activation_height", chainman.GetConsensus().vDeployments[id].min_activation_height);
+    bip9.pushKV("min_activation_height", depparams.min_activation_height);
 
     // BIP9 status
-    bip9.pushKV("status", get_state_name(current_state));
+    bip9.pushKV("status", info.current_state);
-    bip9.pushKV("since", chainman.m_versionbitscache.StateSinceHeight(blockindex->pprev, chainman.GetConsensus(), id));
+    bip9.pushKV("since", info.since);
-    bip9.pushKV("status_next", get_state_name(next_state));
+    bip9.pushKV("status_next", info.next_state);
 
     // BIP9 signalling status, if applicable
-    if (has_signal) {
+    if (info.stats.has_value()) {
         UniValue statsUV(UniValue::VOBJ);
-        std::vector<bool> signals;
+        statsUV.pushKV("period", info.stats->period);
-        BIP9Stats statsStruct = chainman.m_versionbitscache.Statistics(blockindex, chainman.GetConsensus(), id, &signals);
+        statsUV.pushKV("elapsed", info.stats->elapsed);
-        statsUV.pushKV("period", statsStruct.period);
+        statsUV.pushKV("count", info.stats->count);
-        statsUV.pushKV("elapsed", statsStruct.elapsed);
+        if (info.stats->threshold > 0 || info.stats->possible) {
-        statsUV.pushKV("count", statsStruct.count);
+            statsUV.pushKV("threshold", info.stats->threshold);
-        if (ThresholdState::LOCKED_IN != current_state) {
+            statsUV.pushKV("possible", info.stats->possible);
-            statsUV.pushKV("threshold", statsStruct.threshold);
-            statsUV.pushKV("possible", statsStruct.possible);
         }
         bip9.pushKV("statistics", std::move(statsUV));
 
         std::string sig;
-        sig.reserve(signals.size());
+        sig.reserve(info.signalling_blocks.size());
-        for (const bool s : signals) {
+        for (const bool s : info.signalling_blocks) {
             sig.push_back(s ? '#' : '-');
         }
         bip9.pushKV("signalling", sig);
@@ -1293,12 +1276,13 @@ static void SoftForkDescPushBack(const CBlockIndex* blockindex, UniValue& softfo
 
     UniValue rv(UniValue::VOBJ);
     rv.pushKV("type", "bip9");
-    if (ThresholdState::ACTIVE == next_state) {
+    bool is_active = false;
-        rv.pushKV("height", chainman.m_versionbitscache.StateSinceHeight(blockindex, chainman.GetConsensus(), id));
+    if (info.active_since.has_value()) {
+        rv.pushKV("height", *info.active_since);
+        is_active = (*info.active_since <= blockindex->nHeight + 1);
     }
-    rv.pushKV("active", ThresholdState::ACTIVE == next_state);
+    rv.pushKV("active", is_active);
-    rv.pushKV("bip9", std::move(bip9));
+    rv.pushKV("bip9", bip9);
-
     softforks.pushKV(DeploymentName(id), std::move(rv));
 }
 

src/rpc/mining.cpp

@@ -596,10 +596,10 @@ static UniValue BIP22ValidationResult(const BlockValidationState& state)
     return "valid?";
 }
 
-static std::string gbt_vb_name(const Consensus::DeploymentPos pos) {
+static std::string gbt_force_name(const std::string& name, bool gbt_force)
-    const struct VBDeploymentInfo& vbinfo = VersionBitsDeploymentInfo[pos];
+{
-    std::string s = vbinfo.name;
+    std::string s{name};
-    if (!vbinfo.gbt_force) {
+    if (!gbt_force) {
         s.insert(s.begin(), '!');
     }
     return s;
@@ -952,45 +952,33 @@ static RPCHelpMan getblocktemplate()
     }
 
     UniValue vbavailable(UniValue::VOBJ);
-    for (int j = 0; j < (int)Consensus::MAX_VERSION_BITS_DEPLOYMENTS; ++j) {
+    const auto gbtstatus = chainman.m_versionbitscache.GBTStatus(*pindexPrev, consensusParams);
-        Consensus::DeploymentPos pos = Consensus::DeploymentPos(j);
+
-        ThresholdState state = chainman.m_versionbitscache.State(pindexPrev, consensusParams, pos);
+    for (const auto& [name, info] : gbtstatus.signalling) {
-        switch (state) {
+        vbavailable.pushKV(gbt_force_name(name, info.gbt_force), info.bit);
-            case ThresholdState::DEFINED:
+        if (!info.gbt_force && !setClientRules.count(name)) {
-            case ThresholdState::FAILED:
+            // If the client doesn't support this, don't indicate it in the [default] version
-                // Not exposed to GBT at all
+            block.nVersion &= ~info.mask;
-                break;
-            case ThresholdState::LOCKED_IN:
-                // Ensure bit is set in block version
-                block.nVersion |= chainman.m_versionbitscache.Mask(consensusParams, pos);
-                [[fallthrough]];
-            case ThresholdState::STARTED:
-            {
-                const struct VBDeploymentInfo& vbinfo = VersionBitsDeploymentInfo[pos];
-                vbavailable.pushKV(gbt_vb_name(pos), consensusParams.vDeployments[pos].bit);
-                if (setClientRules.find(vbinfo.name) == setClientRules.end()) {
-                    if (!vbinfo.gbt_force) {
-                        // If the client doesn't support this, don't indicate it in the [default] version
-                        block.nVersion &= ~chainman.m_versionbitscache.Mask(consensusParams, pos);
-                    }
-                }
-                break;
-            }
-            case ThresholdState::ACTIVE:
-            {
-                // Add to rules only
-                const struct VBDeploymentInfo& vbinfo = VersionBitsDeploymentInfo[pos];
-                aRules.push_back(gbt_vb_name(pos));
-                if (setClientRules.find(vbinfo.name) == setClientRules.end()) {
-                    // Not supported by the client; make sure it's safe to proceed
-                    if (!vbinfo.gbt_force) {
-                        throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Support for '%s' rule requires explicit client support", vbinfo.name));
-                    }
-                }
-                break;
-            }
         }
     }
+
+    for (const auto& [name, info] : gbtstatus.locked_in) {
+        block.nVersion |= info.mask;
+        vbavailable.pushKV(gbt_force_name(name, info.gbt_force), info.bit);
+        if (!info.gbt_force && !setClientRules.count(name)) {
+            // If the client doesn't support this, don't indicate it in the [default] version
+            block.nVersion &= ~info.mask;
+        }
+    }
+
+    for (const auto& [name, info] : gbtstatus.active) {
+        aRules.push_back(gbt_force_name(name, info.gbt_force));
+        if (!info.gbt_force && !setClientRules.count(name)) {
+            // Not supported by the client; make sure it's safe to proceed
+            throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Support for '%s' rule requires explicit client support", name));
+        }
+    }
+
     result.pushKV("version", block.nVersion);
     result.pushKV("rules", std::move(aRules));
     result.pushKV("vbavailable", std::move(vbavailable));

src/test/fuzz/versionbits.cpp

@@ -9,6 +9,7 @@
 #include <primitives/block.h>
 #include <util/chaintype.h>
 #include <versionbits.h>
+#include <versionbits_impl.h>
 
 #include <test/fuzz/FuzzedDataProvider.h>
 #include <test/fuzz/fuzz.h>
@@ -20,47 +21,22 @@
 #include <vector>
 
 namespace {
-class TestConditionChecker : public AbstractThresholdConditionChecker
+class TestConditionChecker : public VersionBitsConditionChecker
 {
 private:
     mutable ThresholdConditionCache m_cache;
-    const Consensus::Params dummy_params{};
 
 public:
-    const int64_t m_begin;
+    TestConditionChecker(const Consensus::BIP9Deployment& dep) : VersionBitsConditionChecker{dep}
-    const int64_t m_end;
-    const int m_period;
-    const int m_threshold;
-    const int m_min_activation_height;
-    const int m_bit;
-
-    TestConditionChecker(int64_t begin, int64_t end, int period, int threshold, int min_activation_height, int bit)
-        : m_begin{begin}, m_end{end}, m_period{period}, m_threshold{threshold}, m_min_activation_height{min_activation_height}, m_bit{bit}
     {
-        assert(m_period > 0);
+        assert(dep.period > 0);
-        assert(0 <= m_threshold && m_threshold <= m_period);
+        assert(dep.threshold <= dep.period);
-        assert(0 <= m_bit && m_bit < 32 && m_bit < VERSIONBITS_NUM_BITS);
+        assert(0 <= dep.bit && dep.bit < 32 && dep.bit < VERSIONBITS_NUM_BITS);
-        assert(0 <= m_min_activation_height);
+        assert(0 <= dep.min_activation_height);
     }
 
-    bool Condition(const CBlockIndex* pindex, const Consensus::Params& params) const override { return Condition(pindex->nVersion); }
+    ThresholdState GetStateFor(const CBlockIndex* pindexPrev) const { return AbstractThresholdConditionChecker::GetStateFor(pindexPrev, m_cache); }
-    int64_t BeginTime(const Consensus::Params& params) const override { return m_begin; }
+    int GetStateSinceHeightFor(const CBlockIndex* pindexPrev) const { return AbstractThresholdConditionChecker::GetStateSinceHeightFor(pindexPrev, m_cache); }
-    int64_t EndTime(const Consensus::Params& params) const override { return m_end; }
-    int Period(const Consensus::Params& params) const override { return m_period; }
-    int Threshold(const Consensus::Params& params) const override { return m_threshold; }
-    int MinActivationHeight(const Consensus::Params& params) const override { return m_min_activation_height; }
-
-    ThresholdState GetStateFor(const CBlockIndex* pindexPrev) const { return AbstractThresholdConditionChecker::GetStateFor(pindexPrev, dummy_params, m_cache); }
-    int GetStateSinceHeightFor(const CBlockIndex* pindexPrev) const { return AbstractThresholdConditionChecker::GetStateSinceHeightFor(pindexPrev, dummy_params, m_cache); }
-    BIP9Stats GetStateStatisticsFor(const CBlockIndex* pindex, std::vector<bool>* signals=nullptr) const { return AbstractThresholdConditionChecker::GetStateStatisticsFor(pindex, dummy_params, signals); }
-
-    bool Condition(int32_t version) const
-    {
-        uint32_t mask = (uint32_t{1}) << m_bit;
-        return (((version & VERSIONBITS_TOP_MASK) == VERSIONBITS_TOP_BITS) && (version & mask) != 0);
-    }
-
-    bool Condition(const CBlockIndex* pindex) const { return Condition(pindex->nVersion); }
 };
 
 /** Track blocks mined for test */
@@ -121,13 +97,10 @@ FUZZ_TARGET(versionbits, .init = initialize)
     FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
 
     // making period/max_periods larger slows these tests down significantly
-    const int period = 32;
+    const uint32_t period = 32;
     const size_t max_periods = 16;
     const size_t max_blocks = 2 * period * max_periods;
 
-    const int threshold = fuzzed_data_provider.ConsumeIntegralInRange(1, period);
-    assert(0 < threshold && threshold <= period); // must be able to both pass and fail threshold!
-
     // too many blocks at 10min each might cause uint32_t time to overflow if
     // block_start_time is at the end of the range above
     assert(std::numeric_limits<uint32_t>::max() - MAX_START_TIME > interval * max_blocks);
@@ -137,53 +110,57 @@ FUZZ_TARGET(versionbits, .init = initialize)
     // what values for version will we use to signal / not signal?
     const int32_t ver_signal = fuzzed_data_provider.ConsumeIntegral<int32_t>();
     const int32_t ver_nosignal = fuzzed_data_provider.ConsumeIntegral<int32_t>();
+    if (ver_nosignal < 0) return; // negative values are uninteresting
 
-    // select deployment parameters: bit, start time, timeout
+    // Now that we have chosen time and versions, setup to mine blocks
-    const int bit = fuzzed_data_provider.ConsumeIntegralInRange<int>(0, VERSIONBITS_NUM_BITS - 1);
+    Blocks blocks(block_start_time, interval, ver_signal, ver_nosignal);
 
-    bool always_active_test = false;
+    const bool always_active_test = fuzzed_data_provider.ConsumeBool();
-    bool never_active_test = false;
+    const bool never_active_test = !always_active_test && fuzzed_data_provider.ConsumeBool();
-    int64_t start_time;
-    int64_t timeout;
-    if (fuzzed_data_provider.ConsumeBool()) {
-        // pick the timestamp to switch based on a block
-        // note states will change *after* these blocks because mediantime lags
-        int start_block = fuzzed_data_provider.ConsumeIntegralInRange<int>(0, period * (max_periods - 3));
-        int end_block = fuzzed_data_provider.ConsumeIntegralInRange<int>(0, period * (max_periods - 3));
 
-        start_time = block_start_time + start_block * interval;
+    const Consensus::BIP9Deployment dep{[&]() {
-        timeout = block_start_time + end_block * interval;
+        Consensus::BIP9Deployment dep;
+        dep.period = period;
 
-        // allow for times to not exactly match a block
+        dep.threshold = fuzzed_data_provider.ConsumeIntegralInRange<uint32_t>(1, period);
-        if (fuzzed_data_provider.ConsumeBool()) start_time += interval / 2;
+        assert(0 < dep.threshold && dep.threshold <= dep.period); // must be able to both pass and fail threshold!
-        if (fuzzed_data_provider.ConsumeBool()) timeout += interval / 2;
+
-    } else {
+        // select deployment parameters: bit, start time, timeout
-        if (fuzzed_data_provider.ConsumeBool()) {
+        dep.bit = fuzzed_data_provider.ConsumeIntegralInRange<int>(0, VERSIONBITS_NUM_BITS - 1);
-            start_time = Consensus::BIP9Deployment::ALWAYS_ACTIVE;
+
-            always_active_test = true;
+        if (always_active_test) {
+            dep.nStartTime = Consensus::BIP9Deployment::ALWAYS_ACTIVE;
+            dep.nTimeout = fuzzed_data_provider.ConsumeBool() ? Consensus::BIP9Deployment::NO_TIMEOUT : fuzzed_data_provider.ConsumeIntegral<int64_t>();
+        } else if (never_active_test) {
+            dep.nStartTime = Consensus::BIP9Deployment::NEVER_ACTIVE;
+            dep.nTimeout = fuzzed_data_provider.ConsumeBool() ? Consensus::BIP9Deployment::NO_TIMEOUT : fuzzed_data_provider.ConsumeIntegral<int64_t>();
         } else {
-            start_time = Consensus::BIP9Deployment::NEVER_ACTIVE;
+            // pick the timestamp to switch based on a block
-            never_active_test = true;
+            // note states will change *after* these blocks because mediantime lags
-        }
+            int start_block = fuzzed_data_provider.ConsumeIntegralInRange<int>(0, period * (max_periods - 3));
-        timeout = fuzzed_data_provider.ConsumeBool() ? Consensus::BIP9Deployment::NO_TIMEOUT : fuzzed_data_provider.ConsumeIntegral<int64_t>();
+            int end_block = fuzzed_data_provider.ConsumeIntegralInRange<int>(0, period * (max_periods - 3));
-    }
-    int min_activation = fuzzed_data_provider.ConsumeIntegralInRange<int>(0, period * max_periods);
 
-    TestConditionChecker checker(start_time, timeout, period, threshold, min_activation, bit);
+            dep.nStartTime = block_start_time + start_block * interval;
+            dep.nTimeout = block_start_time + end_block * interval;
+
+            // allow for times to not exactly match a block
+            if (fuzzed_data_provider.ConsumeBool()) dep.nStartTime += interval / 2;
+            if (fuzzed_data_provider.ConsumeBool()) dep.nTimeout += interval / 2;
+        }
+        dep.min_activation_height = fuzzed_data_provider.ConsumeIntegralInRange<int>(0, period * max_periods);
+        return dep;
+    }()};
+    TestConditionChecker checker(dep);
 
     // Early exit if the versions don't signal sensibly for the deployment
     if (!checker.Condition(ver_signal)) return;
     if (checker.Condition(ver_nosignal)) return;
-    if (ver_nosignal < 0) return;
 
     // TOP_BITS should ensure version will be positive and meet min
     // version requirement
     assert(ver_signal > 0);
     assert(ver_signal >= VERSIONBITS_LAST_OLD_BLOCK_VERSION);
 
-    // Now that we have chosen time and versions, setup to mine blocks
-    Blocks blocks(block_start_time, interval, ver_signal, ver_nosignal);
-
     /* Strategy:
      *  * we will mine a final period worth of blocks, with
      *    randomised signalling according to a mask
@@ -203,7 +180,7 @@ FUZZ_TARGET(versionbits, .init = initialize)
     while (fuzzed_data_provider.remaining_bytes() > 0) { // early exit; no need for LIMITED_WHILE
         // all blocks in these periods either do or don't signal
         bool signal = fuzzed_data_provider.ConsumeBool();
-        for (int b = 0; b < period; ++b) {
+        for (uint32_t b = 0; b < period; ++b) {
             blocks.mine_block(signal);
         }
 
@@ -215,7 +192,7 @@ FUZZ_TARGET(versionbits, .init = initialize)
     // now we mine the final period and check that everything looks sane
 
     // count the number of signalling blocks
-    int blocks_sig = 0;
+    uint32_t blocks_sig = 0;
 
     // get the info for the first block of the period
     CBlockIndex* prev = blocks.tip();
@@ -225,23 +202,23 @@ FUZZ_TARGET(versionbits, .init = initialize)
     // get statistics from end of previous period, then reset
     BIP9Stats last_stats;
     last_stats.period = period;
-    last_stats.threshold = threshold;
+    last_stats.threshold = dep.threshold;
     last_stats.count = last_stats.elapsed = 0;
-    last_stats.possible = (period >= threshold);
+    last_stats.possible = (period >= dep.threshold);
     std::vector<bool> last_signals{};
 
     int prev_next_height = (prev == nullptr ? 0 : prev->nHeight + 1);
     assert(exp_since <= prev_next_height);
 
     // mine (period-1) blocks and check state
-    for (int b = 1; b < period; ++b) {
+    for (uint32_t b = 1; b < period; ++b) {
         const bool signal = (signalling_mask >> (b % 32)) & 1;
         if (signal) ++blocks_sig;
 
         CBlockIndex* current_block = blocks.mine_block(signal);
 
         // verify that signalling attempt was interpreted correctly
-        assert(checker.Condition(current_block) == signal);
+        assert(checker.Condition(current_block->nVersion) == signal);
 
         // state and since don't change within the period
         const ThresholdState state = checker.GetStateFor(current_block);
@@ -258,10 +235,10 @@ FUZZ_TARGET(versionbits, .init = initialize)
                && stats.possible == stats_no_signals.possible);
 
         assert(stats.period == period);
-        assert(stats.threshold == threshold);
+        assert(stats.threshold == dep.threshold);
         assert(stats.elapsed == b);
         assert(stats.count == last_stats.count + (signal ? 1 : 0));
-        assert(stats.possible == (stats.count + period >= stats.elapsed + threshold));
+        assert(stats.possible == (stats.count + period >= stats.elapsed + dep.threshold));
         last_stats = stats;
 
         assert(signals.size() == last_signals.size() + 1);
@@ -272,21 +249,21 @@ FUZZ_TARGET(versionbits, .init = initialize)
 
     if (exp_state == ThresholdState::STARTED) {
         // double check that stats.possible is sane
-        if (blocks_sig >= threshold - 1) assert(last_stats.possible);
+        if (blocks_sig >= dep.threshold - 1) assert(last_stats.possible);
     }
 
     // mine the final block
     bool signal = (signalling_mask >> (period % 32)) & 1;
     if (signal) ++blocks_sig;
     CBlockIndex* current_block = blocks.mine_block(signal);
-    assert(checker.Condition(current_block) == signal);
+    assert(checker.Condition(current_block->nVersion) == signal);
 
     const BIP9Stats stats = checker.GetStateStatisticsFor(current_block);
     assert(stats.period == period);
-    assert(stats.threshold == threshold);
+    assert(stats.threshold == dep.threshold);
     assert(stats.elapsed == period);
     assert(stats.count == blocks_sig);
-    assert(stats.possible == (stats.count + period >= stats.elapsed + threshold));
+    assert(stats.possible == (stats.count + period >= stats.elapsed + dep.threshold));
 
     // More interesting is whether the state changed.
     const ThresholdState state = checker.GetStateFor(current_block);
@@ -306,33 +283,33 @@ FUZZ_TARGET(versionbits, .init = initialize)
     case ThresholdState::DEFINED:
         assert(since == 0);
         assert(exp_state == ThresholdState::DEFINED);
-        assert(current_block->GetMedianTimePast() < checker.m_begin);
+        assert(current_block->GetMedianTimePast() < dep.nStartTime);
         break;
     case ThresholdState::STARTED:
-        assert(current_block->GetMedianTimePast() >= checker.m_begin);
+        assert(current_block->GetMedianTimePast() >= dep.nStartTime);
         if (exp_state == ThresholdState::STARTED) {
-            assert(blocks_sig < threshold);
+            assert(blocks_sig < dep.threshold);
-            assert(current_block->GetMedianTimePast() < checker.m_end);
+            assert(current_block->GetMedianTimePast() < dep.nTimeout);
         } else {
             assert(exp_state == ThresholdState::DEFINED);
         }
         break;
     case ThresholdState::LOCKED_IN:
         if (exp_state == ThresholdState::LOCKED_IN) {
-            assert(current_block->nHeight + 1 < min_activation);
+            assert(current_block->nHeight + 1 < dep.min_activation_height);
         } else {
             assert(exp_state == ThresholdState::STARTED);
-            assert(blocks_sig >= threshold);
+            assert(blocks_sig >= dep.threshold);
         }
         break;
     case ThresholdState::ACTIVE:
-        assert(always_active_test || min_activation <= current_block->nHeight + 1);
+        assert(always_active_test || dep.min_activation_height <= current_block->nHeight + 1);
         assert(exp_state == ThresholdState::ACTIVE || exp_state == ThresholdState::LOCKED_IN);
         break;
     case ThresholdState::FAILED:
-        assert(never_active_test || current_block->GetMedianTimePast() >= checker.m_end);
+        assert(never_active_test || current_block->GetMedianTimePast() >= dep.nTimeout);
         if (exp_state == ThresholdState::STARTED) {
-            assert(blocks_sig < threshold);
+            assert(blocks_sig < dep.threshold);
         } else {
             assert(exp_state == ThresholdState::FAILED);
         }

src/test/versionbits_tests.cpp

@@ -9,59 +9,48 @@
 #include <test/util/setup_common.h>
 #include <util/chaintype.h>
 #include <versionbits.h>
+#include <versionbits_impl.h>
 
 #include <boost/test/unit_test.hpp>
 
 /* Define a virtual block time, one block per 10 minutes after Nov 14 2014, 0:55:36am */
 static int32_t TestTime(int nHeight) { return 1415926536 + 600 * nHeight; }
 
-static std::string StateName(ThresholdState state)
+class TestConditionChecker final : public VersionBitsConditionChecker
-{
-    switch (state) {
-    case ThresholdState::DEFINED:   return "DEFINED";
-    case ThresholdState::STARTED:   return "STARTED";
-    case ThresholdState::LOCKED_IN: return "LOCKED_IN";
-    case ThresholdState::ACTIVE:    return "ACTIVE";
-    case ThresholdState::FAILED:    return "FAILED";
-    } // no default case, so the compiler can warn about missing cases
-    return "";
-}
-
-static const Consensus::Params paramsDummy = Consensus::Params();
-
-class TestConditionChecker : public AbstractThresholdConditionChecker
 {
 private:
     mutable ThresholdConditionCache cache;
 
 public:
-    int64_t BeginTime(const Consensus::Params& params) const override { return TestTime(10000); }
+    // constructor is implicit to allow for easier initialization of vector<TestConditionChecker>
-    int64_t EndTime(const Consensus::Params& params) const override { return TestTime(20000); }
+    explicit(false) TestConditionChecker(const Consensus::BIP9Deployment& dep) : VersionBitsConditionChecker{dep} { }
-    int Period(const Consensus::Params& params) const override { return 1000; }
+    ~TestConditionChecker() override = default;
-    int Threshold(const Consensus::Params& params) const override { return 900; }
-    bool Condition(const CBlockIndex* pindex, const Consensus::Params& params) const override { return (pindex->nVersion & 0x100); }
 
-    ThresholdState GetStateFor(const CBlockIndex* pindexPrev) const { return AbstractThresholdConditionChecker::GetStateFor(pindexPrev, paramsDummy, cache); }
+    ThresholdState StateFor(const CBlockIndex* pindexPrev) const { return AbstractThresholdConditionChecker::GetStateFor(pindexPrev, cache); }
-    int GetStateSinceHeightFor(const CBlockIndex* pindexPrev) const { return AbstractThresholdConditionChecker::GetStateSinceHeightFor(pindexPrev, paramsDummy, cache); }
+    int StateSinceHeightFor(const CBlockIndex* pindexPrev) const { return AbstractThresholdConditionChecker::GetStateSinceHeightFor(pindexPrev, cache); }
+    void clear() { cache.clear(); }
 };
 
-class TestDelayedActivationConditionChecker : public TestConditionChecker
+namespace {
+struct Deployments
 {
-public:
+    const Consensus::BIP9Deployment normal{
-    int MinActivationHeight(const Consensus::Params& params) const override { return 15000; }
+        .bit = 8,
-};
+        .nStartTime = TestTime(10000),
-
+        .nTimeout = TestTime(20000),
-class TestAlwaysActiveConditionChecker : public TestConditionChecker
+        .min_activation_height = 0,
-{
+        .period = 1000,
-public:
+        .threshold = 900,
-    int64_t BeginTime(const Consensus::Params& params) const override { return Consensus::BIP9Deployment::ALWAYS_ACTIVE; }
+    };
-};
+    Consensus::BIP9Deployment always, never, delayed;
-
+    Deployments()
-class TestNeverActiveConditionChecker : public TestConditionChecker
+    {
-{
+        delayed = normal; delayed.min_activation_height = 15000;
-public:
+        always = normal; always.nStartTime = Consensus::BIP9Deployment::ALWAYS_ACTIVE;
-    int64_t BeginTime(const Consensus::Params& params) const override { return Consensus::BIP9Deployment::NEVER_ACTIVE; }
+        never = normal; never.nStartTime = Consensus::BIP9Deployment::NEVER_ACTIVE;
+    }
 };
+}
 
 #define CHECKERS 6
 
@@ -71,22 +60,28 @@ class VersionBitsTester
     // A fake blockchain
     std::vector<CBlockIndex*> vpblock;
 
+    // Used to automatically set the top bits for manual calls to Mine()
+    const int32_t nVersionBase{0};
+
+    // Setup BIP9Deployment structs for the checkers
+    const Deployments test_deployments;
+
     // 6 independent checkers for the same bit.
     // The first one performs all checks, the second only 50%, the third only 25%, etc...
     // This is to test whether lack of cached information leads to the same results.
-    TestConditionChecker checker[CHECKERS];
+    std::vector<TestConditionChecker> checker{CHECKERS, {test_deployments.normal}};
     // Another 6 that assume delayed activation
-    TestDelayedActivationConditionChecker checker_delayed[CHECKERS];
+    std::vector<TestConditionChecker> checker_delayed{CHECKERS, {test_deployments.delayed}};
     // Another 6 that assume always active activation
-    TestAlwaysActiveConditionChecker checker_always[CHECKERS];
+    std::vector<TestConditionChecker> checker_always{CHECKERS, {test_deployments.always}};
     // Another 6 that assume never active activation
-    TestNeverActiveConditionChecker checker_never[CHECKERS];
+    std::vector<TestConditionChecker> checker_never{CHECKERS, {test_deployments.never}};
 
     // Test counter (to identify failures)
     int num{1000};
 
 public:
-    VersionBitsTester(FastRandomContext& rng) : m_rng{rng} {}
+    explicit VersionBitsTester(FastRandomContext& rng, int32_t nVersionBase=0) : m_rng{rng}, nVersionBase{nVersionBase} { }
 
     VersionBitsTester& Reset() {
         // Have each group of tests be counted by the 1000s part, starting at 1000
@@ -96,10 +91,10 @@ public:
             delete vpblock[i];
         }
         for (unsigned int  i = 0; i < CHECKERS; i++) {
-            checker[i] = TestConditionChecker();
+            checker[i].clear();
-            checker_delayed[i] = TestDelayedActivationConditionChecker();
+            checker_delayed[i].clear();
-            checker_always[i] = TestAlwaysActiveConditionChecker();
+            checker_always[i].clear();
-            checker_never[i] = TestNeverActiveConditionChecker();
+            checker_never[i].clear();
         }
         vpblock.clear();
         return *this;
@@ -115,7 +110,7 @@ public:
             pindex->nHeight = vpblock.size();
             pindex->pprev = Tip();
             pindex->nTime = nTime;
-            pindex->nVersion = nVersion;
+            pindex->nVersion = (nVersionBase | nVersion);
             pindex->BuildSkip();
             vpblock.push_back(pindex);
         }
@@ -132,10 +127,10 @@ public:
         const CBlockIndex* tip = Tip();
         for (int i = 0; i < CHECKERS; i++) {
             if (m_rng.randbits(i) == 0) {
-                BOOST_CHECK_MESSAGE(checker[i].GetStateSinceHeightFor(tip) == height, strprintf("Test %i for StateSinceHeight", num));
+                BOOST_CHECK_MESSAGE(checker[i].StateSinceHeightFor(tip) == height, strprintf("Test %i for StateSinceHeight", num));
-                BOOST_CHECK_MESSAGE(checker_delayed[i].GetStateSinceHeightFor(tip) == height_delayed, strprintf("Test %i for StateSinceHeight (delayed)", num));
+                BOOST_CHECK_MESSAGE(checker_delayed[i].StateSinceHeightFor(tip) == height_delayed, strprintf("Test %i for StateSinceHeight (delayed)", num));
-                BOOST_CHECK_MESSAGE(checker_always[i].GetStateSinceHeightFor(tip) == 0, strprintf("Test %i for StateSinceHeight (always active)", num));
+                BOOST_CHECK_MESSAGE(checker_always[i].StateSinceHeightFor(tip) == 0, strprintf("Test %i for StateSinceHeight (always active)", num));
-                BOOST_CHECK_MESSAGE(checker_never[i].GetStateSinceHeightFor(tip) == 0, strprintf("Test %i for StateSinceHeight (never active)", num));
+                BOOST_CHECK_MESSAGE(checker_never[i].StateSinceHeightFor(tip) == 0, strprintf("Test %i for StateSinceHeight (never active)", num));
             }
         }
         num++;
@@ -158,10 +153,10 @@ public:
         const CBlockIndex* pindex = Tip();
         for (int i = 0; i < CHECKERS; i++) {
             if (m_rng.randbits(i) == 0) {
-                ThresholdState got = checker[i].GetStateFor(pindex);
+                ThresholdState got = checker[i].StateFor(pindex);
-                ThresholdState got_delayed = checker_delayed[i].GetStateFor(pindex);
+                ThresholdState got_delayed = checker_delayed[i].StateFor(pindex);
-                ThresholdState got_always = checker_always[i].GetStateFor(pindex);
+                ThresholdState got_always = checker_always[i].StateFor(pindex);
-                ThresholdState got_never = checker_never[i].GetStateFor(pindex);
+                ThresholdState got_never = checker_never[i].StateFor(pindex);
                 // nHeight of the next block. If vpblock is empty, the next (ie first)
                 // block should be the genesis block with nHeight == 0.
                 int height = pindex == nullptr ? 0 : pindex->nHeight + 1;
@@ -193,7 +188,7 @@ BOOST_AUTO_TEST_CASE(versionbits_test)
 {
     for (int i = 0; i < 64; i++) {
         // DEFINED -> STARTED after timeout reached -> FAILED
-        VersionBitsTester(m_rng).TestDefined().TestStateSinceHeight(0)
+        VersionBitsTester(m_rng, VERSIONBITS_TOP_BITS).TestDefined().TestStateSinceHeight(0)
                            .Mine(1, TestTime(1), 0x100).TestDefined().TestStateSinceHeight(0)
                            .Mine(11, TestTime(11), 0x100).TestDefined().TestStateSinceHeight(0)
                            .Mine(989, TestTime(989), 0x100).TestDefined().TestStateSinceHeight(0)
@@ -260,7 +255,8 @@ BOOST_AUTO_TEST_CASE(versionbits_test)
 }
 
 struct BlockVersionTest : BasicTestingSetup {
-/** Check that ComputeBlockVersion will set the appropriate bit correctly */
+/** Check that ComputeBlockVersion will set the appropriate bit correctly
+ * Also checks IsActiveAfter() behaviour */
 void check_computeblockversion(VersionBitsCache& versionbitscache, const Consensus::Params& params, Consensus::DeploymentPos dep)
 {
     // Clear the cache every time
@@ -270,6 +266,12 @@ void check_computeblockversion(VersionBitsCache& versionbitscache, const Consens
     int64_t nStartTime = params.vDeployments[dep].nStartTime;
     int64_t nTimeout = params.vDeployments[dep].nTimeout;
     int min_activation_height = params.vDeployments[dep].min_activation_height;
+    uint32_t period = params.vDeployments[dep].period;
+    uint32_t threshold = params.vDeployments[dep].threshold;
+
+    BOOST_REQUIRE(period > 0); // no division by zero, thankyou
+    BOOST_REQUIRE(0 < threshold); // must be able to have a window that doesn't activate
+    BOOST_REQUIRE(threshold < period); // must be able to have a window that does activate
 
     // should not be any signalling for first block
     BOOST_CHECK_EQUAL(versionbitscache.ComputeBlockVersion(nullptr, params), VERSIONBITS_TOP_BITS);
@@ -278,6 +280,11 @@ void check_computeblockversion(VersionBitsCache& versionbitscache, const Consens
     if (nStartTime == Consensus::BIP9Deployment::ALWAYS_ACTIVE ||
         nStartTime == Consensus::BIP9Deployment::NEVER_ACTIVE)
     {
+        if (nStartTime == Consensus::BIP9Deployment::ALWAYS_ACTIVE) {
+            BOOST_CHECK(versionbitscache.IsActiveAfter(nullptr, params, dep));
+        } else {
+            BOOST_CHECK(!versionbitscache.IsActiveAfter(nullptr, params, dep));
+        }
         BOOST_CHECK_EQUAL(min_activation_height, 0);
         BOOST_CHECK_EQUAL(nTimeout, Consensus::BIP9Deployment::NO_TIMEOUT);
         return;
@@ -291,10 +298,7 @@ void check_computeblockversion(VersionBitsCache& versionbitscache, const Consens
     BOOST_REQUIRE(((1 << bit) & VERSIONBITS_TOP_MASK) == 0);
     BOOST_REQUIRE(min_activation_height >= 0);
     // Check min_activation_height is on a retarget boundary
-    BOOST_REQUIRE_EQUAL(min_activation_height % params.nMinerConfirmationWindow, 0U);
+    BOOST_REQUIRE_EQUAL(min_activation_height % period, 0U);
-
-    const uint32_t bitmask{versionbitscache.Mask(params, dep)};
-    BOOST_CHECK_EQUAL(bitmask, uint32_t{1} << bit);
 
     // In the first chain, test that the bit is set by CBV until it has failed.
     // In the second chain, test the bit is set by CBV while STARTED and
@@ -311,49 +315,56 @@ void check_computeblockversion(VersionBitsCache& versionbitscache, const Consens
         // since CBlockIndex::nTime is uint32_t we can't represent any
         // earlier time, so will transition from DEFINED to STARTED at the
         // end of the first period by mining blocks at nTime == 0
-        lastBlock = firstChain.Mine(params.nMinerConfirmationWindow - 1, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip();
+        lastBlock = firstChain.Mine(period - 1, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip();
         BOOST_CHECK_EQUAL(versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit), 0);
-        lastBlock = firstChain.Mine(params.nMinerConfirmationWindow, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip();
+        BOOST_CHECK(!versionbitscache.IsActiveAfter(lastBlock, params, dep));
+        lastBlock = firstChain.Mine(period, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip();
         BOOST_CHECK((versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit)) != 0);
+        BOOST_CHECK(!versionbitscache.IsActiveAfter(lastBlock, params, dep));
         // then we'll keep mining at nStartTime...
     } else {
         // use a time 1s earlier than start time to check we stay DEFINED
         --nTime;
 
         // Start generating blocks before nStartTime
-        lastBlock = firstChain.Mine(params.nMinerConfirmationWindow, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip();
+        lastBlock = firstChain.Mine(period, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip();
         BOOST_CHECK_EQUAL(versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit), 0);
+        BOOST_CHECK(!versionbitscache.IsActiveAfter(lastBlock, params, dep));
 
         // Mine more blocks (4 less than the adjustment period) at the old time, and check that CBV isn't setting the bit yet.
-        for (uint32_t i = 1; i < params.nMinerConfirmationWindow - 4; i++) {
+        for (uint32_t i = 1; i < period - 4; i++) {
-            lastBlock = firstChain.Mine(params.nMinerConfirmationWindow + i, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip();
+            lastBlock = firstChain.Mine(period + i, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip();
             BOOST_CHECK_EQUAL(versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit), 0);
+            BOOST_CHECK(!versionbitscache.IsActiveAfter(lastBlock, params, dep));
         }
         // Now mine 5 more blocks at the start time -- MTP should not have passed yet, so
         // CBV should still not yet set the bit.
         nTime = nStartTime;
-        for (uint32_t i = params.nMinerConfirmationWindow - 4; i <= params.nMinerConfirmationWindow; i++) {
+        for (uint32_t i = period - 4; i <= period; i++) {
-            lastBlock = firstChain.Mine(params.nMinerConfirmationWindow + i, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip();
+            lastBlock = firstChain.Mine(period + i, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip();
             BOOST_CHECK_EQUAL(versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit), 0);
+            BOOST_CHECK(!versionbitscache.IsActiveAfter(lastBlock, params, dep));
         }
         // Next we will advance to the next period and transition to STARTED,
     }
 
-    lastBlock = firstChain.Mine(params.nMinerConfirmationWindow * 3, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip();
+    lastBlock = firstChain.Mine(period * 3, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip();
     // so ComputeBlockVersion should now set the bit,
     BOOST_CHECK((versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit)) != 0);
     // and should also be using the VERSIONBITS_TOP_BITS.
     BOOST_CHECK_EQUAL(versionbitscache.ComputeBlockVersion(lastBlock, params) & VERSIONBITS_TOP_MASK, VERSIONBITS_TOP_BITS);
+    BOOST_CHECK(!versionbitscache.IsActiveAfter(lastBlock, params, dep));
 
     // Check that ComputeBlockVersion will set the bit until nTimeout
     nTime += 600;
-    uint32_t blocksToMine = params.nMinerConfirmationWindow * 2; // test blocks for up to 2 time periods
+    uint32_t blocksToMine = period * 2; // test blocks for up to 2 time periods
-    uint32_t nHeight = params.nMinerConfirmationWindow * 3;
+    uint32_t nHeight = period * 3;
     // These blocks are all before nTimeout is reached.
     while (nTime < nTimeout && blocksToMine > 0) {
         lastBlock = firstChain.Mine(nHeight+1, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip();
         BOOST_CHECK((versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit)) != 0);
         BOOST_CHECK_EQUAL(versionbitscache.ComputeBlockVersion(lastBlock, params) & VERSIONBITS_TOP_MASK, VERSIONBITS_TOP_BITS);
+        BOOST_CHECK(!versionbitscache.IsActiveAfter(lastBlock, params, dep));
         blocksToMine--;
         nTime += 600;
         nHeight += 1;
@@ -365,22 +376,25 @@ void check_computeblockversion(VersionBitsCache& versionbitscache, const Consens
         nTime = nTimeout;
 
         // finish the last period before we start timing out
-        while (nHeight % params.nMinerConfirmationWindow != 0) {
+        while (nHeight % period != 0) {
             lastBlock = firstChain.Mine(nHeight+1, nTime - 1, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip();
             BOOST_CHECK((versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit)) != 0);
+            BOOST_CHECK(!versionbitscache.IsActiveAfter(lastBlock, params, dep));
             nHeight += 1;
         }
 
         // FAILED is only triggered at the end of a period, so CBV should be setting
         // the bit until the period transition.
-        for (uint32_t i = 0; i < params.nMinerConfirmationWindow - 1; i++) {
+        for (uint32_t i = 0; i < period - 1; i++) {
             lastBlock = firstChain.Mine(nHeight+1, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip();
             BOOST_CHECK((versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit)) != 0);
+            BOOST_CHECK(!versionbitscache.IsActiveAfter(lastBlock, params, dep));
             nHeight += 1;
         }
         // The next block should trigger no longer setting the bit.
         lastBlock = firstChain.Mine(nHeight+1, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip();
         BOOST_CHECK_EQUAL(versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit), 0);
+        BOOST_CHECK(!versionbitscache.IsActiveAfter(lastBlock, params, dep));
     }
 
     // On a new chain:
@@ -390,31 +404,36 @@ void check_computeblockversion(VersionBitsCache& versionbitscache, const Consens
 
     // Mine one period worth of blocks, and check that the bit will be on for the
     // next period.
-    lastBlock = secondChain.Mine(params.nMinerConfirmationWindow, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip();
+    lastBlock = secondChain.Mine(period, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip();
     BOOST_CHECK((versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit)) != 0);
+    BOOST_CHECK(!versionbitscache.IsActiveAfter(lastBlock, params, dep));
 
     // Mine another period worth of blocks, signaling the new bit.
-    lastBlock = secondChain.Mine(params.nMinerConfirmationWindow * 2, nTime, VERSIONBITS_TOP_BITS | (1<<bit)).Tip();
+    lastBlock = secondChain.Mine(period * 2, nTime, VERSIONBITS_TOP_BITS | (1<<bit)).Tip();
     // After one period of setting the bit on each block, it should have locked in.
     // We keep setting the bit for one more period though, until activation.
     BOOST_CHECK((versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit)) != 0);
+    BOOST_CHECK(!versionbitscache.IsActiveAfter(lastBlock, params, dep));
 
     // Now check that we keep mining the block until the end of this period, and
     // then stop at the beginning of the next period.
-    lastBlock = secondChain.Mine((params.nMinerConfirmationWindow * 3) - 1, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip();
+    lastBlock = secondChain.Mine((period * 3) - 1, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip();
     BOOST_CHECK((versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit)) != 0);
-    lastBlock = secondChain.Mine(params.nMinerConfirmationWindow * 3, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip();
+    BOOST_CHECK(!versionbitscache.IsActiveAfter(lastBlock, params, dep));
+    lastBlock = secondChain.Mine(period * 3, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip();
 
     if (lastBlock->nHeight + 1 < min_activation_height) {
         // check signalling continues while min_activation_height is not reached
         lastBlock = secondChain.Mine(min_activation_height - 1, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip();
         BOOST_CHECK((versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit)) != 0);
+        BOOST_CHECK(!versionbitscache.IsActiveAfter(lastBlock, params, dep));
         // then reach min_activation_height, which was already REQUIRE'd to start a new period
         lastBlock = secondChain.Mine(min_activation_height, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip();
     }
 
     // Check that we don't signal after activation
     BOOST_CHECK_EQUAL(versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit), 0);
+    BOOST_CHECK(versionbitscache.IsActiveAfter(lastBlock, params, dep));
 }
 }; // struct BlockVersionTest
 
@@ -434,7 +453,7 @@ BOOST_FIXTURE_TEST_CASE(versionbits_computeblockversion, BlockVersionTest)
             // not take precedence over STARTED/LOCKED_IN. So all softforks on
             // the same bit might overlap, even when non-overlapping start-end
             // times are picked.
-            const uint32_t dep_mask{vbcache.Mask(chainParams->GetConsensus(), dep)};
+            const uint32_t dep_mask{uint32_t{1} << chainParams->GetConsensus().vDeployments[dep].bit};
             BOOST_CHECK(!(chain_all_vbits & dep_mask));
             chain_all_vbits |= dep_mask;
             check_computeblockversion(vbcache, chainParams->GetConsensus(), dep);

src/validation.cpp

@@ -2366,32 +2366,6 @@ DisconnectResult Chainstate::DisconnectBlock(const CBlock& block, const CBlockIn
     return fClean ? DISCONNECT_OK : DISCONNECT_UNCLEAN;
 }
 
-/**
- * Threshold condition checker that triggers when unknown versionbits are seen on the network.
- */
-class WarningBitsConditionChecker : public AbstractThresholdConditionChecker
-{
-private:
-    const ChainstateManager& m_chainman;
-    int m_bit;
-
-public:
-    explicit WarningBitsConditionChecker(const ChainstateManager& chainman, int bit) : m_chainman{chainman}, m_bit(bit) {}
-
-    int64_t BeginTime(const Consensus::Params& params) const override { return 0; }
-    int64_t EndTime(const Consensus::Params& params) const override { return std::numeric_limits<int64_t>::max(); }
-    int Period(const Consensus::Params& params) const override { return params.nMinerConfirmationWindow; }
-    int Threshold(const Consensus::Params& params) const override { return params.nRuleChangeActivationThreshold; }
-
-    bool Condition(const CBlockIndex* pindex, const Consensus::Params& params) const override
-    {
-        return pindex->nHeight >= params.MinBIP9WarningHeight &&
-               ((pindex->nVersion & VERSIONBITS_TOP_MASK) == VERSIONBITS_TOP_BITS) &&
-               ((pindex->nVersion >> m_bit) & 1) != 0 &&
-               ((m_chainman.m_versionbitscache.ComputeBlockVersion(pindex->pprev, params) >> m_bit) & 1) == 0;
-    }
-};
-
 static unsigned int GetBlockScriptFlags(const CBlockIndex& block_index, const ChainstateManager& chainman)
 {
     const Consensus::Params& consensusparams = chainman.GetConsensus();
@@ -3029,17 +3003,13 @@ void Chainstate::UpdateTip(const CBlockIndex* pindexNew)
 
     std::vector<bilingual_str> warning_messages;
     if (!m_chainman.IsInitialBlockDownload()) {
-        const CBlockIndex* pindex = pindexNew;
+        auto bits = m_chainman.m_versionbitscache.CheckUnknownActivations(pindexNew, m_chainman.GetParams());
-        for (int bit = 0; bit < VERSIONBITS_NUM_BITS; bit++) {
+        for (auto [bit, active] : bits) {
-            WarningBitsConditionChecker checker(m_chainman, bit);
+            const bilingual_str warning = strprintf(_("Unknown new rules activated (versionbit %i)"), bit);
-            ThresholdState state = checker.GetStateFor(pindex, m_chainman.GetConsensus(), m_chainman.m_warningcache.at(bit));
+            if (active) {
-            if (state == ThresholdState::ACTIVE || state == ThresholdState::LOCKED_IN) {
+                m_chainman.GetNotifications().warningSet(kernel::Warning::UNKNOWN_NEW_RULES_ACTIVATED, warning);
-                const bilingual_str warning = strprintf(_("Unknown new rules activated (versionbit %i)"), bit);
+            } else {
-                if (state == ThresholdState::ACTIVE) {
+                warning_messages.push_back(warning);
-                    m_chainman.GetNotifications().warningSet(kernel::Warning::UNKNOWN_NEW_RULES_ACTIVATED, warning);
-                } else {
-                    warning_messages.push_back(warning);
-                }
             }
         }
     }

src/validation.h

@@ -935,8 +935,6 @@ private:
     /** Most recent headers presync progress update, for rate-limiting. */
     MockableSteadyClock::time_point m_last_presync_update GUARDED_BY(GetMutex()){};
 
-    std::array<ThresholdConditionCache, VERSIONBITS_NUM_BITS> m_warningcache GUARDED_BY(::cs_main);
-
     //! Return true if a chainstate is considered usable.
     //!
     //! This is false when a background validation chainstate has completed its

src/versionbits.cpp

@@ -3,16 +3,33 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <consensus/params.h>
+#include <deploymentinfo.h>
+#include <kernel/chainparams.h>
 #include <util/check.h>
 #include <versionbits.h>
+#include <versionbits_impl.h>
 
-ThresholdState AbstractThresholdConditionChecker::GetStateFor(const CBlockIndex* pindexPrev, const Consensus::Params& params, ThresholdConditionCache& cache) const
+using enum ThresholdState;
+
+std::string StateName(ThresholdState state)
 {
-    int nPeriod = Period(params);
+    switch (state) {
-    int nThreshold = Threshold(params);
+    case DEFINED: return "defined";
-    int min_activation_height = MinActivationHeight(params);
+    case STARTED: return "started";
-    int64_t nTimeStart = BeginTime(params);
+    case LOCKED_IN: return "locked_in";
-    int64_t nTimeTimeout = EndTime(params);
+    case ACTIVE: return "active";
+    case FAILED: return "failed";
+    }
+    return "invalid";
+}
+
+ThresholdState AbstractThresholdConditionChecker::GetStateFor(const CBlockIndex* pindexPrev, ThresholdConditionCache& cache) const
+{
+    int nPeriod = Period();
+    int nThreshold = Threshold();
+    int min_activation_height = MinActivationHeight();
+    int64_t nTimeStart = BeginTime();
+    int64_t nTimeTimeout = EndTime();
 
     // Check if this deployment is always active.
     if (nTimeStart == Consensus::BIP9Deployment::ALWAYS_ACTIVE) {
@@ -68,7 +85,7 @@ ThresholdState AbstractThresholdConditionChecker::GetStateFor(const CBlockIndex*
                 const CBlockIndex* pindexCount = pindexPrev;
                 int count = 0;
                 for (int i = 0; i < nPeriod; i++) {
-                    if (Condition(pindexCount, params)) {
+                    if (Condition(pindexCount)) {
                         count++;
                     }
                     pindexCount = pindexCount->pprev;
@@ -99,12 +116,12 @@ ThresholdState AbstractThresholdConditionChecker::GetStateFor(const CBlockIndex*
     return state;
 }
 
-BIP9Stats AbstractThresholdConditionChecker::GetStateStatisticsFor(const CBlockIndex* pindex, const Consensus::Params& params, std::vector<bool>* signalling_blocks) const
+BIP9Stats AbstractThresholdConditionChecker::GetStateStatisticsFor(const CBlockIndex* pindex, std::vector<bool>* signalling_blocks) const
 {
     BIP9Stats stats = {};
 
-    stats.period = Period(params);
+    stats.period = Period();
-    stats.threshold = Threshold(params);
+    stats.threshold = Threshold();
 
     if (pindex == nullptr) return stats;
 
@@ -123,7 +140,7 @@ BIP9Stats AbstractThresholdConditionChecker::GetStateStatisticsFor(const CBlockI
     do {
         ++elapsed;
         --blocks_in_period;
-        if (Condition(currentIndex, params)) {
+        if (Condition(currentIndex)) {
             ++count;
             if (signalling_blocks) signalling_blocks->at(blocks_in_period) = true;
         }
@@ -137,21 +154,21 @@ BIP9Stats AbstractThresholdConditionChecker::GetStateStatisticsFor(const CBlockI
     return stats;
 }
 
-int AbstractThresholdConditionChecker::GetStateSinceHeightFor(const CBlockIndex* pindexPrev, const Consensus::Params& params, ThresholdConditionCache& cache) const
+int AbstractThresholdConditionChecker::GetStateSinceHeightFor(const CBlockIndex* pindexPrev, ThresholdConditionCache& cache) const
 {
-    int64_t start_time = BeginTime(params);
+    int64_t start_time = BeginTime();
     if (start_time == Consensus::BIP9Deployment::ALWAYS_ACTIVE || start_time == Consensus::BIP9Deployment::NEVER_ACTIVE) {
         return 0;
     }
 
-    const ThresholdState initialState = GetStateFor(pindexPrev, params, cache);
+    const ThresholdState initialState = GetStateFor(pindexPrev, cache);
 
     // BIP 9 about state DEFINED: "The genesis block is by definition in this state for each deployment."
     if (initialState == ThresholdState::DEFINED) {
         return 0;
     }
 
-    const int nPeriod = Period(params);
+    const int nPeriod = Period();
 
     // A block's state is always the same as that of the first of its period, so it is computed based on a pindexPrev whose height equals a multiple of nPeriod - 1.
     // To ease understanding of the following height calculation, it helps to remember that
@@ -163,7 +180,7 @@ int AbstractThresholdConditionChecker::GetStateSinceHeightFor(const CBlockIndex*
 
     const CBlockIndex* previousPeriodParent = pindexPrev->GetAncestor(pindexPrev->nHeight - nPeriod);
 
-    while (previousPeriodParent != nullptr && GetStateFor(previousPeriodParent, params, cache) == initialState) {
+    while (previousPeriodParent != nullptr && GetStateFor(previousPeriodParent, cache) == initialState) {
         pindexPrev = previousPeriodParent;
         previousPeriodParent = pindexPrev->GetAncestor(pindexPrev->nHeight - nPeriod);
     }
@@ -172,70 +189,99 @@ int AbstractThresholdConditionChecker::GetStateSinceHeightFor(const CBlockIndex*
     return pindexPrev->nHeight + 1;
 }
 
-namespace
+BIP9Info VersionBitsCache::Info(const CBlockIndex& block_index, const Consensus::Params& params, Consensus::DeploymentPos id)
 {
-/**
+    BIP9Info result;
- * Class to implement versionbits logic.
- */
-class VersionBitsConditionChecker : public AbstractThresholdConditionChecker {
-private:
-    const Consensus::DeploymentPos id;
 
-protected:
+    VersionBitsConditionChecker checker(params, id);
-    int64_t BeginTime(const Consensus::Params& params) const override { return params.vDeployments[id].nStartTime; }
+
-    int64_t EndTime(const Consensus::Params& params) const override { return params.vDeployments[id].nTimeout; }
+    ThresholdState current_state, next_state;
-    int MinActivationHeight(const Consensus::Params& params) const override { return params.vDeployments[id].min_activation_height; }
-    int Period(const Consensus::Params& params) const override { return params.nMinerConfirmationWindow; }
-    int Threshold(const Consensus::Params& params) const override { return params.nRuleChangeActivationThreshold; }
 
-    bool Condition(const CBlockIndex* pindex, const Consensus::Params& params) const override
     {
-        return (((pindex->nVersion & VERSIONBITS_TOP_MASK) == VERSIONBITS_TOP_BITS) && (pindex->nVersion & Mask(params)) != 0);
+        LOCK(m_mutex);
+        current_state = checker.GetStateFor(block_index.pprev, m_caches[id]);
+        next_state = checker.GetStateFor(&block_index, m_caches[id]);
+        result.since = checker.GetStateSinceHeightFor(block_index.pprev, m_caches[id]);
     }
 
-public:
+    result.current_state = StateName(current_state);
-    explicit VersionBitsConditionChecker(Consensus::DeploymentPos id_) : id(id_) {}
+    result.next_state = StateName(next_state);
-    uint32_t Mask(const Consensus::Params& params) const { return (uint32_t{1}) << params.vDeployments[id].bit; }
-};
 
-} // namespace
+    const bool has_signal = (STARTED == current_state || LOCKED_IN == current_state);
+    if (has_signal) {
+        result.stats.emplace(checker.GetStateStatisticsFor(&block_index, &result.signalling_blocks));
+        if (LOCKED_IN == current_state) {
+            result.stats->threshold = 0;
+            result.stats->possible = false;
+        }
+    }
 
-ThresholdState VersionBitsCache::State(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos)
+    if (current_state == ACTIVE) {
+        result.active_since = result.since;
+    } else if (next_state == ACTIVE) {
+        result.active_since = block_index.nHeight + 1;
+    }
+
+    return result;
+}
+
+BIP9GBTStatus VersionBitsCache::GBTStatus(const CBlockIndex& block_index, const Consensus::Params& params)
+{
+    BIP9GBTStatus result;
+
+    LOCK(m_mutex);
+    for (int i = 0; i < (int)Consensus::MAX_VERSION_BITS_DEPLOYMENTS; i++) {
+        auto pos = static_cast<Consensus::DeploymentPos>(i);
+        VersionBitsConditionChecker checker(params, pos);
+        ThresholdState state = checker.GetStateFor(&block_index, m_caches[pos]);
+        const VBDeploymentInfo& vbdepinfo = VersionBitsDeploymentInfo[pos];
+        BIP9GBTStatus::Info gbtinfo{.bit=params.vDeployments[pos].bit, .mask=checker.Mask(), .gbt_force=vbdepinfo.gbt_force};
+
+        switch (state) {
+        case DEFINED:
+        case FAILED:
+            // Not exposed to GBT
+            break;
+        case STARTED:
+            result.signalling.try_emplace(vbdepinfo.name, gbtinfo);
+            break;
+        case LOCKED_IN:
+            result.locked_in.try_emplace(vbdepinfo.name, gbtinfo);
+            break;
+        case ACTIVE:
+            result.active.try_emplace(vbdepinfo.name, gbtinfo);
+            break;
+        }
+    }
+    return result;
+}
+
+bool VersionBitsCache::IsActiveAfter(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos)
 {
     LOCK(m_mutex);
-    return VersionBitsConditionChecker(pos).GetStateFor(pindexPrev, params, m_caches[pos]);
+    return ThresholdState::ACTIVE == VersionBitsConditionChecker(params, pos).GetStateFor(pindexPrev, m_caches[pos]);
 }
 
-BIP9Stats VersionBitsCache::Statistics(const CBlockIndex* pindex, const Consensus::Params& params, Consensus::DeploymentPos pos, std::vector<bool>* signalling_blocks)
+static int32_t ComputeBlockVersion(const CBlockIndex* pindexPrev, const Consensus::Params& params, std::array<ThresholdConditionCache, Consensus::MAX_VERSION_BITS_DEPLOYMENTS>& caches)
 {
-    return VersionBitsConditionChecker(pos).GetStateStatisticsFor(pindex, params, signalling_blocks);
+    int32_t nVersion = VERSIONBITS_TOP_BITS;
-}
 
-int VersionBitsCache::StateSinceHeight(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos)
+    for (int i = 0; i < (int)Consensus::MAX_VERSION_BITS_DEPLOYMENTS; i++) {
-{
+        Consensus::DeploymentPos pos = static_cast<Consensus::DeploymentPos>(i);
-    LOCK(m_mutex);
+        VersionBitsConditionChecker checker(params, pos);
-    return VersionBitsConditionChecker(pos).GetStateSinceHeightFor(pindexPrev, params, m_caches[pos]);
+        ThresholdState state = checker.GetStateFor(pindexPrev, caches[pos]);
-}
+        if (state == ThresholdState::LOCKED_IN || state == ThresholdState::STARTED) {
+            nVersion |= checker.Mask();
+        }
+    }
 
-uint32_t VersionBitsCache::Mask(const Consensus::Params& params, Consensus::DeploymentPos pos)
+    return nVersion;
-{
-    return VersionBitsConditionChecker(pos).Mask(params);
 }
 
 int32_t VersionBitsCache::ComputeBlockVersion(const CBlockIndex* pindexPrev, const Consensus::Params& params)
 {
     LOCK(m_mutex);
-    int32_t nVersion = VERSIONBITS_TOP_BITS;
+    return ::ComputeBlockVersion(pindexPrev, params, m_caches);
-
-    for (int i = 0; i < (int)Consensus::MAX_VERSION_BITS_DEPLOYMENTS; i++) {
-        Consensus::DeploymentPos pos = static_cast<Consensus::DeploymentPos>(i);
-        ThresholdState state = VersionBitsConditionChecker(pos).GetStateFor(pindexPrev, params, m_caches[pos]);
-        if (state == ThresholdState::LOCKED_IN || state == ThresholdState::STARTED) {
-            nVersion |= Mask(params, pos);
-        }
-    }
-
-    return nVersion;
 }
 
 void VersionBitsCache::Clear()
@@ -245,3 +291,55 @@ void VersionBitsCache::Clear()
         m_caches[d].clear();
     }
 }
+
+namespace {
+/**
+ * Threshold condition checker that triggers when unknown versionbits are seen on the network.
+ */
+class WarningBitsConditionChecker : public AbstractThresholdConditionChecker
+{
+private:
+    const Consensus::Params& m_params;
+    std::array<ThresholdConditionCache, Consensus::MAX_VERSION_BITS_DEPLOYMENTS>& m_caches;
+    int m_bit;
+    int period{2016};
+    int threshold{1815}; // 90% threshold used in BIP 341
+
+public:
+    explicit WarningBitsConditionChecker(const CChainParams& chainparams, std::array<ThresholdConditionCache, Consensus::MAX_VERSION_BITS_DEPLOYMENTS>& caches, int bit)
+    : m_params{chainparams.GetConsensus()}, m_caches{caches}, m_bit(bit)
+    {
+        if (chainparams.IsTestChain()) {
+            period = chainparams.GetConsensus().DifficultyAdjustmentInterval();
+            threshold = period * 3 / 4; // 75% for test nets per BIP9 suggestion
+        }
+    }
+
+    int64_t BeginTime() const override { return 0; }
+    int64_t EndTime() const override { return std::numeric_limits<int64_t>::max(); }
+    int Period() const override { return period; }
+    int Threshold() const override { return threshold; }
+
+    bool Condition(const CBlockIndex* pindex) const override
+    {
+        return pindex->nHeight >= m_params.MinBIP9WarningHeight &&
+               ((pindex->nVersion & VERSIONBITS_TOP_MASK) == VERSIONBITS_TOP_BITS) &&
+               ((pindex->nVersion >> m_bit) & 1) != 0 &&
+               ((::ComputeBlockVersion(pindex->pprev, m_params, m_caches) >> m_bit) & 1) == 0;
+    }
+};
+} // anonymous namespace
+
+std::vector<std::pair<int, bool>> VersionBitsCache::CheckUnknownActivations(const CBlockIndex* pindex, const CChainParams& chainparams)
+{
+    LOCK(m_mutex);
+    std::vector<std::pair<int, bool>> result;
+    for (int bit = 0; bit < VERSIONBITS_NUM_BITS; ++bit) {
+        WarningBitsConditionChecker checker(chainparams, m_caches, bit);
+        ThresholdState state = checker.GetStateFor(pindex, m_warning_caches.at(bit));
+        if (state == ACTIVE || state == LOCKED_IN) {
+            result.emplace_back(bit, state == ACTIVE);
+        }
+    }
+    return result;
+}

src/versionbits.h

@@ -8,7 +8,12 @@
 #include <chain.h>
 #include <sync.h>
 
+#include <array>
 #include <map>
+#include <optional>
+#include <vector>
+
+class CChainParams;
 
 /** What block version to use for new blocks (pre versionbits) */
 static const int32_t VERSIONBITS_LAST_OLD_BLOCK_VERSION = 4;
@@ -19,18 +24,8 @@ static const int32_t VERSIONBITS_TOP_MASK = 0xE0000000UL;
 /** Total bits available for versionbits */
 static const int32_t VERSIONBITS_NUM_BITS = 29;
 
-/** BIP 9 defines a finite-state-machine to deploy a softfork in multiple stages.
+/** Opaque type for BIP9 state. See versionbits_impl.h for details. */
- *  State transitions happen during retarget period if conditions are met
+enum class ThresholdState : uint8_t;
- *  In case of reorg, transitions can go backward. Without transition, state is
- *  inherited between periods. All blocks of a period share the same state.
- */
-enum class ThresholdState {
-    DEFINED,   // First state that each softfork starts out as. The genesis block is by definition in this state for each deployment.
-    STARTED,   // For blocks past the starttime.
-    LOCKED_IN, // For at least one retarget period after the first retarget period with STARTED blocks of which at least threshold have the associated bit set in nVersion, until min_activation_height is reached.
-    ACTIVE,    // For all blocks after the LOCKED_IN retarget period (final state)
-    FAILED,    // For all blocks once the first retarget period after the timeout time is hit, if LOCKED_IN wasn't already reached (final state)
-};
 
 // A map that gives the state for blocks whose height is a multiple of Period().
 // The map is indexed by the block's parent, however, so all keys in the map
@@ -40,39 +35,40 @@ typedef std::map<const CBlockIndex*, ThresholdState> ThresholdConditionCache;
 /** Display status of an in-progress BIP9 softfork */
 struct BIP9Stats {
     /** Length of blocks of the BIP9 signalling period */
-    int period;
+    uint32_t period{0};
     /** Number of blocks with the version bit set required to activate the softfork */
-    int threshold;
+    uint32_t threshold{0};
     /** Number of blocks elapsed since the beginning of the current period */
-    int elapsed;
+    uint32_t elapsed{0};
     /** Number of blocks with the version bit set since the beginning of the current period */
-    int count;
+    uint32_t count{0};
     /** False if there are not enough blocks left in this period to pass activation threshold */
-    bool possible;
+    bool possible{false};
 };
 
-/**
+/** Detailed status of an enabled BIP9 deployment */
- * Abstract class that implements BIP9-style threshold logic, and caches results.
+struct BIP9Info {
- */
+    /** Height at which current_state started */
-class AbstractThresholdConditionChecker {
+    int since{0};
-protected:
+    /** String representing the current state */
-    virtual bool Condition(const CBlockIndex* pindex, const Consensus::Params& params) const =0;
+    std::string current_state{};
-    virtual int64_t BeginTime(const Consensus::Params& params) const =0;
+    /** String representing the next block's state */
-    virtual int64_t EndTime(const Consensus::Params& params) const =0;
+    std::string next_state{};
-    virtual int MinActivationHeight(const Consensus::Params& params) const { return 0; }
+    /** For states where signalling is applicable, information about the signalling */
-    virtual int Period(const Consensus::Params& params) const =0;
+    std::optional<BIP9Stats> stats;
-    virtual int Threshold(const Consensus::Params& params) const =0;
+    /** Which blocks signalled; empty if signalling is not applicable */
+    std::vector<bool> signalling_blocks;
+    /** Height at which the deployment is active, if known. May be in the future. */
+    std::optional<int> active_since;
+};
 
-public:
+struct BIP9GBTStatus {
-    /** Returns the numerical statistics of an in-progress BIP9 softfork in the period including pindex
+    struct Info {
-     * If provided, signalling_blocks is set to true/false based on whether each block in the period signalled
+        int bit;
-     */
+        uint32_t mask;
-    BIP9Stats GetStateStatisticsFor(const CBlockIndex* pindex, const Consensus::Params& params, std::vector<bool>* signalling_blocks = nullptr) const;
+        bool gbt_force;
-    /** Returns the state for pindex A based on parent pindexPrev B. Applies any state transition if conditions are present.
+    };
-     *  Caches state from first block of period. */
+    std::map<std::string, const Info, std::less<>> signalling, locked_in, active;
-    ThresholdState GetStateFor(const CBlockIndex* pindexPrev, const Consensus::Params& params, ThresholdConditionCache& cache) const;
-    /** Returns the height since when the ThresholdState has started for pindex A based on parent pindexPrev B, all blocks of a period share the same */
-    int GetStateSinceHeightFor(const CBlockIndex* pindexPrev, const Consensus::Params& params, ThresholdConditionCache& cache) const;
 };
 
 /** BIP 9 allows multiple softforks to be deployed in parallel. We cache
@@ -81,26 +77,25 @@ class VersionBitsCache
 {
 private:
     Mutex m_mutex;
-    ThresholdConditionCache m_caches[Consensus::MAX_VERSION_BITS_DEPLOYMENTS] GUARDED_BY(m_mutex);
+    std::array<ThresholdConditionCache,VERSIONBITS_NUM_BITS> m_warning_caches GUARDED_BY(m_mutex);
+    std::array<ThresholdConditionCache,Consensus::MAX_VERSION_BITS_DEPLOYMENTS> m_caches GUARDED_BY(m_mutex);
 
 public:
-    /** Get the numerical statistics for a given deployment for the signalling period that includes pindex.
+    BIP9Info Info(const CBlockIndex& block_index, const Consensus::Params& params, Consensus::DeploymentPos id) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
-     * If provided, signalling_blocks is set to true/false based on whether each block in the period signalled
-     */
-    static BIP9Stats Statistics(const CBlockIndex* pindex, const Consensus::Params& params, Consensus::DeploymentPos pos, std::vector<bool>* signalling_blocks = nullptr);
 
-    static uint32_t Mask(const Consensus::Params& params, Consensus::DeploymentPos pos);
+    BIP9GBTStatus GBTStatus(const CBlockIndex& block_index, const Consensus::Params& params) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
 
     /** Get the BIP9 state for a given deployment for the block after pindexPrev. */
-    ThresholdState State(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
+    bool IsActiveAfter(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
 
-    /** Get the block height at which the BIP9 deployment switched into the state for the block after pindexPrev. */
+    /** Determine what nVersion a new block should use */
-    int StateSinceHeight(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
-
-    /** Determine what nVersion a new block should use
-     */
     int32_t ComputeBlockVersion(const CBlockIndex* pindexPrev, const Consensus::Params& params) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
 
+    /** Check for unknown activations
+     *  Returns a vector containing the bit number used for signalling and a bool
+     *  indicating the deployment is likely to be ACTIVE, rather than merely LOCKED_IN. */
+    std::vector<std::pair<int,bool>> CheckUnknownActivations(const CBlockIndex* pindex, const CChainParams& chainparams) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
+
     void Clear() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
 };
 

src/versionbits_impl.h

@@ -0,0 +1,85 @@
+// Copyright (c) 2016-2022 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_VERSIONBITS_IMPL_H
+#define BITCOIN_VERSIONBITS_IMPL_H
+
+#include <chain.h>
+#include <sync.h>
+#include <versionbits.h>
+
+/** BIP 9 defines a finite-state-machine to deploy a softfork in multiple stages.
+ *  State transitions happen during retarget period if conditions are met
+ *  In case of reorg, transitions can go backward. Without transition, state is
+ *  inherited between periods. All blocks of a period share the same state.
+ */
+enum class ThresholdState : uint8_t {
+    DEFINED,   // First state that each softfork starts out as. The genesis block is by definition in this state for each deployment.
+    STARTED,   // For blocks past the starttime.
+    LOCKED_IN, // For at least one retarget period after the first retarget period with STARTED blocks of which at least threshold have the associated bit set in nVersion, until min_activation_height is reached.
+    ACTIVE,    // For all blocks after the LOCKED_IN retarget period (final state)
+    FAILED,    // For all blocks once the first retarget period after the timeout time is hit, if LOCKED_IN wasn't already reached (final state)
+};
+
+/** Get a string with the state name */
+std::string StateName(ThresholdState state);
+
+/**
+ * Abstract class that implements BIP9-style threshold logic, and caches results.
+ */
+class AbstractThresholdConditionChecker {
+protected:
+    virtual bool Condition(const CBlockIndex* pindex) const =0;
+    virtual int64_t BeginTime() const =0;
+    virtual int64_t EndTime() const =0;
+    virtual int MinActivationHeight() const { return 0; }
+    virtual int Period() const =0;
+    virtual int Threshold() const =0;
+
+public:
+    virtual ~AbstractThresholdConditionChecker() = default;
+
+    /** Returns the numerical statistics of an in-progress BIP9 softfork in the period including pindex
+     * If provided, signalling_blocks is set to true/false based on whether each block in the period signalled
+     */
+    BIP9Stats GetStateStatisticsFor(const CBlockIndex* pindex, std::vector<bool>* signalling_blocks = nullptr) const;
+    /** Returns the state for pindex A based on parent pindexPrev B. Applies any state transition if conditions are present.
+     *  Caches state from first block of period. */
+    ThresholdState GetStateFor(const CBlockIndex* pindexPrev, ThresholdConditionCache& cache) const;
+    /** Returns the height since when the ThresholdState has started for pindex A based on parent pindexPrev B, all blocks of a period share the same */
+    int GetStateSinceHeightFor(const CBlockIndex* pindexPrev, ThresholdConditionCache& cache) const;
+};
+
+/**
+ * Class to implement versionbits logic.
+ */
+class VersionBitsConditionChecker : public AbstractThresholdConditionChecker {
+private:
+    const Consensus::BIP9Deployment& dep;
+
+protected:
+    int64_t BeginTime() const override { return dep.nStartTime; }
+    int64_t EndTime() const override { return dep.nTimeout; }
+    int MinActivationHeight() const override { return dep.min_activation_height; }
+    int Period() const override { return dep.period; }
+    int Threshold() const override { return dep.threshold; }
+
+    bool Condition(const CBlockIndex* pindex) const override
+    {
+        return Condition(pindex->nVersion);
+    }
+
+public:
+    explicit VersionBitsConditionChecker(const Consensus::BIP9Deployment& dep) : dep{dep} {}
+    explicit VersionBitsConditionChecker(const Consensus::Params& params, Consensus::DeploymentPos id) : VersionBitsConditionChecker{params.vDeployments[id]} {}
+
+    uint32_t Mask() const { return (uint32_t{1}) << dep.bit; }
+
+    bool Condition(int32_t nVersion) const
+    {
+        return (((nVersion & VERSIONBITS_TOP_MASK) == VERSIONBITS_TOP_BITS) && (nVersion & Mask()) != 0);
+    }
+};
+
+#endif // BITCOIN_VERSIONBITS_IMPL_H

test/lint/lint-circular-dependencies.py

@@ -21,6 +21,7 @@ EXPECTED_CIRCULAR_DEPENDENCIES = (
     "qt/transactiontablemodel -> qt/walletmodel -> qt/transactiontablemodel",
     "wallet/wallet -> wallet/walletdb -> wallet/wallet",
     "kernel/coinstats -> validation -> kernel/coinstats",
+    "versionbits -> versionbits_impl -> versionbits",
 
     # Temporary, removed in followup https://github.com/bitcoin/bitcoin/pull/24230
     "index/base -> node/context -> net_processing -> index/blockfilterindex -> index/base",