diff --git a/src/policy/policy.cpp b/src/policy/policy.cpp index fdc2fdb9cdf..48f2a6a7446 100644 --- a/src/policy/policy.cpp +++ b/src/policy/policy.cpp @@ -163,6 +163,35 @@ 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_LEGACY_SIGOPS) { + return false; + } + } + + return true; +} + /** * Check transaction inputs. * @@ -178,6 +207,8 @@ 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) { @@ -185,6 +216,10 @@ bool AreInputsStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs) return true; // Coinbases don't use vin normally } + if (!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/policy/policy.h b/src/policy/policy.h index e62efa02e3e..f9a18561bce 100644 --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -38,6 +38,8 @@ 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_LEGACY_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{1000}; /** Default for -bytespersigop */ diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 54b3216a4c6..5f03641e99e 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -1053,4 +1054,99 @@ BOOST_AUTO_TEST_CASE(test_IsStandard) CheckIsNotStandard(t, "dust"); } +BOOST_AUTO_TEST_CASE(max_standard_legacy_sigops) +{ + CCoinsView coins_dummy; + CCoinsViewCache coins(&coins_dummy); + CKey key; + key.MakeNewKey(true); + + // Create a pathological P2SH script padded with as many sigops as is standard. + CScript max_sigops_redeem_script{CScript() << std::vector{} << key.GetPubKey()}; + for (unsigned i{0}; i < MAX_P2SH_SIGOPS - 1; ++i) max_sigops_redeem_script << OP_2DUP << OP_CHECKSIG << OP_DROP; + max_sigops_redeem_script << OP_CHECKSIG << OP_NOT; + const CScript max_sigops_p2sh{GetScriptForDestination(ScriptHash(max_sigops_redeem_script))}; + + // 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}; + 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); + } + auto prev_txid{tx_create.GetHash()}; + tx_max_sigops.vin.reserve(p2sh_inputs_count); + for (unsigned i{0}; i < p2sh_inputs_count; ++i) { + tx_max_sigops.vin.emplace_back(prev_txid, i, CScript() << ToByteVector(max_sigops_redeem_script)); + } + + // p2sh_inputs_count is truncated to 166 (from 166.6666..) + BOOST_CHECK_LT(p2sh_inputs_count * MAX_P2SH_SIGOPS, MAX_TX_LEGACY_SIGOPS); + AddCoins(coins, CTransaction(tx_create), 0, false); + + // 2490 sigops is below the limit. + BOOST_CHECK_EQUAL(GetP2SHSigOpCount(CTransaction(tx_max_sigops), coins), 2490); + BOOST_CHECK(::AreInputsStandard(CTransaction(tx_max_sigops), coins)); + + // Adding one more input will bump this to 2505, hitting the limit. + tx_create.vout.emplace_back(424242, max_sigops_p2sh); + prev_txid = tx_create.GetHash(); + for (unsigned i{0}; i < p2sh_inputs_count; ++i) { + tx_max_sigops.vin[i] = CTxIn(COutPoint(prev_txid, i), CScript() << ToByteVector(max_sigops_redeem_script)); + } + 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_EQUAL(GetP2SHSigOpCount(CTransaction(tx_max_sigops), coins), 2505); + BOOST_CHECK(!::AreInputsStandard(CTransaction(tx_max_sigops), coins)); + + // 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. + CMutableTransaction tx_create_p2pk; + const auto p2pk_script{CScript() << key.GetPubKey() << OP_CHECKSIG}; + unsigned p2pk_inputs_count{10}; // From 2490 to 2500. + for (unsigned i{0}; i < p2pk_inputs_count; ++i) { + tx_create_p2pk.vout.emplace_back(212121 + i, p2pk_script); + } + prev_txid = tx_create_p2pk.GetHash(); + tx_max_sigops.vin.resize(p2sh_inputs_count); // Drop the extra input. + for (unsigned i{0}; i < p2pk_inputs_count; ++i) { + tx_max_sigops.vin.emplace_back(prev_txid, i); + } + 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(::AreInputsStandard(CTransaction(tx_max_sigops), coins)); + + // 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. + CMutableTransaction tx_create_segwit; + const auto witness_script{CScript() << key.GetPubKey() << OP_CHECKSIG}; + tx_create_segwit.vout.emplace_back(121212, GetScriptForDestination(WitnessV0KeyHash(key.GetPubKey()))); + tx_create_segwit.vout.emplace_back(131313, GetScriptForDestination(WitnessV0ScriptHash(witness_script))); + tx_create_segwit.vout.emplace_back(141414, GetScriptForDestination(WitnessV1Taproot{XOnlyPubKey(key.GetPubKey())})); + prev_txid = tx_create_segwit.GetHash(); + for (unsigned i{0}; i < tx_create_segwit.vout.size(); ++i) { + tx_max_sigops.vin.emplace_back(prev_txid, i); + } + + // 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)); + + // Add one more P2PK input. We'll reach the limit. + tx_create_p2pk.vout.emplace_back(212121, p2pk_script); + prev_txid = tx_create_p2pk.GetHash(); + tx_max_sigops.vin.resize(p2sh_inputs_count); + ++p2pk_inputs_count; + for (unsigned i{0}; i < p2pk_inputs_count; ++i) { + 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(!::AreInputsStandard(CTransaction(tx_max_sigops), coins)); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/test/functional/mempool_sigoplimit.py b/test/functional/mempool_sigoplimit.py index 23ddf0bac9c..6b2a0268c4d 100755 --- a/test/functional/mempool_sigoplimit.py +++ b/test/functional/mempool_sigoplimit.py @@ -3,6 +3,7 @@ # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test sigop limit mempool policy (`-bytespersigop` parameter)""" +from copy import deepcopy from decimal import Decimal from math import ceil @@ -17,23 +18,30 @@ from test_framework.messages import ( ) from test_framework.script import ( CScript, + OP_2DUP, OP_CHECKMULTISIG, OP_CHECKSIG, + OP_DROP, OP_ENDIF, OP_FALSE, OP_IF, + OP_NOT, OP_RETURN, OP_TRUE, ) from test_framework.script_util import ( keys_to_multisig_script, script_to_p2wsh_script, + script_to_p2sh_script, + MAX_STD_LEGACY_SIGOPS, + MAX_STD_P2SH_SIGOPS, ) from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, assert_greater_than, assert_greater_than_or_equal, + assert_raises_rpc_error, ) from test_framework.wallet import MiniWallet from test_framework.wallet_util import generate_keypair @@ -174,6 +182,42 @@ class BytesPerSigOpTest(BitcoinTestFramework): # Transactions are tiny in weight 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.") + + # Restart with the default settings + self.restart_node(0) + + # Create a P2SH script with 15 sigops. + _, dummy_pubkey = generate_keypair() + packed_redeem_script = [dummy_pubkey] + for _ in range(MAX_STD_P2SH_SIGOPS - 1): + packed_redeem_script += [OP_2DUP, OP_CHECKSIG, OP_DROP] + packed_redeem_script = CScript(packed_redeem_script + [OP_CHECKSIG, OP_NOT]) + packed_p2sh_script = script_to_p2sh_script(packed_redeem_script) + + # Create enough outputs to reach the sigops limit when spending them all at once. + outpoints = [] + for _ in range(int(MAX_STD_LEGACY_SIGOPS / MAX_STD_P2SH_SIGOPS) + 1): + res = self.wallet.send_to(from_node=self.nodes[0], scriptPubKey=packed_p2sh_script, amount=1_000) + txid = int.from_bytes(bytes.fromhex(res["txid"]), byteorder="big") + 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. + 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()) + + # Spending one less accounts for 2490 legacy sigops and is 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()]) + def run_test(self): self.wallet = MiniWallet(self.nodes[0]) @@ -191,6 +235,7 @@ class BytesPerSigOpTest(BitcoinTestFramework): self.generate(self.wallet, 1) self.test_sigops_package() + self.test_legacy_sigops_stdness() if __name__ == '__main__': diff --git a/test/functional/test_framework/script_util.py b/test/functional/test_framework/script_util.py index 3f8e1e61906..975785798f3 100755 --- a/test/functional/test_framework/script_util.py +++ b/test/functional/test_framework/script_util.py @@ -35,6 +35,12 @@ from test_framework.script import ( hash160, ) +# Maximum number of potentially executed legacy signature operations in validating a transaction. +MAX_STD_LEGACY_SIGOPS = 2_500 + +# Maximum number of sigops per standard P2SH redeemScript. +MAX_STD_P2SH_SIGOPS = 15 + # To prevent a "tx-size-small" policy rule error, a transaction has to have a # non-witness size of at least 65 bytes (MIN_STANDARD_TX_NONWITNESS_SIZE in # src/policy/policy.h). Considering a Tx with the smallest possible single