Merge #21489: fuzz: cleanups for versionbits fuzzer

aa7f418 fuzz: cleanups for versionbits fuzzer (Anthony Towns)

Pull request description:

  Followups for #21380, shouldn't change coverage. Marking as draft to avoid introducing conflicts for the speedy trial PRs.

ACKs for top commit:
  MarcoFalke:
    cr ACK aa7f418
  practicalswift:
    Tested ACK aa7f418

Tree-SHA512: 6792364e3bb036cc903b4a5f5805d00afceeae475ce84660da962d28335bd98e59d5f45e68718657d3aa526123e351edadda39e99e49f1c6cfab629e98df35ed

Author: fanquake

src/test/fuzz/versionbits.cpp

@@ -25,18 +25,18 @@ class TestConditionChecker : public AbstractThresholdConditionChecker
     const Consensus::Params dummy_params{};
 
 public:
-    const int64_t m_begin = 0;
-    const int64_t m_end = 0;
-    const int m_period = 0;
-    const int m_threshold = 0;
-    const int m_bit = 0;
+    const int64_t m_begin;
+    const int64_t m_end;
+    const int m_period;
+    const int m_threshold;
+    const int m_bit;
 
     TestConditionChecker(int64_t begin, int64_t end, int period, int threshold, int bit)
         : m_begin{begin}, m_end{end}, m_period{period}, m_threshold{threshold}, m_bit{bit}
     {
         assert(m_period > 0);
         assert(0 <= m_threshold && m_threshold <= m_period);
-        assert(0 <= m_bit && m_bit <= 32 && m_bit < VERSIONBITS_NUM_BITS);
+        assert(0 <= m_bit && m_bit < 32 && m_bit < VERSIONBITS_NUM_BITS);
     }
 
     bool Condition(const CBlockIndex* pindex, const Consensus::Params& params) const override { return Condition(pindex->nVersion); }
@@ -49,9 +49,10 @@ class TestConditionChecker : public AbstractThresholdConditionChecker
     int GetStateSinceHeightFor(const CBlockIndex* pindexPrev) const { return AbstractThresholdConditionChecker::GetStateSinceHeightFor(pindexPrev, dummy_params, m_cache); }
     BIP9Stats GetStateStatisticsFor(const CBlockIndex* pindexPrev) const { return AbstractThresholdConditionChecker::GetStateStatisticsFor(pindexPrev, dummy_params); }
 
-    bool Condition(int64_t version) const
+    bool Condition(int32_t version) const
     {
-        return ((version >> m_bit) & 1) != 0 && (version & VERSIONBITS_TOP_MASK) == VERSIONBITS_TOP_BITS;
+        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); }
@@ -94,18 +95,20 @@ class Blocks
     }
 };
 
+std::unique_ptr<const CChainParams> g_params;
+
 void initialize()
 {
-    SelectParams(CBaseChainParams::MAIN);
+    // this is actually comparatively slow, so only do it once
+    g_params = CreateChainParams(ArgsManager{}, CBaseChainParams::MAIN);
+    assert(g_params != nullptr);
 }
-} // namespace
 
-constexpr uint32_t MAX_TIME = 4102444800; // 2100-01-01
+constexpr uint32_t MAX_START_TIME = 4102444800; // 2100-01-01
 
 FUZZ_TARGET_INIT(versionbits, initialize)
 {
-    const CChainParams& params = Params();
-
+    const CChainParams& params = *g_params;
     const int64_t interval = params.GetConsensus().nPowTargetSpacing;
     assert(interval > 1); // need to be able to halve it
     assert(interval < std::numeric_limits<int32_t>::max());
@@ -122,9 +125,9 @@ FUZZ_TARGET_INIT(versionbits, initialize)
 
     // 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_TIME > interval * max_blocks);
+    assert(std::numeric_limits<uint32_t>::max() - MAX_START_TIME > interval * max_blocks);
 
-    const int64_t block_start_time = fuzzed_data_provider.ConsumeIntegralInRange<uint32_t>(params.GenesisBlock().nTime, MAX_TIME);
+    const int64_t block_start_time = fuzzed_data_provider.ConsumeIntegralInRange<uint32_t>(params.GenesisBlock().nTime, MAX_START_TIME);
 
     // what values for version will we use to signal / not signal?
     const int32_t ver_signal = fuzzed_data_provider.ConsumeIntegral<int32_t>();
@@ -173,8 +176,10 @@ FUZZ_TARGET_INIT(versionbits, initialize)
     if (checker.Condition(ver_nosignal)) return;
     if (ver_nosignal < 0) return;
 
-    // TOP_BITS should ensure version will be positive
+    // 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);
@@ -203,7 +208,7 @@ FUZZ_TARGET_INIT(versionbits, initialize)
         }
 
         // don't risk exceeding max_blocks or times may wrap around
-        if (blocks.size() + period*2 > max_blocks) break;
+        if (blocks.size() + 2 * period > max_blocks) break;
     }
     // NOTE: fuzzed_data_provider may be fully consumed at this point and should not be used further
 
@@ -316,7 +321,7 @@ FUZZ_TARGET_INIT(versionbits, initialize)
         assert(false);
     }
 
-    if (blocks.size() >= max_periods * period) {
+    if (blocks.size() >= period * max_periods) {
         // we chose the timeout (and block times) so that by the time we have this many blocks it's all over
         assert(state == ThresholdState::ACTIVE || state == ThresholdState::FAILED);
     }
@@ -343,3 +348,4 @@ FUZZ_TARGET_INIT(versionbits, initialize)
         }
     }
 }
+} // namespace