[refactor] versionbits: make VersionBitsCache a full class

Moves the VersionBits* functions to be methods of the cache class,
and makes the cache and its lock private to the class.

7 files changed, 39 insertions, 34 deletions

diff --git a/src/deploymentstatus.h b/src/deploymentstatus.h
index 3c4f1895a1..84c5e54698 100644
--- a/src/deploymentstatus.h
+++ b/src/deploymentstatus.h
@@ -23,7 +23,7 @@ inline bool DeploymentActiveAfter(const CBlockIndex* pindexPrev, const Consensus
 inline bool DeploymentActiveAfter(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos dep)
 {
     assert(Consensus::ValidDeployment(dep));
-    return ThresholdState::ACTIVE == VersionBitsState(pindexPrev, params, dep, g_versionbitscache);
+    return ThresholdState::ACTIVE == g_versionbitscache.State(pindexPrev, params, dep);
 }
 
 /** Determine if a deployment is active for this block */
diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index 9c9a909d66..b630458f23 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -1372,7 +1372,7 @@ static void SoftForkDescPushBack(const CBlockIndex* active_chain_tip, UniValue&
     if (consensusParams.vDeployments[id].nStartTime == Consensus::BIP9Deployment::NEVER_ACTIVE) return;
 
     UniValue bip9(UniValue::VOBJ);
-    const ThresholdState thresholdState = VersionBitsState(active_chain_tip, consensusParams, id, g_versionbitscache);
+    const ThresholdState thresholdState = g_versionbitscache.State(active_chain_tip, consensusParams, id);
     switch (thresholdState) {
     case ThresholdState::DEFINED: bip9.pushKV("status", "defined"); break;
     case ThresholdState::STARTED: bip9.pushKV("status", "started"); break;
@@ -1386,12 +1386,12 @@ static void SoftForkDescPushBack(const CBlockIndex* active_chain_tip, UniValue&
     }
     bip9.pushKV("start_time", consensusParams.vDeployments[id].nStartTime);
     bip9.pushKV("timeout", consensusParams.vDeployments[id].nTimeout);
-    int64_t since_height = VersionBitsStateSinceHeight(active_chain_tip, consensusParams, id, g_versionbitscache);
+    int64_t since_height = g_versionbitscache.StateSinceHeight(active_chain_tip, consensusParams, id);
     bip9.pushKV("since", since_height);
     if (ThresholdState::STARTED == thresholdState)
     {
         UniValue statsUV(UniValue::VOBJ);
-        BIP9Stats statsStruct = VersionBitsStatistics(active_chain_tip, consensusParams, id);
+        BIP9Stats statsStruct = g_versionbitscache.Statistics(active_chain_tip, consensusParams, id);
         statsUV.pushKV("period", statsStruct.period);
         statsUV.pushKV("threshold", statsStruct.threshold);
         statsUV.pushKV("elapsed", statsStruct.elapsed);
diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
index 04aa459e11..2762d78493 100644
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -841,7 +841,7 @@ static RPCHelpMan getblocktemplate()
     UniValue vbavailable(UniValue::VOBJ);
     for (int j = 0; j < (int)Consensus::MAX_VERSION_BITS_DEPLOYMENTS; ++j) {
         Consensus::DeploymentPos pos = Consensus::DeploymentPos(j);
-        ThresholdState state = VersionBitsState(pindexPrev, consensusParams, pos, g_versionbitscache);
+        ThresholdState state = g_versionbitscache.State(pindexPrev, consensusParams, pos);
         switch (state) {
             case ThresholdState::DEFINED:
             case ThresholdState::FAILED:
@@ -849,7 +849,7 @@ static RPCHelpMan getblocktemplate()
                 break;
             case ThresholdState::LOCKED_IN:
                 // Ensure bit is set in block version
-                pblock->nVersion |= VersionBitsMask(consensusParams, pos);
+                pblock->nVersion |= g_versionbitscache.Mask(consensusParams, pos);
                 // FALL THROUGH to get vbavailable set...
             case ThresholdState::STARTED:
             {
@@ -858,7 +858,7 @@ static RPCHelpMan getblocktemplate()
                 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
-                        pblock->nVersion &= ~VersionBitsMask(consensusParams, pos);
+                        pblock->nVersion &= ~g_versionbitscache.Mask(consensusParams, pos);
                     }
                 }
                 break;
diff --git a/src/test/versionbits_tests.cpp b/src/test/versionbits_tests.cpp
index 49c61f5f2f..946b35f3b9 100644
--- a/src/test/versionbits_tests.cpp
+++ b/src/test/versionbits_tests.cpp
@@ -288,7 +288,7 @@ static void check_computeblockversion(const Consensus::Params& params, Consensus
     // Check min_activation_height is on a retarget boundary
     BOOST_REQUIRE_EQUAL(min_activation_height % params.nMinerConfirmationWindow, 0U);
 
-    const uint32_t bitmask{VersionBitsMask(params, dep)};
+    const uint32_t bitmask{g_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.
@@ -426,7 +426,7 @@ BOOST_AUTO_TEST_CASE(versionbits_computeblockversion)
             // 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{VersionBitsMask(chainParams->GetConsensus(), dep)};
+            const uint32_t dep_mask{g_versionbitscache.Mask(chainParams->GetConsensus(), dep)};
             BOOST_CHECK(!(chain_all_vbits & dep_mask));
             chain_all_vbits |= dep_mask;
             check_computeblockversion(chainParams->GetConsensus(), dep);
diff --git a/src/validation.cpp b/src/validation.cpp
index d2baa92dde..024f643be5 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1611,9 +1611,9 @@ int32_t ComputeBlockVersion(const CBlockIndex* pindexPrev, const Consensus::Para
     int32_t nVersion = VERSIONBITS_TOP_BITS;
 
     for (int i = 0; i < (int)Consensus::MAX_VERSION_BITS_DEPLOYMENTS; i++) {
-        ThresholdState state = VersionBitsState(pindexPrev, params, static_cast<Consensus::DeploymentPos>(i), g_versionbitscache);
+        ThresholdState state = g_versionbitscache.State(pindexPrev, params, static_cast<Consensus::DeploymentPos>(i));
         if (state == ThresholdState::LOCKED_IN || state == ThresholdState::STARTED) {
-            nVersion |= VersionBitsMask(params, static_cast<Consensus::DeploymentPos>(i));
+            nVersion |= g_versionbitscache.Mask(params, static_cast<Consensus::DeploymentPos>(i));
         }
     }
 
diff --git a/src/versionbits.cpp b/src/versionbits.cpp
index 07ecc93c93..0fbad0a64e 100644
--- a/src/versionbits.cpp
+++ b/src/versionbits.cpp
@@ -190,32 +190,32 @@ public:
 
 } // namespace
 
-ThresholdState VersionBitsState(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos, VersionBitsCache& cache)
+ThresholdState VersionBitsCache::State(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos)
 {
-    LOCK(cache.mutex);
-    return VersionBitsConditionChecker(pos).GetStateFor(pindexPrev, params, cache.caches[pos]);
+    LOCK(m_mutex);
+    return VersionBitsConditionChecker(pos).GetStateFor(pindexPrev, params, m_caches[pos]);
 }
 
-BIP9Stats VersionBitsStatistics(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos)
+BIP9Stats VersionBitsCache::Statistics(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos)
 {
     return VersionBitsConditionChecker(pos).GetStateStatisticsFor(pindexPrev, params);
 }
 
-int VersionBitsStateSinceHeight(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos, VersionBitsCache& cache)
+int VersionBitsCache::StateSinceHeight(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos)
 {
-    LOCK(cache.mutex);
-    return VersionBitsConditionChecker(pos).GetStateSinceHeightFor(pindexPrev, params, cache.caches[pos]);
+    LOCK(m_mutex);
+    return VersionBitsConditionChecker(pos).GetStateSinceHeightFor(pindexPrev, params, m_caches[pos]);
 }
 
-uint32_t VersionBitsMask(const Consensus::Params& params, Consensus::DeploymentPos pos)
+uint32_t VersionBitsCache::Mask(const Consensus::Params& params, Consensus::DeploymentPos pos)
 {
     return VersionBitsConditionChecker(pos).Mask(params);
 }
 
 void VersionBitsCache::Clear()
 {
-    LOCK(mutex);
+    LOCK(m_mutex);
     for (unsigned int d = 0; d < Consensus::MAX_VERSION_BITS_DEPLOYMENTS; d++) {
-        caches[d].clear();
+        m_caches[d].clear();
     }
 }
diff --git a/src/versionbits.h b/src/versionbits.h
index 24279a0de4..4ede920803 100644
--- a/src/versionbits.h
+++ b/src/versionbits.h
@@ -73,22 +73,27 @@ public:
     int GetStateSinceHeightFor(const CBlockIndex* pindexPrev, const Consensus::Params& params, ThresholdConditionCache& cache) const;
 };
 
-/** BIP 9 allows multiple softforks to be deployed in parallel. We cache per-period state for every one of them
- *  keyed by the bit position used to signal support. */
-struct VersionBitsCache
+/** BIP 9 allows multiple softforks to be deployed in parallel. We cache
+ *  per-period state for every one of them. */
+class VersionBitsCache
 {
-    Mutex mutex;
-    ThresholdConditionCache caches[Consensus::MAX_VERSION_BITS_DEPLOYMENTS] GUARDED_BY(mutex);
+private:
+    Mutex m_mutex;
+    ThresholdConditionCache m_caches[Consensus::MAX_VERSION_BITS_DEPLOYMENTS] GUARDED_BY(m_mutex);
+
+public:
+    /** Get the numerical statistics for a given deployment for the signalling period that includes the block after pindexPrev. */
+    static BIP9Stats Statistics(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos);
+
+    static uint32_t Mask(const Consensus::Params& params, Consensus::DeploymentPos pos);
+
+    /** 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);
+
+    /** Get the block height at which the BIP9 deployment switched into the state for the block after pindexPrev. */
+    int StateSinceHeight(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos);
 
     void Clear();
 };
 
-/** Get the BIP9 state for a given deployment for the block after pindexPrev. */
-ThresholdState VersionBitsState(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos, VersionBitsCache& cache);
-/** Get the numerical statistics for a given deployment for the signalling period that includes the block after pindexPrev. */
-BIP9Stats VersionBitsStatistics(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos);
-/** Get the block height at which the BIP9 deployment switched into the state for the block after pindexPrev. */
-int VersionBitsStateSinceHeight(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos, VersionBitsCache& cache);
-uint32_t VersionBitsMask(const Consensus::Params& params, Consensus::DeploymentPos pos);
-
 #endif // BITCOIN_VERSIONBITS_H