From 8f651e0b206d32abd41217640ed4dd67a7fe91a1 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Mon, 29 Sep 2025 13:49:16 -0400 Subject: [PATCH 01/24] chainparams: encapsulate deployment configuration logic This encapsulates the soft fork configuration logic as set by the `-testactivationheight` (for buried deployments) and `-vbparams` (for version bits deployments) options which for the moment are regtest-only, in order to make them available on other networks as well in the next commit. Can be reviewed using git's `--color-moved` option with `--color-moved-ws=allow-indentation-change`. --- src/chainparams.cpp | 49 ++++++++++++++++++--------------- src/kernel/chainparams.cpp | 55 +++++++++++++++++++++----------------- src/kernel/chainparams.h | 26 +++++++++++------- 3 files changed, 73 insertions(+), 57 deletions(-) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index eb915b0bec3a..3490f01c93d0 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -23,29 +23,8 @@ using util::SplitString; -void ReadSigNetArgs(const ArgsManager& args, CChainParams::SigNetOptions& options) -{ - if (!args.GetArgs("-signetseednode").empty()) { - options.seeds.emplace(args.GetArgs("-signetseednode")); - } - if (!args.GetArgs("-signetchallenge").empty()) { - const auto signet_challenge = args.GetArgs("-signetchallenge"); - if (signet_challenge.size() != 1) { - throw std::runtime_error("-signetchallenge cannot be multiple values."); - } - const auto val{TryParseHex(signet_challenge[0])}; - if (!val) { - throw std::runtime_error(strprintf("-signetchallenge must be hex, not '%s'.", signet_challenge[0])); - } - options.challenge.emplace(*val); - } -} - -void ReadRegTestArgs(const ArgsManager& args, CChainParams::RegTestOptions& options) +static void HandleDeploymentArgs(const ArgsManager& args, CChainParams::DeploymentOptions& options) { - if (auto value = args.GetBoolArg("-fastprune")) options.fastprune = *value; - if (HasTestOption(args, "bip94")) options.enforce_bip94 = true; - for (const std::string& arg : args.GetArgs("-testactivationheight")) { const auto found{arg.find('@')}; if (found == std::string::npos) { @@ -107,6 +86,32 @@ void ReadRegTestArgs(const ArgsManager& args, CChainParams::RegTestOptions& opti } } +void ReadSigNetArgs(const ArgsManager& args, CChainParams::SigNetOptions& options) +{ + if (!args.GetArgs("-signetseednode").empty()) { + options.seeds.emplace(args.GetArgs("-signetseednode")); + } + if (!args.GetArgs("-signetchallenge").empty()) { + const auto signet_challenge = args.GetArgs("-signetchallenge"); + if (signet_challenge.size() != 1) { + throw std::runtime_error("-signetchallenge cannot be multiple values."); + } + const auto val{TryParseHex(signet_challenge[0])}; + if (!val) { + throw std::runtime_error(strprintf("-signetchallenge must be hex, not '%s'.", signet_challenge[0])); + } + options.challenge.emplace(*val); + } +} + +void ReadRegTestArgs(const ArgsManager& args, CChainParams::RegTestOptions& options) +{ + if (auto value = args.GetBoolArg("-fastprune")) options.fastprune = *value; + if (HasTestOption(args, "bip94")) options.enforce_bip94 = true; + + HandleDeploymentArgs(args, options.dep_opts); +} + static std::unique_ptr globalChainParams; const CChainParams &Params() { diff --git a/src/kernel/chainparams.cpp b/src/kernel/chainparams.cpp index 2cbce2c56c75..1beb15e90d49 100644 --- a/src/kernel/chainparams.cpp +++ b/src/kernel/chainparams.cpp @@ -76,6 +76,35 @@ static CBlock CreateGenesisBlock(uint32_t nTime, uint32_t nNonce, uint32_t nBits return CreateGenesisBlock(pszTimestamp, genesisOutputScript, nTime, nNonce, nBits, nVersion, genesisReward); } +void CChainParams::ApplyDeploymentOptions(const DeploymentOptions& opts) +{ + for (const auto& [dep, height] : opts.activation_heights) { + switch (dep) { + case Consensus::BuriedDeployment::DEPLOYMENT_SEGWIT: + consensus.SegwitHeight = int{height}; + break; + case Consensus::BuriedDeployment::DEPLOYMENT_HEIGHTINCB: + consensus.BIP34Height = int{height}; + break; + case Consensus::BuriedDeployment::DEPLOYMENT_DERSIG: + consensus.BIP66Height = int{height}; + break; + case Consensus::BuriedDeployment::DEPLOYMENT_CLTV: + consensus.BIP65Height = int{height}; + break; + case Consensus::BuriedDeployment::DEPLOYMENT_CSV: + consensus.CSVHeight = int{height}; + break; + } + } + + for (const auto& [deployment_pos, version_bits_params] : opts.version_bits_parameters) { + consensus.vDeployments[deployment_pos].nStartTime = version_bits_params.start_time; + consensus.vDeployments[deployment_pos].nTimeout = version_bits_params.timeout; + consensus.vDeployments[deployment_pos].min_activation_height = version_bits_params.min_activation_height; + } +} + /** * Main network on which people trade goods and services. */ @@ -561,31 +590,7 @@ class CRegTestParams : public CChainParams m_assumed_blockchain_size = 0; m_assumed_chain_state_size = 0; - for (const auto& [dep, height] : opts.activation_heights) { - switch (dep) { - case Consensus::BuriedDeployment::DEPLOYMENT_SEGWIT: - consensus.SegwitHeight = int{height}; - break; - case Consensus::BuriedDeployment::DEPLOYMENT_HEIGHTINCB: - consensus.BIP34Height = int{height}; - break; - case Consensus::BuriedDeployment::DEPLOYMENT_DERSIG: - consensus.BIP66Height = int{height}; - break; - case Consensus::BuriedDeployment::DEPLOYMENT_CLTV: - consensus.BIP65Height = int{height}; - break; - case Consensus::BuriedDeployment::DEPLOYMENT_CSV: - consensus.CSVHeight = int{height}; - break; - } - } - - for (const auto& [deployment_pos, version_bits_params] : opts.version_bits_parameters) { - consensus.vDeployments[deployment_pos].nStartTime = version_bits_params.start_time; - consensus.vDeployments[deployment_pos].nTimeout = version_bits_params.timeout; - consensus.vDeployments[deployment_pos].min_activation_height = version_bits_params.min_activation_height; - } + ApplyDeploymentOptions(opts.dep_opts); genesis = CreateGenesisBlock(1296688602, 2, 0x207fffff, 1, 50 * COIN); consensus.hashGenesisBlock = genesis.GetHash(); diff --git a/src/kernel/chainparams.h b/src/kernel/chainparams.h index 77991d497dd5..83e43a468393 100644 --- a/src/kernel/chainparams.h +++ b/src/kernel/chainparams.h @@ -118,14 +118,6 @@ class CChainParams const ChainTxData& TxData() const { return chainTxData; } - /** - * SigNetOptions holds configurations for creating a signet CChainParams. - */ - struct SigNetOptions { - std::optional> challenge{}; - std::optional> seeds{}; - }; - /** * VersionBitsParameters holds activation parameters */ @@ -135,12 +127,24 @@ class CChainParams int min_activation_height; }; + struct DeploymentOptions { + std::unordered_map version_bits_parameters{}; + std::unordered_map activation_heights{}; + }; + + /** + * SigNetOptions holds configurations for creating a signet CChainParams. + */ + struct SigNetOptions { + std::optional> challenge{}; + std::optional> seeds{}; + }; + /** * RegTestOptions holds configurations for creating a regtest CChainParams. */ struct RegTestOptions { - std::unordered_map version_bits_parameters{}; - std::unordered_map activation_heights{}; + DeploymentOptions dep_opts{}; bool fastprune{false}; bool enforce_bip94{false}; }; @@ -170,6 +174,8 @@ class CChainParams bool m_is_mockable_chain; std::vector m_assumeutxo_data; ChainTxData chainTxData; + + void ApplyDeploymentOptions(const DeploymentOptions& opts); }; std::optional GetNetworkForMagic(const MessageStartChars& pchMessageStart); From 59dd8ec5bcd70ec488e4fe0b287f9edc2fcf3fb7 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Mon, 29 Sep 2025 13:56:32 -0400 Subject: [PATCH 02/24] chainparams: make deployment configuration available on all test networks This allows unit tests to set `-testactivationheight` and `-vbparams` on all networks instead of exclusively on regtest. Those are kept test-network-only when used as startup parameters. --- src/chainparams.cpp | 32 ++++++++++++++++++++++++++------ src/chainparamsbase.cpp | 4 ++-- src/init.cpp | 10 ++++++++++ src/kernel/chainparams.cpp | 26 +++++++++++++++++--------- src/kernel/chainparams.h | 18 +++++++++++++++--- 5 files changed, 70 insertions(+), 20 deletions(-) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 3490f01c93d0..f3092fb351ea 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -86,6 +86,16 @@ static void HandleDeploymentArgs(const ArgsManager& args, CChainParams::Deployme } } +void ReadMainNetArgs(const ArgsManager& args, CChainParams::MainNetOptions& options) +{ + HandleDeploymentArgs(args, options.dep_opts); +} + +void ReadTestNetArgs(const ArgsManager& args, CChainParams::TestNetOptions& options) +{ + HandleDeploymentArgs(args, options.dep_opts); +} + void ReadSigNetArgs(const ArgsManager& args, CChainParams::SigNetOptions& options) { if (!args.GetArgs("-signetseednode").empty()) { @@ -102,6 +112,7 @@ void ReadSigNetArgs(const ArgsManager& args, CChainParams::SigNetOptions& option } options.challenge.emplace(*val); } + HandleDeploymentArgs(args, options.dep_opts); } void ReadRegTestArgs(const ArgsManager& args, CChainParams::RegTestOptions& options) @@ -122,12 +133,21 @@ const CChainParams &Params() { std::unique_ptr CreateChainParams(const ArgsManager& args, const ChainType chain) { switch (chain) { - case ChainType::MAIN: - return CChainParams::Main(); - case ChainType::TESTNET: - return CChainParams::TestNet(); - case ChainType::TESTNET4: - return CChainParams::TestNet4(); + case ChainType::MAIN: { + auto opts = CChainParams::MainNetOptions{}; + ReadMainNetArgs(args, opts); + return CChainParams::Main(opts); + } + case ChainType::TESTNET: { + auto opts = CChainParams::TestNetOptions{}; + ReadTestNetArgs(args, opts); + return CChainParams::TestNet(opts); + } + case ChainType::TESTNET4: { + auto opts = CChainParams::TestNetOptions{}; + ReadTestNetArgs(args, opts); + return CChainParams::TestNet4(opts); + } case ChainType::SIGNET: { auto opts = CChainParams::SigNetOptions{}; ReadSigNetArgs(args, opts); diff --git a/src/chainparamsbase.cpp b/src/chainparamsbase.cpp index d816d1af91c3..f62e455fac45 100644 --- a/src/chainparamsbase.cpp +++ b/src/chainparamsbase.cpp @@ -16,10 +16,10 @@ void SetupChainParamsBaseOptions(ArgsManager& argsman) argsman.AddArg("-chain=", "Use the chain (default: main). Allowed values: " LIST_CHAIN_NAMES, ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS); argsman.AddArg("-regtest", "Enter regression test mode, which uses a special chain in which blocks can be solved instantly. " "This is intended for regression testing tools and app development. Equivalent to -chain=regtest.", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS); - argsman.AddArg("-testactivationheight=name@height.", "Set the activation height of 'name' (segwit, bip34, dersig, cltv, csv). (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); + argsman.AddArg("-testactivationheight=name@height.", "Set the activation height of 'name' (segwit, bip34, dersig, cltv, csv). (test-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-testnet", "Use the testnet3 chain. Equivalent to -chain=test. Support for testnet3 is deprecated and will be removed in an upcoming release. Consider moving to testnet4 now by using -testnet4.", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS); argsman.AddArg("-testnet4", "Use the testnet4 chain. Equivalent to -chain=testnet4.", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS); - argsman.AddArg("-vbparams=deployment:start:end[:min_activation_height]", "Use given start/end times and min_activation_height for specified version bits deployment (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS); + argsman.AddArg("-vbparams=deployment:start:end[:min_activation_height]", "Use given start/end times and min_activation_height for specified version bits deployment (test-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS); argsman.AddArg("-signet", "Use the signet chain. Equivalent to -chain=signet. Note that the network is defined by the -signetchallenge parameter", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS); argsman.AddArg("-signetchallenge", "Blocks must satisfy the given script to be considered valid (only for signet networks; defaults to the global default signet test network challenge)", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::CHAINPARAMS); argsman.AddArg("-signetseednode", "Specify a seed node for the signet network, in the hostname[:port] format, e.g. sig.net:1234 (may be used multiple times to specify multiple seed nodes; defaults to the global default signet test network seed node(s))", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::CHAINPARAMS); diff --git a/src/init.cpp b/src/init.cpp index def3211ef412..779c6f2b67ca 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1089,6 +1089,16 @@ bool AppInitParameterInteraction(const ArgsManager& args) } } + // Prevent setting deployment parameters on mainnet. + if (chainparams.GetChainType() == ChainType::MAIN) { + if (args.IsArgSet("-testactivationheight")) { + return InitError(_("The -testactivationheight option may not be used on mainnet.")); + } + if (args.IsArgSet("-vbparams")) { + return InitError(_("The -vbparams option may not be used on mainnet.")); + } + } + // Also report errors from parsing before daemonization { kernel::Notifications notifications{}; diff --git a/src/kernel/chainparams.cpp b/src/kernel/chainparams.cpp index 1beb15e90d49..96aac1622ee1 100644 --- a/src/kernel/chainparams.cpp +++ b/src/kernel/chainparams.cpp @@ -110,7 +110,7 @@ void CChainParams::ApplyDeploymentOptions(const DeploymentOptions& opts) */ class CMainParams : public CChainParams { public: - CMainParams() { + CMainParams(const MainNetOptions& opts) { m_chain_type = ChainType::MAIN; consensus.signet_blocks = false; consensus.signet_challenge.clear(); @@ -147,6 +147,8 @@ class CMainParams : public CChainParams { consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].threshold = 1815; // 90% consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].period = 2016; + ApplyDeploymentOptions(opts.dep_opts); + consensus.nMinimumChainWork = uint256{"0000000000000000000000000000000000000000dee8e2a309ad8a9820433c68"}; consensus.defaultAssumeValid = uint256{"00000000000000000000611fd22f2df7c8fbd0688745c3a6c3bb5109cc2a12cb"}; // 912683 @@ -231,7 +233,7 @@ class CMainParams : public CChainParams { */ class CTestNetParams : public CChainParams { public: - CTestNetParams() { + CTestNetParams(const TestNetOptions& opts) { m_chain_type = ChainType::TESTNET; consensus.signet_blocks = false; consensus.signet_challenge.clear(); @@ -266,6 +268,8 @@ class CTestNetParams : public CChainParams { consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].threshold = 1512; // 75% consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].period = 2016; + ApplyDeploymentOptions(opts.dep_opts); + consensus.nMinimumChainWork = uint256{"0000000000000000000000000000000000000000000016dd270dd94fac1d7632"}; consensus.defaultAssumeValid = uint256{"0000000000000065c6c38258e201971a3fdfcc2ceee0dd6e85a6c022d45dee34"}; // 4550000 @@ -328,7 +332,7 @@ class CTestNetParams : public CChainParams { */ class CTestNet4Params : public CChainParams { public: - CTestNet4Params() { + CTestNet4Params(const TestNetOptions& opts) { m_chain_type = ChainType::TESTNET4; consensus.signet_blocks = false; consensus.signet_challenge.clear(); @@ -362,6 +366,8 @@ class CTestNet4Params : public CChainParams { consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].threshold = 1512; // 75% consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].period = 2016; + ApplyDeploymentOptions(opts.dep_opts); + consensus.nMinimumChainWork = uint256{"00000000000000000000000000000000000000000000034a4690fe592dc49c7c"}; consensus.defaultAssumeValid = uint256{"000000000000000180a58e7fa3b0db84b5ea76377524894f53660d93ac839d9b"}; // 91000 @@ -501,6 +507,8 @@ class SigNetParams : public CChainParams { consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].threshold = 1815; // 90% consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].period = 2016; + ApplyDeploymentOptions(options.dep_opts); + // message start is defined as the first 4 bytes of the sha256d of the block script HashWriter h{}; h << consensus.signet_challenge; @@ -653,19 +661,19 @@ std::unique_ptr CChainParams::RegTest(const RegTestOptions& return std::make_unique(options); } -std::unique_ptr CChainParams::Main() +std::unique_ptr CChainParams::Main(const MainNetOptions& options) { - return std::make_unique(); + return std::make_unique(options); } -std::unique_ptr CChainParams::TestNet() +std::unique_ptr CChainParams::TestNet(const TestNetOptions& options) { - return std::make_unique(); + return std::make_unique(options); } -std::unique_ptr CChainParams::TestNet4() +std::unique_ptr CChainParams::TestNet4(const TestNetOptions& options) { - return std::make_unique(); + return std::make_unique(options); } std::vector CChainParams::GetAvailableSnapshotHeights() const diff --git a/src/kernel/chainparams.h b/src/kernel/chainparams.h index 83e43a468393..bdfc8baf2c14 100644 --- a/src/kernel/chainparams.h +++ b/src/kernel/chainparams.h @@ -136,6 +136,7 @@ class CChainParams * SigNetOptions holds configurations for creating a signet CChainParams. */ struct SigNetOptions { + DeploymentOptions dep_opts{}; std::optional> challenge{}; std::optional> seeds{}; }; @@ -149,11 +150,22 @@ class CChainParams bool enforce_bip94{false}; }; + struct MainNetOptions { + DeploymentOptions dep_opts{}; + }; + + struct TestNetOptions { + DeploymentOptions dep_opts{}; + }; + static std::unique_ptr RegTest(const RegTestOptions& options); static std::unique_ptr SigNet(const SigNetOptions& options); - static std::unique_ptr Main(); - static std::unique_ptr TestNet(); - static std::unique_ptr TestNet4(); + static std::unique_ptr Main(const MainNetOptions& options); + static std::unique_ptr Main() { const MainNetOptions opts{}; return Main(opts); } + static std::unique_ptr TestNet(const TestNetOptions& options); + static std::unique_ptr TestNet() { const TestNetOptions opts{}; return TestNet(opts); } + static std::unique_ptr TestNet4(const TestNetOptions& options); + static std::unique_ptr TestNet4() { const TestNetOptions opts{}; return TestNet4(opts); } protected: CChainParams() = default; From feeef2f164e86e76e7a9ff27eb679cb40e51c761 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Thu, 30 Apr 2026 17:04:41 -0400 Subject: [PATCH 03/24] qa: use NORMAL_GBT_REQUEST_PARAMS consistently Some functional tests were still hardcoding parameters. Using the constant allows to change the rules in a single place if necessary. --- test/functional/feature_segwit.py | 7 ++++--- test/functional/mining_basic.py | 2 +- test/functional/mining_getblocktemplate_longpoll.py | 9 +++++---- test/functional/mining_prioritisetransaction.py | 5 +++-- test/functional/mining_template_verification.py | 7 ++++--- 5 files changed, 17 insertions(+), 13 deletions(-) diff --git a/test/functional/feature_segwit.py b/test/functional/feature_segwit.py index 80266aa2e728..fb077c953994 100755 --- a/test/functional/feature_segwit.py +++ b/test/functional/feature_segwit.py @@ -11,6 +11,7 @@ script_to_p2wsh, ) from test_framework.blocktools import ( + NORMAL_GBT_REQUEST_PARAMS, send_to_witness, witness_script, ) @@ -116,7 +117,7 @@ def run_test(self): self.log.info("Verify sigops are counted in GBT with pre-BIP141 rules before the fork") txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1) - tmpl = self.nodes[0].getblocktemplate({'rules': ['segwit']}) + tmpl = self.nodes[0].getblocktemplate(NORMAL_GBT_REQUEST_PARAMS) assert_equal(tmpl['sizelimit'], 1000000) assert 'weightlimit' not in tmpl assert_equal(tmpl['sigoplimit'], 20000) @@ -228,7 +229,7 @@ def run_test(self): self.log.info("Verify sigops are counted in GBT with BIP141 rules after the fork") txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1) raw_tx = self.nodes[0].getrawtransaction(txid, True) - tmpl = self.nodes[0].getblocktemplate({'rules': ['segwit']}) + tmpl = self.nodes[0].getblocktemplate(NORMAL_GBT_REQUEST_PARAMS) assert_greater_than_or_equal(tmpl['sizelimit'], 3999577) # actual maximum size is lower due to minimum mandatory non-witness data assert_equal(tmpl['weightlimit'], 4000000) assert_equal(tmpl['sigoplimit'], 80000) @@ -282,7 +283,7 @@ def run_test(self): assert txid3 in self.nodes[0].getrawmempool() # Check that getblocktemplate includes all transactions. - template = self.nodes[0].getblocktemplate({"rules": ["segwit"]}) + template = self.nodes[0].getblocktemplate(NORMAL_GBT_REQUEST_PARAMS) template_txids = [t['txid'] for t in template['transactions']] assert txid1 in template_txids assert txid2 in template_txids diff --git a/test/functional/mining_basic.py b/test/functional/mining_basic.py index 7e71761ae654..409942b60092 100755 --- a/test/functional/mining_basic.py +++ b/test/functional/mining_basic.py @@ -230,7 +230,7 @@ def test_timewarp(self): assert_equal(node.getblocktemplate(template_request={ 'data': block.serialize().hex(), 'mode': 'proposal', - 'rules': ['segwit'], + **NORMAL_GBT_REQUEST_PARAMS, }), None) bad_block = copy.deepcopy(block) diff --git a/test/functional/mining_getblocktemplate_longpoll.py b/test/functional/mining_getblocktemplate_longpoll.py index 2d15151e6508..b8fde5f2b41f 100755 --- a/test/functional/mining_getblocktemplate_longpoll.py +++ b/test/functional/mining_getblocktemplate_longpoll.py @@ -7,6 +7,7 @@ import random import threading +from test_framework.blocktools import NORMAL_GBT_REQUEST_PARAMS from test_framework.test_framework import BitcoinTestFramework from test_framework.util import get_rpc_proxy from test_framework.wallet import MiniWallet @@ -16,14 +17,14 @@ class LongpollThread(threading.Thread): def __init__(self, node): threading.Thread.__init__(self) # query current longpollid - template = node.getblocktemplate({'rules': ['segwit']}) + template = node.getblocktemplate(NORMAL_GBT_REQUEST_PARAMS) self.longpollid = template['longpollid'] # create a new connection to the node, we can't use the same # connection from two threads self.node = get_rpc_proxy(node.url, 1, timeout=600, coveragedir=node.coverage_dir) def run(self): - self.node.getblocktemplate({'longpollid': self.longpollid, 'rules': ['segwit']}) + self.node.getblocktemplate({'longpollid': self.longpollid, **NORMAL_GBT_REQUEST_PARAMS}) class GetBlockTemplateLPTest(BitcoinTestFramework): def set_test_params(self): @@ -34,9 +35,9 @@ def run_test(self): self.log.info("Warning: this test will take about 70 seconds in the best case. Be patient.") self.log.info("Test that longpollid doesn't change between successive getblocktemplate() invocations if nothing else happens") self.generate(self.nodes[0], 10) - template = self.nodes[0].getblocktemplate({'rules': ['segwit']}) + template = self.nodes[0].getblocktemplate(NORMAL_GBT_REQUEST_PARAMS) longpollid = template['longpollid'] - template2 = self.nodes[0].getblocktemplate({'rules': ['segwit']}) + template2 = self.nodes[0].getblocktemplate(NORMAL_GBT_REQUEST_PARAMS) assert template2['longpollid'] == longpollid self.log.info("Test that longpoll waits if we do nothing") diff --git a/test/functional/mining_prioritisetransaction.py b/test/functional/mining_prioritisetransaction.py index a8646d491814..4bbcdc7e938d 100755 --- a/test/functional/mining_prioritisetransaction.py +++ b/test/functional/mining_prioritisetransaction.py @@ -7,6 +7,7 @@ from decimal import Decimal import time +from test_framework.blocktools import NORMAL_GBT_REQUEST_PARAMS from test_framework.messages import ( COIN, MAX_BLOCK_WEIGHT, @@ -293,14 +294,14 @@ def run_test(self): # getblocktemplate to (eventually) return a new block. mock_time = int(time.time()) self.nodes[0].setmocktime(mock_time) - template = self.nodes[0].getblocktemplate({'rules': ['segwit']}) + template = self.nodes[0].getblocktemplate(NORMAL_GBT_REQUEST_PARAMS) self.nodes[0].prioritisetransaction(txid=tx_id, fee_delta=-int(self.relayfee*COIN)) # Calling prioritisetransaction with the inverse amount should delete its prioritisation entry assert tx_id not in self.nodes[0].getprioritisedtransactions() self.nodes[0].setmocktime(mock_time+10) - new_template = self.nodes[0].getblocktemplate({'rules': ['segwit']}) + new_template = self.nodes[0].getblocktemplate(NORMAL_GBT_REQUEST_PARAMS) assert_not_equal(template, new_template) diff --git a/test/functional/mining_template_verification.py b/test/functional/mining_template_verification.py index de0833c596df..616fdc943c9b 100755 --- a/test/functional/mining_template_verification.py +++ b/test/functional/mining_template_verification.py @@ -15,6 +15,7 @@ create_block, create_coinbase, add_witness_commitment, + NORMAL_GBT_REQUEST_PARAMS, ) from test_framework.test_framework import BitcoinTestFramework @@ -39,7 +40,7 @@ def assert_template(node, block, expect, *, rehash=True, submit=True, solve=True rsp = node.getblocktemplate(template_request={ 'data': block.serialize().hex(), 'mode': 'proposal', - 'rules': ['segwit'], + **NORMAL_GBT_REQUEST_PARAMS, }) assert_equal(rsp, expect) # Only attempt to submit invalid templates @@ -82,7 +83,7 @@ def truncated_final_transaction_test(self, node, block): template_request={ "data": block.serialize()[:-1].hex(), "mode": "proposal", - "rules": ["segwit"], + **NORMAL_GBT_REQUEST_PARAMS, } ) @@ -115,7 +116,7 @@ def bad_tx_count_test(self, node, block): assert_raises_rpc_error(-22, "Block decode failed", node.getblocktemplate, { 'data': bad_block_sn.hex(), 'mode': 'proposal', - 'rules': ['segwit'], + **NORMAL_GBT_REQUEST_PARAMS, }) def nbits_test(self, node, block): From 92121a489c454547f5649518632f772b6dabbdfc Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Tue, 14 Oct 2025 16:22:42 -0400 Subject: [PATCH 04/24] ======= Consensus Cleanup BEGINS HERE ======= Prior commits are preparatory work. Following commits is the implementation of BIP54. From 608d43ac1bdf6994f5e170e1602cf15afad43011 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Thu, 30 Apr 2026 17:11:12 -0400 Subject: [PATCH 05/24] chainparams: add versionbits deployment for BIP 54 --- src/consensus/params.h | 1 + src/deploymentinfo.cpp | 4 +++ src/kernel/chainparams.cpp | 28 ++++++++++++++++++++ src/rpc/blockchain.cpp | 1 + test/functional/rpc_blockchain.py | 13 +++++++++ test/functional/test_framework/blocktools.py | 2 +- 6 files changed, 48 insertions(+), 1 deletion(-) diff --git a/src/consensus/params.h b/src/consensus/params.h index 6344349b8661..cdab7d6f40a0 100644 --- a/src/consensus/params.h +++ b/src/consensus/params.h @@ -33,6 +33,7 @@ constexpr bool ValidDeployment(BuriedDeployment dep) { return dep <= DEPLOYMENT_ enum DeploymentPos : uint16_t { DEPLOYMENT_TESTDUMMY, DEPLOYMENT_TAPROOT, // Deployment of Schnorr/Taproot (BIPs 340-342) + DEPLOYMENT_CONSENSUSCLEANUP, // Deployment of BIP 54 // NOTE: Also add new deployments to VersionBitsDeploymentInfo in deploymentinfo.cpp MAX_VERSION_BITS_DEPLOYMENTS }; diff --git a/src/deploymentinfo.cpp b/src/deploymentinfo.cpp index 5c4795505be0..32977052b444 100644 --- a/src/deploymentinfo.cpp +++ b/src/deploymentinfo.cpp @@ -17,6 +17,10 @@ const std::array Versi .name = "taproot", .gbt_optional_rule = true, }, + VBDeploymentInfo{ + .name = "consensuscleanup", + .gbt_optional_rule = false, + }, }; std::string DeploymentName(Consensus::BuriedDeployment dep) diff --git a/src/kernel/chainparams.cpp b/src/kernel/chainparams.cpp index 96aac1622ee1..5a7feb594c7b 100644 --- a/src/kernel/chainparams.cpp +++ b/src/kernel/chainparams.cpp @@ -147,6 +147,11 @@ class CMainParams : public CChainParams { consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].threshold = 1815; // 90% consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].period = 2016; + // Deployment of the Consensus Cleanup (BIP 54) + consensus.vDeployments[Consensus::DEPLOYMENT_CONSENSUSCLEANUP].bit = 3; + consensus.vDeployments[Consensus::DEPLOYMENT_CONSENSUSCLEANUP].nStartTime = Consensus::BIP9Deployment::NEVER_ACTIVE; + consensus.vDeployments[Consensus::DEPLOYMENT_CONSENSUSCLEANUP].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT; + ApplyDeploymentOptions(opts.dep_opts); consensus.nMinimumChainWork = uint256{"0000000000000000000000000000000000000000dee8e2a309ad8a9820433c68"}; @@ -268,6 +273,11 @@ class CTestNetParams : public CChainParams { consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].threshold = 1512; // 75% consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].period = 2016; + // Deployment of the Consensus Cleanup (BIP 54) + consensus.vDeployments[Consensus::DEPLOYMENT_CONSENSUSCLEANUP].bit = 3; + consensus.vDeployments[Consensus::DEPLOYMENT_CONSENSUSCLEANUP].nStartTime = Consensus::BIP9Deployment::NEVER_ACTIVE; + consensus.vDeployments[Consensus::DEPLOYMENT_CONSENSUSCLEANUP].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT; + ApplyDeploymentOptions(opts.dep_opts); consensus.nMinimumChainWork = uint256{"0000000000000000000000000000000000000000000016dd270dd94fac1d7632"}; @@ -366,6 +376,11 @@ class CTestNet4Params : public CChainParams { consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].threshold = 1512; // 75% consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].period = 2016; + // Deployment of the Consensus Cleanup (BIP 54) + consensus.vDeployments[Consensus::DEPLOYMENT_CONSENSUSCLEANUP].bit = 3; + consensus.vDeployments[Consensus::DEPLOYMENT_CONSENSUSCLEANUP].nStartTime = Consensus::BIP9Deployment::NEVER_ACTIVE; + consensus.vDeployments[Consensus::DEPLOYMENT_CONSENSUSCLEANUP].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT; + ApplyDeploymentOptions(opts.dep_opts); consensus.nMinimumChainWork = uint256{"00000000000000000000000000000000000000000000034a4690fe592dc49c7c"}; @@ -507,6 +522,11 @@ class SigNetParams : public CChainParams { consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].threshold = 1815; // 90% consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].period = 2016; + // Deployment of the Consensus Cleanup (BIP 54) + consensus.vDeployments[Consensus::DEPLOYMENT_CONSENSUSCLEANUP].bit = 3; + consensus.vDeployments[Consensus::DEPLOYMENT_CONSENSUSCLEANUP].nStartTime = Consensus::BIP9Deployment::NEVER_ACTIVE; + consensus.vDeployments[Consensus::DEPLOYMENT_CONSENSUSCLEANUP].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT; + ApplyDeploymentOptions(options.dep_opts); // message start is defined as the first 4 bytes of the sha256d of the block script @@ -586,6 +606,14 @@ class CRegTestParams : public CChainParams consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].threshold = 108; // 75% consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].period = 144; + // Deployment of the Consensus Cleanup (BIP 54) + consensus.vDeployments[Consensus::DEPLOYMENT_CONSENSUSCLEANUP].bit = 3; + consensus.vDeployments[Consensus::DEPLOYMENT_CONSENSUSCLEANUP].nStartTime = Consensus::BIP9Deployment::ALWAYS_ACTIVE; + consensus.vDeployments[Consensus::DEPLOYMENT_CONSENSUSCLEANUP].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT; + consensus.vDeployments[Consensus::DEPLOYMENT_CONSENSUSCLEANUP].min_activation_height = 0; // No activation delay + consensus.vDeployments[Consensus::DEPLOYMENT_CONSENSUSCLEANUP].threshold = 108; // 75% + consensus.vDeployments[Consensus::DEPLOYMENT_CONSENSUSCLEANUP].period = 144; + consensus.nMinimumChainWork = uint256{}; consensus.defaultAssumeValid = uint256{}; diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index a20affafee74..04bba4c6ec06 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1454,6 +1454,7 @@ UniValue DeploymentInfo(const CBlockIndex* blockindex, const ChainstateManager& SoftForkDescPushBack(blockindex, softforks, chainman, Consensus::DEPLOYMENT_SEGWIT); SoftForkDescPushBack(blockindex, softforks, chainman, Consensus::DEPLOYMENT_TESTDUMMY); SoftForkDescPushBack(blockindex, softforks, chainman, Consensus::DEPLOYMENT_TAPROOT); + SoftForkDescPushBack(blockindex, softforks, chainman, Consensus::DEPLOYMENT_CONSENSUSCLEANUP); return softforks; } } // anon namespace diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py index 84fa8ebccf13..9bc8bef9c2d2 100755 --- a/test/functional/rpc_blockchain.py +++ b/test/functional/rpc_blockchain.py @@ -252,6 +252,19 @@ def check_signalling_deploymentinfo_result(self, gdi_result, height, blockhash, }, 'height': 0, 'active': True + }, + 'consensuscleanup': { + 'type': 'bip9', + 'bip9': { + 'start_time': -1, + 'timeout': 9223372036854775807, + 'min_activation_height': 0, + 'status': 'active', + 'status_next': 'active', + 'since': 0, + }, + 'height': 0, + 'active': True } } }) diff --git a/test/functional/test_framework/blocktools.py b/test/functional/test_framework/blocktools.py index eb1d3b0542b4..b5073b96e31a 100644 --- a/test/functional/test_framework/blocktools.py +++ b/test/functional/test_framework/blocktools.py @@ -63,7 +63,7 @@ # From BIP141 WITNESS_COMMITMENT_HEADER = b"\xaa\x21\xa9\xed" -NORMAL_GBT_REQUEST_PARAMS = {"rules": ["segwit"]} +NORMAL_GBT_REQUEST_PARAMS = {"rules": ["segwit", "consensuscleanup"]} VERSIONBITS_LAST_OLD_BLOCK_VERSION = 4 MIN_BLOCKS_TO_KEEP = 288 From 41d20c4d31292f3a55a0ee97ba508dafe96ce532 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Tue, 20 Jan 2026 16:38:25 -0500 Subject: [PATCH 06/24] scripted-diff: rename MAX_TX_LEGACY_SIGOPS to MAX_TX_BIP54_SIGOPS BIP54 counts sigops differently from existing sigops-based checks. Since we are overloading the sigops term, make clear the constant refers to BIP54-sigops, not other kinds of pre-existing sigops. -BEGIN VERIFY SCRIPT- sed -i 's/MAX_TX_LEGACY_SIGOPS/MAX_TX_BIP54_SIGOPS/g' $(git grep -l MAX_TX_LEGACY_SIGOPS src/) -END VERIFY SCRIPT- --- src/policy/policy.cpp | 2 +- src/policy/policy.h | 2 +- src/test/transaction_tests.cpp | 10 +++++----- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/policy/policy.cpp b/src/policy/policy.cpp index 3da6cb7489b1..33328a28e3e3 100644 --- a/src/policy/policy.cpp +++ b/src/policy/policy.cpp @@ -184,7 +184,7 @@ static bool CheckSigopsBIP54(const CTransaction& tx, const CCoinsViewCache& inpu sigops += txin.scriptSig.GetSigOpCount(/*fAccurate=*/true); sigops += prev_txo.scriptPubKey.GetSigOpCount(txin.scriptSig); - if (sigops > MAX_TX_LEGACY_SIGOPS) { + if (sigops > MAX_TX_BIP54_SIGOPS) { return false; } } diff --git a/src/policy/policy.h b/src/policy/policy.h index 23993dd705e5..780899e76d64 100644 --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -39,7 +39,7 @@ static constexpr unsigned int MAX_P2SH_SIGOPS{15}; /** The maximum number of sigops we're willing to relay/mine in a single tx */ static constexpr unsigned int MAX_STANDARD_TX_SIGOPS_COST{MAX_BLOCK_SIGOPS_COST/5}; /** The maximum number of potentially executed legacy signature operations in a single standard tx */ -static constexpr unsigned int MAX_TX_LEGACY_SIGOPS{2'500}; +static constexpr unsigned int MAX_TX_BIP54_SIGOPS{2'500}; /** Default for -incrementalrelayfee, which sets the minimum feerate increase for mempool limiting or replacement **/ static constexpr unsigned int DEFAULT_INCREMENTAL_RELAY_FEE{100}; /** Default for -bytespersigop */ diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index c0443f3a0a45..2a899c499487 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -1070,7 +1070,7 @@ BOOST_AUTO_TEST_CASE(max_standard_legacy_sigops) // Create a transaction fanning out as many such P2SH outputs as is standard to spend in a // single transaction, and a transaction spending them. CMutableTransaction tx_create, tx_max_sigops; - const unsigned p2sh_inputs_count{MAX_TX_LEGACY_SIGOPS / MAX_P2SH_SIGOPS}; + const unsigned p2sh_inputs_count{MAX_TX_BIP54_SIGOPS / MAX_P2SH_SIGOPS}; tx_create.vout.reserve(p2sh_inputs_count); for (unsigned i{0}; i < p2sh_inputs_count; ++i) { tx_create.vout.emplace_back(424242 + i, max_sigops_p2sh); @@ -1082,7 +1082,7 @@ BOOST_AUTO_TEST_CASE(max_standard_legacy_sigops) } // p2sh_inputs_count is truncated to 166 (from 166.6666..) - BOOST_CHECK_LT(p2sh_inputs_count * MAX_P2SH_SIGOPS, MAX_TX_LEGACY_SIGOPS); + BOOST_CHECK_LT(p2sh_inputs_count * MAX_P2SH_SIGOPS, MAX_TX_BIP54_SIGOPS); AddCoins(coins, CTransaction(tx_create), 0, false); // 2490 sigops is below the limit. @@ -1097,7 +1097,7 @@ BOOST_AUTO_TEST_CASE(max_standard_legacy_sigops) } tx_max_sigops.vin.emplace_back(prev_txid, p2sh_inputs_count, CScript() << ToByteVector(max_sigops_redeem_script)); AddCoins(coins, CTransaction(tx_create), 0, false); - BOOST_CHECK_GT((p2sh_inputs_count + 1) * MAX_P2SH_SIGOPS, MAX_TX_LEGACY_SIGOPS); + BOOST_CHECK_GT((p2sh_inputs_count + 1) * MAX_P2SH_SIGOPS, MAX_TX_BIP54_SIGOPS); BOOST_CHECK_EQUAL(GetP2SHSigOpCount(CTransaction(tx_max_sigops), coins), 2505); BOOST_CHECK(!::AreInputsStandard(CTransaction(tx_max_sigops), coins)); @@ -1117,7 +1117,7 @@ BOOST_AUTO_TEST_CASE(max_standard_legacy_sigops) AddCoins(coins, CTransaction(tx_create_p2pk), 0, false); // The transaction now contains exactly 2500 sigops, the check should pass. - BOOST_CHECK_EQUAL(p2sh_inputs_count * MAX_P2SH_SIGOPS + p2pk_inputs_count * 1, MAX_TX_LEGACY_SIGOPS); + BOOST_CHECK_EQUAL(p2sh_inputs_count * MAX_P2SH_SIGOPS + p2pk_inputs_count * 1, MAX_TX_BIP54_SIGOPS); BOOST_CHECK(::AreInputsStandard(CTransaction(tx_max_sigops), coins)); // Now, add some Segwit inputs. We add one for each defined Segwit output type. The limit @@ -1145,7 +1145,7 @@ BOOST_AUTO_TEST_CASE(max_standard_legacy_sigops) tx_max_sigops.vin.emplace_back(prev_txid, i); } AddCoins(coins, CTransaction(tx_create_p2pk), 0, false); - BOOST_CHECK_GT(p2sh_inputs_count * MAX_P2SH_SIGOPS + p2pk_inputs_count * 1, MAX_TX_LEGACY_SIGOPS); + BOOST_CHECK_GT(p2sh_inputs_count * MAX_P2SH_SIGOPS + p2pk_inputs_count * 1, MAX_TX_BIP54_SIGOPS); BOOST_CHECK(!::AreInputsStandard(CTransaction(tx_max_sigops), coins)); } From 1b9ba0b1dde6da1a50e4bb4ce61c60da24a34d92 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Thu, 4 Sep 2025 16:14:42 -0400 Subject: [PATCH 07/24] moveonly: move CheckSigopsBIP54 from policy to consensus Move the function that checks whether a transaction respects the BIP54 sigops rule to the consensus folder (along with the accompanying constant), as it will be made consensus-critical in the next commit. Can be reviewed with git's --color-moved option. --- src/CMakeLists.txt | 2 +- src/consensus/consensus.h | 3 +++ src/consensus/tx_verify.cpp | 26 ++++++++++++++++++++++++++ src/consensus/tx_verify.h | 5 +++++ src/policy/policy.cpp | 32 ++------------------------------ src/policy/policy.h | 2 -- 6 files changed, 37 insertions(+), 33 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index f9934bb59060..846a1c7e131e 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -106,6 +106,7 @@ add_library(bitcoin_common STATIC EXCLUDE_FROM_ALL common/system.cpp common/url.cpp compressor.cpp + consensus/tx_verify.cpp core_read.cpp core_write.cpp deploymentinfo.cpp @@ -182,7 +183,6 @@ add_library(bitcoin_node STATIC EXCLUDE_FROM_ALL bip324.cpp blockencodings.cpp blockfilter.cpp - consensus/tx_verify.cpp dbwrapper.cpp deploymentstatus.cpp flatfile.cpp diff --git a/src/consensus/consensus.h b/src/consensus/consensus.h index 71b5fe2468d9..ae230ce47b28 100644 --- a/src/consensus/consensus.h +++ b/src/consensus/consensus.h @@ -34,4 +34,7 @@ static constexpr unsigned int LOCKTIME_VERIFY_SEQUENCE = (1 << 0); */ static constexpr int64_t MAX_TIMEWARP = 600; +/** The maximum number of potentially executed legacy signature operations in a single tx */ +static constexpr unsigned int MAX_TX_BIP54_SIGOPS{2'500}; + #endif // BITCOIN_CONSENSUS_CONSENSUS_H diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp index 95466b759cbb..59a662859356 100644 --- a/src/consensus/tx_verify.cpp +++ b/src/consensus/tx_verify.cpp @@ -161,6 +161,32 @@ int64_t GetTransactionSigOpCost(const CTransaction& tx, const CCoinsViewCache& i return nSigOps; } +bool Consensus::CheckSigopsBIP54(const CTransaction& tx, const CCoinsViewCache& inputs) +{ + Assert(!tx.IsCoinBase()); + + unsigned int sigops{0}; + for (const auto& txin: tx.vin) { + const auto& prev_txo{inputs.AccessCoin(txin.prevout).out}; + + // Unlike the existing block wide sigop limit which counts sigops present in the block + // itself (including the scriptPubKey which is not executed until spending later), BIP54 + // counts sigops in the block where they are potentially executed (only). + // This means sigops in the spent scriptPubKey count toward the limit. + // `fAccurate` means correctly accounting sigops for CHECKMULTISIGs(VERIFY) with 16 pubkeys + // or fewer. This method of accounting was introduced by BIP16, and BIP54 reuses it. + // The GetSigOpCount call on the previous scriptPubKey counts both bare and P2SH sigops. + sigops += txin.scriptSig.GetSigOpCount(/*fAccurate=*/true); + sigops += prev_txo.scriptPubKey.GetSigOpCount(txin.scriptSig); + + if (sigops > MAX_TX_BIP54_SIGOPS) { + return false; + } + } + + return true; +} + bool Consensus::CheckTxInputs(const CTransaction& tx, TxValidationState& state, const CCoinsViewCache& inputs, int nSpendHeight, CAmount& txfee) { // are the actual inputs available? diff --git a/src/consensus/tx_verify.h b/src/consensus/tx_verify.h index 8b7e6d6b385f..212ea26205be 100644 --- a/src/consensus/tx_verify.h +++ b/src/consensus/tx_verify.h @@ -18,6 +18,11 @@ class TxValidationState; /** Transaction validation functions */ namespace Consensus { +/** + * Check the total number of non-witness sigops across the whole transaction, as per BIP54. + */ +bool CheckSigopsBIP54(const CTransaction& tx, const CCoinsViewCache& inputs); + /** * Check whether all inputs of this transaction are valid (no double spends and amounts) * This does not modify the UTXO set. This does not check scripts and sigs. diff --git a/src/policy/policy.cpp b/src/policy/policy.cpp index 33328a28e3e3..5623fe0188a5 100644 --- a/src/policy/policy.cpp +++ b/src/policy/policy.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -163,35 +164,6 @@ bool IsStandardTx(const CTransaction& tx, const std::optional& max_dat return true; } -/** - * Check the total number of non-witness sigops across the whole transaction, as per BIP54. - */ -static bool CheckSigopsBIP54(const CTransaction& tx, const CCoinsViewCache& inputs) -{ - Assert(!tx.IsCoinBase()); - - unsigned int sigops{0}; - for (const auto& txin: tx.vin) { - const auto& prev_txo{inputs.AccessCoin(txin.prevout).out}; - - // Unlike the existing block wide sigop limit which counts sigops present in the block - // itself (including the scriptPubKey which is not executed until spending later), BIP54 - // counts sigops in the block where they are potentially executed (only). - // This means sigops in the spent scriptPubKey count toward the limit. - // `fAccurate` means correctly accounting sigops for CHECKMULTISIGs(VERIFY) with 16 pubkeys - // or fewer. This method of accounting was introduced by BIP16, and BIP54 reuses it. - // The GetSigOpCount call on the previous scriptPubKey counts both bare and P2SH sigops. - sigops += txin.scriptSig.GetSigOpCount(/*fAccurate=*/true); - sigops += prev_txo.scriptPubKey.GetSigOpCount(txin.scriptSig); - - if (sigops > MAX_TX_BIP54_SIGOPS) { - return false; - } - } - - return true; -} - /** * Check transaction inputs. * @@ -216,7 +188,7 @@ bool AreInputsStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs) return true; // Coinbases don't use vin normally } - if (!CheckSigopsBIP54(tx, mapInputs)) { + if (!Consensus::CheckSigopsBIP54(tx, mapInputs)) { return false; } diff --git a/src/policy/policy.h b/src/policy/policy.h index 780899e76d64..2562e18de967 100644 --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -38,8 +38,6 @@ static constexpr unsigned int MIN_STANDARD_TX_NONWITNESS_SIZE{65}; static constexpr unsigned int MAX_P2SH_SIGOPS{15}; /** The maximum number of sigops we're willing to relay/mine in a single tx */ static constexpr unsigned int MAX_STANDARD_TX_SIGOPS_COST{MAX_BLOCK_SIGOPS_COST/5}; -/** The maximum number of potentially executed legacy signature operations in a single standard tx */ -static constexpr unsigned int MAX_TX_BIP54_SIGOPS{2'500}; /** Default for -incrementalrelayfee, which sets the minimum feerate increase for mempool limiting or replacement **/ static constexpr unsigned int DEFAULT_INCREMENTAL_RELAY_FEE{100}; /** Default for -bytespersigop */ From ce8e00c43c61c503c45f36f21d0b931ec840a340 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Tue, 14 Oct 2025 16:20:07 -0400 Subject: [PATCH 08/24] validation: make BIP54 sigops check consensus-critical 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. --- src/consensus/tx_verify.cpp | 6 +++++- src/consensus/tx_verify.h | 3 ++- src/policy/policy.cpp | 6 ------ src/test/fuzz/coins_view.cpp | 7 ++++++- src/test/transaction_tests.cpp | 19 ++++++++++++++----- src/txmempool.cpp | 2 +- src/validation.cpp | 5 +++-- test/functional/mempool_sigoplimit.py | 12 ++++++------ 8 files changed, 37 insertions(+), 23 deletions(-) diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp index 59a662859356..398faef5804e 100644 --- a/src/consensus/tx_verify.cpp +++ b/src/consensus/tx_verify.cpp @@ -187,7 +187,7 @@ bool Consensus::CheckSigopsBIP54(const CTransaction& tx, const CCoinsViewCache& return true; } -bool Consensus::CheckTxInputs(const CTransaction& tx, TxValidationState& state, const CCoinsViewCache& inputs, int nSpendHeight, CAmount& txfee) +bool Consensus::CheckTxInputs(const CTransaction& tx, TxValidationState& state, const CCoinsViewCache& inputs, int nSpendHeight, CAmount& txfee, bool enforce_bip54) { // are the actual inputs available? if (!inputs.HaveInputs(tx)) { @@ -195,6 +195,10 @@ bool Consensus::CheckTxInputs(const CTransaction& tx, TxValidationState& state, strprintf("%s: inputs missing/spent", __func__)); } + if (enforce_bip54 && !Consensus::CheckSigopsBIP54(tx, inputs)) { + return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-legacy-sigops", "too many legacy sigops (BIP54)"); + } + CAmount nValueIn = 0; for (unsigned int i = 0; i < tx.vin.size(); ++i) { const COutPoint &prevout = tx.vin[i].prevout; diff --git a/src/consensus/tx_verify.h b/src/consensus/tx_verify.h index 212ea26205be..8499fc6d6397 100644 --- a/src/consensus/tx_verify.h +++ b/src/consensus/tx_verify.h @@ -27,9 +27,10 @@ bool CheckSigopsBIP54(const CTransaction& tx, const CCoinsViewCache& inputs); * Check whether all inputs of this transaction are valid (no double spends and amounts) * This does not modify the UTXO set. This does not check scripts and sigs. * @param[out] txfee Set to the transaction fee if successful. + * @param[in] enforce_bip54 Whether to perform the BIP54 sigops check. * Preconditions: tx.IsCoinBase() is false. */ -[[nodiscard]] bool CheckTxInputs(const CTransaction& tx, TxValidationState& state, const CCoinsViewCache& inputs, int nSpendHeight, CAmount& txfee); +[[nodiscard]] bool CheckTxInputs(const CTransaction& tx, TxValidationState& state, const CCoinsViewCache& inputs, int nSpendHeight, CAmount& txfee, bool enforce_bip54); } // namespace Consensus /** Auxiliary functions for transaction validation (ideally should not be exposed) */ diff --git a/src/policy/policy.cpp b/src/policy/policy.cpp index 5623fe0188a5..eacb3dcfcd11 100644 --- a/src/policy/policy.cpp +++ b/src/policy/policy.cpp @@ -179,8 +179,6 @@ bool IsStandardTx(const CTransaction& tx, const std::optional& max_dat * as potential new upgrade hooks. * * Note that only the non-witness portion of the transaction is checked here. - * - * We also check the total number of non-witness sigops across the whole transaction, as per BIP54. */ bool AreInputsStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs) { @@ -188,10 +186,6 @@ bool AreInputsStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs) return true; // Coinbases don't use vin normally } - if (!Consensus::CheckSigopsBIP54(tx, mapInputs)) { - return false; - } - for (unsigned int i = 0; i < tx.vin.size(); i++) { const CTxOut& prev = mapInputs.AccessCoin(tx.vin[i].prevout).out; diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp index 2b9b81e03068..04c5dc54f480 100644 --- a/src/test/fuzz/coins_view.cpp +++ b/src/test/fuzz/coins_view.cpp @@ -268,7 +268,12 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsView& backend // It is not allowed to call CheckTxInputs if CheckTransaction failed return; } - if (Consensus::CheckTxInputs(transaction, state, coins_view_cache, fuzzed_data_provider.ConsumeIntegralInRange(0, std::numeric_limits::max()), tx_fee_out)) { + if (transaction.IsCoinBase()) { + // It is not allowed to call CheckTxInputs on a coinbase transaction. + return; + } + const bool enforce_bip54{fuzzed_data_provider.ConsumeBool()}; + if (Consensus::CheckTxInputs(transaction, state, coins_view_cache, fuzzed_data_provider.ConsumeIntegralInRange(0, std::numeric_limits::max()), tx_fee_out, enforce_bip54)) { assert(MoneyRange(tx_fee_out)); } }, diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 2a899c499487..b751cdd75212 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -1056,6 +1056,9 @@ BOOST_AUTO_TEST_CASE(test_IsStandard) BOOST_AUTO_TEST_CASE(max_standard_legacy_sigops) { + TxValidationState state; + const int dummy_height{0}; + CAmount dummy_fee; CCoinsView coins_dummy; CCoinsViewCache coins(&coins_dummy); CKey key; @@ -1087,7 +1090,8 @@ BOOST_AUTO_TEST_CASE(max_standard_legacy_sigops) // 2490 sigops is below the limit. BOOST_CHECK_EQUAL(GetP2SHSigOpCount(CTransaction(tx_max_sigops), coins), 2490); - BOOST_CHECK(::AreInputsStandard(CTransaction(tx_max_sigops), coins)); + BOOST_CHECK(Consensus::CheckTxInputs(CTransaction(tx_max_sigops), state, coins, dummy_height, dummy_fee, /*enforce_bip54=*/true)); + BOOST_CHECK(state.IsValid()); // Adding one more input will bump this to 2505, hitting the limit. tx_create.vout.emplace_back(424242, max_sigops_p2sh); @@ -1099,7 +1103,9 @@ BOOST_AUTO_TEST_CASE(max_standard_legacy_sigops) AddCoins(coins, CTransaction(tx_create), 0, false); BOOST_CHECK_GT((p2sh_inputs_count + 1) * MAX_P2SH_SIGOPS, MAX_TX_BIP54_SIGOPS); BOOST_CHECK_EQUAL(GetP2SHSigOpCount(CTransaction(tx_max_sigops), coins), 2505); - BOOST_CHECK(!::AreInputsStandard(CTransaction(tx_max_sigops), coins)); + BOOST_CHECK(!Consensus::CheckTxInputs(CTransaction(tx_max_sigops), state, coins, dummy_height, dummy_fee, /*enforce_bip54=*/true)); + BOOST_CHECK(state.IsInvalid()); + state = TxValidationState{}; // Now, check the limit can be reached with regular P2PK outputs too. Use a separate // preparation transaction, to demonstrate spending coins from a single tx is irrelevant. @@ -1118,7 +1124,8 @@ BOOST_AUTO_TEST_CASE(max_standard_legacy_sigops) // The transaction now contains exactly 2500 sigops, the check should pass. BOOST_CHECK_EQUAL(p2sh_inputs_count * MAX_P2SH_SIGOPS + p2pk_inputs_count * 1, MAX_TX_BIP54_SIGOPS); - BOOST_CHECK(::AreInputsStandard(CTransaction(tx_max_sigops), coins)); + BOOST_CHECK(Consensus::CheckTxInputs(CTransaction(tx_max_sigops), state, coins, dummy_height, dummy_fee, /*enforce_bip54=*/true)); + BOOST_CHECK(state.IsValid()); // Now, add some Segwit inputs. We add one for each defined Segwit output type. The limit // is exclusively on non-witness sigops and therefore those should not be counted. @@ -1134,7 +1141,8 @@ BOOST_AUTO_TEST_CASE(max_standard_legacy_sigops) // The transaction now still contains exactly 2500 sigops, the check should pass. AddCoins(coins, CTransaction(tx_create_segwit), 0, false); - BOOST_REQUIRE(::AreInputsStandard(CTransaction(tx_max_sigops), coins)); + BOOST_CHECK(Consensus::CheckTxInputs(CTransaction(tx_max_sigops), state, coins, dummy_height, dummy_fee, /*enforce_bip54=*/true)); + BOOST_CHECK(state.IsValid()); // Add one more P2PK input. We'll reach the limit. tx_create_p2pk.vout.emplace_back(212121, p2pk_script); @@ -1146,7 +1154,8 @@ BOOST_AUTO_TEST_CASE(max_standard_legacy_sigops) } AddCoins(coins, CTransaction(tx_create_p2pk), 0, false); BOOST_CHECK_GT(p2sh_inputs_count * MAX_P2SH_SIGOPS + p2pk_inputs_count * 1, MAX_TX_BIP54_SIGOPS); - BOOST_CHECK(!::AreInputsStandard(CTransaction(tx_max_sigops), coins)); + BOOST_CHECK(!Consensus::CheckTxInputs(CTransaction(tx_max_sigops), state, coins, dummy_height, dummy_fee, /*enforce_bip54=*/true)); + BOOST_CHECK(state.IsInvalid()); } /** Sanity check the return value of SpendsNonAnchorWitnessProg for various output types. */ diff --git a/src/txmempool.cpp b/src/txmempool.cpp index cae16a675bd4..70fded4d93c7 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -777,7 +777,7 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei TxValidationState dummy_state; // Not used. CheckTxInputs() should always pass CAmount txfee = 0; assert(!tx.IsCoinBase()); - assert(Consensus::CheckTxInputs(tx, dummy_state, mempoolDuplicate, spendheight, txfee)); + assert(Consensus::CheckTxInputs(tx, dummy_state, mempoolDuplicate, spendheight, txfee, /*enforce_bip54=*/true)); for (const auto& input: tx.vin) mempoolDuplicate.SpendCoin(input.prevout); AddCoins(mempoolDuplicate, tx, std::numeric_limits::max()); } diff --git a/src/validation.cpp b/src/validation.cpp index 21ccf045fbfc..66d923de4771 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -900,7 +900,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) } // The mempool holds txs for the next block, so pass height+1 to CheckTxInputs - if (!Consensus::CheckTxInputs(tx, state, m_view, m_active_chainstate.m_chain.Height() + 1, ws.m_base_fees)) { + if (!Consensus::CheckTxInputs(tx, state, m_view, m_active_chainstate.m_chain.Height() + 1, ws.m_base_fees, /*enforce_bip54=*/true)) { return false; // state filled in by CheckTxInputs } @@ -2588,6 +2588,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, std::optional> control; if (auto& queue = m_chainman.GetCheckQueue(); queue.HasThreads() && fScriptChecks) control.emplace(queue); + const bool enforce_bip54{DeploymentActiveAt(*pindex, m_chainman, Consensus::DEPLOYMENT_CONSENSUSCLEANUP)}; std::vector prevheights; CAmount nFees = 0; int nInputs = 0; @@ -2604,7 +2605,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, { CAmount txfee = 0; TxValidationState tx_state; - if (!Consensus::CheckTxInputs(tx, tx_state, view, pindex->nHeight, txfee)) { + if (!Consensus::CheckTxInputs(tx, tx_state, view, pindex->nHeight, txfee, /*enforce_bip54=*/enforce_bip54)) { // Any transaction validation failure in ConnectBlock is a block consensus failure state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, tx_state.GetRejectReason(), diff --git a/test/functional/mempool_sigoplimit.py b/test/functional/mempool_sigoplimit.py index 6b2a0268c4da..330e06bc3fe5 100755 --- a/test/functional/mempool_sigoplimit.py +++ b/test/functional/mempool_sigoplimit.py @@ -183,7 +183,7 @@ def create_bare_multisig_tx(utxo_to_spend=None): assert_greater_than(2000, tx_parent.get_weight() + tx_child.get_weight()) def test_legacy_sigops_stdness(self): - self.log.info("Test a transaction with too many legacy sigops in its inputs is non-standard.") + self.log.info("Test a transaction with too many legacy sigops in its inputs is invalid.") # Restart with the default settings self.restart_node(0) @@ -204,19 +204,19 @@ def test_legacy_sigops_stdness(self): outpoints.append(COutPoint(txid, res["sent_vout"])) self.generate(self.nodes[0], 1) - # Spending all these outputs at once accounts for 2505 legacy sigops and is non-standard. + # Spending all these outputs at once accounts for 2505 legacy sigops and is invalid. nonstd_tx = CTransaction() nonstd_tx.vin = [CTxIn(op, CScript([b"", packed_redeem_script])) for op in outpoints] nonstd_tx.vout = [CTxOut(0, CScript([OP_RETURN, b""]))] - assert_raises_rpc_error(-26, "bad-txns-nonstandard-inputs", self.nodes[0].sendrawtransaction, nonstd_tx.serialize().hex()) + assert_raises_rpc_error(-26, "bad-txns-legacy-sigops", self.nodes[0].sendrawtransaction, nonstd_tx.serialize().hex()) - # Spending one less accounts for 2490 legacy sigops and is standard. + # Spending one less accounts for 2490 legacy sigops and is valid and standard. std_tx = deepcopy(nonstd_tx) std_tx.vin.pop() self.nodes[0].sendrawtransaction(std_tx.serialize().hex()) - # Make sure the original, non-standard, transaction can be mined. - self.generateblock(self.nodes[0], output="raw(42)", transactions=[nonstd_tx.serialize().hex()]) + # The invalid transaction also cannot appear in a block. + assert_raises_rpc_error(-25, "bad-txns-legacy-sigops", self.generateblock, self.nodes[0], "raw(42)", [nonstd_tx.serialize().hex()]) def run_test(self): self.wallet = MiniWallet(self.nodes[0]) From d4d46971f64c8b6d8bb23d585a5dd6dabe8a290c Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Tue, 16 Sep 2025 13:09:04 -0400 Subject: [PATCH 09/24] qa: add to utilities a version of SignSignature for Taproot inputs In Taproot the signature commits to the list of spent outputs. --- src/test/util/transaction_utils.cpp | 14 ++++++++++++++ src/test/util/transaction_utils.h | 3 +++ 2 files changed, 17 insertions(+) diff --git a/src/test/util/transaction_utils.cpp b/src/test/util/transaction_utils.cpp index a588e6194410..71a3d4b73622 100644 --- a/src/test/util/transaction_utils.cpp +++ b/src/test/util/transaction_utils.cpp @@ -91,6 +91,20 @@ void BulkTransaction(CMutableTransaction& tx, int32_t target_weight) assert(GetTransactionWeight(CTransaction(tx)) <= target_weight + 3); } +bool SignSignature(const SigningProvider &provider, const CScript& fromPubKey, CMutableTransaction& txTo, unsigned int nIn, + const CAmount& amount, std::vector&& spent_outputs, int nHashType, SignatureData& sig_data) +{ + assert(nIn < txTo.vin.size()); + + PrecomputedTransactionData txdata; + txdata.Init(txTo, std::forward>(spent_outputs), /*force=*/true); + MutableTransactionSignatureCreator creator(txTo, nIn, amount, &txdata, nHashType); + + bool ret = ProduceSignature(provider, creator, fromPubKey, sig_data); + UpdateInput(txTo.vin.at(nIn), sig_data); + return ret; +} + bool SignSignature(const SigningProvider &provider, const CScript& fromPubKey, CMutableTransaction& txTo, unsigned int nIn, const CAmount& amount, int nHashType, SignatureData& sig_data) { assert(nIn < txTo.vin.size()); diff --git a/src/test/util/transaction_utils.h b/src/test/util/transaction_utils.h index 4a18ab6ab49d..ddc5350852be 100644 --- a/src/test/util/transaction_utils.h +++ b/src/test/util/transaction_utils.h @@ -49,5 +49,8 @@ bool SignSignature(const SigningProvider &provider, const CScript& fromPubKey, C unsigned int nIn, const CAmount& amount, int nHashType, SignatureData& sig_data); bool SignSignature(const SigningProvider &provider, const CTransaction& txFrom, CMutableTransaction& txTo, unsigned int nIn, int nHashType, SignatureData& sig_data); +bool SignSignature(const SigningProvider &provider, const CScript& fromPubKey, CMutableTransaction& txTo, + unsigned int nIn, const CAmount& amount, std::vector&& spent_outputs, int nHashType, + SignatureData& sig_data); #endif // BITCOIN_TEST_UTIL_TRANSACTION_UTILS_H From bfb43481be4c0660ba20662e9f1d0aa2f23e4436 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Mon, 22 Sep 2025 13:53:14 -0400 Subject: [PATCH 10/24] qa: extensive unit tests for BIP54 legacy sigops limit Test the newly introduced limit with various combinations of inputs and outputs types, historical transactions, and exercise some implementation-specific edge cases. Record each test case and optionally write them to disk as JSON to generate the BIP test vectors. --- src/test/CMakeLists.txt | 1 + src/test/bip54_tests.cpp | 1324 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 1325 insertions(+) create mode 100644 src/test/bip54_tests.cpp diff --git a/src/test/CMakeLists.txt b/src/test/CMakeLists.txt index aacefb3f85cc..db0483f71f4c 100644 --- a/src/test/CMakeLists.txt +++ b/src/test/CMakeLists.txt @@ -18,6 +18,7 @@ add_executable(test_bitcoin bech32_tests.cpp bip32_tests.cpp bip324_tests.cpp + bip54_tests.cpp blockchain_tests.cpp blockencodings_tests.cpp blockfilter_index_tests.cpp diff --git a/src/test/bip54_tests.cpp b/src/test/bip54_tests.cpp new file mode 100644 index 000000000000..8fdc09318087 --- /dev/null +++ b/src/test/bip54_tests.cpp @@ -0,0 +1,1324 @@ +// Copyright (c) The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +#include +#include +#include +#include +#include +#include +#include +#include