Remove arbitrary restrictions on OP_RETURN by default

Any number or size of data-carrying OP_RETURN outputs are allowed, and the
`-datacarrier` option is removed. For those who want to limit data carrying
outputs, `-datacarriersize` is still supported, and has the functionality of
applying the specified data carrier limit as well as limiting the number of
data carrying OP_RETURN outputs to one. If `-datacarriersize=0` is set, no data
carrying output is allowed.

Rational: there's lots of ways for people to publish data in the Bitcoin chain,
and lots of data has been published.  There's no reason for us to place
artificial limits on this particular method.

src/init.cpp

@@ -589,8 +589,7 @@ void SetupServerArgs(ArgsManager& argsman)
     argsman.AddArg("-dustrelayfee=<amt>", 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<std::chrono::hours>(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("-datacarriersize", strprintf("Maximum size of data in data carrier transactions we relay and mine (default: %u)", MAX_OP_RETURN_RELAY), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY);
+    argsman.AddArg("-datacarriersize", "Limit the size of data in data carrier transactions we relay and mine", ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY);
     argsman.AddArg("-mempoolfullrbf", strprintf("Accept transaction replace-by-fee without requiring replaceability signaling (default: %u)", DEFAULT_MEMPOOL_FULL_RBF), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY);
     argsman.AddArg("-permitbaremultisig", strprintf("Relay non-P2SH multisig (default: %u)", DEFAULT_PERMIT_BAREMULTISIG), ArgsManager::ALLOW_ANY,
                    OptionsCategory::NODE_RELAY);

src/kernel/mempool_options.h

@@ -49,9 +49,9 @@ struct MemPoolOptions {
      * type is designated as TxoutType::NULL_DATA.
      *
      * Maximum size of TxoutType::NULL_DATA scripts that this node considers standard.
-     * If nullopt, any size is nonstandard.
+     * If nullopt, there is no limit on the size or number of outputs.
      */
-    std::optional<unsigned> max_datacarrier_bytes{DEFAULT_ACCEPT_DATACARRIER ? std::optional{MAX_OP_RETURN_RELAY} : std::nullopt};
+    std::optional<unsigned> max_datacarrier_bytes{std::nullopt};
     bool permit_bare_multisig{DEFAULT_PERMIT_BAREMULTISIG};
     bool require_standard{true};
     bool full_rbf{DEFAULT_MEMPOOL_FULL_RBF};

src/node/mempool_args.cpp

@@ -81,11 +81,7 @@ util::Result<void> ApplyArgsManOptions(const ArgsManager& argsman, const CChainP
 
     mempool_opts.permit_bare_multisig = argsman.GetBoolArg("-permitbaremultisig", DEFAULT_PERMIT_BAREMULTISIG);
 
-    if (argsman.GetBoolArg("-datacarrier", DEFAULT_ACCEPT_DATACARRIER)) {
-        mempool_opts.max_datacarrier_bytes = argsman.GetIntArg("-datacarriersize", MAX_OP_RETURN_RELAY);
-    } else {
-        mempool_opts.max_datacarrier_bytes = std::nullopt;
-    }
+    mempool_opts.max_datacarrier_bytes = argsman.GetIntArg("-datacarriersize");
 
     mempool_opts.require_standard = !argsman.GetBoolArg("-acceptnonstdtxn", !chainparams.RequireStandard());
     if (!chainparams.IsTestChain() && !mempool_opts.require_standard) {

src/policy/policy.cpp

@@ -83,7 +83,7 @@ bool IsStandard(const CScript& scriptPubKey, const std::optional<unsigned>& max_
         if (m < 1 || m > n)
             return false;
     } else if (whichType == TxoutType::NULL_DATA) {
-        if (!max_datacarrier_bytes || scriptPubKey.size() > *max_datacarrier_bytes + 1) {
+        if (max_datacarrier_bytes && scriptPubKey.size() > *max_datacarrier_bytes + 1) {
             return false;
         }
     }
@@ -136,7 +136,8 @@ bool IsStandardTx(const CTransaction& tx, const std::optional<unsigned>& max_dat
             return false;
         }
 
-        if (whichType == TxoutType::NULL_DATA)
+        // data carrying outputs only count if they actually carry data
+        if (whichType == TxoutType::NULL_DATA && txout.scriptPubKey.size() > 1)
             nDataOut++;
         else if ((whichType == TxoutType::MULTISIG) && (!permit_bare_multisig)) {
             reason = "bare-multisig";
@@ -147,8 +148,9 @@ bool IsStandardTx(const CTransaction& tx, const std::optional<unsigned>& max_dat
         }
     }
 
-    // only one OP_RETURN txout is permitted
-    if (nDataOut > 1) {
+    // only one data carrying OP_RETURN txout is permitted if datacarrier is
+    // limited
+    if (max_datacarrier_bytes && nDataOut > 1) {
         reason = "multi-op-return";
         return false;
     }

src/script/standard.h

@@ -32,12 +32,6 @@ class CScriptID : public BaseHash<uint160>
     explicit CScriptID(const ScriptHash& in);
 };
 
-/**
- * Default setting for -datacarriersize. 80 bytes of data, +2 for the pushdata
- * opcodes.
- */
-static const unsigned int MAX_OP_RETURN_RELAY = 82;
-
 /**
  * Mandatory script verification flags that all new blocks must comply with for
  * them to be valid. (but old blocks may not comply with) Currently just P2SH,

src/test/transaction_tests.cpp

@@ -766,12 +766,12 @@ BOOST_AUTO_TEST_CASE(test_IsStandard)
 
     constexpr auto CheckIsStandard = [](const auto& t) {
         std::string reason;
-        BOOST_CHECK(IsStandardTx(CTransaction{t}, MAX_OP_RETURN_RELAY, g_bare_multi, g_dust, reason));
+        BOOST_CHECK(IsStandardTx(CTransaction{t}, 82, g_bare_multi, g_dust, reason));
         BOOST_CHECK(reason.empty());
     };
     constexpr auto CheckIsNotStandard = [](const auto& t, const std::string& reason_in) {
         std::string reason;
-        BOOST_CHECK(!IsStandardTx(CTransaction{t}, MAX_OP_RETURN_RELAY, g_bare_multi, g_dust, reason));
+        BOOST_CHECK(!IsStandardTx(CTransaction{t}, 82, g_bare_multi, g_dust, reason));
         BOOST_CHECK_EQUAL(reason_in, reason);
     };
 
@@ -820,12 +820,12 @@ BOOST_AUTO_TEST_CASE(test_IsStandard)
 
     // MAX_OP_RETURN_RELAY-byte TxoutType::NULL_DATA (standard)
     t.vout[0].scriptPubKey = CScript() << OP_RETURN << ParseHex("04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef3804678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38");
-    BOOST_CHECK_EQUAL(MAX_OP_RETURN_RELAY + 1, t.vout[0].scriptPubKey.size());
+    BOOST_CHECK_EQUAL(83, t.vout[0].scriptPubKey.size());
     CheckIsStandard(t);
 
     // MAX_OP_RETURN_RELAY+1-byte TxoutType::NULL_DATA (non-standard)
     t.vout[0].scriptPubKey = CScript() << OP_RETURN << ParseHex("04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef3804678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef3800");
-    BOOST_CHECK_EQUAL(MAX_OP_RETURN_RELAY + 2, t.vout[0].scriptPubKey.size());
+    BOOST_CHECK_EQUAL(84, t.vout[0].scriptPubKey.size());
     CheckIsNotStandard(t, "scriptpubkey");
 
     // Data payload can be encoded in any way...
@@ -856,14 +856,6 @@ BOOST_AUTO_TEST_CASE(test_IsStandard)
     t.vout[1].nValue = 0;
     CheckIsNotStandard(t, "multi-op-return");
 
-    t.vout[0].scriptPubKey = CScript() << OP_RETURN << ParseHex("04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38");
-    t.vout[1].scriptPubKey = CScript() << OP_RETURN;
-    CheckIsNotStandard(t, "multi-op-return");
-
-    t.vout[0].scriptPubKey = CScript() << OP_RETURN;
-    t.vout[1].scriptPubKey = CScript() << OP_RETURN;
-    CheckIsNotStandard(t, "multi-op-return");
-
     // Check large scriptSig (non-standard if size is >1650 bytes)
     t.vout.resize(1);
     t.vout[0].nValue = MAX_MONEY;

test/functional/mempool_datacarrier.py

@@ -5,7 +5,6 @@
 """Test datacarrier functionality"""
 from test_framework.messages import (
     CTxOut,
-    MAX_OP_RETURN_RELAY,
 )
 from test_framework.script import (
     CScript,
@@ -25,13 +24,31 @@ def set_test_params(self):
         self.num_nodes = 3
         self.extra_args = [
             [],
-            ["-datacarrier=0"],
-            ["-datacarrier=1", f"-datacarriersize={MAX_OP_RETURN_RELAY - 1}"]
+            ["-datacarriersize=0"],
+            ["-datacarriersize=100"]
         ]
 
-    def test_null_data_transaction(self, node: TestNode, data: bytes, success: bool) -> None:
+    def test_null_data_transaction(self, node: TestNode, datas: [bytes], success: bool) -> None:
         tx = self.wallet.create_self_transfer(fee_rate=0)["tx"]
-        tx.vout.append(CTxOut(nValue=0, scriptPubKey=CScript([OP_RETURN, data])))
+
+        for data in datas:
+            tx.vout.append(CTxOut(nValue=0, scriptPubKey=CScript([OP_RETURN, data])))
+
+        tx.vout[0].nValue -= tx.get_vsize()  # simply pay 1sat/vbyte fee
+
+        tx_hex = tx.serialize().hex()
+
+        if success:
+            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)
+
+    def test_bare_op_return(self, node: TestNode, n: int, success: bool) -> None:
+        tx = self.wallet.create_self_transfer(fee_rate=0)["tx"]
+
+        for i in range(0, n):
+            tx.vout.append(CTxOut(nValue=0, scriptPubKey=CScript([OP_RETURN])))
         tx.vout[0].nValue -= tx.get_vsize()  # simply pay 1sat/vbyte fee
 
         tx_hex = tx.serialize().hex()
@@ -45,26 +62,33 @@ def test_null_data_transaction(self, node: TestNode, data: bytes, success: bool)
     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 = random_bytes(MAX_OP_RETURN_RELAY - 3)
-        too_long_data = random_bytes(MAX_OP_RETURN_RELAY - 2)
-        small_data = random_bytes(MAX_OP_RETURN_RELAY - 4)
+        # Pushdata opcode takes up 2 bytes
+        exact_size_data = random_bytes(100 - 2)
+        too_large_data = random_bytes(100 - 1)
+        small_data = random_bytes(100 - 3)
 
-        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 null data transaction without -datacarriersize limit.")
+        self.test_null_data_transaction(node=self.nodes[0], datas=[random_bytes(99800)], 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 multiple null data outputs without -datacarriersize limit.")
+        self.test_null_data_transaction(node=self.nodes[0], datas=[random_bytes(100), random_bytes(100)], 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.log.info("Testing a null data transaction with -datacarriersize=0.")
+        self.test_null_data_transaction(node=self.nodes[1], datas=[b''], 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], datas=[too_large_data], success=False)
+
+        self.log.info("Testing a null data transaction with a size exactly equal to accepted by -datacarriersize.")
+        self.test_null_data_transaction(node=self.nodes[2], datas=[exact_size_data], success=True)
 
         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.test_null_data_transaction(node=self.nodes[2], datas=[small_data], success=True)
 
+        self.log.info("Testing that a single and multiple bare OP_RETURN is always accepted")
+        for n in (1, 2):
+            for node in self.nodes:
+                self.test_bare_op_return(node=node, n=n, success=True)
 
 if __name__ == '__main__':
     DataCarrierTest().main()

test/functional/test_framework/messages.py

@@ -68,9 +68,6 @@
 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_MEMPOOL_EXPIRY_HOURS = 336  # hours
 
 def sha256(s):