Skip to content

Commit

Permalink
Merge #29292: rpc: improve submitpackage documentation and other impr…
Browse files Browse the repository at this point in the history
…ovements

78e52f6 doc: rpc: fix submitpackage examples (stickies-v)
1a875d4 rpc: update min package size error message in submitpackage (stickies-v)
f9ece25 doc: rpc: submitpackage takes sorted array (stickies-v)
17f7451 test: add bounds checking for submitpackage RPC (stickies-v)

Pull request description:

  `submitpackage` requires the package to be topologically sorted with the child being the last element in the array, but this is not documented in the RPC method or the error messages.

  Also sneaking in some other minor improvements that I found while going through the code:
  - Informing the user that `package` needs to be an array of length between `1` and `MAX_PACKAGE_COUNT` is confusing when `IsChildWithPackage()` requires that the package size >= 2. Remove this check to avoid code duplication and sending a confusing error message.
  - fixups to the `submitpackage` examples

ACKs for top commit:
  fjahr:
    re-ACK 78e52f6
  instagibbs:
    ACK 78e52f6
  achow101:
    ACK 78e52f6
  glozow:
    utACK 78e52f6

Tree-SHA512: a8845621bb1cbf784167fc7c82cb8ceb105868b65b26d3465f072d1c04ef3699e85a21a524ade805d423bcecbc34f7d5bff12f2c21cbd902ae1fb154193ebdc9
  • Loading branch information
achow101 committed May 8, 2024
2 parents 8a45f57 + 78e52f6 commit 4300325
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 6 deletions.
13 changes: 7 additions & 6 deletions src/rpc/mempool.cpp
Expand Up @@ -813,13 +813,14 @@ static RPCHelpMan submitpackage()
{
return RPCHelpMan{"submitpackage",
"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 any transaction passes, it 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: 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.",
{"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 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 @@ -860,15 +861,15 @@ static RPCHelpMan submitpackage()
},
},
RPCExamples{
HelpExampleCli("testmempoolaccept", "[rawtx1, rawtx2]") +
HelpExampleCli("submitpackage", "[rawtx1, rawtx2]")
HelpExampleRpc("submitpackage", R"(["rawtx1", "rawtx2"])") +
HelpExampleCli("submitpackage", R"('["rawtx1", "rawtx2"]')")
},
[&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
{
const UniValue raw_transactions = request.params[0].get_array();
if (raw_transactions.size() < 1 || raw_transactions.size() > MAX_PACKAGE_COUNT) {
if (raw_transactions.size() < 2 || raw_transactions.size() > MAX_PACKAGE_COUNT) {
throw JSONRPCError(RPC_INVALID_PARAMETER,
"Array must contain between 1 and " + ToString(MAX_PACKAGE_COUNT) + " transactions.");
"Array must contain between 2 and " + ToString(MAX_PACKAGE_COUNT) + " transactions.");
}

// Fee check needs to be run with chainstate and package context
Expand Down
10 changes: 10 additions & 0 deletions test/functional/rpc_packages.py
Expand Up @@ -26,6 +26,9 @@
)


MAX_PACKAGE_COUNT = 25


class RPCPackagesTest(BitcoinTestFramework):
def set_test_params(self):
self.num_nodes = 1
Expand Down Expand Up @@ -344,6 +347,13 @@ 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 2 and {MAX_PACKAGE_COUNT} transactions.",
node.submitpackage, [chain_hex[0]] * (MAX_PACKAGE_COUNT + 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 4300325

Please sign in to comment.