From 17f74512f0d19cb452ed79a4bff5a222fcdb53c4 Mon Sep 17 00:00:00 2001 From: stickies-v Date: Thu, 25 Jan 2024 12:04:24 +0000 Subject: [PATCH 1/4] test: add bounds checking for submitpackage RPC --- test/functional/rpc_packages.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/functional/rpc_packages.py b/test/functional/rpc_packages.py index 664f2df3f19..76a08171b58 100755 --- a/test/functional/rpc_packages.py +++ b/test/functional/rpc_packages.py @@ -25,6 +25,9 @@ from test_framework.wallet import ( ) +MAX_PACKAGE_COUNT = 25 + + class RPCPackagesTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 @@ -340,6 +343,13 @@ class RPCPackagesTest(BitcoinTestFramework): 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 1 and {MAX_PACKAGE_COUNT} transactions.", node.submitpackage, []) + assert_raises_rpc_error(-25, "package topology disallowed", node.submitpackage, [chain_hex[0]] * 1) + assert_raises_rpc_error( + -8, f"Array must contain between 1 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") From f9ece258aa868d0776caa86b94e71ba05a9b287e Mon Sep 17 00:00:00 2001 From: stickies-v Date: Mon, 22 Jan 2024 15:45:43 +0000 Subject: [PATCH 2/4] doc: rpc: submitpackage takes sorted array --- src/rpc/mempool.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index c1753a1f6e2..a3452c78865 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -815,13 +815,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, ""}, }, From 1a875d4049574730d4a53a1b68bd29b80ad96d38 Mon Sep 17 00:00:00 2001 From: stickies-v Date: Mon, 22 Jan 2024 15:51:31 +0000 Subject: [PATCH 3/4] rpc: update min package size error message in submitpackage Currently, the only allowed package topology has a min size of 2. Update the error message to reflect that. --- src/rpc/mempool.cpp | 4 ++-- test/functional/rpc_packages.py | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index a3452c78865..9fa3c32a8fc 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -861,9 +861,9 @@ static RPCHelpMan submitpackage() [&](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."); } std::vector txns; diff --git a/test/functional/rpc_packages.py b/test/functional/rpc_packages.py index 76a08171b58..ab679eec9a8 100755 --- a/test/functional/rpc_packages.py +++ b/test/functional/rpc_packages.py @@ -343,10 +343,10 @@ class RPCPackagesTest(BitcoinTestFramework): 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 1 and {MAX_PACKAGE_COUNT} transactions.", node.submitpackage, []) - assert_raises_rpc_error(-25, "package topology disallowed", node.submitpackage, [chain_hex[0]] * 1) + 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.", + -8, f"Array must contain between 2 and {MAX_PACKAGE_COUNT} transactions.", node.submitpackage, [chain_hex[0]] * (MAX_PACKAGE_COUNT + 1) ) From 78e52f663f3e3ac86260b913dad777fd7218f210 Mon Sep 17 00:00:00 2001 From: stickies-v Date: Mon, 22 Jan 2024 16:01:25 +0000 Subject: [PATCH 4/4] doc: rpc: fix submitpackage examples --- src/rpc/mempool.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index 9fa3c32a8fc..b04e7c65428 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -855,8 +855,8 @@ 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 {