merge bitcoin#19940: Return fee and vsize from testmempoolaccept

doc/release-notes-5834.md

@@ -0,0 +1,5 @@
+Updated RPCs
+------------
+
+- The `testmempoolaccept` RPC returns `vsize` and a `fee` object with the `base` fee
+  if the transaction passes validation.

src/rpc/rawtransaction.cpp

@@ -1174,6 +1174,11 @@ static UniValue testmempoolaccept(const JSONRPCRequest& request)
                         {
                             {RPCResult::Type::STR_HEX, "txid", "The transaction hash in hex"},
                             {RPCResult::Type::BOOL, "allowed", "If the mempool allows this tx to be inserted"},
+                            {RPCResult::Type::NUM, "vsize", "Virtual transaction size as defined in BIP 141"},
+                            {RPCResult::Type::OBJ, "fees", "Transaction fees (only present if 'allowed' is true)",
+                            {
+                                {RPCResult::Type::STR_AMOUNT, "base", "transaction fee in " + CURRENCY_UNIT},
+                            }},
                             {RPCResult::Type::STR, "reject-reason", "Rejection string (only present when 'allowed' is false)"},
                         }},
                     }
@@ -1222,14 +1227,23 @@ static UniValue testmempoolaccept(const JSONRPCRequest& request)
 
     TxValidationState state;
     bool test_accept_res;
+    CAmount fee;
     {
         ChainstateManager& chainman = EnsureChainman(node);
         LOCK(cs_main);
         test_accept_res = AcceptToMemoryPool(chainman.ActiveChainstate(), mempool, state, std::move(tx),
-            false /* bypass_limits */, max_raw_tx_fee, /* test_accept */ true);
+            false /* bypass_limits */, max_raw_tx_fee, /* test_accept */ true, &fee);
     }
     result_0.pushKV("allowed", test_accept_res);
-    if (!test_accept_res) {
+
+    // Only return the fee and vsize if the transaction would pass ATMP.
+    // These can be used to calculate the feerate.
+    if (test_accept_res) {
+        result_0.pushKV("vsize", virtual_size);
+        UniValue fees(UniValue::VOBJ);
+        fees.pushKV("base", ValueFromAmount(fee));
+        result_0.pushKV("fees", fees);
+    } else {
         if (state.IsInvalid()) {
             if (state.GetResult() == TxValidationResult::TX_MISSING_INPUTS) {
                 result_0.pushKV("reject-reason", "missing-inputs");

src/validation.cpp

@@ -558,6 +558,7 @@ public:
          */
         std::vector<COutPoint>& m_coins_to_uncache;
         const bool m_test_accept;
+        CAmount* m_fee_out;
     };
 
     // Single transaction acceptance
@@ -764,6 +765,11 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
         return error("%s: Consensus::CheckTxInputs: %s, %s", __func__, tx.GetHash().ToString(), state.ToString());
     }
 
+    // If fee_out is passed, return the fee to the caller
+    if (args.m_fee_out) {
+        *args.m_fee_out = nFees;
+    }
+
     // Check for non-standard pay-to-script-hash in inputs
     if (fRequireStandard && !AreInputsStandard(tx, m_view)) {
         return state.Invalid(TxValidationResult::TX_INPUTS_NOT_STANDARD, "bad-txns-nonstandard-inputs");
@@ -986,10 +992,10 @@ bool MemPoolAccept::AcceptSingleTransaction(const CTransactionRef& ptx, ATMPArgs
 /** (try to) add transaction to memory pool with a specified acceptance time **/
 static bool AcceptToMemoryPoolWithTime(const CChainParams& chainparams, CTxMemPool& pool, CChainState& active_chainstate, TxValidationState &state, const CTransactionRef &tx,
                         int64_t nAcceptTime, bool bypass_limits,
-                        const CAmount nAbsurdFee, bool test_accept) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
+                        const CAmount nAbsurdFee, bool test_accept, CAmount* fee_out=nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
 {
     std::vector<COutPoint> coins_to_uncache;
-    MemPoolAccept::ATMPArgs args { chainparams, state, nAcceptTime, bypass_limits, nAbsurdFee, coins_to_uncache, test_accept };
+    MemPoolAccept::ATMPArgs args { chainparams, state, nAcceptTime, bypass_limits, nAbsurdFee, coins_to_uncache, test_accept, fee_out };
 
     assert(std::addressof(::ChainstateActive()) == std::addressof(active_chainstate));
     bool res = MemPoolAccept(pool, active_chainstate).AcceptSingleTransaction(tx, args);
@@ -1011,11 +1017,11 @@ static bool AcceptToMemoryPoolWithTime(const CChainParams& chainparams, CTxMemPo
 }
 
 bool AcceptToMemoryPool(CChainState& active_chainstate, CTxMemPool& pool, TxValidationState &state, const CTransactionRef &tx,
-                        bool bypass_limits, const CAmount nAbsurdFee, bool test_accept)
+                        bool bypass_limits, const CAmount nAbsurdFee, bool test_accept, CAmount* fee_out)
 {
     const CChainParams& chainparams = Params();
     assert(std::addressof(::ChainstateActive()) == std::addressof(active_chainstate));
-    return AcceptToMemoryPoolWithTime(chainparams, pool, active_chainstate, state, tx, GetTime(), bypass_limits, nAbsurdFee, test_accept);
+    return AcceptToMemoryPoolWithTime(chainparams, pool, active_chainstate, state, tx, GetTime(), bypass_limits, nAbsurdFee, test_accept, fee_out);
 }
 
 bool GetTimestampIndex(const unsigned int &high, const unsigned int &low, std::vector<uint256> &hashes)

src/validation.h

@@ -221,10 +221,12 @@ void UnlinkPrunedFiles(const std::set<int>& setFilesToPrune);
 /** Prune block files up to a given height */
 void PruneBlockFilesManual(CChainState& active_chainstate, int nManualPruneHeight);
 
-/** (try to) add transaction to memory pool */
+/** (try to) add transaction to memory pool
+ * plTxnReplaced will be appended to with all transactions replaced from mempool
+ * @param[out] fee_out optional argument to return tx fee to the caller **/
 bool AcceptToMemoryPool(CChainState& active_chainstate, CTxMemPool& pool, TxValidationState &state, const CTransactionRef &tx,
                         bool bypass_limits,
-                        const CAmount nAbsurdFee, bool test_accept=false) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
+                        const CAmount nAbsurdFee, bool test_accept=false, CAmount* fee_out=nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 
 bool GetUTXOCoin(const COutPoint& outpoint, Coin& coin);
 int GetUTXOHeight(const COutPoint& outpoint);

test/functional/feature_asset_locks.py

@@ -272,7 +272,7 @@ class AssetLocksTest(DashTestFramework):
         asset_lock_tx = self.create_assetlock(coin, locked_1, pubkey)
 
 
-        self.check_mempool_result(tx=asset_lock_tx, result_expected={'allowed': True})
+        self.check_mempool_result(tx=asset_lock_tx, result_expected={'allowed': True, 'vsize': asset_lock_tx.get_vsize(), 'fees': {'base': Decimal(str(tiny_amount / COIN))}})
         self.validate_credit_pool_balance(0)
         txid_in_block = self.send_tx(asset_lock_tx)
         assert "assetLockTx" in node.getrawtransaction(txid_in_block, 1)
@@ -331,7 +331,7 @@ class AssetLocksTest(DashTestFramework):
         asset_unlock_tx_duplicate_index.vout[0].nValue += COIN
         too_late_height = node.getblockcount() + 48
 
-        self.check_mempool_result(tx=asset_unlock_tx, result_expected={'allowed': True})
+        self.check_mempool_result(tx=asset_unlock_tx, result_expected={'allowed': True, 'vsize': asset_unlock_tx.get_vsize(), 'fees': {'base': Decimal(str(tiny_amount / COIN))}})
         self.check_mempool_result(tx=asset_unlock_tx_too_big_fee,
                 result_expected={'allowed': False, 'reject-reason' : 'absurdly-high-fee'})
         self.check_mempool_result(tx=asset_unlock_tx_zero_fee,
@@ -374,7 +374,7 @@ class AssetLocksTest(DashTestFramework):
         self.mine_quorum(llmq_type_name="llmq_test_platform", llmq_type=106)
         self.log.info("Checking credit pool amount is same...")
         self.validate_credit_pool_balance(locked_1 - 1 * COIN)
-        self.check_mempool_result(tx=asset_unlock_tx_late, result_expected={'allowed': True})
+        self.check_mempool_result(tx=asset_unlock_tx_late, result_expected={'allowed': True, 'vsize': asset_unlock_tx_late.get_vsize(), 'fees': {'base': Decimal(str(tiny_amount / COIN))}})
         self.log.info("Checking credit pool amount still is same...")
         self.validate_credit_pool_balance(locked_1 - 1 * COIN)
         self.send_tx(asset_unlock_tx_late)
@@ -384,7 +384,7 @@ class AssetLocksTest(DashTestFramework):
 
         self.log.info("Generating many blocks to make quorum far behind (even still active)...")
         self.slowly_generate_batch(too_late_height - node.getblockcount() - 1)
-        self.check_mempool_result(tx=asset_unlock_tx_too_late, result_expected={'allowed': True})
+        self.check_mempool_result(tx=asset_unlock_tx_too_late, result_expected={'allowed': True, 'vsize': asset_unlock_tx_too_late.get_vsize(), 'fees': {'base': Decimal(str(tiny_amount / COIN))}})
         node.generate(1)
         self.sync_all()
         self.check_mempool_result(tx=asset_unlock_tx_too_late,
@@ -421,7 +421,7 @@ class AssetLocksTest(DashTestFramework):
 
         self.log.info("Checking that transaction with exceeding amount accepted by mempool...")
         # Mempool doesn't know about the size of the credit pool
-        self.check_mempool_result(tx=asset_unlock_tx_full, result_expected={'allowed': True })
+        self.check_mempool_result(tx=asset_unlock_tx_full, result_expected={'allowed': True, 'vsize': asset_unlock_tx_full.get_vsize(), 'fees': {'base': Decimal(str(tiny_amount / COIN))}})
 
         txid_in_block = self.send_tx(asset_unlock_tx_full)
         node.generate(1)
@@ -434,7 +434,7 @@ class AssetLocksTest(DashTestFramework):
 
         self.mempool_size += 1
         asset_unlock_tx_full = self.create_assetunlock(301, self.get_credit_pool_balance(), pubkey)
-        self.check_mempool_result(tx=asset_unlock_tx_full, result_expected={'allowed': True })
+        self.check_mempool_result(tx=asset_unlock_tx_full, result_expected={'allowed': True, 'vsize': asset_unlock_tx_full.get_vsize(), 'fees': {'base': Decimal(str(tiny_amount / COIN))}})
 
         txid_in_block = self.send_tx(asset_unlock_tx_full)
         node.generate(1)

test/functional/mempool_accept.py

@@ -93,20 +93,22 @@ class MempoolAcceptanceTest(BitcoinTestFramework):
         tx.deserialize(BytesIO(hex_str_to_bytes(raw_tx_0)))
         txid_0 = tx.rehash()
         self.check_mempool_result(
-            result_expected=[{'txid': txid_0, 'allowed': True}],
+            result_expected=[{'txid': txid_0, 'allowed': True, 'vsize': tx.get_vsize(), 'fees': {'base': Decimal(str(fee))}}],
             rawtxs=[raw_tx_0],
         )
 
         self.log.info('A final transaction not in the mempool')
         coin = coins.pop()  # Pick a random coin(base) to spend
+        output_amount = 0.025
         raw_tx_final = node.signrawtransactionwithwallet(node.createrawtransaction(
             inputs=[{'txid': coin['txid'], 'vout': coin['vout'], "sequence": 0xffffffff}],  # SEQUENCE_FINAL
-            outputs=[{node.getnewaddress(): 0.025}],
+            outputs=[{node.getnewaddress(): output_amount}],
             locktime=node.getblockcount() + 2000,  # Can be anything
         ))['hex']
         tx.deserialize(BytesIO(hex_str_to_bytes(raw_tx_final)))
+        fee_expected = int(coin['amount']) - output_amount
         self.check_mempool_result(
-            result_expected=[{'txid': tx.rehash(), 'allowed': True}],
+            result_expected=[{'txid': tx.rehash(), 'allowed': True, 'vsize': tx.get_vsize(), 'fees': {'base': Decimal(str(fee_expected))}}],
             rawtxs=[tx.serialize().hex()],
             maxfeerate=0,
         )
@@ -190,7 +192,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework):
         tx.deserialize(BytesIO(hex_str_to_bytes(raw_tx_reference)))
         # Reference tx should be valid on itself
         self.check_mempool_result(
-            result_expected=[{'txid': tx.rehash(), 'allowed': True}],
+            result_expected=[{'txid': tx.rehash(), 'allowed': True, 'vsize': tx.get_vsize(), 'fees': { 'base': Decimal(str(0.1 - 0.05))}}],
             rawtxs=[tx.serialize().hex()],
             maxfeerate=0,
         )

test/functional/test_framework/messages.py

@@ -508,6 +508,11 @@ class CTransaction:
                 return False
         return True
 
+    # Calculate the virtual transaction size using
+    # serialization size (does NOT use sigops).
+    def get_vsize(self):
+        return len(self.serialize())
+
     def __repr__(self):
         return "CTransaction(nVersion=%i vin=%s vout=%s nLockTime=%i)" \
                % (self.nVersion, repr(self.vin), repr(self.vout), self.nLockTime)