From 2254ce6b0bc0f0a0b756a81ce055e1e2ad457803 Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Sun, 17 Dec 2023 19:07:17 +1000 Subject: [PATCH] tests: refactor versionbits fuzz test Test `VersionBitsConditionChecker` behaviour directly, rather than reimplementing it, thus slightly improving fuzz test coverage of the real code. --- src/test/fuzz/versionbits.cpp | 144 +++++++++++++++------------------- src/versionbits_impl.h | 7 +- 2 files changed, 68 insertions(+), 83 deletions(-) diff --git a/src/test/fuzz/versionbits.cpp b/src/test/fuzz/versionbits.cpp index 10c88fc7872f6..5a7722d870866 100644 --- a/src/test/fuzz/versionbits.cpp +++ b/src/test/fuzz/versionbits.cpp @@ -21,43 +21,22 @@ #include namespace { -class TestConditionChecker : public AbstractThresholdConditionChecker +class TestConditionChecker : public VersionBitsConditionChecker { private: mutable ThresholdConditionCache m_cache; public: - const int64_t m_begin; - 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} + TestConditionChecker(const Consensus::BIP9Deployment& dep) : VersionBitsConditionChecker{dep} { - 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_min_activation_height); + assert(dep.period > 0); + assert(dep.threshold <= dep.period); + assert(0 <= dep.bit && dep.bit < 32 && dep.bit < VERSIONBITS_NUM_BITS); + assert(0 <= dep.min_activation_height); } - bool Condition(const CBlockIndex* pindex) const override { return Condition(pindex->nVersion); } - int64_t BeginTime() const override { return m_begin; } - int64_t EndTime() const override { return m_end; } - int Period() const override { return m_period; } - int Threshold() const override { return m_threshold; } - int MinActivationHeight() const override { return m_min_activation_height; } - ThresholdState GetStateFor(const CBlockIndex* pindexPrev) const { return AbstractThresholdConditionChecker::GetStateFor(pindexPrev, m_cache); } int GetStateSinceHeightFor(const CBlockIndex* pindexPrev) const { return AbstractThresholdConditionChecker::GetStateSinceHeightFor(pindexPrev, m_cache); } - - 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); - } }; /** Track blocks mined for test */ @@ -122,9 +101,6 @@ FUZZ_TARGET(versionbits, .init = initialize) const size_t max_periods = 16; const size_t max_blocks = 2 * period * max_periods; - const uint32_t 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::max() - MAX_START_TIME > interval * max_blocks); @@ -134,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(); const int32_t ver_nosignal = fuzzed_data_provider.ConsumeIntegral(); + if (ver_nosignal < 0) return; // negative values are uninteresting - // select deployment parameters: bit, start time, timeout - const int bit = fuzzed_data_provider.ConsumeIntegralInRange(0, VERSIONBITS_NUM_BITS - 1); - - bool always_active_test = false; - bool never_active_test = false; - 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(0, period * (max_periods - 3)); - int end_block = fuzzed_data_provider.ConsumeIntegralInRange(0, period * (max_periods - 3)); - - start_time = block_start_time + start_block * interval; - timeout = block_start_time + end_block * interval; - - // allow for times to not exactly match a block - if (fuzzed_data_provider.ConsumeBool()) start_time += interval / 2; - if (fuzzed_data_provider.ConsumeBool()) timeout += interval / 2; - } else { - if (fuzzed_data_provider.ConsumeBool()) { - start_time = Consensus::BIP9Deployment::ALWAYS_ACTIVE; - always_active_test = true; + // Now that we have chosen time and versions, setup to mine blocks + Blocks blocks(block_start_time, interval, ver_signal, ver_nosignal); + + const bool always_active_test = fuzzed_data_provider.ConsumeBool(); + const bool never_active_test = !always_active_test && fuzzed_data_provider.ConsumeBool(); + + const Consensus::BIP9Deployment dep{[&]() { + Consensus::BIP9Deployment dep; + dep.period = period; + + dep.threshold = fuzzed_data_provider.ConsumeIntegralInRange(1, period); + assert(0 < dep.threshold && dep.threshold <= dep.period); // must be able to both pass and fail threshold! + + // select deployment parameters: bit, start time, timeout + dep.bit = fuzzed_data_provider.ConsumeIntegralInRange(0, VERSIONBITS_NUM_BITS - 1); + + if (always_active_test) { + dep.nStartTime = Consensus::BIP9Deployment::ALWAYS_ACTIVE; + dep.nTimeout = fuzzed_data_provider.ConsumeBool() ? Consensus::BIP9Deployment::NO_TIMEOUT : fuzzed_data_provider.ConsumeIntegral(); + } 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(); } else { - start_time = Consensus::BIP9Deployment::NEVER_ACTIVE; - never_active_test = true; - } - timeout = fuzzed_data_provider.ConsumeBool() ? Consensus::BIP9Deployment::NO_TIMEOUT : fuzzed_data_provider.ConsumeIntegral(); - } - int min_activation = fuzzed_data_provider.ConsumeIntegralInRange(0, period * max_periods); + // 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(0, period * (max_periods - 3)); + int end_block = fuzzed_data_provider.ConsumeIntegralInRange(0, period * (max_periods - 3)); - 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(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 @@ -222,9 +202,9 @@ 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 last_signals{}; int prev_next_height = (prev == nullptr ? 0 : prev->nHeight + 1); @@ -238,7 +218,7 @@ FUZZ_TARGET(versionbits, .init = initialize) 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); @@ -255,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); @@ -269,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); @@ -303,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(current_block->GetMedianTimePast() < checker.m_end); + assert(blocks_sig < dep.threshold); + 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); } diff --git a/src/versionbits_impl.h b/src/versionbits_impl.h index 3f80c7b547d19..2a9404f0b9feb 100644 --- a/src/versionbits_impl.h +++ b/src/versionbits_impl.h @@ -70,7 +70,7 @@ class VersionBitsConditionChecker : public AbstractThresholdConditionChecker { bool Condition(const CBlockIndex* pindex) const override { - return (((pindex->nVersion & VERSIONBITS_TOP_MASK) == VERSIONBITS_TOP_BITS) && (pindex->nVersion & Mask()) != 0); + return Condition(pindex->nVersion); } public: @@ -78,6 +78,11 @@ class VersionBitsConditionChecker : public AbstractThresholdConditionChecker { 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