Skip to content

Commit

Permalink
AcceptMultipleTransactions: Fix workspace not being set as client_max…
Browse files Browse the repository at this point in the history
…feerate failure
  • Loading branch information
instagibbs committed Mar 26, 2024
1 parent aaa5125 commit 5ae4dce
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 6 deletions.
2 changes: 2 additions & 0 deletions src/validation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1366,6 +1366,8 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std::
// Individual modified feerate exceeded caller-defined max; abort
// N.B. this doesn't take into account CPFPs. Chunk-aware validation may be more robust.
if (args.m_client_maxfeerate && CFeeRate(ws.m_modified_fees, ws.m_vsize) > args.m_client_maxfeerate.value()) {
// Need to set failure here both individually and at package level
ws.m_state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "max feerate exceeded", "");
package_state.Invalid(PackageValidationResult::PCKG_TX, "max feerate exceeded");
// Exit early to avoid doing pointless work. Update the failed tx result; the rest are unfinished.
results.emplace(ws.m_ptx->GetWitnessHash(), MempoolAcceptResult::Failure(ws.m_state));
Expand Down
53 changes: 47 additions & 6 deletions test/functional/rpc_packages.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
assert_equal,
assert_fee_amount,
assert_raises_rpc_error,
fill_mempool,
)
from test_framework.wallet import (
DEFAULT_FEE,
Expand All @@ -31,6 +32,11 @@ def set_test_params(self):
self.setup_clean_chain = True
# whitelist peers to speed up tx relay / mempool sync
self.noban_tx_relay = True
# Needed for fill_mempool
self.extra_args = [[
"-datacarriersize=100000",
"-maxmempool=5",
]]

def assert_testres_equal(self, package_hex, testres_expected):
"""Shuffle package_hex and assert that the testmempoolaccept result matches testres_expected. This should only
Expand Down Expand Up @@ -369,23 +375,58 @@ def test_maxfeerate_maxburn_submitpackage(self):
minrate_btc_kvb = min([chained_txn["fee"] / chained_txn["tx"].get_vsize() * 1000 for chained_txn in chained_txns])
chain_hex = [t["hex"] for t in chained_txns]
pkg_result = node.submitpackage(chain_hex, maxfeerate=minrate_btc_kvb - Decimal("0.00000001"))

# First tx failed in single transaction evaluation, so package message is generic
assert_equal(pkg_result["package_msg"], "transaction failed")
assert_equal(pkg_result["tx-results"][chained_txns[0]["wtxid"]]["error"], "max feerate exceeded")
assert_equal(pkg_result["tx-results"][chained_txns[1]["wtxid"]]["error"], "bad-txns-inputs-missingorspent")
assert_equal(node.getrawmempool(), [])

# Make chain of two transactions where parent doesn't make minfee threshold
# but child is too high fee
fill_mempool(self, node, self.wallet)
while node.getrawmempool() != []:
self.generate(node, 1)

minrelay = node.getmempoolinfo()["minrelaytxfee"]
parent = self.wallet.create_self_transfer(
fee_rate=minrelay,
)

child = self.wallet.create_self_transfer(
fee_rate=DEFAULT_FEE,
utxo_to_spend=parent["new_utxo"],
)

pkg_result = node.submitpackage([parent["hex"], child["hex"]], maxfeerate=DEFAULT_FEE - Decimal("0.00000001"))

# Package-wide error reported for package evaluation rejection
assert_equal(pkg_result["package_msg"], "max feerate exceeded")
assert "mempool min fee not met" in pkg_result["tx-results"][parent["wtxid"]]["error"]
assert_equal(pkg_result["tx-results"][child["wtxid"]]["error"], "max feerate exceeded")
assert_equal(node.getrawmempool(), [])

# Reset minfee
self.restart_node(0)

self.log.info("Submitpackage maxburnamount arg testing")
tx = tx_from_hex(chain_hex[1])
chained_txns_burn = self.wallet.create_self_transfer_chain(chain_length=2)
chained_burn_hex = [t["hex"] for t in chained_txns_burn]

tx = tx_from_hex(chained_burn_hex[1])
tx.vout[-1].scriptPubKey = b'a' * 10001 # scriptPubKey bigger than 10k IsUnspendable
chain_hex = [chain_hex[0], tx.serialize().hex()]
chained_burn_hex = [chained_burn_hex[0], tx.serialize().hex()]
# burn test is run before any package evaluation; nothing makes it in and we get broader exception
assert_raises_rpc_error(-25, "Unspendable output exceeds maximum configured by user", node.submitpackage, chain_hex, 0, chained_txns[1]["new_utxo"]["value"] - Decimal("0.00000001"))
assert_raises_rpc_error(-25, "Unspendable output exceeds maximum configured by user", node.submitpackage, chained_burn_hex, 0, chained_txns_burn[1]["new_utxo"]["value"] - Decimal("0.00000001"))
assert_equal(node.getrawmempool(), [])

minrate_btc_kvb_burn = min([chained_txn_burn["fee"] / chained_txn_burn["tx"].get_vsize() * 1000 for chained_txn_burn in chained_txns_burn])

# Relax the restrictions for both and send it; parent gets through as own subpackage
pkg_result = node.submitpackage(chain_hex, maxfeerate=minrate_btc_kvb, maxburnamount=chained_txns[1]["new_utxo"]["value"])
assert "error" not in pkg_result["tx-results"][chained_txns[0]["wtxid"]]
pkg_result = node.submitpackage(chained_burn_hex, maxfeerate=minrate_btc_kvb_burn, maxburnamount=chained_txns_burn[1]["new_utxo"]["value"])
assert "error" not in pkg_result["tx-results"][chained_txns_burn[0]["wtxid"]]
assert_equal(pkg_result["tx-results"][tx.getwtxid()]["error"], "scriptpubkey")
assert_equal(node.getrawmempool(), [chained_txns[0]["txid"]])
assert_equal(node.getrawmempool(), [chained_txns_burn[0]["txid"]])

if __name__ == "__main__":
RPCPackagesTest().main()

0 comments on commit 5ae4dce

Please sign in to comment.