What the developer wrote
Authored by Fabian Jahr
index, refactor: DRY coinbase check
Also marks a few additional variables const.
What changed, and why it matters
This is a small internal cleanup in Bitcoin Core's coin statistics index. It removes repeated calls checking whether a transaction is a coinbase transaction by storing the result once in a local variable, and marks a few objects as const so the compiler can enforce they are not modified. There is no change to network rules, consensus logic, or user-facing behavior.
No security action needed. This is a routine refactor; normal code review is sufficient.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit refactors CoinStatsIndex::CustomAppend and CoinStatsIndex::RevertBlock in src/index/coinstatsindex.cpp. It introduces a local const bool is_coinbase = tx->IsCoinBase() and replaces multiple tx->IsCoinBase() calls with that variable. It also adds const to several Coin and COutPoint declarations. The order of the BIP30 check’s conjunction is swapped (is_coinbase && IsBIP30Unspendable(…)), which is semantically equivalent to the original. No functional behavior is altered.
Changed components
src/index/coinstatsindex.cppInspect captured patch +14 / −13
diff --git a/src/index/coinstatsindex.cpp b/src/index/coinstatsindex.cpp
index ce3f566d..48d4c749 100644
--- a/src/index/coinstatsindex.cpp
+++ b/src/index/coinstatsindex.cpp
@@ -140,9 +140,10 @@ bool CoinStatsIndex::CustomAppend(const interfaces::BlockInfo& block)
assert(block.data);
for (size_t i = 0; i < block.data->vtx.size(); ++i) {
const auto& tx{block.data->vtx.at(i)};
+ const bool is_coinbase{tx->IsCoinBase()};
// Skip duplicate txid coinbase transactions (BIP30).
- if (IsBIP30Unspendable(block.hash, block.height) && tx->IsCoinBase()) {
+ if (is_coinbase && IsBIP30Unspendable(block.hash, block.height)) {
m_total_unspendable_amount += block_subsidy;
m_total_unspendables_bip30 += block_subsidy;
continue;
@@ -150,8 +151,8 @@ bool CoinStatsIndex::CustomAppend(const interfaces::BlockInfo& block)
for (uint32_t j = 0; j < tx->vout.size(); ++j) {
const CTxOut& out{tx->vout[j]};
- Coin coin{out, block.height, tx->IsCoinBase()};
- COutPoint outpoint{tx->GetHash(), j};
+ const Coin coin{out, block.height, is_coinbase};
+ const COutPoint outpoint{tx->GetHash(), j};
// Skip unspendable coins
if (coin.out.scriptPubKey.IsUnspendable()) {
@@ -162,7 +163,7 @@ bool CoinStatsIndex::CustomAppend(const interfaces::BlockInfo& block)
ApplyCoinHash(m_muhash, outpoint, coin);
- if (tx->IsCoinBase()) {
+ if (is_coinbase) {
m_total_coinbase_amount += coin.out.nValue;
} else {
m_total_new_outputs_ex_coinbase_amount += coin.out.nValue;
@@ -174,12 +175,12 @@ bool CoinStatsIndex::CustomAppend(const interfaces::BlockInfo& block)
}
// The coinbase tx has no undo data since no former output is spent
- if (!tx->IsCoinBase()) {
+ if (!is_coinbase) {
const auto& tx_undo{Assert(block.undo_data)->vtxundo.at(i - 1)};
for (size_t j = 0; j < tx_undo.vprevout.size(); ++j) {
- Coin coin{tx_undo.vprevout[j]};
- COutPoint outpoint{tx->vin[j].prevout.hash, tx->vin[j].prevout.n};
+ const Coin coin{tx_undo.vprevout[j]};
+ const COutPoint outpoint{tx->vin[j].prevout.hash, tx->vin[j].prevout.n};
RemoveCoinHash(m_muhash, outpoint, coin);
@@ -413,11 +414,12 @@ bool CoinStatsIndex::RevertBlock(const interfaces::BlockInfo& block)
assert(block.undo_data);
for (size_t i = 0; i < block.data->vtx.size(); ++i) {
const auto& tx{block.data->vtx.at(i)};
+ const bool is_coinbase{tx->IsCoinBase()};
for (uint32_t j = 0; j < tx->vout.size(); ++j) {
const CTxOut& out{tx->vout[j]};
- COutPoint outpoint{tx->GetHash(), j};
- Coin coin{out, block.height, tx->IsCoinBase()};
+ const COutPoint outpoint{tx->GetHash(), j};
+ const Coin coin{out, block.height, is_coinbase};
// Skip unspendable coins
if (coin.out.scriptPubKey.IsUnspendable()) {
@@ -440,13 +442,12 @@ bool CoinStatsIndex::RevertBlock(const interfaces::BlockInfo& block)
}
// The coinbase tx has no undo data since no former output is spent
- if (!tx->IsCoinBase()) {
+ if (!is_coinbase) {
const auto& tx_undo{block.undo_data->vtxundo.at(i - 1)};
for (size_t j = 0; j < tx_undo.vprevout.size(); ++j) {
- Coin coin{tx_undo.vprevout[j]};
- COutPoint outpoint{tx->vin[j].prevout.hash, tx->vin[j].prevout.n};
-
+ const Coin coin{tx_undo.vprevout[j]};
+ const COutPoint outpoint{tx->vin[j].prevout.hash, tx->vin[j].prevout.n};
ApplyCoinHash(m_muhash, outpoint, coin);
m_total_prevout_spent_amount -= coin.out.nValue;
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.