Merge bitcoin/bitcoin#27609: rpc: allow submitpackage to be called ou…

…tside of regtest

5b878be [doc] add release note for submitpackage (glozow)
7a9bb2a [rpc] allow submitpackage to be called outside of regtest (glozow)
5b9087a [rpc] require package to be a tree in submitpackage (glozow)
e32ba15 [txpackages] IsChildWithParentsTree() (glozow)
b4f28cc [doc] parent pay for child in aggregate CheckFeeRate (glozow)

Pull request description:

  Permit (restricted topology) submitpackage RPC outside of regtest. Suggested in bitcoin/bitcoin#26933 (comment)

  This RPC should be safe but still experimental - interface may change, not all features (e.g. package RBF) are implemented, etc. If a miner wants to expose this to people, they can effectively use "package relay" before the p2p changes are implemented. However, please note **this is not package relay**; transactions submitted this way will not relay to other nodes if the feerates are below their mempool min fee. Users should put this behind some kind of rate limit or permissions.

ACKs for top commit:
  instagibbs:
    ACK 5b878be
  achow101:
    ACK 5b878be
  dergoegge:
    Code review ACK 5b878be
  ajtowns:
    ACK 5b878be
  ariard:
    Code Review ACK  5b878be. Though didn’t manually test the PR.

Tree-SHA512: 610365c0b2ffcccd55dedd1151879c82de1027e3319712bcb11d54f2467afaae4d05dca5f4b25f03354c80845fef538d3938b958174dda8b14c10670537a6524

doc/release-notes-27609.md

@@ -0,0 +1,14 @@
+- A new RPC, `submitpackage`, has been added. It can be used to submit a list of raw hex
+  transactions to the mempool to be evaluated as a package using consensus and mempool policy rules.
+These policies include package CPFP, allowing a child with high fees to bump a parent below the
+mempool minimum feerate (but not minimum relay feerate).
+
+    - Warning: successful submission does not mean the transactions will propagate throughout the
+      network, as package relay is not supported.
+
+    - Not all features are available. The package is limited to a child with all of its
+      unconfirmed parents, and no parent may spend the output of another parent.  Also, package
+      RBF is not supported. Refer to doc/policy/packages.md for more details on package policies
+      and limitations.
+
+    - This RPC is experimental. Its interface may change.

src/policy/packages.cpp

@@ -88,3 +88,18 @@ bool IsChildWithParents(const Package& package)
     return std::all_of(package.cbegin(), package.cend() - 1,
                        [&input_txids](const auto& ptx) { return input_txids.count(ptx->GetHash()) > 0; });
 }
+
+bool IsChildWithParentsTree(const Package& package)
+{
+    if (!IsChildWithParents(package)) return false;
+    std::unordered_set<uint256, SaltedTxidHasher> parent_txids;
+    std::transform(package.cbegin(), package.cend() - 1, std::inserter(parent_txids, parent_txids.end()),
+                   [](const auto& ptx) { return ptx->GetHash(); });
+    // Each parent must not have an input who is one of the other parents.
+    return std::all_of(package.cbegin(), package.cend() - 1, [&](const auto& ptx) {
+        for (const auto& input : ptx->vin) {
+            if (parent_txids.count(input.prevout.hash) > 0) return false;
+        }
+        return true;
+    });
+}

src/policy/packages.h

@@ -63,4 +63,8 @@ bool CheckPackage(const Package& txns, PackageValidationState& state);
  */
 bool IsChildWithParents(const Package& package);
 
+/** Context-free check that a package IsChildWithParents() and none of the parents depend on each
+ * other (the package is a "tree").
+ */
+bool IsChildWithParentsTree(const Package& package);
 #endif // BITCOIN_POLICY_PACKAGES_H

src/rpc/mempool.cpp

@@ -819,11 +819,11 @@ static RPCHelpMan savemempool()
 static RPCHelpMan submitpackage()
 {
     return RPCHelpMan{"submitpackage",
-        "Submit a package of raw transactions (serialized, hex-encoded) to local node (-regtest only).\n"
+        "Submit a package of raw transactions (serialized, hex-encoded) to local node.\n"
+        "The package must consist of a child with its parents, and none of the parents may depend on one another.\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"
+        "Warning: successful submission does not mean the transactions will propagate throughout the network.\n"
         ,
         {
             {"package", RPCArg::Type::ARR, RPCArg::Optional::NO, "An array of raw transactions.",
@@ -862,9 +862,6 @@ static RPCHelpMan submitpackage()
         },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
         {
-            if (Params().GetChainType() != ChainType::REGTEST) {
-                throw std::runtime_error("submitpackage is for regression testing (-regtest mode) only");
-            }
             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,
@@ -881,6 +878,9 @@ static RPCHelpMan submitpackage()
                 }
                 txns.emplace_back(MakeTransactionRef(std::move(mtx)));
             }
+            if (!IsChildWithParentsTree(txns)) {
+                throw JSONRPCTransactionError(TransactionError::INVALID_PACKAGE, "package topology disallowed. not child-with-parents or parents depend on each other.");
+            }
 
             NodeContext& node = EnsureAnyNodeContext(request.context);
             CTxMemPool& mempool = EnsureMemPool(node);
@@ -983,7 +983,7 @@ void RegisterMempoolRPCCommands(CRPCTable& t)
         {"blockchain", &getrawmempool},
         {"blockchain", &importmempool},
         {"blockchain", &savemempool},
-        {"hidden", &submitpackage},
+        {"rawtransactions", &submitpackage},
     };
     for (const auto& c : commands) {
         t.appendCommand(c.name, &c);

src/test/txpackage_tests.cpp

@@ -162,6 +162,7 @@ BOOST_FIXTURE_TEST_CASE(noncontextual_package_tests, TestChain100Setup)
         BOOST_CHECK_EQUAL(state.GetResult(), PackageValidationResult::PCKG_POLICY);
         BOOST_CHECK_EQUAL(state.GetRejectReason(), "package-not-sorted");
         BOOST_CHECK(IsChildWithParents({tx_parent, tx_child}));
+        BOOST_CHECK(IsChildWithParentsTree({tx_parent, tx_child}));
     }
 
     // 24 Parents and 1 Child
@@ -187,6 +188,7 @@ BOOST_FIXTURE_TEST_CASE(noncontextual_package_tests, TestChain100Setup)
         PackageValidationState state;
         BOOST_CHECK(CheckPackage(package, state));
         BOOST_CHECK(IsChildWithParents(package));
+        BOOST_CHECK(IsChildWithParentsTree(package));
 
         package.erase(package.begin());
         BOOST_CHECK(IsChildWithParents(package));
@@ -219,6 +221,7 @@ BOOST_FIXTURE_TEST_CASE(noncontextual_package_tests, TestChain100Setup)
         BOOST_CHECK(IsChildWithParents({tx_parent, tx_parent_also_child}));
         BOOST_CHECK(IsChildWithParents({tx_parent, tx_child}));
         BOOST_CHECK(IsChildWithParents({tx_parent, tx_parent_also_child, tx_child}));
+        BOOST_CHECK(!IsChildWithParentsTree({tx_parent, tx_parent_also_child, tx_child}));
         // IsChildWithParents does not detect unsorted parents.
         BOOST_CHECK(IsChildWithParents({tx_parent_also_child, tx_parent, tx_child}));
         BOOST_CHECK(CheckPackage({tx_parent, tx_parent_also_child, tx_child}, state));

src/validation.cpp

@@ -1287,6 +1287,12 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std::
     // Transactions must meet two minimum feerates: the mempool minimum fee and min relay fee.
     // For transactions consisting of exactly one child and its parents, it suffices to use the
     // package feerate (total modified fees / total virtual size) to check this requirement.
+    // Note that this is an aggregate feerate; this function has not checked that there are transactions
+    // too low feerate to pay for themselves, or that the child transactions are higher feerate than
+    // their parents. Using aggregate feerate may allow "parents pay for child" behavior and permit
+    // a child that is below mempool minimum feerate. To avoid these behaviors, callers of
+    // AcceptMultipleTransactions need to restrict txns topology (e.g. to ancestor sets) and check
+    // the feerates of individuals and subsets.
     const auto m_total_vsize = std::accumulate(workspaces.cbegin(), workspaces.cend(), int64_t{0},
         [](int64_t sum, auto& ws) { return sum + ws.m_vsize; });
     const auto m_total_modified_fees = std::accumulate(workspaces.cbegin(), workspaces.cend(), CAmount{0},

test/functional/rpc_packages.py

@@ -335,7 +335,7 @@ def test_submitpackage(self):
         self.log.info("Submitpackage only allows packages of 1 child with its parents")
         # Chain of 3 transactions has too many generations
         chain_hex = [t["hex"] for t in self.wallet.create_self_transfer_chain(chain_length=25)]
-        assert_raises_rpc_error(-25, "not-child-with-parents", node.submitpackage, chain_hex)
+        assert_raises_rpc_error(-25, "package topology disallowed", node.submitpackage, chain_hex)
 
 
 if __name__ == "__main__":