Merge bitcoin#24836: add RPC (-regtest only) for testing package policy

e866f0d [functional test] submitrawpackage RPC (glozow)
fa07651 [rpc] add new submitpackage RPC (glozow)

Pull request description:

  It would be nice for LN/wallet/app devs to test out package policy, package RBF, etc., but the only interface to do so right now is through unit tests. This PR adds a `-regtest` only RPC interface so people can test by submitting raw transaction data. It is regtest-only, as it would be unsafe/confusing to create an actual mainnet interface while package relay doesn't exist.

  Note that the functional tests are there to ensure the RPC interface is working properly; they aren't for testing policy itself. See src/test/txpackage_tests.cpp.

ACKs for top commit:
  t-bast:
    Tested ACK against eclair bitcoin@e866f0d
  ariard:
    Code Review ACK e866f0d
  instagibbs:
    code review ACK e866f0d

Tree-SHA512: 824a26b10d2240e0fd85e5dd25bf499ee3dd9ba8ef4f522533998fcf767ddded9f001f7a005fe3ab07ec95e696448484e26599803e6034ed2733125c8c376c84

src/rpc/client.cpp

@@ -129,6 +129,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
     { "sendrawtransaction", 3, "bypasslimits" },
     { "testmempoolaccept", 0, "rawtxs" },
     { "testmempoolaccept", 1, "maxfeerate" },
+    { "submitpackage", 0, "package" },
     { "combinerawtransaction", 0, "txs" },
     { "fundrawtransaction", 1, "options" },
     { "walletcreatefundedpsbt", 0, "inputs" },

src/rpc/mempool.cpp

@@ -5,6 +5,7 @@
 
 #include <rpc/blockchain.h>
 
+#include <chainparams.h>
 #include <core_io.h>
 #include <fs.h>
 #include <policy/settings.h>
@@ -465,6 +466,150 @@ RPCHelpMan savemempool()
     };
 }
 
+static RPCHelpMan submitpackage()
+{
+    return RPCHelpMan{"submitpackage",
+        "Submit a package of raw transactions (serialized, hex-encoded) to local node (-regtest only).\n"
+        "The package will be validated according to consensus and mempool policy rules. If all transactions pass, they will be accepted to mempool.\n"
+        "This RPC is experimental and the interface may be unstable. Refer to doc/policy/packages.md for documentation on package policies.\n"
+        "Warning: until package relay is in use, successful submission does not mean the transaction will propagate to other nodes on the network.\n"
+        "Currently, each transaction is broadcasted individually after submission, which means they must meet other nodes' feerate requirements alone.\n"
+        ,
+        {
+            {"package", RPCArg::Type::ARR, RPCArg::Optional::NO, "An array of raw transactions.",
+                {
+                    {"rawtx", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, ""},
+                },
+            },
+        },
+        RPCResult{
+            RPCResult::Type::OBJ, "", "",
+            {
+                {RPCResult::Type::OBJ_DYN, "tx-results", "transaction results keyed by wtxid",
+                {
+                    {RPCResult::Type::OBJ, "wtxid", "transaction wtxid", {
+                        {RPCResult::Type::STR_HEX, "txid", "The transaction hash in hex"},
+                        {RPCResult::Type::STR_HEX, "other-wtxid", /*optional=*/true, "The wtxid of a different transaction with the same txid but different witness found in the mempool. This means the submitted transaction was ignored."},
+                        {RPCResult::Type::NUM, "vsize", "Virtual transaction size as defined in BIP 141."},
+                        {RPCResult::Type::OBJ, "fees", "Transaction fees", {
+                            {RPCResult::Type::STR_AMOUNT, "base", "transaction fee in " + CURRENCY_UNIT},
+                        }},
+                    }}
+                }},
+                {RPCResult::Type::STR_AMOUNT, "package-feerate", /*optional=*/true, "package feerate used for feerate checks in " + CURRENCY_UNIT + " per KvB. Excludes transactions which were deduplicated or accepted individually."},
+                {RPCResult::Type::ARR, "replaced-transactions", /*optional=*/true, "List of txids of replaced transactions",
+                {
+                    {RPCResult::Type::STR_HEX, "", "The transaction id"},
+                }},
+            },
+        },
+        RPCExamples{
+            HelpExampleCli("testmempoolaccept", "[rawtx1, rawtx2]") +
+            HelpExampleCli("submitpackage", "[rawtx1, rawtx2]")
+        },
+        [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
+        {
+            if (!Params().IsMockableChain()) {
+                throw std::runtime_error("submitpackage is for regression testing (-regtest mode) only");
+            }
+            RPCTypeCheck(request.params, {
+                UniValue::VARR,
+            });
+            const UniValue raw_transactions = request.params[0].get_array();
+            if (raw_transactions.size() < 1 || raw_transactions.size() > MAX_PACKAGE_COUNT) {
+                throw JSONRPCError(RPC_INVALID_PARAMETER,
+                                   "Array must contain between 1 and " + ToString(MAX_PACKAGE_COUNT) + " transactions.");
+            }
+
+            std::vector<CTransactionRef> txns;
+            txns.reserve(raw_transactions.size());
+            for (const auto& rawtx : raw_transactions.getValues()) {
+                CMutableTransaction mtx;
+                if (!DecodeHexTx(mtx, rawtx.get_str())) {
+                    throw JSONRPCError(RPC_DESERIALIZATION_ERROR,
+                                       "TX decode failed: " + rawtx.get_str() + " Make sure the tx has at least one input.");
+                }
+                txns.emplace_back(MakeTransactionRef(std::move(mtx)));
+            }
+
+            NodeContext& node = EnsureAnyNodeContext(request.context);
+            CTxMemPool& mempool = EnsureMemPool(node);
+            CChainState& chainstate = EnsureChainman(node).ActiveChainstate();
+            const auto package_result = WITH_LOCK(::cs_main, return ProcessNewPackage(chainstate, mempool, txns, /*test_accept=*/ false));
+
+            // First catch any errors.
+            switch(package_result.m_state.GetResult()) {
+                case PackageValidationResult::PCKG_RESULT_UNSET: break;
+                case PackageValidationResult::PCKG_POLICY:
+                {
+                    throw JSONRPCTransactionError(TransactionError::INVALID_PACKAGE,
+                        package_result.m_state.GetRejectReason());
+                }
+                case PackageValidationResult::PCKG_MEMPOOL_ERROR:
+                {
+                    throw JSONRPCTransactionError(TransactionError::MEMPOOL_ERROR,
+                        package_result.m_state.GetRejectReason());
+                }
+                case PackageValidationResult::PCKG_TX:
+                {
+                    for (const auto& tx : txns) {
+                        auto it = package_result.m_tx_results.find(tx->GetWitnessHash());
+                        if (it != package_result.m_tx_results.end() && it->second.m_state.IsInvalid()) {
+                            throw JSONRPCTransactionError(TransactionError::MEMPOOL_REJECTED,
+                                strprintf("%s failed: %s", tx->GetHash().ToString(), it->second.m_state.GetRejectReason()));
+                        }
+                    }
+                    // If a PCKG_TX error was returned, there must have been an invalid transaction.
+                    NONFATAL_UNREACHABLE();
+                }
+            }
+            for (const auto& tx : txns) {
+                size_t num_submitted{0};
+                std::string err_string;
+                const auto err = BroadcastTransaction(node, tx, err_string, 0, true, true);
+                if (err != TransactionError::OK) {
+                    throw JSONRPCTransactionError(err,
+                        strprintf("transaction broadcast failed: %s (all transactions were submitted, %d transactions were broadcast successfully)",
+                            err_string, num_submitted));
+                }
+            }
+            UniValue rpc_result{UniValue::VOBJ};
+            UniValue tx_result_map{UniValue::VOBJ};
+            std::set<uint256> replaced_txids;
+            for (const auto& tx : txns) {
+                auto it = package_result.m_tx_results.find(tx->GetWitnessHash());
+                CHECK_NONFATAL(it != package_result.m_tx_results.end());
+                UniValue result_inner{UniValue::VOBJ};
+                result_inner.pushKV("txid", tx->GetHash().GetHex());
+                if (it->second.m_result_type == MempoolAcceptResult::ResultType::DIFFERENT_WITNESS) {
+                    result_inner.pushKV("other-wtxid", it->second.m_other_wtxid.value().GetHex());
+                }
+                if (it->second.m_result_type == MempoolAcceptResult::ResultType::VALID ||
+                    it->second.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY) {
+                    result_inner.pushKV("vsize", int64_t{it->second.m_vsize.value()});
+                    UniValue fees(UniValue::VOBJ);
+                    fees.pushKV("base", ValueFromAmount(it->second.m_base_fees.value()));
+                    result_inner.pushKV("fees", fees);
+                    if (it->second.m_replaced_transactions.has_value()) {
+                        for (const auto& ptx : it->second.m_replaced_transactions.value()) {
+                            replaced_txids.insert(ptx->GetHash());
+                        }
+                    }
+                }
+                tx_result_map.pushKV(tx->GetWitnessHash().GetHex(), result_inner);
+            }
+            rpc_result.pushKV("tx-results", tx_result_map);
+            if (package_result.m_package_feerate.has_value()) {
+                rpc_result.pushKV("package-feerate", ValueFromAmount(package_result.m_package_feerate.value().GetFeePerK()));
+            }
+            UniValue replaced_list(UniValue::VARR);
+            for (const uint256& hash : replaced_txids) replaced_list.push_back(hash.ToString());
+            rpc_result.pushKV("replaced-transactions", replaced_list);
+            return rpc_result;
+        },
+    };
+}
+
 void RegisterMempoolRPCCommands(CRPCTable& t)
 {
     static const CRPCCommand commands[]{
@@ -476,6 +621,7 @@ void RegisterMempoolRPCCommands(CRPCTable& t)
         {"blockchain", &getmempoolinfo},
         {"blockchain", &getrawmempool},
         {"blockchain", &savemempool},
+        {"hidden", &submitpackage},
     };
     for (const auto& c : commands) {
         t.appendCommand(c.name, &c);

src/test/fuzz/rpc.cpp

@@ -158,6 +158,7 @@ const std::vector<std::string> RPC_COMMANDS_SAFE_FOR_FUZZING{
     "signrawtransactionwithkey",
     "submitblock",
     "submitheader",
+    "submitpackage",
     "syncwithvalidationinterfacequeue",
     "testmempoolaccept",
     "uptime",

src/util/error.cpp

@@ -31,6 +31,8 @@ bilingual_str TransactionErrorString(const TransactionError err)
             return Untranslated("Specified sighash value does not match value stored in PSBT");
         case TransactionError::MAX_FEE_EXCEEDED:
             return Untranslated("Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)");
+        case TransactionError::INVALID_PACKAGE:
+            return Untranslated("Transaction rejected due to invalid package");
         // no default case, so the compiler can warn about missing cases
     }
     assert(false);

src/util/error.h

@@ -30,6 +30,7 @@ enum class TransactionError {
     PSBT_MISMATCH,
     SIGHASH_MISMATCH,
     MAX_FEE_EXCEEDED,
+    INVALID_PACKAGE,
 };
 
 bilingual_str TransactionErrorString(const TransactionError error);

test/functional/rpc_packages.py

@@ -12,16 +12,20 @@
 from test_framework.messages import (
     tx_from_hex,
 )
+from test_framework.p2p import P2PTxInvStore
 from test_framework.script import (
     CScript,
     OP_TRUE,
 )
 from test_framework.util import (
     assert_equal,
+    assert_fee_amount,
+    assert_raises_rpc_error,
 )
 from test_framework.wallet import (
     create_child_with_parents,
     create_raw_chain,
+    DEFAULT_FEE,
     make_chain,
 )
 
@@ -48,7 +52,7 @@ def run_test(self):
         self.address = node.get_deterministic_priv_key().address
         self.coins = []
         # The last 100 coinbase transactions are premature
-        for b in self.generatetoaddress(node, 200, self.address)[:100]:
+        for b in self.generatetoaddress(node, 220, self.address)[:-100]:
             coinbase = node.getblock(blockhash=b, verbosity=2)["tx"][0]
             self.coins.append({
                 "txid": coinbase["txid"],
@@ -79,6 +83,7 @@ def run_test(self):
         self.test_multiple_parents()
         self.test_conflicting()
 
+        self.test_submitpackage()
 
     def test_independent(self):
         self.log.info("Test multiple independent transactions in a package")
@@ -129,8 +134,7 @@ def test_independent(self):
 
     def test_chain(self):
         node = self.nodes[0]
-        first_coin = self.coins.pop()
-        (chain_hex, chain_txns) = create_raw_chain(node, first_coin, self.address, self.privkeys)
+        (chain_hex, chain_txns) = create_raw_chain(node, self.coins.pop(), self.address, self.privkeys)
         self.log.info("Check that testmempoolaccept requires packages to be sorted by dependency")
         assert_equal(node.testmempoolaccept(rawtxs=chain_hex[::-1]),
                 [{"txid": tx.rehash(), "package-error": "package-not-sorted"} for tx in chain_txns[::-1]])
@@ -263,5 +267,127 @@ def test_conflicting(self):
             {"txid": tx2.rehash(), "package-error": "conflict-in-package"}
         ])
 
+    def assert_equal_package_results(self, node, testmempoolaccept_result, submitpackage_result):
+        """Assert that a successful submitpackage result is consistent with testmempoolaccept
+        results and getmempoolentry info. Note that the result structs are different and, due to
+        policy differences between testmempoolaccept and submitpackage (i.e. package feerate),
+        some information may be different.
+        """
+        for testres_tx in testmempoolaccept_result:
+            # Grab this result from the submitpackage_result
+            submitres_tx = submitpackage_result["tx-results"][testres_tx["wtxid"]]
+            assert_equal(submitres_tx["txid"], testres_tx["txid"])
+            # No "allowed" if the tx was already in the mempool
+            if "allowed" in testres_tx and testres_tx["allowed"]:
+                assert_equal(submitres_tx["vsize"], testres_tx["vsize"])
+                assert_equal(submitres_tx["fees"]["base"], testres_tx["fees"]["base"])
+            entry_info = node.getmempoolentry(submitres_tx["txid"])
+            assert_equal(submitres_tx["vsize"], entry_info["vsize"])
+            assert_equal(submitres_tx["fees"]["base"], entry_info["fees"]["base"])
+
+    def test_submit_child_with_parents(self, num_parents, partial_submit):
+        node = self.nodes[0]
+        peer = node.add_p2p_connection(P2PTxInvStore())
+        # Test a package with num_parents parents and 1 child transaction.
+        package_hex = []
+        package_txns = []
+        values = []
+        scripts = []
+        for _ in range(num_parents):
+            parent_coin = self.coins.pop()
+            value = parent_coin["amount"]
+            (tx, txhex, value, spk) = make_chain(node, self.address, self.privkeys, parent_coin["txid"], value)
+            package_hex.append(txhex)
+            package_txns.append(tx)
+            values.append(value)
+            scripts.append(spk)
+            if partial_submit and random.choice([True, False]):
+                node.sendrawtransaction(txhex)
+        child_hex = create_child_with_parents(node, self.address, self.privkeys, package_txns, values, scripts)
+        package_hex.append(child_hex)
+        package_txns.append(tx_from_hex(child_hex))
+
+        testmempoolaccept_result = node.testmempoolaccept(rawtxs=package_hex)
+        submitpackage_result = node.submitpackage(package=package_hex)
+
+        # Check that each result is present, with the correct size and fees
+        for i in range(num_parents + 1):
+            tx = package_txns[i]
+            wtxid = tx.getwtxid()
+            assert wtxid in submitpackage_result["tx-results"]
+            tx_result = submitpackage_result["tx-results"][wtxid]
+            assert_equal(tx_result, {
+                "txid": tx.rehash(),
+                "vsize": tx.get_vsize(),
+                "fees": {
+                    "base": DEFAULT_FEE,
+                }
+            })
+
+        # submitpackage result should be consistent with testmempoolaccept and getmempoolentry
+        self.assert_equal_package_results(node, testmempoolaccept_result, submitpackage_result)
+
+        # Package feerate is calculated for the remaining transactions after deduplication and
+        # individual submission. If only 0 or 1 transaction is left, e.g. because all transactions
+        # had high-feerates or were already in the mempool, no package feerate is provided.
+        # In this case, since all of the parents have high fees, each is accepted individually.
+        assert "package-feerate" not in submitpackage_result
+
+        # The node should announce each transaction. No guarantees for propagation.
+        peer.wait_for_broadcast([tx.getwtxid() for tx in package_txns])
+        self.generate(node, 1)
+
+
+    def test_submit_cpfp(self):
+        node = self.nodes[0]
+        peer = node.add_p2p_connection(P2PTxInvStore())
+
+        # 2 parent 1 child CPFP. First parent pays high fees, second parent pays 0 fees and is
+        # fee-bumped by the child.
+        coin_rich = self.coins.pop()
+        coin_poor = self.coins.pop()
+        tx_rich, hex_rich, value_rich, spk_rich = make_chain(node, self.address, self.privkeys, coin_rich["txid"], coin_rich["amount"])
+        tx_poor, hex_poor, value_poor, spk_poor = make_chain(node, self.address, self.privkeys, coin_poor["txid"], coin_poor["amount"], fee=0)
+        package_txns = [tx_rich, tx_poor]
+        hex_child = create_child_with_parents(node, self.address, self.privkeys, package_txns, [value_rich, value_poor], [spk_rich, spk_poor])
+        tx_child = tx_from_hex(hex_child)
+        package_txns.append(tx_child)
+
+        submitpackage_result = node.submitpackage([hex_rich, hex_poor, hex_child])
+
+        rich_parent_result = submitpackage_result["tx-results"][tx_rich.getwtxid()]
+        poor_parent_result = submitpackage_result["tx-results"][tx_poor.getwtxid()]
+        child_result = submitpackage_result["tx-results"][tx_child.getwtxid()]
+        assert_equal(rich_parent_result["fees"]["base"], DEFAULT_FEE)
+        assert_equal(poor_parent_result["fees"]["base"], 0)
+        assert_equal(child_result["fees"]["base"], DEFAULT_FEE)
+        # Package feerate is calculated for the remaining transactions after deduplication and
+        # individual submission. Since this package had a 0-fee parent, package feerate must have
+        # been used and returned.
+        assert "package-feerate" in submitpackage_result
+        assert_fee_amount(DEFAULT_FEE, rich_parent_result["vsize"] + child_result["vsize"], submitpackage_result["package-feerate"])
+
+        # The node will broadcast each transaction, still abiding by its peer's fee filter
+        peer.wait_for_broadcast([tx.getwtxid() for tx in package_txns])
+        self.generate(node, 1)
+
+
+    def test_submitpackage(self):
+        node = self.nodes[0]
+
+        self.log.info("Submitpackage valid packages with 1 child and some number of parents")
+        for num_parents in [1, 2, 24]:
+            self.test_submit_child_with_parents(num_parents, False)
+            self.test_submit_child_with_parents(num_parents, True)
+
+        self.log.info("Submitpackage valid packages with CPFP")
+        self.test_submit_cpfp()
+
+        self.log.info("Submitpackage only allows packages of 1 child with its parents")
+        # Chain of 3 transactions has too many generations
+        chain_hex, _ = create_raw_chain(node, self.coins.pop(), self.address, self.privkeys, 3)
+        assert_raises_rpc_error(-25, "not-child-with-parents", node.submitpackage, chain_hex)
+
+
 if __name__ == "__main__":
     RPCPackagesTest().main()

test/functional/test_runner.py

@@ -153,6 +153,7 @@
     'wallet_abandonconflict.py --legacy-wallet',
     'wallet_abandonconflict.py --descriptors',
     'feature_csv_activation.py',
+    'rpc_packages.py',
     'feature_reindex.py',
     'feature_abortnode.py',
     # vv Tests less than 30s vv
@@ -264,7 +265,6 @@
     'mempool_package_onemore.py',
     'rpc_createmultisig.py --legacy-wallet',
     'rpc_createmultisig.py --descriptors',
-    'rpc_packages.py',
     'mempool_package_limits.py',
     'feature_versionbits_warning.py',
     'rpc_preciousblock.py',