Skip to content

Commit

Permalink
Merge bitcoin#18772: rpc: calculate fees in getblock using BlockUndo …
Browse files Browse the repository at this point in the history
…data

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

Pull request description:

  This change is progress towards bitcoin#18771 .  It adapts the fee calculation part of bitcoin#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 bitcoin#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 66d012a
  fjahr:
    tACK 66d012a
  MarcoFalke:
    review ACK 66d012a 🗜

Tree-SHA512: be1fe4b866946a8dc36427f7dc72a20e10860e320a28fa49bc85bd2a93a0d699768179be29fa52e18b2ed8505d3ec272e586753ef2239b4230e0aefd233acaa2
  • Loading branch information
MarcoFalke authored and knst committed Apr 9, 2024
1 parent 41a1e10 commit e36eacd
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 11 deletions.
3 changes: 2 additions & 1 deletion src/core_io.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ class CTransaction;
struct CMutableTransaction;
class uint256;
class UniValue;
class CTxUndo;

struct CSpentIndexTxInfo;

Expand Down Expand Up @@ -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
32 changes: 28 additions & 4 deletions src/core_write.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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());
Expand All @@ -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);
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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());

Expand Down
18 changes: 12 additions & 6 deletions src/rpc/blockchain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -219,19 +219,24 @@ 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)
{
if (txDetails) {
CBlockUndo blockUndo;
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()) {
Expand Down Expand Up @@ -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"},
}},
}},
}},
Expand Down
44 changes: 44 additions & 0 deletions test/functional/rpc_blockchain.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

from decimal import Decimal
import http.client
import os
import subprocess

from test_framework.blocktools import (
Expand All @@ -42,8 +43,10 @@
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):
Expand All @@ -65,6 +68,7 @@ def run_test(self):
self._test_getnetworkhashps()
self._test_stopatheight()
self._test_waitforblockheight()
self._test_getblock()
assert self.nodes[0].verifychain(4, 0)

def mine_chain(self):
Expand Down Expand Up @@ -397,6 +401,46 @@ def assert_waitforheight(height, timeout=2):
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()

0 comments on commit e36eacd

Please sign in to comment.