Merge bitcoin/bitcoin#27609: rpc: allow submitpackage to be called outside of regtest

5b878be742dbfcd232d949d2df1fff4743aec3d8 [doc] add release note for submitpackage (glozow)
7a9bb2a2a59ba49f80519c8435229abec2432486 [rpc] allow submitpackage to be called outside of regtest (glozow)
5b9087a9a7da2602485e85e0b163dc3cbd2daf31 [rpc] require package to be a tree in submitpackage (glozow)
e32ba1599c599e75b1da3393f71f633de860505f [txpackages] IsChildWithParentsTree() (glozow)
b4f28cc345ef9c5261c4a8d743654a44784c7802 [doc] parent pay for child in aggregate CheckFeeRate (glozow)

Pull request description:

  Permit (restricted topology) submitpackage RPC outside of regtest. Suggested in https://github.com/bitcoin/bitcoin/pull/26933#issuecomment-1510851570

  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 5b878be742dbfcd232d949d2df1fff4743aec3d8
  achow101:
    ACK 5b878be742dbfcd232d949d2df1fff4743aec3d8
  dergoegge:
    Code review ACK 5b878be742dbfcd232d949d2df1fff4743aec3d8
  ajtowns:
    ACK 5b878be742dbfcd232d949d2df1fff4743aec3d8
  ariard:
    Code Review ACK  5b878be742. Though didn’t manually test the PR.

Tree-SHA512: 610365c0b2ffcccd55dedd1151879c82de1027e3319712bcb11d54f2467afaae4d05dca5f4b25f03354c80845fef538d3938b958174dda8b14c10670537a6524

7 files changed, 50 insertions, 8 deletions

diff --git a/doc/release-notes-27609.md b/doc/release-notes-27609.md
new file mode 100644
index 0000000000..b8cecbd882
--- /dev/null
+++ b/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.
diff --git a/src/policy/packages.cpp b/src/policy/packages.cpp
index fd272a2642..47a9274a31 100644
--- a/src/policy/packages.cpp
+++ b/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;
+    });
+}
diff --git a/src/policy/packages.h b/src/policy/packages.h
index 702667b8ad..cf37857e4b 100644
--- a/src/policy/packages.h
+++ b/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
diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp
index 705608bd47..136969eb87 100644
--- a/src/rpc/mempool.cpp
+++ b/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);
diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp
index 571b58156f..a8318e9fdb 100644
--- a/src/test/txpackage_tests.cpp
+++ b/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));
diff --git a/src/validation.cpp b/src/validation.cpp
index 30b3dde74f..e24c187591 100644
--- a/src/validation.cpp
+++ b/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},
diff --git a/test/functional/rpc_packages.py b/test/functional/rpc_packages.py
index 9c4960aa1e..5644a9f5a8 100755
--- a/test/functional/rpc_packages.py
+++ b/test/functional/rpc_packages.py
@@ -335,7 +335,7 @@ class RPCPackagesTest(BitcoinTestFramework):
         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__":