Skip to content

Commit

Permalink
Package validation: accept packages of size 1
Browse files Browse the repository at this point in the history
Users may want to entirely switch to submitpackage
even for loose transactions. We should let them.
  • Loading branch information
instagibbs committed Oct 21, 2024
1 parent 48cf3da commit 92ed1bd
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 12 deletions.
2 changes: 1 addition & 1 deletion doc/policy/packages.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ The following rules are only enforced for packages to be submitted to the mempoo
enforced for test accepts):

* Packages must be child-with-unconfirmed-parents packages. This also means packages must contain at
least 2 transactions. (#22674)
least 1 transaction. (#31096)

- *Rationale*: This allows for fee-bumping by CPFP. Allowing multiple parents makes it possible
to fee-bump a batch of transactions. Restricting packages to a defined topology is easier to
Expand Down
3 changes: 2 additions & 1 deletion src/policy/packages.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,9 @@ bool IsWellFormedPackage(const Package& txns, PackageValidationState& state, boo

bool IsChildWithParents(const Package& package)
{
if (package.empty()) return false;
assert(std::all_of(package.cbegin(), package.cend(), [](const auto& tx){return tx != nullptr;}));
if (package.size() < 2) return false;
if (package.size() < 2) return true;

// The package is expected to be sorted, so the last transaction is the child.
const auto& child = package.back();
Expand Down
6 changes: 3 additions & 3 deletions src/rpc/mempool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -923,7 +923,7 @@ static RPCHelpMan submitpackage()
,
{
{"package", RPCArg::Type::ARR, RPCArg::Optional::NO, "An array of raw transactions.\n"
"The package must solely consist of a child and its parents. None of the parents may depend on each other.\n"
"The package must solely consist of a child transaction and all of its unconfirmed parents, if any. None of the parents may depend on each other.\n"
"The package must be topologically sorted, with the child being the last element in the array.",
{
{"rawtx", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, ""},
Expand Down Expand Up @@ -971,9 +971,9 @@ static RPCHelpMan submitpackage()
[&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
{
const UniValue raw_transactions = request.params[0].get_array();
if (raw_transactions.size() < 2 || raw_transactions.size() > MAX_PACKAGE_COUNT) {
if (raw_transactions.empty() || raw_transactions.size() > MAX_PACKAGE_COUNT) {
throw JSONRPCError(RPC_INVALID_PARAMETER,
"Array must contain between 2 and " + ToString(MAX_PACKAGE_COUNT) + " transactions.");
"Array must contain between 1 and " + ToString(MAX_PACKAGE_COUNT) + " transactions.");
}

// Fee check needs to be run with chainstate and package context
Expand Down
8 changes: 8 additions & 0 deletions src/test/txpackage_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,14 @@ BOOST_AUTO_TEST_CASE(noncontextual_package_tests)
BOOST_CHECK(GetPackageHash({tx_parent}) != GetPackageHash({tx_child}));
BOOST_CHECK(GetPackageHash({tx_child, tx_child}) != GetPackageHash({tx_child}));
BOOST_CHECK(GetPackageHash({tx_child, tx_parent}) != GetPackageHash({tx_child, tx_child}));

// IsChildWithParents* can also be a singleton
BOOST_CHECK(IsChildWithParents({tx_child}));
BOOST_CHECK(IsChildWithParentsTree({tx_child}));

// But must not be empty
BOOST_CHECK(!IsChildWithParents({}));
BOOST_CHECK(!IsChildWithParentsTree({}));
}

// 24 Parents and 1 Child
Expand Down
5 changes: 3 additions & 2 deletions src/validation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1695,8 +1695,9 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package,
return PackageMempoolAcceptResult(package_state_quit_early, {});
}

// IsChildWithParents() guarantees the package is > 1 transactions.
assert(package.size() > 1);
// IsChildWithParents() guarantees the package is not empty.
assert(!package.empty());

// The package must be 1 child with all of its unconfirmed parents. The package is expected to
// be sorted, so the last transaction is the child.
const auto& child = package.back();
Expand Down
9 changes: 4 additions & 5 deletions test/functional/rpc_packages.py
Original file line number Diff line number Diff line change
Expand Up @@ -369,8 +369,8 @@ def test_submit_child_with_parents(self, num_parents, partial_submit):
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.log.info("Submitpackage valid packages with 1 child and some number of parents (or none)")
for num_parents in [0, 1, 2, 24]:
self.test_submit_child_with_parents(num_parents, False)
self.test_submit_child_with_parents(num_parents, True)

Expand All @@ -381,10 +381,9 @@ def test_submitpackage(self):
assert_raises_rpc_error(-25, "package topology disallowed", node.submitpackage, chain_hex)
assert_equal(legacy_pool, node.getrawmempool())

assert_raises_rpc_error(-8, f"Array must contain between 2 and {MAX_PACKAGE_COUNT} transactions.", node.submitpackage, [])
assert_raises_rpc_error(-8, f"Array must contain between 2 and {MAX_PACKAGE_COUNT} transactions.", node.submitpackage, [chain_hex[0]] * 1)
assert_raises_rpc_error(-8, f"Array must contain between 1 and {MAX_PACKAGE_COUNT} transactions.", node.submitpackage, [])
assert_raises_rpc_error(
-8, f"Array must contain between 2 and {MAX_PACKAGE_COUNT} transactions.",
-8, f"Array must contain between 1 and {MAX_PACKAGE_COUNT} transactions.",
node.submitpackage, [chain_hex[0]] * (MAX_PACKAGE_COUNT + 1)
)

Expand Down

0 comments on commit 92ed1bd

Please sign in to comment.