From 1691eaa818f7a7b22907f756490b842d80a9a21d Mon Sep 17 00:00:00 2001 From: glozow Date: Wed, 28 Sep 2022 14:07:56 +0100 Subject: [PATCH] [rpc] return effective-feerate in testmempoolaccept and submitpackage --- src/rpc/mempool.cpp | 10 +++++++ test/functional/mempool_accept.py | 5 +++- test/functional/p2p_segwit.py | 10 ++++--- test/functional/rpc_packages.py | 45 ++++++++++++++++++++----------- 4 files changed, 50 insertions(+), 20 deletions(-) diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index 58e71a0604a..5a8724196ae 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -126,6 +126,7 @@ static RPCHelpMan testmempoolaccept() {RPCResult::Type::OBJ, "fees", /*optional=*/true, "Transaction fees (only present if 'allowed' is true)", { {RPCResult::Type::STR_AMOUNT, "base", "transaction fee in " + CURRENCY_UNIT}, + {RPCResult::Type::STR_AMOUNT, "effective-feerate", /*optional=*/false, "the effective feerate in " + CURRENCY_UNIT + " per KvB. May differ from the base feerate if, for example, there are modified fees from prioritisetransaction or a package feerate was used."}, }}, {RPCResult::Type::STR, "reject-reason", /*optional=*/true, "Rejection string (only present when 'allowed' is false)"}, }}, @@ -217,6 +218,7 @@ static RPCHelpMan testmempoolaccept() result_inner.pushKV("vsize", virtual_size); UniValue fees(UniValue::VOBJ); fees.pushKV("base", ValueFromAmount(fee)); + fees.pushKV("effective-feerate", ValueFromAmount(tx_result.m_effective_feerate.value().GetFeePerK())); result_inner.pushKV("fees", fees); } } else { @@ -768,6 +770,7 @@ static RPCHelpMan submitpackage() {RPCResult::Type::NUM, "vsize", "Virtual transaction size as defined in BIP 141."}, {RPCResult::Type::OBJ, "fees", "Transaction fees", { {RPCResult::Type::STR_AMOUNT, "base", "transaction fee in " + CURRENCY_UNIT}, + {RPCResult::Type::STR_AMOUNT, "effective-feerate", /*optional=*/true, "if the transaction was not already in the mempool, the effective feerate in " + CURRENCY_UNIT + " per KvB. For example, the package feerate and/or feerate with modified fees from prioritisetransaction."}, }}, }} }}, @@ -856,6 +859,7 @@ static RPCHelpMan submitpackage() CHECK_NONFATAL(it != package_result.m_tx_results.end()); UniValue result_inner{UniValue::VOBJ}; result_inner.pushKV("txid", tx->GetHash().GetHex()); + const auto& tx_result = it->second; if (it->second.m_result_type == MempoolAcceptResult::ResultType::DIFFERENT_WITNESS) { result_inner.pushKV("other-wtxid", it->second.m_other_wtxid.value().GetHex()); } @@ -864,6 +868,12 @@ static RPCHelpMan submitpackage() result_inner.pushKV("vsize", int64_t{it->second.m_vsize.value()}); UniValue fees(UniValue::VOBJ); fees.pushKV("base", ValueFromAmount(it->second.m_base_fees.value())); + if (tx_result.m_result_type == MempoolAcceptResult::ResultType::VALID) { + // Effective feerate is not provided for MEMPOOL_ENTRY transactions even + // though modified fees is known, because it is unknown whether package + // feerate was used when it was originally submitted. + fees.pushKV("effective-feerate", ValueFromAmount(tx_result.m_effective_feerate.value().GetFeePerK())); + } result_inner.pushKV("fees", fees); if (it->second.m_replaced_transactions.has_value()) { for (const auto& ptx : it->second.m_replaced_transactions.value()) { diff --git a/test/functional/mempool_accept.py b/test/functional/mempool_accept.py index abbca15bed7..12ea7a00b13 100755 --- a/test/functional/mempool_accept.py +++ b/test/functional/mempool_accept.py @@ -58,7 +58,10 @@ class MempoolAcceptanceTest(BitcoinTestFramework): """Wrapper to check result of testmempoolaccept on node_0's mempool""" result_test = self.nodes[0].testmempoolaccept(*args, **kwargs) for r in result_test: - r.pop('wtxid') # Skip check for now + # Skip these checks for now + r.pop('wtxid') + if "fees" in r: + r["fees"].pop("effective-feerate") assert_equal(result_expected, result_test) assert_equal(self.nodes[0].getmempoolinfo()['size'], self.mempool_size) # Must not change mempool state diff --git a/test/functional/p2p_segwit.py b/test/functional/p2p_segwit.py index 2ddc09b13b4..0072478f1cf 100755 --- a/test/functional/p2p_segwit.py +++ b/test/functional/p2p_segwit.py @@ -622,8 +622,9 @@ class SegWitTest(BitcoinTestFramework): if not self.segwit_active: # Just check mempool acceptance, but don't add the transaction to the mempool, since witness is disallowed # in blocks and the tx is impossible to mine right now. - assert_equal( - self.nodes[0].testmempoolaccept([tx3.serialize_with_witness().hex()]), + testres3 = self.nodes[0].testmempoolaccept([tx3.serialize_with_witness().hex()]) + testres3[0]["fees"].pop("effective-feerate") + assert_equal(testres3, [{ 'txid': tx3.hash, 'wtxid': tx3.getwtxid(), @@ -639,8 +640,9 @@ class SegWitTest(BitcoinTestFramework): tx3 = tx tx3.vout = [tx3_out] tx3.rehash() - assert_equal( - self.nodes[0].testmempoolaccept([tx3.serialize_with_witness().hex()]), + testres3_replaced = self.nodes[0].testmempoolaccept([tx3.serialize_with_witness().hex()]) + testres3_replaced[0]["fees"].pop("effective-feerate") + assert_equal(testres3_replaced, [{ 'txid': tx3.hash, 'wtxid': tx3.getwtxid(), diff --git a/test/functional/rpc_packages.py b/test/functional/rpc_packages.py index f1352f88d88..f3b5f4d2dc2 100755 --- a/test/functional/rpc_packages.py +++ b/test/functional/rpc_packages.py @@ -20,6 +20,7 @@ from test_framework.util import ( assert_raises_rpc_error, ) from test_framework.wallet import ( + COIN, DEFAULT_FEE, MiniWallet, ) @@ -239,11 +240,13 @@ class RPCPackagesTest(BitcoinTestFramework): coin = self.wallet.get_utxo() fee = Decimal("0.00125000") replaceable_tx = self.wallet.create_self_transfer(utxo_to_spend=coin, sequence=MAX_BIP125_RBF_SEQUENCE, fee = fee) - testres_replaceable = node.testmempoolaccept([replaceable_tx["hex"]]) - assert_equal(testres_replaceable, [ - {"txid": replaceable_tx["txid"], "wtxid": replaceable_tx["wtxid"], - "allowed": True, "vsize": replaceable_tx["tx"].get_vsize(), "fees": { "base": fee }} - ]) + testres_replaceable = node.testmempoolaccept([replaceable_tx["hex"]])[0] + assert_equal(testres_replaceable["txid"], replaceable_tx["txid"]) + assert_equal(testres_replaceable["wtxid"], replaceable_tx["wtxid"]) + assert testres_replaceable["allowed"] + assert_equal(testres_replaceable["vsize"], replaceable_tx["tx"].get_vsize()) + assert_equal(testres_replaceable["fees"]["base"], fee) + assert_fee_amount(fee, replaceable_tx["tx"].get_vsize(), testres_replaceable["fees"]["effective-feerate"]) # Replacement transaction is identical except has double the fee replacement_tx = self.wallet.create_self_transfer(utxo_to_spend=coin, sequence=MAX_BIP125_RBF_SEQUENCE, fee = 2 * fee) @@ -287,11 +290,13 @@ class RPCPackagesTest(BitcoinTestFramework): peer = node.add_p2p_connection(P2PTxInvStore()) package_txns = [] + presubmitted_wtxids = set() for _ in range(num_parents): parent_tx = self.wallet.create_self_transfer(fee=DEFAULT_FEE) package_txns.append(parent_tx) if partial_submit and random.choice([True, False]): node.sendrawtransaction(parent_tx["hex"]) + presubmitted_wtxids.add(parent_tx["wtxid"]) child_tx = self.wallet.create_self_transfer_multi(utxos_to_spend=[tx["new_utxo"] for tx in package_txns], fee_per_output=10000) #DEFAULT_FEE package_txns.append(child_tx) @@ -302,14 +307,14 @@ class RPCPackagesTest(BitcoinTestFramework): for package_txn in package_txns: tx = package_txn["tx"] assert tx.getwtxid() in submitpackage_result["tx-results"] - tx_result = submitpackage_result["tx-results"][tx.getwtxid()] - assert_equal(tx_result, { - "txid": package_txn["txid"], - "vsize": tx.get_vsize(), - "fees": { - "base": DEFAULT_FEE, - } - }) + wtxid = tx.getwtxid() + assert wtxid in submitpackage_result["tx-results"] + tx_result = submitpackage_result["tx-results"][wtxid] + assert_equal(tx_result["txid"], tx.rehash()) + assert_equal(tx_result["vsize"], tx.get_vsize()) + assert_equal(tx_result["fees"]["base"], DEFAULT_FEE) + if wtxid not in presubmitted_wtxids: + assert_fee_amount(DEFAULT_FEE, tx.get_vsize(), tx_result["fees"]["effective-feerate"]) # submitpackage result should be consistent with testmempoolaccept and getmempoolentry self.assert_equal_package_results(node, testmempoolaccept_result, submitpackage_result) @@ -328,8 +333,11 @@ class RPCPackagesTest(BitcoinTestFramework): node = self.nodes[0] peer = node.add_p2p_connection(P2PTxInvStore()) + # Package with 2 parents and 1 child. One parent pays for itself using modified fees, and + # another has 0 fees but is bumped by child. tx_poor = self.wallet.create_self_transfer(fee=0, fee_rate=0) - tx_rich = self.wallet.create_self_transfer(fee=DEFAULT_FEE) + tx_rich = self.wallet.create_self_transfer(fee=0, fee_rate=0) + node.prioritisetransaction(tx_rich["txid"], 0, int(DEFAULT_FEE * COIN)) package_txns = [tx_rich, tx_poor] coins = [tx["new_utxo"] for tx in package_txns] tx_child = self.wallet.create_self_transfer_multi(utxos_to_spend=coins, fee_per_output=10000) #DEFAULT_FEE @@ -340,9 +348,16 @@ class RPCPackagesTest(BitcoinTestFramework): rich_parent_result = submitpackage_result["tx-results"][tx_rich["wtxid"]] poor_parent_result = submitpackage_result["tx-results"][tx_poor["wtxid"]] child_result = submitpackage_result["tx-results"][tx_child["tx"].getwtxid()] - assert_equal(rich_parent_result["fees"]["base"], DEFAULT_FEE) + assert_equal(rich_parent_result["fees"]["base"], 0) assert_equal(poor_parent_result["fees"]["base"], 0) assert_equal(child_result["fees"]["base"], DEFAULT_FEE) + # The "rich" parent does not require CPFP so its effective feerate. + assert_fee_amount(DEFAULT_FEE, tx_rich["tx"].get_vsize(), rich_parent_result["fees"]["effective-feerate"]) + # The "poor" parent and child's effective feerates are the same, composed of the child's fee + # divided by their combined vsize. + assert_fee_amount(DEFAULT_FEE, tx_poor["tx"].get_vsize() + tx_child["tx"].get_vsize(), poor_parent_result["fees"]["effective-feerate"]) + assert_fee_amount(DEFAULT_FEE, tx_poor["tx"].get_vsize() + tx_child["tx"].get_vsize(), child_result["fees"]["effective-feerate"]) + # Package feerate is calculated for the remaining transactions after deduplication and # individual submission. Since this package had a 0-fee parent, package feerate must have # been used and returned.