What’s the reason for doing these calculations in the middle of the method - is it to avoid calculating the subsidy when there’s a read error?
I wouldn’t optimize for that usecase: leaving the block_subsidy
declaration at the beginning and only updating total_subsidy
at the very end would avoid repeating the block.height > 0
condition and the diff would also be smaller.
Also, since we’re reading and writing a DBVal
anyway, could use that to hold the state instead of keeping track of the individual primitives and copying back and forth?
I mean something like:
0diff --git a/src/index/coinstatsindex.cpp b/src/index/coinstatsindex.cpp
1--- a/src/index/coinstatsindex.cpp (revision a53063a6d93103926509d3f95dc791140aae58c6)
2+++ b/src/index/coinstatsindex.cpp (date 1754333742030)
3@@ -114,56 +114,27 @@
4
5 bool CoinStatsIndex::CustomAppend(const interfaces::BlockInfo& block)
6 {
7- uint64_t transaction_output_count{0};
8- uint64_t bogo_size{0};
9- CAmount total_amount{0};
10- CAmount total_subsidy{0};
11- CAmount total_unspendable_amount{0};
12- CAmount total_prevout_spent_amount{0};
13- CAmount total_new_outputs_ex_coinbase_amount{0};
14- CAmount total_coinbase_amount{0};
15- CAmount total_unspendables_genesis_block{0};
16- CAmount total_unspendables_bip30{0};
17- CAmount total_unspendables_scripts{0};
18- CAmount total_unspendables_unclaimed_rewards{0};
19+ const CAmount block_subsidy{GetBlockSubsidy(block.height, Params().GetConsensus())};
20+ std::pair<uint256, DBVal> val{};
21
22- // Ignore genesis block
23 if (block.height > 0) {
24- std::pair<uint256, DBVal> read_out;
25- if (!m_db->Read(DBHeightKey(block.height - 1), read_out)) {
26+ if (!m_db->Read(DBHeightKey(block.height - 1), val)) {
27 return false;
28 }
29
30 uint256 expected_block_hash{*Assert(block.prev_hash)};
31- if (read_out.first != expected_block_hash) {
32+ if (val.first != expected_block_hash) {
33 LogWarning("previous block header belongs to unexpected block %s; expected %s",
34- read_out.first.ToString(), expected_block_hash.ToString());
35+ val.first.ToString(), expected_block_hash.ToString());
36
37- if (!m_db->Read(DBHashKey(expected_block_hash), read_out)) {
38+ if (!m_db->Read(DBHashKey(expected_block_hash), val)) {
39 LogError("previous block header not found; expected %s",
40 expected_block_hash.ToString());
41 return false;
42 }
43 }
44
45- transaction_output_count = read_out.second.transaction_output_count;
46- bogo_size = read_out.second.bogo_size;
47- total_amount = read_out.second.total_amount;
48- total_subsidy = read_out.second.total_subsidy;
49- total_unspendable_amount = read_out.second.total_unspendable_amount;
50- total_prevout_spent_amount = read_out.second.total_prevout_spent_amount;
51- total_new_outputs_ex_coinbase_amount = read_out.second.total_new_outputs_ex_coinbase_amount;
52- total_coinbase_amount = read_out.second.total_coinbase_amount;
53- total_unspendables_genesis_block = read_out.second.total_unspendables_genesis_block;
54- total_unspendables_bip30 = read_out.second.total_unspendables_bip30;
55- total_unspendables_scripts = read_out.second.total_unspendables_scripts;
56- total_unspendables_unclaimed_rewards = read_out.second.total_unspendables_unclaimed_rewards;
57- }
58-
59- const CAmount block_subsidy{GetBlockSubsidy(block.height, Params().GetConsensus())};
60- total_subsidy += block_subsidy;
61-
62- if (block.height > 0) {
63 // Add the new utxos created from the block
64 assert(block.data);
65 for (size_t i = 0; i < block.data->vtx.size(); ++i) {
66@@ -171,8 +142,8 @@
67
68 // Skip duplicate txid coinbase transactions (BIP30).
69 if (IsBIP30Unspendable(block.hash, block.height) && tx->IsCoinBase()) {
70- total_unspendable_amount += block_subsidy;
71- total_unspendables_bip30 += block_subsidy;
72+ val.second.total_unspendable_amount += block_subsidy;
73+ val.second.total_unspendables_bip30 += block_subsidy;
74 continue;
75 }
76
77@@ -183,22 +154,22 @@
78
79 // Skip unspendable coins
80 if (coin.out.scriptPubKey.IsUnspendable()) {
81- total_unspendable_amount += coin.out.nValue;
82- total_unspendables_scripts += coin.out.nValue;
83+ val.second.total_unspendable_amount += coin.out.nValue;
84+ val.second.total_unspendables_scripts += coin.out.nValue;
85 continue;
86 }
87
88 ApplyCoinHash(m_muhash, outpoint, coin);
89
90 if (tx->IsCoinBase()) {
91- total_coinbase_amount += coin.out.nValue;
92+ val.second.total_coinbase_amount += coin.out.nValue;
93 } else {
94- total_new_outputs_ex_coinbase_amount += coin.out.nValue;
95+ val.second.total_new_outputs_ex_coinbase_amount += coin.out.nValue;
96 }
97
98- ++transaction_output_count;
99- total_amount += coin.out.nValue;
100- bogo_size += GetBogoSize(coin.out.scriptPubKey);
101+ ++val.second.transaction_output_count;
102+ val.second.total_amount += coin.out.nValue;
103+ val.second.bogo_size += GetBogoSize(coin.out.scriptPubKey);
104 }
105
106 // The coinbase tx has no undo data since no former output is spent
107@@ -211,50 +182,39 @@
108
109 RemoveCoinHash(m_muhash, outpoint, coin);
110
111- total_prevout_spent_amount += coin.out.nValue;
112+ val.second.total_prevout_spent_amount += coin.out.nValue;
113
114- --transaction_output_count;
115- total_amount -= coin.out.nValue;
116- bogo_size -= GetBogoSize(coin.out.scriptPubKey);
117+ --val.second.transaction_output_count;
118+ val.second.total_amount -= coin.out.nValue;
119+ val.second.bogo_size -= GetBogoSize(coin.out.scriptPubKey);
120 }
121 }
122 }
123 } else {
124 // genesis block
125- total_unspendable_amount += block_subsidy;
126- total_unspendables_genesis_block += block_subsidy;
127+ val.second.total_unspendable_amount += block_subsidy;
128+ val.second.total_unspendables_genesis_block += block_subsidy;
129 }
130+
131+ val.second.total_subsidy += block_subsidy;
132
133 // If spent prevouts + block subsidy are still a higher amount than
134 // new outputs + coinbase + current unspendable amount this means
135 // the miner did not claim the full block reward. Unclaimed block
136 // rewards are also unspendable.
137- const CAmount unclaimed_rewards{(total_prevout_spent_amount + total_subsidy) - (total_new_outputs_ex_coinbase_amount + total_coinbase_amount + total_unspendable_amount)};
138- total_unspendable_amount += unclaimed_rewards;
139- total_unspendables_unclaimed_rewards += unclaimed_rewards;
140+ const CAmount unclaimed_rewards{(val.second.total_prevout_spent_amount + val.second.total_subsidy) - (val.second.total_new_outputs_ex_coinbase_amount + val.second.total_coinbase_amount + val.second.total_unspendable_amount)};
141+ val.second.total_unspendable_amount += unclaimed_rewards;
142+ val.second.total_unspendables_unclaimed_rewards += unclaimed_rewards;
143
144- std::pair<uint256, DBVal> value;
145- value.first = block.hash;
146- value.second.transaction_output_count = transaction_output_count;
147- value.second.bogo_size = bogo_size;
148- value.second.total_amount = total_amount;
149- value.second.total_subsidy = total_subsidy;
150- value.second.total_unspendable_amount = total_unspendable_amount;
151- value.second.total_prevout_spent_amount = total_prevout_spent_amount;
152- value.second.total_new_outputs_ex_coinbase_amount = total_new_outputs_ex_coinbase_amount;
153- value.second.total_coinbase_amount = total_coinbase_amount;
154- value.second.total_unspendables_genesis_block = total_unspendables_genesis_block;
155- value.second.total_unspendables_bip30 = total_unspendables_bip30;
156- value.second.total_unspendables_scripts = total_unspendables_scripts;
157- value.second.total_unspendables_unclaimed_rewards = total_unspendables_unclaimed_rewards;
158+ val.first = block.hash;
159
160 uint256 out;
161 m_muhash.Finalize(out);
162- value.second.muhash = out;
163+ val.second.muhash = out;
164
165 // Intentionally do not update DB_MUHASH here so it stays in sync with
166 // DB_BEST_BLOCK, and the index is not corrupted if there is an unclean shutdown.
167- return m_db->Write(DBHeightKey(block.height), value);
168+ return m_db->Write(DBHeightKey(block.height), val);
169 }
170
171 [[nodiscard]] static bool CopyHeightIndexToHashIndex(CDBIterator& db_it, CDBBatch& batch,
(val.second
can of course be extracted if you find it harder to read)
Also note that return m_db->Write(DBHeightKey(block.height), val);
is always true
currently, it’s not actually returning false
on error, see: #33042)