From 1658b8f82b99f9ba3b42a50ba1f72b19a38c8e75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Fri, 31 Oct 2025 14:51:04 +0100 Subject: [PATCH] refactor: rename `CTransaction::GetTotalSize` to signal that it's not cached Transaction hashes are cached, it may not be intuitive that their sizes are actually recalculated every time. This is done before the other refactors to clarify why we want to avoid calling this method; Co-authored-by: maflcko <6399679+maflcko@users.noreply.github.com> --- src/bench/duplicate_inputs.cpp | 2 +- src/blockencodings.cpp | 2 +- src/core_io.cpp | 2 +- src/net_processing.cpp | 2 +- src/primitives/transaction.cpp | 2 +- src/primitives/transaction.h | 4 ++-- src/qt/transactiondesc.cpp | 2 +- src/rpc/blockchain.cpp | 2 +- src/rpc/mempool.cpp | 2 +- src/test/fuzz/transaction.cpp | 4 ++-- 10 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/bench/duplicate_inputs.cpp b/src/bench/duplicate_inputs.cpp index b59d14af892..5b1ca5f552f 100644 --- a/src/bench/duplicate_inputs.cpp +++ b/src/bench/duplicate_inputs.cpp @@ -58,7 +58,7 @@ static void DuplicateInputs(benchmark::Bench& bench) naughtyTx.vout[0].nValue = 0; naughtyTx.vout[0].scriptPubKey = SCRIPT_PUB; - uint64_t n_inputs = (((MAX_BLOCK_SERIALIZED_SIZE / WITNESS_SCALE_FACTOR) - (CTransaction(coinbaseTx).GetTotalSize() + CTransaction(naughtyTx).GetTotalSize())) / 41) - 100; + uint64_t n_inputs = (((MAX_BLOCK_SERIALIZED_SIZE / WITNESS_SCALE_FACTOR) - (CTransaction(coinbaseTx).ComputeTotalSize() + CTransaction(naughtyTx).ComputeTotalSize())) / 41) - 100; for (uint64_t x = 0; x < (n_inputs - 1); ++x) { naughtyTx.vin.emplace_back(Txid::FromUint256(GetRandHash()), 0, CScript(), 0); } diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp index 49900482f98..eebf7bf466a 100644 --- a/src/blockencodings.cpp +++ b/src/blockencodings.cpp @@ -203,7 +203,7 @@ ReadStatus PartiallyDownloadedBlock::FillBlock(CBlock& block, const std::vector< if (vtx_missing.size() <= tx_missing_offset) return READ_STATUS_INVALID; block.vtx[i] = vtx_missing[tx_missing_offset++]; - tx_missing_size += block.vtx[i]->GetTotalSize(); + tx_missing_size += block.vtx[i]->ComputeTotalSize(); } else block.vtx[i] = std::move(txn_available[i]); } diff --git a/src/core_io.cpp b/src/core_io.cpp index a789d5ca947..1171b4c9594 100644 --- a/src/core_io.cpp +++ b/src/core_io.cpp @@ -432,7 +432,7 @@ void TxToUniv(const CTransaction& tx, const uint256& block_hash, UniValue& entry entry.pushKV("txid", tx.GetHash().GetHex()); entry.pushKV("hash", tx.GetWitnessHash().GetHex()); entry.pushKV("version", tx.version); - entry.pushKV("size", tx.GetTotalSize()); + entry.pushKV("size", tx.ComputeTotalSize()); entry.pushKV("vsize", (GetTransactionWeight(tx) + WITNESS_SCALE_FACTOR - 1) / WITNESS_SCALE_FACTOR); entry.pushKV("weight", GetTransactionWeight(tx)); entry.pushKV("locktime", (int64_t)tx.nLockTime); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 3fff1db522b..89f93781831 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2577,7 +2577,7 @@ void PeerManagerImpl::SendBlockTransactions(CNode& pfrom, Peer& peer, const CBlo return; } resp.txn[i] = block.vtx[req.indexes[i]]; - tx_requested_size += resp.txn[i]->GetTotalSize(); + tx_requested_size += resp.txn[i]->ComputeTotalSize(); } LogDebug(BCLog::CMPCTBLOCK, "Peer %d sent us a GETBLOCKTXN for block %s, sending a BLOCKTXN with %u txns. (%u bytes)\n", pfrom.GetId(), block.GetHash().ToString(), resp.txn.size(), tx_requested_size); diff --git a/src/primitives/transaction.cpp b/src/primitives/transaction.cpp index c8bbd9f821e..fc8a70bf01a 100644 --- a/src/primitives/transaction.cpp +++ b/src/primitives/transaction.cpp @@ -107,7 +107,7 @@ CAmount CTransaction::GetValueOut() const return nValueOut; } -unsigned int CTransaction::GetTotalSize() const +unsigned int CTransaction::ComputeTotalSize() const { return ::GetSerializeSize(TX_WITH_WITNESS(*this)); } diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h index 2ccdb4e44bc..7d4c6bf5b9a 100644 --- a/src/primitives/transaction.h +++ b/src/primitives/transaction.h @@ -332,11 +332,11 @@ public: CAmount GetValueOut() const; /** - * Get the total transaction size in bytes, including witness data. + * Calculate the total transaction size in bytes, including witness data. * "Total Size" defined in BIP141 and BIP144. * @return Total transaction size in bytes */ - unsigned int GetTotalSize() const; + unsigned int ComputeTotalSize() const; bool IsCoinBase() const { diff --git a/src/qt/transactiondesc.cpp b/src/qt/transactiondesc.cpp index 918d0af95d1..d9c97d85297 100644 --- a/src/qt/transactiondesc.cpp +++ b/src/qt/transactiondesc.cpp @@ -279,7 +279,7 @@ QString TransactionDesc::toHTML(interfaces::Node& node, interfaces::Wallet& wall strHTML += "
" + tr("Comment") + ":
" + GUIUtil::HtmlEscape(wtx.value_map["comment"], true) + "
"; strHTML += "" + tr("Transaction ID") + ": " + rec->getTxHash() + "
"; - strHTML += "" + tr("Transaction total size") + ": " + QString::number(wtx.tx->GetTotalSize()) + " bytes
"; + strHTML += "" + tr("Transaction total size") + ": " + QString::number(wtx.tx->ComputeTotalSize()) + " bytes
"; strHTML += "" + tr("Transaction virtual size") + ": " + QString::number(GetVirtualTransactionSize(*wtx.tx)) + " bytes
"; strHTML += "" + tr("Output index") + ": " + QString::number(rec->getOutputIndex()) + "
"; diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 763de836890..7de01ac641a 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -2053,7 +2053,7 @@ static RPCHelpMan getblockstats() int64_t tx_size = 0; if (do_calculate_size) { - tx_size = tx->GetTotalSize(); + tx_size = tx->ComputeTotalSize(); if (do_mediantxsize) { txsize_array.push_back(tx_size); } diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index 363e5e382e9..7657910512a 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -1031,7 +1031,7 @@ static UniValue OrphanToJSON(const node::TxOrphanage::OrphanInfo& orphan) UniValue o(UniValue::VOBJ); o.pushKV("txid", orphan.tx->GetHash().ToString()); o.pushKV("wtxid", orphan.tx->GetWitnessHash().ToString()); - o.pushKV("bytes", orphan.tx->GetTotalSize()); + o.pushKV("bytes", orphan.tx->ComputeTotalSize()); o.pushKV("vsize", GetVirtualTransactionSize(*orphan.tx)); o.pushKV("weight", GetTransactionWeight(*orphan.tx)); UniValue from(UniValue::VARR); diff --git a/src/test/fuzz/transaction.cpp b/src/test/fuzz/transaction.cpp index b96fb71b1f4..da915713f7e 100644 --- a/src/test/fuzz/transaction.cpp +++ b/src/test/fuzz/transaction.cpp @@ -68,7 +68,7 @@ FUZZ_TARGET(transaction, .init = initialize_transaction) } (void)tx.GetHash(); - (void)tx.GetTotalSize(); + (void)tx.ComputeTotalSize(); try { (void)tx.GetValueOut(); } catch (const std::runtime_error&) { @@ -92,7 +92,7 @@ FUZZ_TARGET(transaction, .init = initialize_transaction) (void)AreInputsStandard(tx, coins_view_cache); (void)IsWitnessStandard(tx, coins_view_cache); - if (tx.GetTotalSize() < 250'000) { // Avoid high memory usage (with msan) due to json encoding + if (tx.ComputeTotalSize() < 250'000) { // Avoid high memory usage (with msan) due to json encoding { UniValue u{UniValue::VOBJ}; TxToUniv(tx, /*block_hash=*/uint256::ZERO, /*entry=*/u);