[rpc] return effective-feerate in testmempoolaccept and submitpackage

Author: glozow

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()) {

test/functional/mempool_accept.py

@@ -58,7 +58,10 @@ def check_mempool_result(self, result_expected, *args, **kwargs):
         """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
 

test/functional/p2p_segwit.py

@@ -622,8 +622,9 @@ def test_standardness_v0(self):
         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 @@ def test_standardness_v0(self):
             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(),

test/functional/rpc_packages.py

@@ -20,6 +20,7 @@
     assert_raises_rpc_error,
 )
 from test_framework.wallet import (
+    COIN,
     DEFAULT_FEE,
     MiniWallet,
 )
@@ -239,11 +240,13 @@ def test_rbf(self):
         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 @@ def test_submit_child_with_parents(self, num_parents, partial_submit):
         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 @@ def test_submit_child_with_parents(self, num_parents, partial_submit):
         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 @@ def test_submit_cpfp(self):
         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 @@ def test_submit_cpfp(self):
         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.