policy: uncap datacarrier by default

Datacarrier output script sizes and output counts are now
uncapped by default.

To avoid introducing another startup argument, we modify the
OP_RETURN accounting to "budget" the spk sizes.

If a user has set a custom default, this results in that
budget being spent over the sum of all OP_RETURN outputs'
scripts in the transaction, no longer capping the number
of OP_RETURN outputs themselves. This should allow a
superset of current behavior while respecting the passed
argument in terms of total arbitrary data storage.

Co-authored-by: Anthony Towns <aj@erisian.com.au>

test/functional/mempool_accept.py

@@ -326,13 +326,6 @@ class MempoolAcceptanceTest(BitcoinTestFramework):
             result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': 'dust'}],
             rawtxs=[tx.serialize().hex()],
         )
-        tx = tx_from_hex(raw_tx_reference)
-        tx.vout[0].scriptPubKey = CScript([OP_RETURN, b'\xff'])
-        tx.vout = [tx.vout[0]] * 2
-        self.check_mempool_result(
-            result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': 'multi-op-return'}],
-            rawtxs=[tx.serialize().hex()],
-        )
 
         self.log.info('A timelocked transaction')
         tx = tx_from_hex(raw_tx_reference)

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)

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