diff --git a/doc/release-notes-32406.md b/doc/release-notes-32406.md new file mode 100644 index 00000000000..f1bbf188aff --- /dev/null +++ b/doc/release-notes-32406.md @@ -0,0 +1,3 @@ +- `-datacarriersize` is increased to 100,000 which effectively uncaps the limit (as the maximum transaction size limit will be hit first). It can be overridden with -datacarriersize=83 to revert to the limit enforced in previous versions. Both `-datacarrier` and `-datacarriersize` options have been marked as deprecated and are expected to be removed in a future release. (#32406) + +- Multiple data carrier (OP_RETURN) outputs in a transaction are now permitted for relay and mining. The `-datacarriersize` limit applies to the aggregate size of the scriptPubKeys across all such outputs in a transaction, not including the scriptPubKey size itself. (#32406) diff --git a/src/init.cpp b/src/init.cpp index 831a7f5fc40..4817af5f01e 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -630,10 +630,10 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc) argsman.AddArg("-dustrelayfee=", strprintf("Fee rate (in %s/kvB) used to define dust, the value of an output such that it will cost more than its value in fees at this fee rate to spend it. (default: %s)", CURRENCY_UNIT, FormatMoney(DUST_RELAY_TX_FEE)), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::NODE_RELAY); argsman.AddArg("-acceptstalefeeestimates", strprintf("Read fee estimates even if they are stale (%sdefault: %u) fee estimates are considered stale if they are %s hours old", "regtest only; ", DEFAULT_ACCEPT_STALE_FEE_ESTIMATES, Ticks(MAX_FILE_AGE)), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-bytespersigop", strprintf("Equivalent bytes per sigop in transactions for relay and mining (default: %u)", DEFAULT_BYTES_PER_SIGOP), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); - argsman.AddArg("-datacarrier", strprintf("Relay and mine data carrier transactions (default: %u)", DEFAULT_ACCEPT_DATACARRIER), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); + argsman.AddArg("-datacarrier", strprintf("(DEPRECATED) Relay and mine data carrier transactions (default: %u)", DEFAULT_ACCEPT_DATACARRIER), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); argsman.AddArg("-datacarriersize", - strprintf("Relay and mine transactions whose data-carrying raw scriptPubKey " - "is of this size or less (default: %u)", + strprintf("(DEPRECATED) Relay and mine transactions whose data-carrying raw scriptPubKeys in aggregate " + "are of this size or less, allowing multiple outputs (default: %u)", MAX_OP_RETURN_RELAY), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); argsman.AddArg("-permitbaremultisig", strprintf("Relay transactions creating non-P2SH multisig outputs (default: %u)", DEFAULT_PERMIT_BAREMULTISIG), ArgsManager::ALLOW_ANY, @@ -875,6 +875,10 @@ bool AppInitParameterInteraction(const ArgsManager& args) InitWarning(_("Option '-checkpoints' is set but checkpoints were removed. This option has no effect.")); } + if (args.IsArgSet("-datacarriersize") || args.IsArgSet("-datacarrier")) { + InitWarning(_("Options '-datacarrier' or '-datacarriersize' are set but are marked as deprecated. They will be removed in a future version.")); + } + // Error if network-specific options (-addnode, -connect, etc) are // specified in default section of config file, but not overridden // on the command line or in this chain's section of the config file. diff --git a/src/policy/policy.cpp b/src/policy/policy.cpp index 32a31ea653c..fdc2fdb9cdf 100644 --- a/src/policy/policy.cpp +++ b/src/policy/policy.cpp @@ -76,7 +76,7 @@ std::vector GetDust(const CTransaction& tx, CFeeRate dust_relay_rate) return dust_outputs; } -bool IsStandard(const CScript& scriptPubKey, const std::optional& max_datacarrier_bytes, TxoutType& whichType) +bool IsStandard(const CScript& scriptPubKey, TxoutType& whichType) { std::vector > vSolutions; whichType = Solver(scriptPubKey, vSolutions); @@ -91,10 +91,6 @@ bool IsStandard(const CScript& scriptPubKey, const std::optional& max_ return false; if (m < 1 || m > n) return false; - } else if (whichType == TxoutType::NULL_DATA) { - if (!max_datacarrier_bytes || scriptPubKey.size() > *max_datacarrier_bytes) { - return false; - } } return true; @@ -137,17 +133,22 @@ bool IsStandardTx(const CTransaction& tx, const std::optional& max_dat } } - unsigned int nDataOut = 0; + unsigned int datacarrier_bytes_left = max_datacarrier_bytes.value_or(0); TxoutType whichType; for (const CTxOut& txout : tx.vout) { - if (!::IsStandard(txout.scriptPubKey, max_datacarrier_bytes, whichType)) { + if (!::IsStandard(txout.scriptPubKey, whichType)) { reason = "scriptpubkey"; return false; } - if (whichType == TxoutType::NULL_DATA) - nDataOut++; - else if ((whichType == TxoutType::MULTISIG) && (!permit_bare_multisig)) { + if (whichType == TxoutType::NULL_DATA) { + unsigned int size = txout.scriptPubKey.size(); + if (size > datacarrier_bytes_left) { + reason = "datacarrier"; + return false; + } + datacarrier_bytes_left -= size; + } else if ((whichType == TxoutType::MULTISIG) && (!permit_bare_multisig)) { reason = "bare-multisig"; return false; } @@ -159,12 +160,6 @@ bool IsStandardTx(const CTransaction& tx, const std::optional& max_dat return false; } - // only one OP_RETURN txout is permitted - if (nDataOut > 1) { - reason = "multi-op-return"; - return false; - } - return true; } diff --git a/src/policy/policy.h b/src/policy/policy.h index 2151ec13dd0..e62efa02e3e 100644 --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -73,10 +73,9 @@ static constexpr unsigned int DEFAULT_DESCENDANT_SIZE_LIMIT_KVB{101}; /** Default for -datacarrier */ static const bool DEFAULT_ACCEPT_DATACARRIER = true; /** - * Default setting for -datacarriersize. 80 bytes of data, +1 for OP_RETURN, - * +2 for the pushdata opcodes. + * Default setting for -datacarriersize in vbytes. */ -static const unsigned int MAX_OP_RETURN_RELAY = 83; +static const unsigned int MAX_OP_RETURN_RELAY = MAX_STANDARD_TX_WEIGHT / WITNESS_SCALE_FACTOR; /** * An extra transaction can be added to a package, as long as it only has one * ancestor and is no larger than this. Not really any reason to make this @@ -136,7 +135,7 @@ CAmount GetDustThreshold(const CTxOut& txout, const CFeeRate& dustRelayFee); bool IsDust(const CTxOut& txout, const CFeeRate& dustRelayFee); -bool IsStandard(const CScript& scriptPubKey, const std::optional& max_datacarrier_bytes, TxoutType& whichType); +bool IsStandard(const CScript& scriptPubKey, TxoutType& whichType); /** Get the vout index numbers of all dust outputs */ std::vector GetDust(const CTransaction& tx, CFeeRate dust_relay_rate); diff --git a/src/test/fuzz/key.cpp b/src/test/fuzz/key.cpp index d851e33adc5..e4bedff8b51 100644 --- a/src/test/fuzz/key.cpp +++ b/src/test/fuzz/key.cpp @@ -150,12 +150,12 @@ FUZZ_TARGET(key, .init = initialize_key) assert(fillable_signing_provider_pub.HaveKey(pubkey.GetID())); TxoutType which_type_tx_pubkey; - const bool is_standard_tx_pubkey = IsStandard(tx_pubkey_script, std::nullopt, which_type_tx_pubkey); + const bool is_standard_tx_pubkey = IsStandard(tx_pubkey_script, which_type_tx_pubkey); assert(is_standard_tx_pubkey); assert(which_type_tx_pubkey == TxoutType::PUBKEY); TxoutType which_type_tx_multisig; - const bool is_standard_tx_multisig = IsStandard(tx_multisig_script, std::nullopt, which_type_tx_multisig); + const bool is_standard_tx_multisig = IsStandard(tx_multisig_script, which_type_tx_multisig); assert(is_standard_tx_multisig); assert(which_type_tx_multisig == TxoutType::MULTISIG); diff --git a/src/test/fuzz/script.cpp b/src/test/fuzz/script.cpp index 07a49e039fb..9aa50559cbd 100644 --- a/src/test/fuzz/script.cpp +++ b/src/test/fuzz/script.cpp @@ -53,7 +53,7 @@ FUZZ_TARGET(script, .init = initialize_script) } TxoutType which_type; - bool is_standard_ret = IsStandard(script, std::nullopt, which_type); + bool is_standard_ret = IsStandard(script, which_type); if (!is_standard_ret) { assert(which_type == TxoutType::NONSTANDARD || which_type == TxoutType::NULL_DATA || diff --git a/src/test/multisig_tests.cpp b/src/test/multisig_tests.cpp index 29a73d03d25..b996f3cc061 100644 --- a/src/test/multisig_tests.cpp +++ b/src/test/multisig_tests.cpp @@ -144,7 +144,7 @@ BOOST_AUTO_TEST_CASE(multisig_IsStandard) const auto is_standard{[](const CScript& spk) { TxoutType type; - bool res{::IsStandard(spk, std::nullopt, type)}; + bool res{::IsStandard(spk, type)}; if (res) { BOOST_CHECK_EQUAL(type, TxoutType::MULTISIG); } diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 7a60ea25d3f..54b3216a4c6 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -797,14 +797,14 @@ BOOST_AUTO_TEST_CASE(test_IsStandard) CKey key = GenerateRandomKey(); t.vout[0].scriptPubKey = GetScriptForDestination(PKHash(key.GetPubKey())); - constexpr auto CheckIsStandard = [](const auto& t) { + constexpr auto CheckIsStandard = [](const auto& t, const unsigned int max_op_return_relay = MAX_OP_RETURN_RELAY) { std::string reason; - BOOST_CHECK(IsStandardTx(CTransaction{t}, MAX_OP_RETURN_RELAY, g_bare_multi, g_dust, reason)); + BOOST_CHECK(IsStandardTx(CTransaction{t}, max_op_return_relay, g_bare_multi, g_dust, reason)); BOOST_CHECK(reason.empty()); }; - constexpr auto CheckIsNotStandard = [](const auto& t, const std::string& reason_in) { + constexpr auto CheckIsNotStandard = [](const auto& t, const std::string& reason_in, const unsigned int max_op_return_relay = MAX_OP_RETURN_RELAY) { std::string reason; - BOOST_CHECK(!IsStandardTx(CTransaction{t}, MAX_OP_RETURN_RELAY, g_bare_multi, g_dust, reason)); + BOOST_CHECK(!IsStandardTx(CTransaction{t}, max_op_return_relay, g_bare_multi, g_dust, reason)); BOOST_CHECK_EQUAL(reason_in, reason); }; @@ -858,15 +858,13 @@ BOOST_AUTO_TEST_CASE(test_IsStandard) t.vout[0].scriptPubKey = CScript() << OP_1; CheckIsNotStandard(t, "scriptpubkey"); - // MAX_OP_RETURN_RELAY-byte TxoutType::NULL_DATA (standard) + // Custom 83-byte TxoutType::NULL_DATA (standard with max_op_return_relay of 83) t.vout[0].scriptPubKey = CScript() << OP_RETURN << "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef3804678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38"_hex; - BOOST_CHECK_EQUAL(MAX_OP_RETURN_RELAY, t.vout[0].scriptPubKey.size()); - CheckIsStandard(t); + BOOST_CHECK_EQUAL(83, t.vout[0].scriptPubKey.size()); + CheckIsStandard(t, /*max_op_return_relay=*/83); - // MAX_OP_RETURN_RELAY+1-byte TxoutType::NULL_DATA (non-standard) - t.vout[0].scriptPubKey = CScript() << OP_RETURN << "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef3804678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef3800"_hex; - BOOST_CHECK_EQUAL(MAX_OP_RETURN_RELAY + 1, t.vout[0].scriptPubKey.size()); - CheckIsNotStandard(t, "scriptpubkey"); + // Non-standard if max_op_return_relay datacarrier arg is one less + CheckIsNotStandard(t, "datacarrier", /*max_op_return_relay=*/82); // Data payload can be encoded in any way... t.vout[0].scriptPubKey = CScript() << OP_RETURN << ""_hex; @@ -888,21 +886,28 @@ BOOST_AUTO_TEST_CASE(test_IsStandard) t.vout[0].scriptPubKey = CScript() << OP_RETURN; CheckIsStandard(t); - // Only one TxoutType::NULL_DATA permitted in all cases + // Multiple TxoutType::NULL_DATA are permitted t.vout.resize(2); t.vout[0].scriptPubKey = CScript() << OP_RETURN << "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38"_hex; t.vout[0].nValue = 0; t.vout[1].scriptPubKey = CScript() << OP_RETURN << "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38"_hex; t.vout[1].nValue = 0; - CheckIsNotStandard(t, "multi-op-return"); + CheckIsStandard(t); t.vout[0].scriptPubKey = CScript() << OP_RETURN << "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38"_hex; t.vout[1].scriptPubKey = CScript() << OP_RETURN; - CheckIsNotStandard(t, "multi-op-return"); + CheckIsStandard(t); t.vout[0].scriptPubKey = CScript() << OP_RETURN; t.vout[1].scriptPubKey = CScript() << OP_RETURN; - CheckIsNotStandard(t, "multi-op-return"); + CheckIsStandard(t); + + t.vout[0].scriptPubKey = CScript() << OP_RETURN << "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef3804678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38"_hex; + t.vout[1].scriptPubKey = CScript() << OP_RETURN << "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef3804678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38"_hex; + const auto datacarrier_size = t.vout[0].scriptPubKey.size() + t.vout[1].scriptPubKey.size(); + CheckIsStandard(t); // Default max relay should never trigger + CheckIsStandard(t, /*max_op_return_relay=*/datacarrier_size); + CheckIsNotStandard(t, "datacarrier", /*max_op_return_relay=*/datacarrier_size-1); // Check large scriptSig (non-standard if size is >1650 bytes) t.vout.resize(1); diff --git a/test/functional/feature_blocksxor.py b/test/functional/feature_blocksxor.py index 9824bf97151..ab5b0071b86 100755 --- a/test/functional/feature_blocksxor.py +++ b/test/functional/feature_blocksxor.py @@ -23,7 +23,6 @@ class BlocksXORTest(BitcoinTestFramework): self.extra_args = [[ '-blocksxor=1', '-fastprune=1', # use smaller block files - '-datacarriersize=100000', # needed to pad transaction with MiniWallet ]] def run_test(self): diff --git a/test/functional/feature_maxuploadtarget.py b/test/functional/feature_maxuploadtarget.py index 1d7a7bfc55f..b72090b4363 100755 --- a/test/functional/feature_maxuploadtarget.py +++ b/test/functional/feature_maxuploadtarget.py @@ -51,7 +51,6 @@ class MaxUploadTest(BitcoinTestFramework): self.num_nodes = 1 self.extra_args = [[ f"-maxuploadtarget={UPLOAD_TARGET_MB}M", - "-datacarriersize=100000", ]] self.supports_cli = False diff --git a/test/functional/mempool_accept.py b/test/functional/mempool_accept.py index 9014ab260e1..d9c01161869 100755 --- a/test/functional/mempool_accept.py +++ b/test/functional/mempool_accept.py @@ -9,6 +9,7 @@ from decimal import Decimal import math from test_framework.test_framework import BitcoinTestFramework +from test_framework.blocktools import MAX_STANDARD_TX_WEIGHT from test_framework.messages import ( MAX_BIP125_RBF_SEQUENCE, COIN, @@ -326,11 +327,52 @@ class MempoolAcceptanceTest(BitcoinTestFramework): result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': 'dust'}], rawtxs=[tx.serialize().hex()], ) + + # OP_RETURN followed by non-push tx = tx_from_hex(raw_tx_reference) - tx.vout[0].scriptPubKey = CScript([OP_RETURN, b'\xff']) - tx.vout = [tx.vout[0]] * 2 + tx.vout[0].scriptPubKey = CScript([OP_RETURN, OP_HASH160]) self.check_mempool_result( - result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': 'multi-op-return'}], + result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': 'scriptpubkey'}], + rawtxs=[tx.serialize().hex()], + ) + + # Multiple OP_RETURN and more than 83 bytes, even if over MAX_SCRIPT_ELEMENT_SIZE + # are standard since v30 + tx = tx_from_hex(raw_tx_reference) + tx.vout.append(CTxOut(0, CScript([OP_RETURN, b'\xff']))) + tx.vout.append(CTxOut(0, CScript([OP_RETURN, b'\xff' * 50000]))) + + self.check_mempool_result( + result_expected=[{'txid': tx.rehash(), 'allowed': True, 'vsize': tx.get_vsize(), 'fees': {'base': Decimal('0.05')}}], + rawtxs=[tx.serialize().hex()], + maxfeerate=0 + ) + + self.log.info("A transaction with several OP_RETURN outputs.") + tx = tx_from_hex(raw_tx_reference) + op_return_count = 42 + tx.vout[0].nValue = int(tx.vout[0].nValue / op_return_count) + tx.vout[0].scriptPubKey = CScript([OP_RETURN, b'\xff']) + tx.vout = [tx.vout[0]] * op_return_count + self.check_mempool_result( + result_expected=[{"txid": tx.rehash(), "allowed": True, "vsize": tx.get_vsize(), "fees": {"base": Decimal("0.05000026")}}], + rawtxs=[tx.serialize().hex()], + ) + + self.log.info("A transaction with an OP_RETURN output that bumps into the max standardness tx size.") + tx = tx_from_hex(raw_tx_reference) + tx.vout[0].scriptPubKey = CScript([OP_RETURN]) + data_len = int(MAX_STANDARD_TX_WEIGHT / 4) - tx.get_vsize() - 5 - 4 # -5 for PUSHDATA4 and -4 for script size + tx.vout[0].scriptPubKey = CScript([OP_RETURN, b"\xff" * (data_len)]) + assert_equal(tx.get_vsize(), int(MAX_STANDARD_TX_WEIGHT / 4)) + self.check_mempool_result( + result_expected=[{"txid": tx.rehash(), "allowed": True, "vsize": tx.get_vsize(), "fees": {"base": Decimal("0.1") - Decimal("0.05")}}], + rawtxs=[tx.serialize().hex()], + ) + tx.vout[0].scriptPubKey = CScript([OP_RETURN, b"\xff" * (data_len + 1)]) + assert_greater_than(tx.get_vsize(), int(MAX_STANDARD_TX_WEIGHT / 4)) + self.check_mempool_result( + result_expected=[{"txid": tx.rehash(), "allowed": False, "reject-reason": "tx-size"}], rawtxs=[tx.serialize().hex()], ) diff --git a/test/functional/mempool_datacarrier.py b/test/functional/mempool_datacarrier.py index 48e636caa22..8eab4ce6b28 100755 --- a/test/functional/mempool_datacarrier.py +++ b/test/functional/mempool_datacarrier.py @@ -18,14 +18,16 @@ from test_framework.wallet import MiniWallet from random import randbytes +# The historical maximum, now used to test coverage +CUSTOM_DATACARRIER_ARG = 83 class DataCarrierTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 4 self.extra_args = [ - [], - ["-datacarrier=0"], - ["-datacarrier=1", f"-datacarriersize={MAX_OP_RETURN_RELAY - 1}"], + [], # default is uncapped + ["-datacarrier=0"], # no relay of datacarrier + ["-datacarrier=1", f"-datacarriersize={CUSTOM_DATACARRIER_ARG}"], ["-datacarrier=1", "-datacarriersize=2"], ] @@ -41,32 +43,33 @@ class DataCarrierTest(BitcoinTestFramework): self.wallet.sendrawtransaction(from_node=node, tx_hex=tx_hex) assert tx.rehash() in node.getrawmempool(True), f'{tx_hex} not in mempool' else: - assert_raises_rpc_error(-26, "scriptpubkey", self.wallet.sendrawtransaction, from_node=node, tx_hex=tx_hex) + assert_raises_rpc_error(-26, "datacarrier", self.wallet.sendrawtransaction, from_node=node, tx_hex=tx_hex) def run_test(self): self.wallet = MiniWallet(self.nodes[0]) - # By default, only 80 bytes are used for data (+1 for OP_RETURN, +2 for the pushdata opcodes). - default_size_data = randbytes(MAX_OP_RETURN_RELAY - 3) - too_long_data = randbytes(MAX_OP_RETURN_RELAY - 2) - small_data = randbytes(MAX_OP_RETURN_RELAY - 4) + # By default, any size is allowed. + + # If it is custom set to 83, the historical value, + # only 80 bytes are used for data (+1 for OP_RETURN, +2 for the pushdata opcodes). + custom_size_data = randbytes(CUSTOM_DATACARRIER_ARG - 3) + too_long_data = randbytes(CUSTOM_DATACARRIER_ARG - 2) + extremely_long_data = randbytes(MAX_OP_RETURN_RELAY - 200) one_byte = randbytes(1) zero_bytes = randbytes(0) - self.log.info("Testing null data transaction with default -datacarrier and -datacarriersize values.") - self.test_null_data_transaction(node=self.nodes[0], data=default_size_data, success=True) - - self.log.info("Testing a null data transaction larger than allowed by the default -datacarriersize value.") - self.test_null_data_transaction(node=self.nodes[0], data=too_long_data, success=False) + self.log.info("Testing a null data transaction succeeds for default arg regardless of size.") + self.test_null_data_transaction(node=self.nodes[0], data=too_long_data, success=True) + self.test_null_data_transaction(node=self.nodes[0], data=extremely_long_data, success=True) self.log.info("Testing a null data transaction with -datacarrier=false.") - self.test_null_data_transaction(node=self.nodes[1], data=default_size_data, success=False) + self.test_null_data_transaction(node=self.nodes[1], data=custom_size_data, success=False) self.log.info("Testing a null data transaction with a size larger than accepted by -datacarriersize.") - self.test_null_data_transaction(node=self.nodes[2], data=default_size_data, success=False) + self.test_null_data_transaction(node=self.nodes[2], data=too_long_data, success=False) - self.log.info("Testing a null data transaction with a size smaller than accepted by -datacarriersize.") - self.test_null_data_transaction(node=self.nodes[2], data=small_data, success=True) + self.log.info("Testing a null data transaction with a size equal to -datacarriersize.") + self.test_null_data_transaction(node=self.nodes[2], data=custom_size_data, success=True) self.log.info("Testing a null data transaction with no data.") self.test_null_data_transaction(node=self.nodes[0], data=None, success=True) @@ -86,6 +89,17 @@ class DataCarrierTest(BitcoinTestFramework): self.test_null_data_transaction(node=self.nodes[2], data=one_byte, success=True) self.test_null_data_transaction(node=self.nodes[3], data=one_byte, success=False) + # Clean shutdown boilerplate due to deprecation + self.expected_stderr = [ + "", # node 0 has no deprecated options + "Warning: Options '-datacarrier' or '-datacarriersize' are set but are marked as deprecated. They will be removed in a future version.", + "Warning: Options '-datacarrier' or '-datacarriersize' are set but are marked as deprecated. They will be removed in a future version.", + "Warning: Options '-datacarrier' or '-datacarriersize' are set but are marked as deprecated. They will be removed in a future version.", + ] + + for i in range(self.num_nodes): + self.stop_node(i, expected_stderr=self.expected_stderr[i]) + if __name__ == '__main__': DataCarrierTest(__file__).main() diff --git a/test/functional/mempool_limit.py b/test/functional/mempool_limit.py index 85d158d6111..791654a8c04 100755 --- a/test/functional/mempool_limit.py +++ b/test/functional/mempool_limit.py @@ -29,7 +29,6 @@ class MempoolLimitTest(BitcoinTestFramework): self.setup_clean_chain = True self.num_nodes = 1 self.extra_args = [[ - "-datacarriersize=100000", "-maxmempool=5", ]] self.supports_cli = False diff --git a/test/functional/mempool_package_limits.py b/test/functional/mempool_package_limits.py index 3290ff43c49..63106f346f3 100755 --- a/test/functional/mempool_package_limits.py +++ b/test/functional/mempool_package_limits.py @@ -51,9 +51,6 @@ class MempoolPackageLimitsTest(BitcoinTestFramework): self.test_anc_count_limits() self.test_anc_count_limits_2() self.test_anc_count_limits_bushy() - - # The node will accept (nonstandard) extra large OP_RETURN outputs - self.restart_node(0, extra_args=["-datacarriersize=100000"]) self.test_anc_size_limits() self.test_desc_size_limits() diff --git a/test/functional/mempool_package_rbf.py b/test/functional/mempool_package_rbf.py index b638c5f512a..f760e427fe9 100755 --- a/test/functional/mempool_package_rbf.py +++ b/test/functional/mempool_package_rbf.py @@ -33,7 +33,6 @@ class PackageRBFTest(BitcoinTestFramework): self.setup_clean_chain = True # Required for fill_mempool() self.extra_args = [[ - "-datacarriersize=100000", "-maxmempool=5", ]] * self.num_nodes diff --git a/test/functional/mempool_sigoplimit.py b/test/functional/mempool_sigoplimit.py index 47df0c614ae..94c13f62a21 100755 --- a/test/functional/mempool_sigoplimit.py +++ b/test/functional/mempool_sigoplimit.py @@ -44,8 +44,6 @@ MAX_PUBKEYS_PER_MULTISIG = 20 class BytesPerSigOpTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 - # allow large datacarrier output to pad transactions - self.extra_args = [['-datacarriersize=100000']] def create_p2wsh_spending_tx(self, witness_script, output_script): """Create a 1-input-1-output P2WSH spending transaction with only the @@ -139,7 +137,7 @@ class BytesPerSigOpTest(BitcoinTestFramework): self.log.info("Test a overly-large sigops-vbyte hits package limits") # Make a 2-transaction package which fails vbyte checks even though # separately they would work. - self.restart_node(0, extra_args=["-bytespersigop=5000","-permitbaremultisig=1"] + self.extra_args[0]) + self.restart_node(0, extra_args=["-bytespersigop=5000","-permitbaremultisig=1"]) def create_bare_multisig_tx(utxo_to_spend=None): _, pubkey = generate_keypair() @@ -185,7 +183,7 @@ class BytesPerSigOpTest(BitcoinTestFramework): else: bytespersigop_parameter = f"-bytespersigop={bytes_per_sigop}" self.log.info(f"Test sigops limit setting {bytespersigop_parameter}...") - self.restart_node(0, extra_args=[bytespersigop_parameter] + self.extra_args[0]) + self.restart_node(0, extra_args=[bytespersigop_parameter]) for num_sigops in (69, 101, 142, 183, 222): self.test_sigops_limit(bytes_per_sigop, num_sigops) diff --git a/test/functional/mempool_truc.py b/test/functional/mempool_truc.py index c2a65f9ade7..8a4f34e3669 100755 --- a/test/functional/mempool_truc.py +++ b/test/functional/mempool_truc.py @@ -49,7 +49,7 @@ class MempoolTRUC(BitcoinTestFramework): assert_equal(len(txids), len(mempool_contents)) assert all([txid in txids for txid in mempool_contents]) - @cleanup(extra_args=["-datacarriersize=20000"]) + @cleanup() def test_truc_max_vsize(self): node = self.nodes[0] self.log.info("Test TRUC-specific maximum transaction vsize") @@ -63,7 +63,7 @@ class MempoolTRUC(BitcoinTestFramework): tx_v2_heavy = self.wallet.send_self_transfer(from_node=node, target_vsize=TRUC_MAX_VSIZE + 1, version=2) self.check_mempool([tx_v2_heavy["txid"]]) - @cleanup(extra_args=["-datacarriersize=1000"]) + @cleanup() def test_truc_acceptance(self): node = self.nodes[0] self.log.info("Test a child of a TRUC transaction cannot be more than 1000vB") @@ -160,7 +160,7 @@ class MempoolTRUC(BitcoinTestFramework): self.check_mempool([tx_v3_bip125_rbf_v2["txid"], tx_v3_parent["txid"], tx_v3_child["txid"]]) - @cleanup(extra_args=["-datacarriersize=40000"]) + @cleanup() def test_truc_reorg(self): node = self.nodes[0] self.log.info("Test that, during a reorg, TRUC rules are not enforced") @@ -182,7 +182,7 @@ class MempoolTRUC(BitcoinTestFramework): node.reconsiderblock(block[0]) - @cleanup(extra_args=["-limitdescendantsize=10", "-datacarriersize=40000"]) + @cleanup(extra_args=["-limitdescendantsize=10"]) def test_nondefault_package_limits(self): """ Max standard tx size + TRUC rules imply the ancestor/descendant rules (at their default @@ -215,7 +215,7 @@ class MempoolTRUC(BitcoinTestFramework): self.generate(node, 1) self.log.info("Test that a decreased limitancestorsize also applies to v3 parent") - self.restart_node(0, extra_args=["-limitancestorsize=10", "-datacarriersize=40000"]) + self.restart_node(0, extra_args=["-limitancestorsize=10"]) tx_v3_parent_large2 = self.wallet.send_self_transfer( from_node=node, target_vsize=parent_target_vsize, @@ -235,7 +235,7 @@ class MempoolTRUC(BitcoinTestFramework): assert_raises_rpc_error(-26, "too-long-mempool-chain, exceeds ancestor size limit", node.sendrawtransaction, tx_v3_child_large2["hex"]) self.check_mempool([tx_v3_parent_large2["txid"]]) - @cleanup(extra_args=["-datacarriersize=1000"]) + @cleanup() def test_truc_ancestors_package(self): self.log.info("Test that TRUC ancestor limits are checked within the package") node = self.nodes[0] @@ -384,7 +384,7 @@ class MempoolTRUC(BitcoinTestFramework): assert_equal(result_package_cpfp["tx-results"][tx_sibling_3['wtxid']]['error'], expected_error_cpfp) - @cleanup(extra_args=["-datacarriersize=1000"]) + @cleanup() def test_truc_package_inheritance(self): self.log.info("Test that TRUC inheritance is checked within package") node = self.nodes[0] diff --git a/test/functional/mempool_updatefromblock.py b/test/functional/mempool_updatefromblock.py index 46b69d67387..4ceeb3f149a 100755 --- a/test/functional/mempool_updatefromblock.py +++ b/test/functional/mempool_updatefromblock.py @@ -28,7 +28,7 @@ class MempoolUpdateFromBlockTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 # Ancestor and descendant limits depend on transaction_graph_test requirements - self.extra_args = [['-limitdescendantsize=1000', '-limitancestorsize=1000', f'-limitancestorcount={CUSTOM_ANCESTOR_COUNT}', f'-limitdescendantcount={CUSTOM_DESCENDANT_COUNT}', '-datacarriersize=100000']] + self.extra_args = [['-limitdescendantsize=1000', '-limitancestorsize=1000', f'-limitancestorcount={CUSTOM_ANCESTOR_COUNT}', f'-limitdescendantcount={CUSTOM_DESCENDANT_COUNT}']] def create_empty_fork(self, fork_length): ''' diff --git a/test/functional/mining_basic.py b/test/functional/mining_basic.py index 89f633cf7f0..12f1a4ddf5b 100755 --- a/test/functional/mining_basic.py +++ b/test/functional/mining_basic.py @@ -187,6 +187,9 @@ class MiningTest(BitcoinTestFramework): assert tx_below_min_feerate['txid'] not in block_template_txids assert tx_below_min_feerate['txid'] not in block_txids + # Restart node to clear mempool for the next test + self.restart_node(0) + def test_timewarp(self): self.log.info("Test timewarp attack mitigation (BIP94)") node = self.nodes[0] @@ -280,11 +283,9 @@ class MiningTest(BitcoinTestFramework): def test_block_max_weight(self): self.log.info("Testing default and custom -blockmaxweight startup options.") - # Restart the node to allow large transactions LARGE_TXS_COUNT = 10 LARGE_VSIZE = int(((MAX_BLOCK_WEIGHT - DEFAULT_BLOCK_RESERVED_WEIGHT) / WITNESS_SCALE_FACTOR) / LARGE_TXS_COUNT) HIGH_FEERATE = Decimal("0.0003") - self.restart_node(0, extra_args=[f"-datacarriersize={LARGE_VSIZE}"]) # Ensure the mempool is empty assert_equal(len(self.nodes[0].getrawmempool()), 0) @@ -312,7 +313,7 @@ class MiningTest(BitcoinTestFramework): # Test block template creation with custom -blockmaxweight custom_block_weight = MAX_BLOCK_WEIGHT - 2000 # Reducing the weight by 2000 units will prevent 1 large transaction from fitting into the block. - self.restart_node(0, extra_args=[f"-datacarriersize={LARGE_VSIZE}", f"-blockmaxweight={custom_block_weight}"]) + self.restart_node(0, extra_args=[f"-blockmaxweight={custom_block_weight}"]) self.log.info("Testing the block template with custom -blockmaxweight to include 9 large and 2 normal transactions.") self.verify_block_template( @@ -322,7 +323,7 @@ class MiningTest(BitcoinTestFramework): # Ensure the block weight does not exceed the maximum self.log.info(f"Testing that the block weight will never exceed {MAX_BLOCK_WEIGHT - DEFAULT_BLOCK_RESERVED_WEIGHT}.") - self.restart_node(0, extra_args=[f"-datacarriersize={LARGE_VSIZE}", f"-blockmaxweight={MAX_BLOCK_WEIGHT}"]) + self.restart_node(0, extra_args=[f"-blockmaxweight={MAX_BLOCK_WEIGHT}"]) self.log.info("Sending 2 additional normal transactions to fill the mempool to the maximum block weight.") self.send_transactions(utxos[LARGE_TXS_COUNT + 2:], NORMAL_FEERATE, NORMAL_VSIZE) self.log.info(f"Testing that the mempool's weight matches the maximum block weight: {MAX_BLOCK_WEIGHT}.") @@ -336,7 +337,7 @@ class MiningTest(BitcoinTestFramework): self.log.info("Test -blockreservedweight startup option.") # Lowering the -blockreservedweight by 4000 will allow for two more transactions. - self.restart_node(0, extra_args=[f"-datacarriersize={LARGE_VSIZE}", "-blockreservedweight=4000"]) + self.restart_node(0, extra_args=["-blockreservedweight=4000"]) self.verify_block_template( expected_tx_count=12, expected_weight=MAX_BLOCK_WEIGHT - 4000, diff --git a/test/functional/mining_prioritisetransaction.py b/test/functional/mining_prioritisetransaction.py index 2cb5d85b44f..a8646d49181 100755 --- a/test/functional/mining_prioritisetransaction.py +++ b/test/functional/mining_prioritisetransaction.py @@ -27,7 +27,6 @@ class PrioritiseTransactionTest(BitcoinTestFramework): self.num_nodes = 1 self.extra_args = [[ "-printpriority=1", - "-datacarriersize=100000", ]] * self.num_nodes self.supports_cli = False diff --git a/test/functional/p2p_1p1c_network.py b/test/functional/p2p_1p1c_network.py index e48e5b88b67..b098e4cf436 100755 --- a/test/functional/p2p_1p1c_network.py +++ b/test/functional/p2p_1p1c_network.py @@ -41,7 +41,6 @@ class PackageRelayTest(BitcoinTestFramework): # hugely speeds up the test, as it involves multiple hops of tx relay. self.noban_tx_relay = True self.extra_args = [[ - "-datacarriersize=100000", "-maxmempool=5", ]] * self.num_nodes self.supports_cli = False diff --git a/test/functional/p2p_opportunistic_1p1c.py b/test/functional/p2p_opportunistic_1p1c.py index addb5027f37..0cdec4275fd 100755 --- a/test/functional/p2p_opportunistic_1p1c.py +++ b/test/functional/p2p_opportunistic_1p1c.py @@ -59,7 +59,6 @@ class PackageRelayTest(BitcoinTestFramework): self.setup_clean_chain = True self.num_nodes = 1 self.extra_args = [[ - "-datacarriersize=100000", "-maxmempool=5", ]] self.supports_cli = False diff --git a/test/functional/p2p_tx_download.py b/test/functional/p2p_tx_download.py index 2b75c1193d1..3da781dee2e 100755 --- a/test/functional/p2p_tx_download.py +++ b/test/functional/p2p_tx_download.py @@ -68,7 +68,7 @@ class ConnectionType(Enum): class TxDownloadTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 2 - self.extra_args= [['-datacarriersize=100000', '-maxmempool=5', '-persistmempool=0']] * self.num_nodes + self.extra_args= [['-maxmempool=5', '-persistmempool=0']] * self.num_nodes def test_tx_requests(self): self.log.info("Test that we request transactions from all our peers, eventually") diff --git a/test/functional/rpc_packages.py b/test/functional/rpc_packages.py index a2f9210f94d..119268213f4 100755 --- a/test/functional/rpc_packages.py +++ b/test/functional/rpc_packages.py @@ -438,7 +438,6 @@ class RPCPackagesTest(BitcoinTestFramework): # but child is too high fee # Lower mempool limit to make it easier to fill_mempool self.restart_node(0, extra_args=[ - "-datacarriersize=100000", "-maxmempool=5", "-persistmempool=0", ]) @@ -467,7 +466,7 @@ class RPCPackagesTest(BitcoinTestFramework): assert parent["txid"] not in node.getrawmempool() assert child["txid"] not in node.getrawmempool() - # Reset maxmempool, datacarriersize, reset dynamic mempool minimum feerate, and empty mempool. + # Reset maxmempool, reset dynamic mempool minimum feerate, and empty mempool. self.restart_node(0) self.wallet.rescan_utxos() diff --git a/test/functional/test_framework/mempool_util.py b/test/functional/test_framework/mempool_util.py index 0e9c821e2ea..ef30b31b249 100644 --- a/test/functional/test_framework/mempool_util.py +++ b/test/functional/test_framework/mempool_util.py @@ -41,7 +41,7 @@ def fill_mempool(test_framework, node, *, tx_sync_fun=None): """Fill mempool until eviction. Allows for simpler testing of scenarios with floating mempoolminfee > minrelay - Requires -datacarriersize=100000 and -maxmempool=5 and assumes -minrelaytxfee + Requires -maxmempool=5 and assumes -minrelaytxfee is 1 sat/vbyte. To avoid unintentional tx dependencies, the mempool filling txs are created with a tagged ephemeral miniwallet instance. diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index e6f72f43b54..5549d8f753c 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -73,8 +73,10 @@ WITNESS_SCALE_FACTOR = 4 DEFAULT_ANCESTOR_LIMIT = 25 # default max number of in-mempool ancestors DEFAULT_DESCENDANT_LIMIT = 25 # default max number of in-mempool descendants -# Default setting for -datacarriersize. 80 bytes of data, +1 for OP_RETURN, +2 for the pushdata opcodes. -MAX_OP_RETURN_RELAY = 83 + +# Default setting for -datacarriersize. +MAX_OP_RETURN_RELAY = 100_000 + DEFAULT_MEMPOOL_EXPIRY_HOURS = 336 # hours