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 16, 2024
1 parent 48cf3da commit 72872c7
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 8 deletions.
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
4 changes: 2 additions & 2 deletions src/rpc/mempool.cpp
Original file line number Diff line number Diff line change
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_parent}));
BOOST_CHECK(IsChildWithParentsTree({tx_parent}));

// 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
8 changes: 5 additions & 3 deletions test/functional/rpc_packages.py
Original file line number Diff line number Diff line change
Expand Up @@ -381,13 +381,15 @@ 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)
)

# Singleton is ok
node.submitpackage([chain_hex[0]] * 1)

# Create a transaction chain such as only the parent gets accepted (by making the child's
# version non-standard). Make sure the parent does get broadcast.
self.log.info("If a package is partially submitted, transactions included in mempool get broadcast")
Expand Down

0 comments on commit 72872c7

Please sign in to comment.