RPC submitpackage: change return format to allow partial errors

Behavior prior to this commit allows some transactions to
enter into the local mempool but not be reported to the user
when encountering a PackageValidationResult::PCKG_TX result.

This is further compounded with the fact that any transactions
submitted to the mempool during this call would also not be
relayed to peers, resulting in unexpected behavior.

Fix this by, if encountering a package error, reporting all
wtxids, along with a new error field, and broadcasting every
transaction that was found in the mempool after submission.

Note that this also changes fees and vsize to optional,
which should also remove an issue with other-wtxid cases.

3 files changed, 31 insertions, 7 deletions

diff --git a/test/functional/mempool_limit.py b/test/functional/mempool_limit.py
index a1147f70f3..6215610c31 100755
--- a/test/functional/mempool_limit.py
+++ b/test/functional/mempool_limit.py
@@ -125,8 +125,9 @@ class MempoolLimitTest(BitcoinTestFramework):
             utxo_to_spend=tx_B["new_utxo"],
             confirmed_only=True
         )
-
-        assert_raises_rpc_error(-26, "too-long-mempool-chain", node.submitpackage, [tx_B["hex"], tx_C["hex"]])
+        res = node.submitpackage([tx_B["hex"], tx_C["hex"]])
+        assert_equal(res["package_msg"], "transaction failed")
+        assert "too-long-mempool-chain" in res["tx-results"][tx_C["wtxid"]]["error"]
 
     def test_mid_package_eviction(self):
         node = self.nodes[0]
@@ -205,7 +206,7 @@ class MempoolLimitTest(BitcoinTestFramework):
 
         # Package should be submitted, temporarily exceeding maxmempool, and then evicted.
         with node.assert_debug_log(expected_msgs=["rolling minimum fee bumped"]):
-            assert_raises_rpc_error(-26, "mempool full", node.submitpackage, package_hex)
+            assert_equal(node.submitpackage(package_hex)["package_msg"], "transaction failed")
 
         # Maximum size must never be exceeded.
         assert_greater_than(node.getmempoolinfo()["maxmempool"], node.getmempoolinfo()["bytes"])
@@ -273,7 +274,9 @@ class MempoolLimitTest(BitcoinTestFramework):
         package_hex = [cpfp_parent["hex"], replacement_tx["hex"], child["hex"]]
 
         # Package should be submitted, temporarily exceeding maxmempool, and then evicted.
-        assert_raises_rpc_error(-26, "bad-txns-inputs-missingorspent", node.submitpackage, package_hex)
+        res = node.submitpackage(package_hex)
+        assert_equal(res["package_msg"], "transaction failed")
+        assert len([tx_res for _, tx_res in res["tx-results"].items() if "error" in tx_res and tx_res["error"] == "bad-txns-inputs-missingorspent"])
 
         # Maximum size must never be exceeded.
         assert_greater_than(node.getmempoolinfo()["maxmempool"], node.getmempoolinfo()["bytes"])
@@ -321,6 +324,7 @@ class MempoolLimitTest(BitcoinTestFramework):
         package_txns.append(tx_child)
 
         submitpackage_result = node.submitpackage([tx["hex"] for tx in package_txns])
+        assert_equal(submitpackage_result["package_msg"], "success")
 
         rich_parent_result = submitpackage_result["tx-results"][tx_rich["wtxid"]]
         poor_parent_result = submitpackage_result["tx-results"][tx_poor["wtxid"]]
@@ -366,7 +370,9 @@ class MempoolLimitTest(BitcoinTestFramework):
         assert_greater_than(worst_feerate_btcvb, (parent_fee + child_fee) / (tx_parent_just_below["tx"].get_vsize() + tx_child_just_above["tx"].get_vsize()))
         assert_greater_than(mempoolmin_feerate, (parent_fee) / (tx_parent_just_below["tx"].get_vsize()))
         assert_greater_than((parent_fee + child_fee) / (tx_parent_just_below["tx"].get_vsize() + tx_child_just_above["tx"].get_vsize()), mempoolmin_feerate / 1000)
-        assert_raises_rpc_error(-26, "mempool full", node.submitpackage, [tx_parent_just_below["hex"], tx_child_just_above["hex"]])
+        res = node.submitpackage([tx_parent_just_below["hex"], tx_child_just_above["hex"]])
+        for wtxid in [tx_parent_just_below["wtxid"], tx_child_just_above["wtxid"]]:
+            assert_equal(res["tx-results"][wtxid]["error"], "mempool full")
 
         self.log.info('Test passing a value below the minimum (5 MB) to -maxmempool throws an error')
         self.stop_node(0)
diff --git a/test/functional/mempool_sigoplimit.py b/test/functional/mempool_sigoplimit.py
index fbec6d0dc8..14dd9cc80e 100755
--- a/test/functional/mempool_sigoplimit.py
+++ b/test/functional/mempool_sigoplimit.py
@@ -34,7 +34,6 @@ from test_framework.util import (
     assert_equal,
     assert_greater_than,
     assert_greater_than_or_equal,
-    assert_raises_rpc_error,
 )
 from test_framework.wallet import MiniWallet
 from test_framework.wallet_util import generate_keypair
@@ -169,7 +168,8 @@ class BytesPerSigOpTest(BitcoinTestFramework):
         assert_equal([x["package-error"] for x in packet_test], ["package-mempool-limits", "package-mempool-limits"])
 
         # When we actually try to submit, the parent makes it into the mempool, but the child would exceed ancestor vsize limits
-        assert_raises_rpc_error(-26, "too-long-mempool-chain", self.nodes[0].submitpackage, [tx_parent.serialize().hex(), tx_child.serialize().hex()])
+        res = self.nodes[0].submitpackage([tx_parent.serialize().hex(), tx_child.serialize().hex()])
+        assert "too-long-mempool-chain" in res["tx-results"][tx_child.getwtxid()]["error"]
         assert tx_parent.rehash() in self.nodes[0].getrawmempool()
 
         # Transactions are tiny in weight
diff --git a/test/functional/rpc_packages.py b/test/functional/rpc_packages.py
index 5644a9f5a8..6feebcd124 100755
--- a/test/functional/rpc_packages.py
+++ b/test/functional/rpc_packages.py
@@ -304,6 +304,7 @@ class RPCPackagesTest(BitcoinTestFramework):
         submitpackage_result = node.submitpackage(package=[tx["hex"] for tx in package_txns])
 
         # Check that each result is present, with the correct size and fees
+        assert_equal(submitpackage_result["package_msg"], "success")
         for package_txn in package_txns:
             tx = package_txn["tx"]
             assert tx.getwtxid() in submitpackage_result["tx-results"]
@@ -334,9 +335,26 @@ class RPCPackagesTest(BitcoinTestFramework):
 
         self.log.info("Submitpackage only allows packages of 1 child with its parents")
         # Chain of 3 transactions has too many generations
+        legacy_pool = node.getrawmempool()
         chain_hex = [t["hex"] for t in self.wallet.create_self_transfer_chain(chain_length=25)]
         assert_raises_rpc_error(-25, "package topology disallowed", node.submitpackage, chain_hex)
+        assert_equal(legacy_pool, node.getrawmempool())
 
+        # 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")
+        peer = node.add_p2p_connection(P2PTxInvStore())
+        txs = self.wallet.create_self_transfer_chain(chain_length=2)
+        bad_child = tx_from_hex(txs[1]["hex"])
+        bad_child.nVersion = -1
+        hex_partial_acceptance = [txs[0]["hex"], bad_child.serialize().hex()]
+        res = node.submitpackage(hex_partial_acceptance)
+        assert_equal(res["package_msg"], "transaction failed")
+        first_wtxid = txs[0]["tx"].getwtxid()
+        assert "error" not in res["tx-results"][first_wtxid]
+        sec_wtxid = bad_child.getwtxid()
+        assert_equal(res["tx-results"][sec_wtxid]["error"], "version")
+        peer.wait_for_broadcast([first_wtxid])
 
 if __name__ == "__main__":
     RPCPackagesTest().main()