versionbits refactoring

ajtowns commented at 4:34 am on December 9, 2023: contributor

Increases the encapsulation/modularity of the versionbits code, moving more of the logic into the versionbits module rather than having it scattered across validation and rpc code. Updates unit/fuzz tests to test the actual code used rather than just a close approximation of it.

DrahtBot commented at 4:34 am on December 9, 2023: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/29039.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	TheCharlatan, darosior, achow101
Concept NACK	luke-jr
Concept ACK	naumenkogs

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#32247 (BIP-348 (OP_CHECKSIGFROMSTACK) (regtest only) by jamesob)
#31989 (BIP-119 (OP_CHECKTEMPLATEVERIFY) (regtest only) by jamesob)
#31974 (Drop testnet3 by Sjors)
#26201 (Remove Taproot activation height by Sjors)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

ajtowns renamed this:
~~202312 vbits simplify~~
versionbits refactoring
on Dec 9, 2023

ajtowns added the label Refactoring on Dec 9, 2023

ajtowns force-pushed on Dec 9, 2023

DrahtBot added the label CI failed on Dec 9, 2023

ajtowns force-pushed on Dec 9, 2023

ajtowns marked this as a draft on Dec 9, 2023

ajtowns commented at 4:59 am on December 9, 2023: contributor

At the end of this sequence of patches the VersionBitsCache object provides a standalone interface to all the “versionbits” features needed by bitcoin core, which means that if the BIP9/speedy trial implementation is changed in future, it should be possible to do that in one place, without needing to change validation/RPC code as well.

This also simplifies and modernises some of the code, much of which has been proposed previously (eg, removing the params arguments from AbstractThresholdConditionChecker was included in the first versions of #21380; making threshold a per-deployment params was part of #19573 and #21392).

After these changes, it should be possible to simplify the unit/fuzz tests a little, as they should now be able to use VersionBitsConditionChecker directly, since it no longer depends on access to Consensus::Params. (actually does this now)

ajtowns force-pushed on Dec 9, 2023

DrahtBot removed the label CI failed on Dec 9, 2023

ajtowns force-pushed on Dec 9, 2023

naumenkogs commented at 9:41 am on December 11, 2023: member

Concept ACK. Looks cleaner. Will look in more detail when you undraft it.

ajtowns force-pushed on Dec 15, 2023

luke-jr commented at 0:49 am on December 17, 2023: member

Concept NACK, refactoring without a purpose, and probably makes more work for merging BIP8.

ajtowns force-pushed on Dec 18, 2023

ajtowns force-pushed on Dec 20, 2023

ajtowns marked this as ready for review on Dec 20, 2023

DrahtBot added the label CI failed on Jan 16, 2024

ajtowns force-pushed on Feb 13, 2024

DrahtBot removed the label CI failed on Feb 13, 2024

achow101 requested review from dergoegge on Apr 9, 2024

DrahtBot added the label CI failed on Apr 20, 2024

DrahtBot removed the label CI failed on Apr 23, 2024

DrahtBot added the label CI failed on May 12, 2024

DrahtBot commented at 11:07 am on May 12, 2024: contributor

🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the documentation.

Possibly this is due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

Leave a comment here, if you need help tracking down a confusing failure.

Debug: https://github.com/bitcoin/bitcoin/runs/21508984099

DrahtBot added the label Needs rebase on May 23, 2024

ajtowns force-pushed on Aug 22, 2024

DrahtBot removed the label Needs rebase on Aug 22, 2024

ajtowns force-pushed on Aug 22, 2024

DrahtBot removed the label CI failed on Aug 22, 2024

DrahtBot added the label Needs rebase on Aug 28, 2024

ajtowns force-pushed on Aug 28, 2024

DrahtBot removed the label Needs rebase on Aug 28, 2024

DrahtBot added the label Needs rebase on Sep 2, 2024

ajtowns force-pushed on Sep 2, 2024

DrahtBot removed the label Needs rebase on Sep 3, 2024

DrahtBot added the label Needs rebase on Sep 16, 2024

ajtowns force-pushed on Oct 12, 2024

DrahtBot removed the label Needs rebase on Oct 12, 2024

Kjpl0916 approved

TheCharlatan commented at 8:59 am on December 17, 2024: contributor

Concept ACK

DrahtBot added the label Needs rebase on Dec 18, 2024

versionbits: Use std::array instead of C-style arrays 9bc41f1b48

versionbits: Remove params from AbstractThresholdConditionChecker

For an abstract class, specifying parameters in detail serves no point;
and for the concrete implementation, changing the consensus parameters
between invocations doesn't make sense. So simplify the class by removing
the consensus params from the method arguments, and just make it a member
variable in the concrete object where needed. This also allows dropping
dummy parameters from the unit/fuzz tests.

e9d617095d

ajtowns force-pushed on Dec 18, 2024

DrahtBot removed the label Needs rebase on Dec 18, 2024

ajtowns commented at 11:06 pm on December 18, 2024: contributor

Rebased. EDIT: Also updated for some of the corecheck complaints.

ajtowns force-pushed on Dec 19, 2024

ajtowns force-pushed on Dec 20, 2024

TheCharlatan commented at 2:46 pm on January 13, 2025: contributor

and probably makes more work for merging BIP8.

Is there a PR for such logic?

in src/deploymentinfo.h:21 in 9bc41f1b48 outdated

17@@ -17,7 +18,7 @@ struct VBDeploymentInfo {
18     bool gbt_force;
19 };
20 
21-extern const VBDeploymentInfo VersionBitsDeploymentInfo[Consensus::MAX_VERSION_BITS_DEPLOYMENTS];
22+extern const std::array<VBDeploymentInfo,Consensus::MAX_VERSION_BITS_DEPLOYMENTS> VersionBitsDeploymentInfo;

TheCharlatan commented at 11:05 am on January 15, 2025:

In commit 9bc41f1b48b2e0cc6abf9714e860a29989d7809c Nit: Could make this a constexpr, and rename to indicate its global nature?

ajtowns commented at 1:47 pm on January 17, 2025:

It’s extern const rather than constexpr because its defined in the cpp file, rather than the header – for constexpr stuff you need to include its value in the header.

in src/consensus/params.h:60 in dc37009c62 outdated

52@@ -53,6 +53,14 @@ struct BIP9Deployment {
53      *  boundary.
54      */
55     int min_activation_height{0};
56+    /** Period of blocks to check signalling in (usually retarget period, ie params.DifficultyAdjustmentInterval()) */
57+    uint32_t period{2016};
58+    /**
59+     * Minimum blocks including miner confirmation of the total of 2016 blocks in a retargeting period,
60+     *  which is also used for BIP9 deployments.

TheCharlatan commented at 11:23 am on January 15, 2025:

Typo nit: Extra whitespace.

in src/validation.cpp:2382 in dc37009c62 outdated

2377@@ -2378,8 +2378,20 @@ class WarningBitsConditionChecker : public AbstractThresholdConditionChecker
2378 
2379     int64_t BeginTime() const override { return 0; }
2380     int64_t EndTime() const override { return std::numeric_limits<int64_t>::max(); }
2381-    int Period() const override { return m_chainman.GetConsensus().nMinerConfirmationWindow; }
2382-    int Threshold() const override { return m_chainman.GetConsensus().nRuleChangeActivationThreshold; }
2383+    int Period() const override {
2384+        if (m_chainman.GetParams().IsTestChain()) {

TheCharlatan commented at 11:39 am on January 15, 2025:

In commit dc37009c621f3ab851e4b78e3a797cb868d1a2ed Doesn’t this now return 144 for any non-mainnet chain? Wouldn’t it be more correct to return return m_chainman.GetConsensus().vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].period;?

ajtowns commented at 5:19 pm on January 17, 2025:

Changed it to set period = chainparams.GetConsensus().DifficultyAdjustmentInterval(); which is much the same, but doesn’t rely on the TESTDUMMY deployment.

in src/validation.cpp:3009 in 6e47d20852 outdated

3013+        for (auto [bit, active] : bits) {
3014+            const bilingual_str warning = strprintf(_("Unknown new rules activated (versionbit %i)"), bit);
3015+            if (active) {
3016+                m_chainman.GetNotifications().warningSet(kernel::Warning::UNKNOWN_NEW_RULES_ACTIVATED, warning);
3017+            } else {
3018                     warning_messages.push_back(warning);

TheCharlatan commented at 12:09 pm on January 15, 2025:

Nit: Fix the indentation here?

in src/versionbits.cpp:269 in 6e47d20852 outdated

268+private:
269+    const Consensus::Params& m_params;
270+    std::array<ThresholdConditionCache, Consensus::MAX_VERSION_BITS_DEPLOYMENTS>& m_caches;
271+    int m_bit;
272+    int period{2016};
273+    int threshold{0};

TheCharlatan commented at 12:31 pm on January 15, 2025:

Why did you choose 0 here and 2016 for the period?

ajtowns commented at 2:22 pm on January 17, 2025:

Changed this, should be more obvious now.

in src/versionbits.h:91 in 6e47d20852 outdated

104@@ -102,6 +105,8 @@ class VersionBitsCache
105      */
106     int32_t ComputeBlockVersion(const CBlockIndex* pindexPrev, const Consensus::Params& params) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
107 
108+    std::vector<std::pair<int,bool>> CheckUnknownActivations(const CBlockIndex* pindex, const CChainParams& chainparams) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);

TheCharlatan commented at 12:45 pm on January 15, 2025:

Can you add a docstring here? I think the semantics of the return type should be explained.

in src/versionbits.cpp:340 in 6e47d20852 outdated

305+    LOCK(m_mutex);
306+    std::vector<std::pair<int, bool>> result;
307+    for (int bit = 0; bit < VERSIONBITS_NUM_BITS; ++bit) {
308+        WarningBitsConditionChecker checker(chainparams, m_caches, bit);
309+        ThresholdState state = checker.GetStateFor(pindex, m_warning_caches.at(bit));
310+        if (state == ACTIVE || state == LOCKED_IN) {

TheCharlatan commented at 12:53 pm on January 15, 2025:

Nit: Select ACTIVE and LOCKED_IN from the enum class name?

ajtowns commented at 4:20 pm on January 17, 2025:

Ah, a later commit had added using enum ThresholdState to avoid the need for this in new code, guess I didn’t move it quite early enough. Moved it earlier and changed the following line for consistency.

in src/versionbits.cpp:257 in af817ebb2a outdated

253@@ -247,6 +254,7 @@ BIP9Info VersionBitsCache::Info(const CBlockIndex& block_index, const Consensus:
254     return result;
255 }
256 
257+

TheCharlatan commented at 9:59 am on January 16, 2025:

In commit af817ebb2af5870c082984e41731569c090ff3fc Nit: Unneeded whitespace change.

in src/versionbits_impl.h:1 in 49afd714b4 outdated

0@@ -0,0 +1,49 @@
1+// Copyright (c) 2016-2022 The Bitcoin Core developers

TheCharlatan commented at 10:15 am on January 16, 2025:

In commit 49afd714b40115b15ea0ef146b48e9d7a403a596

Can you give some more motivation for splitting this out into its own header, especially since it seems to be introducing a new circular dependency? It also not immediately clear to me why a file called implementation only holds abstract declarations. From what I understand this is supposed to hold declarations that are only relevant for external use by tests.

ajtowns commented at 4:30 pm on January 17, 2025:

They’re implementation details that aren’t required to use the interface, but also can’t just go in the .cpp file (because of tests). addrman_impl.h is similar (though I see it’s been included in rpc/net.h because AddrInfo has been added to addrman.h’s interface without the class being moved into that file…) It’s just one module, split into three files, so the “circular dependency” is just a false positive.

TheCharlatan commented at 10:30 am on January 16, 2025: contributor

Reviewed up to the last commit. This looks very nice, I especially like the move of business logic out of the rpc into a more appropriate module. I also think the new place for m_warning_caches is much less confusing.

in src/versionbits.cpp:314 in ae37d1233a outdated

345+    explicit WarningBitsConditionChecker(const CChainParams& chainparams, std::array<ThresholdConditionCache, Consensus::MAX_VERSION_BITS_DEPLOYMENTS>& caches, int bit)
346+    : m_params{chainparams.GetConsensus()}, m_caches{caches}, m_bit(bit)
347+    {
348+        if (chainparams.IsTestChain()) {
349+            period = 144;
350+            threshold = 108;

ajtowns commented at 2:19 pm on January 17, 2025:

This period is probably too low for testnet3 which would have ~20k periods analysed and cached rather than the ~1400 that it should have with the BIP9 recommended period size of 2016.

ajtowns force-pushed on Jan 17, 2025

DrahtBot added the label CI failed on Jan 17, 2025

consensus/params: Move version bits period/threshold to bip9 param

Rather than having the rule change period/threshold be constant for all
potential deployments on a chain, have it be specific to the deployment
itself. This both matches history (BIP 9 specified a 2016 block period
and 1916 block threshold; BIP 91 specified a 336 block period and 269
block threshold; and BIP 341 specified a 2016 block period and 1815
block threshold), and allows the code to be simplified, as only the
BIP9Deployment structure is needed, not the full Consensus::Params
structure.

a679040ec1

versionbits: Change BIP9Stats to uint32_t types 5da119e5d0

versionbits: Move WarningBits logic from validation to versionbits 3bd32c2055

versionbits: Move getdeploymentinfo logic to versionbits

Rather than having the RPC code have knowledge about how BIP9 is
implemented, create a reporting function in the versionbits code, and
limit the RPC code to coverting the result of that into Univalue/JSON.

b1e967c3ec

versionbits: Move BIP9 status logic for getblocktemplate to versionbits

Rather than having the RPC code have knowledge about how BIP9 is
implemented, create a reporting function in the versionbits code, and
limit the RPC code to coverting the result of that into the appropriate
output for getblocktemplate.

1198e7d2fd

versionbits: Simplify VersionBitsCache API

Replaces State() (which returned ACTIVE/STARTED/etc) with IsActiveAfter()
which just returns a bool, as this was all State was actually used
for. Drops Mask(), which was only used in tests and can be replaced with
`1<<bit`, and also drops StateSinceHeight() and Statistics(), which are
now only used internally for Info().

37b9b67a39

versionbits: Split out internal details into impl header d00d1ed52c

versionbits: Expose StateName function

Rather than essentially duplicating StateName in the unit tests, expose
it via the impl header.

e74a7049b4

versionbits: Expose VersionBitsConditionChecker via impl header 525c00f91b

tests: refactor versionbits unit test

Base the unit test directly on `VersionBitsConditionChecker`, slightly
improving coverage, in particular adding coverage for the the logic
regarding setting the TOP_BITS.

2e4e9b9608

tests: refactor versionbits fuzz test

Test `VersionBitsConditionChecker` behaviour directly, rather than
reimplementing it, thus slightly improving fuzz test coverage of the
real code.

7565563bc7

ajtowns force-pushed on Jan 20, 2025

DrahtBot removed the label CI failed on Jan 20, 2025

in src/validation.cpp:2386 in a679040ec1 outdated

2383+    int Period() const override {
2384+        if (m_chainman.GetParams().IsTestChain()) {
2385+            return m_chainman.GetConsensus().DifficultyAdjustmentInterval();
2386+        } else {
2387+            return 2016;
2388+        }

darosior commented at 9:00 pm on March 5, 2025:

No need for the condition on test network and the seemingly-magic 2016 value here, you could just always return DifficultyAdjustmentInterval()?

ajtowns commented at 9:47 pm on March 7, 2025:

For non-testnets, the 2016 and 1815 figures WarningBitsConditionChecker in versionbits.cpp (by the end of this PR) are constrained by what we might expect to see in future from consensus change coordination signalling; while that currently matches DifficultyAdjustmentInterval, it doesn’t need to – for instance BIP 91 had a window of 336 blocks, and I think in practice would not have (did not) trigger this warning code. So I think it makes sense to leave them as constants, though perhaps some in-line documentation would help?

darosior commented at 3:40 pm on March 14, 2025:

BIP9 does not define the period as configurable, it’s always the retarget period. No strong opinion, but i find it surprising that the BIP9 warning mechanism should support (a specific type of) non-BIP9 deployments.

Sjors commented at 9:59 am on April 30, 2025:

a679040ec19ef17f3f03988a52207f1c03af701e: I find this intermediate state a bit confusing, but in the final result it looks fine to me: a default value for the period instance variable combined with a constructor that overrides it only on test networks

in src/versionbits.cpp:237 in b1e967c3ec outdated

232+    if (has_signal) {
233+        result.stats.emplace(Statistics(&block_index, params, id, &result.signalling_blocks));
234+        if (LOCKED_IN == current_state) {
235+            result.stats->threshold = 0;
236+            result.stats->possible = false;
237+        }

darosior commented at 9:17 pm on March 6, 2025:

This is a bit confusing, don’t you think it’s preferable to directly have another member in BIP9Info to convey this to the RPC command rather than (ab)using the stats’ members?

ajtowns commented at 9:50 pm on March 7, 2025:

This is just clearing those members because they don’t make sense in the LOCKED_IN state. The earlier line result.current_state = StateName(current_state) is what’s reporting the state. Make sense?

darosior commented at 6:24 pm on March 14, 2025:

I’m not talking about the state but the stats. My comment was about how in this commit you replace if (ThresholdState::LOCKED_IN != current_state) by if (info.stats->threshold > 0 || info.stats->possible) { to show the threshold and possible fields in the RPC, and this relies on these specific lines to not change behaviour. I thought the only reason you had this logic here was to convey to the RPC whether to show these fields, which is a bit convoluted. I don’t think this is worth touching at this point.

ajtowns commented at 6:51 am on March 15, 2025:

I thought the only reason you had this logic here was to convey to the RPC whether to show these fields, which is a bit convoluted. I don’t think this is worth touching at this point.

Ah, no, it’s two separate things in my head: the versionbits code quoted above is making sure the values returned in the structure are as sane as they can be, and the RPC code is taking a special case that wouldn’t make sense (threshold=0 always implies possible=true if the signalling means anything) and trying to avoid confusing people. Just reporting it always and leaving it up to the caller to ignore the data would be possible, would make the statistics object not have any optional fields.

I don’t think this is worth touching at this point.

Okay, will mark as resolved.

darosior commented at 10:23 pm on March 6, 2025: member

Concept ACK. A couple comments as i was reading through, will delve deeper soon.

in src/versionbits.h:61 in b1e967c3ec outdated

61+    bool possible{false};
62+};
63+
64+/** Detailed status of an enabled BIP9 deployment */
65+struct BIP9Info {
66+    int since{0};

TheCharlatan commented at 1:05 pm on March 13, 2025:

Doc nit: Maybe add some docstrings here, just like currently done with BIP9Stats?

ajtowns commented at 4:01 am on March 14, 2025:

Added.

in src/test/versionbits_tests.cpp:18 in 2e4e9b9608 outdated

15@@ -16,39 +16,41 @@
16 /* Define a virtual block time, one block per 10 minutes after Nov 14 2014, 0:55:36am */
17 static int32_t TestTime(int nHeight) { return 1415926536 + 600 * nHeight; }
18

TheCharlatan commented at 2:59 pm on March 13, 2025:

nit: Since corecheck is reporting IsActiveAfter as not being covered, how about (though not very sure the checks make a lot of sense there):

 0diff --git a/src/test/versionbits_tests.cpp b/src/test/versionbits_tests.cpp
 1index bcacac025d..3218ed0811 100644
 2--- a/src/test/versionbits_tests.cpp
 3+++ b/src/test/versionbits_tests.cpp
 4@@ -407,11 +407,13 @@ void check_computeblockversion(VersionBitsCache& versionbitscache, const Consens
 5         // check signalling continues while min_activation_height is not reached
 6         lastBlock = secondChain.Mine(min_activation_height - 1, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip();
 7         BOOST_CHECK((versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit)) != 0);
 8+        BOOST_CHECK(!versionbitscache.IsActiveAfter(lastBlock, params, dep));
 9         // then reach min_activation_height, which was already REQUIRE'd to start a new period
10         lastBlock = secondChain.Mine(min_activation_height, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip();
11     }
12 
13     // Check that we don't signal after activation
14+    BOOST_CHECK(versionbitscache.IsActiveAfter(lastBlock, params, dep));
15     BOOST_CHECK_EQUAL(versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit), 0);
16 }
17 }; // struct BlockVersionTest

Might also be good to similarly check the active state in the fuzz test.

ajtowns commented at 4:01 am on March 14, 2025:

Added a bunch of checks to the unit test. The fuzz tester checks at a lower level (ThresholdConditionCache vs VersionBitsCache) so that’s not really suitable.

TheCharlatan approved

TheCharlatan commented at 4:01 pm on March 13, 2025: contributor

ACK 7565563bc7a5bb98ebf03a7d6881912a74d3f302

DrahtBot requested review from naumenkogs on Mar 13, 2025

DrahtBot requested review from darosior on Mar 13, 2025

versionbits: docstrings for BIP9Info 60950f77c3

test: add IsActiveAfter tests for versionbits e3014017ba

TheCharlatan approved

TheCharlatan commented at 9:09 am on March 14, 2025: contributor

Re-ACK e3014017bacff42d8d69f3061ce1ee621aaa450a

Just addressed nits; added some more docs and tests for IsActiveAfter.

darosior approved

darosior commented at 11:24 pm on March 14, 2025: member

ACK e3014017bacff42d8d69f3061ce1ee621aaa450a

achow101 commented at 8:54 pm on April 29, 2025: member

ACK e3014017bacff42d8d69f3061ce1ee621aaa450a

refactoring without a purpose, and probably makes more work for merging BIP8.

While this currently doesn’t have much purpose other than cleaning things up, I think it will make it easier to implement deployments in the future. The test coverage improvements are also a useful change.

I think right now is the best time to merge this PR. There’s no soft fork that is in active deployment, nothing that is really relying on this code right now, and no urgency for this code to be used at this time. Thus cleaning it up and better encapsulating things makes sense right now before it conflicts with some soft fork that we’re all expecting to happen. I also think that this will make it easier to use both BIP 8 and 9 deployments.

achow101 merged this on Apr 29, 2025

achow101 closed this on Apr 29, 2025

in src/rpc/mining.cpp:568 in 1198e7d2fd outdated

564@@ -565,10 +565,10 @@ static UniValue BIP22ValidationResult(const BlockValidationState& state)
565     return "valid?";
566 }
567 
568-static std::string gbt_vb_name(const Consensus::DeploymentPos pos) {
569-    const struct VBDeploymentInfo& vbinfo = VersionBitsDeploymentInfo[pos];
570-    std::string s = vbinfo.name;
571-    if (!vbinfo.gbt_force) {
572+static std::string gbt_force_name(const std::string& name, bool gbt_force)

Sjors commented at 11:42 am on April 30, 2025:

1198e7d2fd665bf2bc49fd26773d4fd5fbc2b716: I find this new function name confusing, adding to the already confusingly named gbt_force. Opened #32386 to rename them.

Sjors referenced this in commit 378bb8c3a7 on Apr 30, 2025

Sjors referenced this in commit d04bbff7ff on Apr 30, 2025

Sjors commented at 12:33 pm on April 30, 2025: member

Post merge ACK (did not study the test and fuzz changes). Agree this was a good time to merge.

The changes in 1198e7d2fd665bf2bc49fd26773d4fd5fbc2b716 which move logic from RPC to versionbits.cpp should make it easier to implement version bit signalling support in Stratum v2 (IPC). And reminded me that I actually need to look into that.

3bd32c20550e69688a4ff02409fb34b9a637b9c4 would have been easier to follow if had been split between one commit that moves and another that modified it (adding m_caches, period, etc). But perhaps this was hard to avoid due to losing access to m_chainman.

I ran a partial IBD with -assumevalid=0 to sanity check that Taproot still activates at height 709,632.

Sjors referenced this in commit fbb7936d38 on Apr 30, 2025

Sjors referenced this in commit 5e87c3ec09 on Apr 30, 2025

versionbits refactoring #29039

Code Coverage & Benchmarks

Reviews

Conflicts