From b4f28cc345ef9c5261c4a8d743654a44784c7802 Mon Sep 17 00:00:00 2001 From: glozow Date: Mon, 2 Oct 2023 10:04:37 +0100 Subject: [PATCH 1/5] [doc] parent pay for child in aggregate CheckFeeRate --- src/validation.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/validation.cpp b/src/validation.cpp index 357b4d422d23a..8166808c27b1e 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1285,6 +1285,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}, From e32ba1599c599e75b1da3393f71f633de860505f Mon Sep 17 00:00:00 2001 From: glozow Date: Thu, 11 May 2023 17:50:05 +0100 Subject: [PATCH 2/5] [txpackages] IsChildWithParentsTree() Many edge cases exist when parents in a child-with-parents package can spend each other. However, this pattern should also be uncommon in normal use cases. --- src/policy/packages.cpp | 15 +++++++++++++++ src/policy/packages.h | 4 ++++ src/test/txpackage_tests.cpp | 3 +++ 3 files changed, 22 insertions(+) diff --git a/src/policy/packages.cpp b/src/policy/packages.cpp index fd272a2642e4c..47a9274a31107 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 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 702667b8ade8d..cf37857e4bb65 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/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp index 571b58156f737..a8318e9fdbe09 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)); From 5b9087a9a7da2602485e85e0b163dc3cbd2daf31 Mon Sep 17 00:00:00 2001 From: glozow Date: Thu, 11 May 2023 17:54:39 +0100 Subject: [PATCH 3/5] [rpc] require package to be a tree in submitpackage --- src/rpc/mempool.cpp | 4 ++++ test/functional/rpc_packages.py | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index 705608bd476a3..173127e01425d 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -820,6 +820,7 @@ static RPCHelpMan submitpackage() { return RPCHelpMan{"submitpackage", "Submit a package of raw transactions (serialized, hex-encoded) to local node (-regtest only).\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" @@ -881,6 +882,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); diff --git a/test/functional/rpc_packages.py b/test/functional/rpc_packages.py index 9c4960aa1ea92..5644a9f5a8c92 100755 --- a/test/functional/rpc_packages.py +++ b/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__": From 7a9bb2a2a59ba49f80519c8435229abec2432486 Mon Sep 17 00:00:00 2001 From: glozow Date: Tue, 11 Apr 2023 16:07:34 +0100 Subject: [PATCH 4/5] [rpc] allow submitpackage to be called outside of regtest --- src/rpc/mempool.cpp | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index 173127e01425d..136969eb87419 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -819,12 +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.", @@ -863,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, @@ -987,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); From 5b878be742dbfcd232d949d2df1fff4743aec3d8 Mon Sep 17 00:00:00 2001 From: glozow Date: Wed, 10 May 2023 16:43:05 +0100 Subject: [PATCH 5/5] [doc] add release note for submitpackage --- doc/release-notes-27609.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 doc/release-notes-27609.md diff --git a/doc/release-notes-27609.md b/doc/release-notes-27609.md new file mode 100644 index 0000000000000..b8cecbd88252b --- /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.