Merge #18772: rpc: calculate fees in getblock using BlockUndo data

66d012ad7f9381bacfc9b8388fa2ebf82cb86c9e test: RPC: getblock fee calculations (Elliott Jin)
bf7d6e31b1062ab5f90e14e83c56309f499fa2e9 RPC: getblock: tx fee calculation for verbosity 2 via Undo data (Elliott Jin)

Pull request description:

  This change is progress towards #18771 .  It adapts the fee calculation part of #16083 and addresses some feedback.  The additional "verbosity level 3" features are planned for a future PR.

  **Original PR description:**

  > Using block undo data (like in #14802) we can now show fee information for each transaction in a block without the need for additional -txindex and/or a ton of costly lookups. For a start we'll add transaction fee information to getblock verbosity level 2. This comes at a negligible speed penalty (<1%).

ACKs for top commit:
  luke-jr:
    tACK 66d012ad7f9381bacfc9b8388fa2ebf82cb86c9e
  fjahr:
    tACK 66d012ad7f9381bacfc9b8388fa2ebf82cb86c9e
  MarcoFalke:
    review ACK 66d012ad7f9381bacfc9b8388fa2ebf82cb86c9e 🗜

Tree-SHA512: be1fe4b866946a8dc36427f7dc72a20e10860e320a28fa49bc85bd2a93a0d699768179be29fa52e18b2ed8505d3ec272e586753ef2239b4230e0aefd233acaa2

src/core_io.h

@@ -18,6 +18,7 @@ class CTransaction;
 struct CMutableTransaction;
 class uint256;
 class UniValue;
+class CTxUndo;
 
 struct CSpentIndexTxInfo;
 
@@ -46,6 +47,6 @@ std::string EncodeHexTx(const CTransaction& tx);
 std::string SighashToStr(unsigned char sighash_type);
 void ScriptPubKeyToUniv(const CScript& scriptPubKey, UniValue& out, bool fIncludeHex);
 void ScriptToUniv(const CScript& script, UniValue& out, bool include_address);
-void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex = true, const CSpentIndexTxInfo* ptxSpentInfo = nullptr);
+void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex = true, const CSpentIndexTxInfo* ptxSpentInfo = nullptr, const CTxUndo* txundo = nullptr);
 
 #endif // BITCOIN_CORE_IO_H

src/core_write.cpp

@@ -11,7 +11,9 @@
 #include <script/standard.h>
 #include <serialize.h>
 #include <streams.h>
+#include <undo.h>
 #include <univalue.h>
+#include <util/check.h>
 #include <util/strencodings.h>
 
 #include <addressindex.h>
@@ -186,7 +188,7 @@ void ScriptPubKeyToUniv(const CScript& scriptPubKey,
     out.pushKV("addresses", a);
 }
 
-void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex, const CSpentIndexTxInfo* ptxSpentInfo)
+void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex, const CSpentIndexTxInfo* ptxSpentInfo, const CTxUndo* txundo)
 {
     uint256 txid = tx.GetHash();
     entry.pushKV("txid", txid.GetHex());
@@ -198,11 +200,19 @@ void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry,
     entry.pushKV("locktime", (int64_t)tx.nLockTime);
 
     UniValue vin(UniValue::VARR);
-    for (const CTxIn& txin : tx.vin) {
+
+    // If available, use Undo data to calculate the fee. Note that txundo == nullptr
+    // for coinbase transactions and for transactions where undo data is unavailable.
+    const bool calculate_fee = txundo != nullptr;
+    CAmount amt_total_in = 0;
+    CAmount amt_total_out = 0;
+
+    for (unsigned int i = 0; i < tx.vin.size(); i++) {
+        const CTxIn& txin = tx.vin[i];
         UniValue in(UniValue::VOBJ);
-        if (tx.IsCoinBase())
+        if (tx.IsCoinBase()) {
             in.pushKV("coinbase", HexStr(txin.scriptSig));
-        else {
+        } else {
             in.pushKV("txid", txin.prevout.hash.GetHex());
             in.pushKV("vout", (int64_t)txin.prevout.n);
             UniValue o(UniValue::VOBJ);
@@ -226,6 +236,10 @@ void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry,
                 }
             }
         }
+        if (calculate_fee) {
+            const CTxOut& prev_txout = txundo->vprevout[i].out;
+            amt_total_in += prev_txout.nValue;
+        }
         in.pushKV("sequence", (int64_t)txin.nSequence);
         vin.push_back(in);
     }
@@ -257,6 +271,10 @@ void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry,
             }
         }
         vout.push_back(out);
+
+        if (calculate_fee) {
+            amt_total_out += txout.nValue;
+        }
     }
     entry.pushKV("vout", vout);
 
@@ -303,6 +321,12 @@ void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry,
         }
     }
 
+    if (calculate_fee) {
+        const CAmount fee = amt_total_in - amt_total_out;
+        CHECK_NONFATAL(MoneyRange(fee));
+        entry.pushKV("fee", ValueFromAmount(fee));
+    }
+
     if (!hashBlock.IsNull())
         entry.pushKV("blockhash", hashBlock.GetHex());
 

src/rpc/blockchain.cpp

@@ -219,20 +219,25 @@ UniValue blockToJSON(const CBlock& block, const CBlockIndex* tip, const CBlockIn
 
     result.pushKV("size", (int)::GetSerializeSize(block, PROTOCOL_VERSION));
     UniValue txs(UniValue::VARR);
-    for(const auto& tx : block.vtx)
+    if (txDetails) {
-    {
+        CBlockUndo blockUndo;
-        if(txDetails)
+        const bool have_undo = !IsBlockPruned(blockindex) && UndoReadFromDisk(blockUndo, blockindex);
-        {
+        for (size_t i = 0; i < block.vtx.size(); ++i) {
+            const CTransactionRef& tx = block.vtx.at(i);
+            // coinbase transaction (i == 0) doesn't have undo data
+            const CTxUndo* txundo = (have_undo && i) ? &blockUndo.vtxundo.at(i - 1) : nullptr;
             UniValue objTx(UniValue::VOBJ);
-            TxToUniv(*tx, uint256(), objTx, true);
+            TxToUniv(*tx, uint256(), objTx, true, nullptr, txundo);
             bool fLocked = isman.IsLocked(tx->GetHash());
             objTx.pushKV("instantlock", fLocked || result["chainlock"].get_bool());
             objTx.pushKV("instantlock_internal", fLocked);
             txs.push_back(objTx);
         }
-        else
+    } else {
+        for (const CTransactionRef& tx : block.vtx) {
             txs.push_back(tx->GetHash().GetHex());
         }
+    }
     result.pushKV("tx", txs);
     if (!block.vtx[0]->vExtraPayload.empty()) {
         if (const auto opt_cbTx = GetTxPayload<CCbTx>(block.vtx[0]->vExtraPayload)) {
@@ -1222,6 +1227,7 @@ static RPCHelpMan getblock()
                                 {RPCResult::Type::OBJ, "", "",
                                 {
                                     {RPCResult::Type::ELISION, "", "The transactions in the format of the getrawtransaction RPC. Different from verbosity = 1 \"tx\" result"},
+                                    {RPCResult::Type::NUM, "fee", "The transaction fee in " + CURRENCY_UNIT + ", omitted if block undo data is not available"},
                                 }},
                             }},
                         }},

test/functional/rpc_blockchain.py

@@ -20,6 +20,7 @@ Tests correspond to code in rpc/blockchain.cpp.
 
 from decimal import Decimal
 import http.client
+import os
 import subprocess
 
 from test_framework.blocktools import (
@@ -42,8 +43,10 @@ from test_framework.util import (
     assert_raises_rpc_error,
     assert_is_hex_string,
     assert_is_hash_string,
+    get_datadir_path,
     set_node_times,
 )
+from test_framework.wallet import MiniWallet
 
 
 class BlockchainTest(BitcoinTestFramework):
@@ -65,6 +68,7 @@ class BlockchainTest(BitcoinTestFramework):
         self._test_getnetworkhashps()
         self._test_stopatheight()
         self._test_waitforblockheight()
+        self._test_getblock()
         assert self.nodes[0].verifychain(4, 0)
 
     def mine_chain(self):
@@ -397,6 +401,46 @@ class BlockchainTest(BitcoinTestFramework):
         assert_waitforheight(current_height)
         assert_waitforheight(current_height + 1)
 
+    def _test_getblock(self):
+        node = self.nodes[0]
+
+        miniwallet = MiniWallet(node)
+        miniwallet.generate(5)
+        node.generate(100)
+
+        fee_per_byte = Decimal('0.00000010')
+        fee_per_kb = 1000 * fee_per_byte
+
+        miniwallet.send_self_transfer(fee_rate=fee_per_kb, from_node=node)
+        blockhash = node.generate(1)[0]
+
+        self.log.info("Test that getblock with verbosity 1 doesn't include fee")
+        block = node.getblock(blockhash, 1)
+        assert 'fee' not in block['tx'][1]
+
+        self.log.info('Test that getblock with verbosity 2 includes expected fee')
+        block = node.getblock(blockhash, 2)
+        tx = block['tx'][1]
+        assert 'fee' in tx
+        assert_equal(tx['fee'], tx['size'] * fee_per_byte)
+
+        self.log.info("Test that getblock with verbosity 2 still works with pruned Undo data")
+        datadir = get_datadir_path(self.options.tmpdir, 0)
+
+        def move_block_file(old, new):
+            old_path = os.path.join(datadir, self.chain, 'blocks', old)
+            new_path = os.path.join(datadir, self.chain, 'blocks', new)
+            os.rename(old_path, new_path)
+
+        # Move instead of deleting so we can restore chain state afterwards
+        move_block_file('rev00000.dat', 'rev_wrong')
+
+        block = node.getblock(blockhash, 2)
+        assert 'fee' not in block['tx'][1]
+
+        # Restore chain state
+        move_block_file('rev_wrong', 'rev00000.dat')
+
 
 if __name__ == '__main__':
     BlockchainTest().main()