#35793 Implement BIP 54 (Consensus Cleanup) without mainnet activation
| Reviewability | Reviews | Agreement | Size | ||||
|---|---|---|---|---|---|---|---|
| #35793 Implement BIP 54 (Consensus Cleanup) without mainnet activation | Needs rebase | 2 | Mild | 151 + 6,433 tests | |||
|
|
|
|
| |||
Reviewability: ReadyStalePaused Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs. Size: lines added or modified outside tests, then in tests; greener = smaller. | |||||||
Goal
Implements the BIP 54 (Consensus Cleanup) validation rules across transaction verification and block header validation, enabling them exclusively on regtest via BIP 9 versionbits deployment bit 3 while setting them to never active on mainnet, testnet, and signet. The rules constrain difficulty adjustment period timestamps to block timewarp and Murch–Zawy attacks, require coinbase transactions to be timelocked to block height minus one, restrict non-witness sigops per transaction to 2,500, and invalidate 64-byte transactions to prevent Merkle tree leaf-internal node ambiguity. Over 95% of the added lines consist of unit tests, JSON test vectors, fuzz harnesses, and end-to-end functional test coverage.
Problem: Bitcoin consensus retains vulnerabilities to timewarp difficulty manipulation, negative difficulty adjustment periods, Merkle tree collisions from 64-byte transactions (CVE-2017-12842), and legacy sigops validation CPU exhaustion. Bitcoin Core currently has no implementation or test suite for the BIP 54 rules, preventing testing and evaluation of the consensus cleanup package within the repository.
Evidence of importance:
- PR description: 'Roughly 95% of the added lines are tests or test data... The bip54_tests unit test module exercises each mitigation extensively in isolation. The feature_bip54.py functional test verifies all of them end-to-end after and prior to activation.'
- Commit 25e9699b9f note on consensus behavior: 'When BIP54 is active, enforce that block transactions do not violate the BIP54 limit on the number of legacy sigops present in Scripts that get executed during block validation.'
- Commit 689f45ae12: '64-byte transactions are invalid (BIP 54) due to serious flaws in the Merkle tree algorithm that make it so that such transactions may be re-interpreted as inner tree nodes.'
Scope: Roughly 5,800 lines of the ~6,500 lines added are test vectors and JSON fixtures (such as bip54_timestamps.json and bip54_coinbases.json) and unit/functional test files.
Risk of a mistake: high (consensus, validation). A flaw in the consensus checks or versionbits deployment parameters could cause a chain split or prematurely invalidate valid blocks.
Changed since the previous assessment: Rebased without architectural changes; maintains regtest-only activation and BIP 54 validation rules while updating test adjustments.
Reviewability
Ready · Needs rebase
Conflicts with master; review the logic now, the rebase is mechanical.
Waiting on (from the thread read):
- author: author reply to review objections, suggestions, and questions from optout21 and ViniciusCestarii 2026-10-27
Agreement
Mild
objection that needs an answer has none yet (optout21)
Thread notes: Added review and suggestions from ViniciusCestarii on test assertions and timestamp calculation in feature_bip54.py.
Objections, suggestions, and questions:
| Reviewer | Type | Harm, request, or question | Status | Must be | Author replied | Quote |
|---|---|---|---|---|---|---|
| ariard (contributor) | suggestion, maintenance | Define what is understood as legacy signature ops in consensus.h | answered · author agreed to replace with "non-Segwit signature operations" | answered | yes | 2026-07-29: 'there could be a definition of what is understood as a "legacy signature ops" even if it's just echoing the bip doc' Status from: 2026-07-31: darosior: 'Ok, i can replace with "non-Segwit signature operations".' |
| ariard (contributor) | question, approach | What is the rationale for the asymmetry in legacy signature accounting for CHECKMULTISIG? | answered · author explained it reuses BIP 16 accounting to avoid adding another opcode counting method | answered | yes | 2026-07-29: 'curious if there is any rational somewhere in all the design notes, that's explaining that choice for the asymmetry in the legacy signature accounting for `CHECKMULTISIG`' Status from: 2026-08-04: darosior: 'the rationale is to reuse BIP 16 accounting, instead of introducing another opcode counting method in consensus rules.' |
| fjahr (member) | objection, correctness | Miner does not enforce Murch-Zawy rule, risking invalid block creation under BIP 54 | fixed · fix pushed · addressed by merged PR #35949 and rebase | changed | yes | 2026-08-03: 'The miner currently does not enforce the murch-zawy rule but I think it should.' Status from: 2026-09-13: darosior: 'Rebased now that #35949 is merged.' |
| l0rinc (member) | question, approach | Should the BIP text or code clarify that difficulty adjustment period constants derive from network adjustment period? | answered · author explained code already uses network params and BIP text should optimize for mainnet clarity | answered | yes | 2026-08-06: 'could we adjust the BIP text, or add a note here to clarify that these values are derived from the network's adjustment period rather than fixed constants?' Status from: 2026-09-11: darosior: 'I think the BIP should always optimize to be maximally clear for mainnet.' |
| ariard (contributor) | suggestion, correctness | Add functional test case for transaction with 2501 sigops to verify boundary check | answered · author explained sigops bounds checking is already extensively covered in unit tests | answered | yes | 2026-08-08: 'there could be a test for the case where the tx has `2501 sigops` and as such is invalid per BIP54 new rule.' Status from: 2026-08-10: darosior: 'Bound checking for sigops accounting is tested extensively in unit tests' |
| ariard (contributor) | suggestion, correctness | Check that fuzz harness covers nested control flow scripts to exercise consensus sigops parser | answered · author replied that fuzz guidance improvements for stateful logic are orthogonal to this PR | answered | yes | 2026-08-08: 'i would check the test before if it's covered though getting tortuous and valid control flow redeemScript with 10-depth OP_IF then the OP_CHECKSIG' Status from: 2026-09-11: darosior: 'What are you suggesting doing here? I agree code coverage guidance is sometimes lacking when fuzzing stateful logic.' |
| ariard (contributor) | objection, safety | Lax mempools could generate invalid block templates across the activation boundary | accepted · reviewer accepted explanation that non-standard rule enforcement and mempool sanity checks mitigate risk | answered | yes | 2026-08-08: 'in fact it's a dumb nuisance attack vector for the few transition blocks during activation if the miners are running "lax" mempools.' Status from: 2026-09-10: ariard: 'Okay if `-acceptnonstdtxs` effect starts to kickoff as short as there is a release including this code, I think this is' |
| polespinasa (member) | objection, interface | Categorizing BIP54 check failures as TX_CONSENSUS before activation misrepresents policy rejections | answered · author explained rationale in commit message and noted divergence from Inquisition is not worthwhile | answered | yes | 2026-08-24: 'This signals the state as TX_CONSENSUS even if this is a policy check. Does not affect in debug/log messages or the rejected reason message, but feels weird to miss-categorize it.' Status from: 2026-09-13: darosior: 'Sure. I gave the rationale for this decision in the commit message though, because i could not find a satisfactory place' |
| ariard (contributor) | suggestion, maintenance | Adjust consensus description comment to specify rule is only applied to testnet4 | answered · author explained constant name already has TESTNET4 suffix and cites BIP 94 | answered | yes | 2026-09-09: 'I think the description comment can be adjusted above to say it's only applied to testnet4 and not to signet, regtest, testnet3' Status from: 2026-09-13: darosior: 'The constant was renamed with a TESTNET4 a suffix with the purpose of making it clear it's only used for testnet4.' |
| ariard (contributor) | suggestion, maintenance | Define comment mathematically to clarify whether equality is included in timewarp check | fixed · fix pushed · author updated comment to positive phrasing using "superior or equal" | answered | yes | 2026-09-10: 'the comment could be "strictly inferior" which is more defined mathematically than "must not be more than 2 hours"' Status from: 2026-09-13: darosior: 'fair enough, i updated the comment to use "superior or equal" language' |
| ariard (contributor) | suggestion, maintenance | Update BLOCK_INVALID_HEADER comment in validation.h to include time too old | answered · author pointed out validation.h already contains this note | answered | yes | 2026-09-10: '`BLOCK_INVALID_HEADER` comment in `src/consensus/validation.h` could be updated to include "invalid proof of work or time too old".' Status from: 2026-09-13: darosior: 'This is already what the comment says' |
| ariard (contributor) | question, safety | Are there potential edge case issues or missing test coverage for caches and mempool reorg under the new consensus rules? | open | answered | no | 2026-09-29: 'I was musing if there can be issues with the caches, including the `disconnectpool` one with the novel consensus rules... I don't remember to have seen test coverage for all those edge cases' |
| murchandamus (member) | question, approach | Should BIP 54 activation start at height 1 rather than the genesis block? | answered · author explained that ALWAYS_ACTIVE takes priority and changing min activation height would be misleading | answered | yes | 2026-10-16: 'Given that Genesis blocks of chains are hardcoded, should it be active for the Genesis block? Wouldn’t height 1 make more sense?' Status from: 2026-10-22: darosior: 'The ALWAYS_ACTIVE start time always takes priority, so while it can't be validated on the genesis block, changing this v' |
| instagibbs (member) | question, safety | Should the assert be placed near the top of CheckContextualBlock given DEPLOYMENT_HEIGHTINCB check? | fixed · fix pushed · author moved the assert near the top of CheckContextualBlock | answered | yes | 2026-10-21: 'Since DEPLOYMENT_HEIGHTINCB check above would already violate this, do we want to do this assert near the top of CheckContextualBlock?' Status from: 2026-10-22: darosior: 'Makes sense, done.' |
| instagibbs (member) | suggestion, maintenance | Add self-documenting assert on witness-stripped serialization before returning coinbase in blocktools | answered · author declined, noting tests using it would fail anyway | answered | yes | 2026-10-21: 'self-documenting assert on witness-stripped serialization before returning the coinbase?' Status from: 2026-10-22: darosior: 'Meh, if that didn't hold, tests using this would all fail anyways.' |
| ViniciusCestarii (contributor) | suggestion, correctness | Add pre-activation mempool test coverage for legacy sigops limit rejection in feature_bip54.py | contested · reviewer pushed back that tests bypass PreChecks on mainnet before activation | answered | yes | 2026-10-21: 'Nothing covers the pre-activation mempool rejection anymore, which is the rule in effect on mainnet until activation. Maybe add a `testmempoolaccept` rejection (`bad-txns-legacy-sigops`) for the many-' Status from: 2026-10-22: ViniciusCestarii: 'Yes, on regtest, but the mutant changes behaviour on mainnet before activation and no test catches that.' |
| fametrano (none) | suggestion, maintenance | Correct description of the 61-byte legacy transaction case in txsize.json which is actually 60 bytes stripped and 64 with witness | open | answered | no | 2026-10-25: 'Nit: the `txsize.json` case "A 61-byte legacy transaction with a witness" is 60 bytes stripped and 64 with its witness' |
| fametrano (none) | suggestion, correctness | Add test in feature_bip54.py using -vbparams to catch pre-activation mempool rejection of 2501-sigop transaction on regtest | open | answered | no | 2026-10-25: 'That check could go in `feature_bip54.py`. I can share it and my other scripts as a gist.' |
| ViniciusCestarii (contributor) | suggestion, correctness | Add tests to kill 3 live mutants shifting BIP 54 enforcement by one block at activation | open | answered | no | 2026-10-27: 'I ran some mutation testing and found 3 live mutants besides https://github.com/bitcoin/bitcoin/pull/35793#discussion_r4338365979. These 3 move BIP54 enforcement by one block at activation.' |
| optout21 (contributor) | objection, interface | Commit breaks getblocktemplate calls on Regtest when consensuscleanup is not explicitly passed in rules parameter | open | answered | no | 2026-10-27: 'This commit breaks existing `getblocktemplate` calls on Regtest. With `gbt_optional_rule = false` and `ALWAYS_ACTIVE` on Regtest, a `getblocktemplate` call now has to include `"consensuscleanup"` in t' |
| optout21 (contributor) | suggestion, interface | Use bad-txns-size instead of txn-size-64 for consistency with ContextualCheckBlock | open | answered | no | 2026-10-27: 'Elsewhere (in `ContextualCheckBlock`) the value `bad-txns-size` is used. I propose using the same value here as well, and dropping `txn-size-64`, for consistency.' |
| optout21 (contributor) | suggestion, approach | Move 64-byte invalid check before IsStandardTx so validity check precedes standardness check | open | answered | no | 2026-10-27: 'Order of checks: I believe any validity check should come before standardness checks. Otherwise a tx that fails both checks is reported as non-standard, instead of the stronger invalid.' |
| optout21 (contributor) | suggestion, correctness | Add assertion in feature_bip54.py that the transaction is 64 bytes | open | answered | no | 2026-10-27: 'Nit: There is no assert here that indeed the tx is 64 bytes, I suggest adding one, like in `submit_block_64byte` above.' |
| optout21 (contributor) | question, maintenance | Is the transaction size in bip54_tests.cpp 60 bytes rather than 61? | open | answered | no | 2026-10-27: 'Minor: Isn't this 60 bytes, not 61? `INVALID_TX_NONWITNESS_SIZE - 4`' |
| optout21 (contributor) | suggestion, maintenance | Explicitly document inconsistency of mempool consensus error rejection on non-BIP54-active chains in comments, commit message, and release notes | open | answered | no | 2026-10-27: 'Unconditional consensus error at mempool check. 64-byte transactions are rejected from mempool with consensus error now, regardless of deployment (so on mainnet as well)... Nonetheless, I think it wou' |
| optout21 (contributor) | suggestion, correctness | Add test on mainnet to verify non-activation of 64-byte rule at current heights | open | answered | no | 2026-10-27: 'The 64-byte rule is tested on BIP54-active chain (RegTest) only, not on non-active chain. I recommend adding a test on mainnet, to verify non-activation on mainnet (at current heights).' |
| optout21 (contributor) | suggestion, approach | Implement a custom IsSizeInvalid method to optimize 64-byte size check | open | answered | no | 2026-10-27: 'Minor optimization observation: a custom, `IsSizeInvalid()` method could perform faster than the generic compute-size-and-compare-to-64.' |
| ViniciusCestarii (contributor) | suggestion, correctness | Add assertion in feature_bip54.py that the coinbase transaction stripped size is 64 bytes | open | answered | no | 2026-10-28: 'nit: could assert: assert_equal(len(block.vtx[0].serialize_without_witness()), 64)' |
| ViniciusCestarii (contributor) | suggestion, correctness | Adjust period 2 timestamp calculation in feature_bip54.py so difficulty adjusts as intended | open | answered | no | 2026-10-28: 'nit: this should be `start_time + 2 * 4 * interval_expected_sec`. As is, period 2's elapsed time is 0' |
Support:
- Christewart (contributor) (2026-07-24): Excited to see this work make it to this point 🎉 [not substantive]
- dergoegge (member) (Concept ACK, 2026-07-29): Concept ACK [not substantive]
- fanquake (member) (Concept ACK, 2026-07-30): Concept ACK [not substantive]
- polespinasa (member) (Concept ACK, 2026-07-30): Concept ACK [not substantive]
- theStack (member) (Concept ACK, 2026-07-30): Concept ACK [not substantive]
- hsjoberg (contributor) (Concept ACK, 2026-07-30): Concept ACK [not substantive]
- fjahr (member) (Concept ACK, 2026-08-01): Concept ACK [not substantive]
- stickies-v (member) (Concept ACK, 2026-08-07): Concept ACK [not substantive]
- nervana21 (contributor) (Concept ACK, 2026-09-21): Concept ACK [not substantive]
- 0xB10C (member) (Concept ACK, 2026-10-15): on implementing BIP 54 code and functionality here without mainnet activation to have it available
- instagibbs (member) (2026-10-21) [both]: reviewed logic against Inquisition branch, conducted extensive mutation testing across all BIP 54 rules, and found only minor issues
- ViniciusCestarii (contributor) (Concept ACK, 2026-10-21): commits are well structured so far
- instagibbs (member) (ACK, 2026-10-22) [read: CheckContextualBlock]: Minor changes only, including hte `Assert()` move in `CheckContextualBlock`
- fametrano (none) (Tested ACK, 2026-10-25) [tested]: verified sigop counting against btclib on published and custom vectors, verified size, timestamp, and coinbase vectors against BIP text, and ran unit tests, functional tests, and bip54_sigops fuzz harness
- ViniciusCestarii (contributor) (2026-10-28) [read]: Reviewed the rest of the code, looks good. Left some nits. [not substantive]
Participants:
- Christewart (contributor, support, 1 statements 2026-07-24 to 2026-07-24): excited to see the work reach this point
- ariard (contributor, objection, 16 statements 2026-07-29 to 2026-09-29): raised nuisance attack objection, multiple suggestions on tests/comments, and questions on sigop accounting and caches
- dergoegge (member, support, 1 statements 2026-07-29 to 2026-07-29): Concept ACK
- fanquake (member, support, 1 statements 2026-07-30 to 2026-07-30): Concept ACK
- hsjoberg (contributor, support, 1 statements 2026-07-30 to 2026-07-30): Concept ACK
- polespinasa (member, objection, 5 statements 2026-07-30 to 2026-09-12): Concept ACK, but objected to categorizing BIP 54 check failures as TX_CONSENSUS before activation
- theStack (member, support, 1 statements 2026-07-30 to 2026-07-30): Concept ACK
- fjahr (member, objection, 3 statements 2026-08-01 to 2026-08-11): Concept ACK, but objected that miner should enforce Murch-Zawy rule
- l0rinc (member, question, 2 statements 2026-08-06 to 2026-08-12): asked whether to clarify difficulty adjustment period constants in the BIP or comments
- stickies-v (member, support, 1 statements 2026-08-07 to 2026-08-07): Concept ACK
- nervana21 (contributor, support, 1 statements 2026-09-21 to 2026-09-21): Concept ACK
- 0xB10C (member, support, 1 statements 2026-10-15 to 2026-10-15): Concept ACK on implementing BIP 54 functionality without mainnet activation
- murchandamus (member, question, 4 statements 2026-10-16 to 2026-10-20): asked whether activation should start at height 1 instead of genesis block
- ViniciusCestarii (contributor, support, 10 statements 2026-10-21 to 2026-10-28): Concept ACK; reviewed code, reported live mutants and missing test coverage, and suggested test assertions
- instagibbs (member, support, 6 statements 2026-10-21 to 2026-10-22): ACK; reviewed logic against Inquisition branch, ran mutation testing, and suggested assertions
- fametrano (none, support, 1 statements 2026-10-25 to 2026-10-25): Tested ACK; verified sigop counting, size, timestamp, and coinbase vectors against btclib and BIP text; suggested test additions
- optout21 (contributor, objection, 10 statements 2026-10-27 to 2026-10-27): objected to getblocktemplate breakage on Regtest, and raised suggestions on 64-byte tx check ordering, error codes, and tests
Dependencies
Enables:
- Regtest testing and potential future deployment of BIP 54 on public networks
Files
6495 lines under test/bench/ci.
- src/test/data/bip54_timestamps.json +4134/-0
- src/test/bip54_tests.cpp +1608/-0
- test/functional/feature_bip54.py +417/-0
- src/test/data/bip54_coinbases.json +80/-0
- src/validation.cpp +54/-14
- src/test/fuzz/bip54.cpp +54/-0
- src/test/transaction_tests.cpp +26/-24
- src/policy/policy.cpp +1/-36
- src/consensus/tx_verify.cpp +31/-1
- src/kernel/chainparams.cpp +28/-0
- test/functional/data/invalid_txs.py +17/-3
- test/functional/p2p_segwit.py +11/-8
- src/consensus/consensus.h +16/-1
- test/functional/mempool_sigoplimit.py +8/-9
- src/test/util/transaction_utils.cpp +14/-0
- test/functional/rpc_blockchain.py +13/-0
- test/functional/feature_block.py +9/-1
- test/functional/mining_basic.py +5/-5
- src/consensus/tx_verify.h +7/-1
- test/functional/feature_taproot.py +5/-3
- src/test/fuzz/coins_view.cpp +6/-1
- src/node/miner.cpp +3/-3
- test/functional/test_framework/blocktools.py +4/-2
- test/functional/wallet_migration.py +4/-2
- src/policy/policy.h +2/-3
- test/functional/wallet_ancient_migration.py +4/-1
- src/deploymentinfo.cpp +4/-0
- test/functional/test_framework/script_util.py +2/-2
- src/test/CMakeLists.txt +3/-0
- src/test/util/transaction_utils.h +3/-0
- test/functional/rpc_getblockstats.py +3/-0
- src/CMakeLists.txt +1/-1
- src/txmempool.cpp +1/-1
- test/functional/mempool_accept.py +1/-1
- doc/bips.md +1/-0
- src/consensus/params.h +1/-0
- src/rpc/blockchain.cpp +1/-0
- src/test/fuzz/CMakeLists.txt +1/-0
- test/functional/test_runner.py +1/-0
Card
The short text the ranking pass compares across PRs.
This PR implements BIP 54 (Consensus Cleanup) validation rules in Bitcoin Core, configured as active exclusively on regtest and never active on mainnet, testnet, and signet. It mitigates quadratic legacy sigops CPU exhaustion, blocks timewarp and negative difficulty retargeting, enforces coinbase height timelocks, and rejects 64-byte transactions to prevent Merkle tree collisions. Approximately 95% of the line additions consist of test vectors, unit tests, a fuzzing target, and functional tests ported and refined from Bitcoin Inquisition. It enables testing and review of consensus cleanup logic without altering live network consensus.
Data
dossier JSON · extract JSON · model ledger, generated 2026-10-29T01:23, confidence high, input hash 60fc8295babf1312
thread read through 2026-10-28 (91 statements), head 3f0f142b1d
labels: Needs rebase
thread read through 2026-10-28 (91 statements), head 3f0f142b1d
labels: Needs rebase
Categories
Validation
P2 · DoS protection (ranking pass: Moved down slightly: regtest-only soft-fork implementation ranks below concrete bug/perf fixes, but first among the soft-fork group since it mitigates real attack classes)
Implements validation rules mitigating timewarp attacks, Merkle tree collisions (CVE-2017-12842), and legacy sigops validation CPU exhaustion. While restricted to regtest deployment without mainnet activation, providing a complete consensus specification and test suite in Core has durable strategic value for evaluating the cleanup soft fork.
Membership: Implements BIP 54 consensus validation rules in src/validation.cpp, src/consensus/tx_verify.cpp, and src/consensus/consensus.h.
Factors: security/stability 2, bug 1, performance 0, user value 0, leverage 2