index: Remove unused coinstatsindex recovery code
What the developer wrote
Authored by Fabian Jahr
index: Remove unused coinstatsindex recovery code
The coinstatsindex currently looks for block data at a hash key if the prev block in CustomAppend is different than expected. This is not needed since base index should always prevent us ending up in this scenario since it should rewind the index before calling CustomAppend in this case. But even if we run into this and our belt-and-suspenders code is getting hit, the index could not recover properly from the hash key index data so it can be removed without any real impact.
What changed, and why it matters
This commit removes a rarely-used recovery path in the CoinStats index. Previously, if the index noticed an unexpected previous block hash, it would try to read older block data from the database to recover. The change makes it simply log an error and stop instead. The commit message says the base index should already prevent this situation, and the recovery code did not work properly anyway. There is no direct evidence this is a security fix; it appears to be a code-cleanup and robustness change.
Treat as routine maintenance. Reviewers may want to confirm that BaseIndex indeed rewinds the index before invoking CustomAppend when the chain reorganizes or the index is behind the tip, so the removed recovery path is truly redundant and not a needed safety net.
Security signals we found
Removal of defensive/recovery code path
Change of log severity from warning to error
Potential change in failure mode from attempted recovery to immediate abort
Evidence from the diff
In CoinStatsIndex::CustomAppend, the code used to compare m_current_block_hash against the expected previous block hash. On mismatch, it logged a warning and attempted to read a DBHashKey(expected_block_hash) entry to recover state. The patch removes that read/recovery branch, upgrades the log to LogError, and returns false immediately. The commit rationale is that BaseIndex should rewind before CustomAppend when needed, making the recovery path unreachable in normal operation, and that the recovery path could not actually recover correctly because the DBVal stored under the hash key does not contain the data needed to reconstruct the running coinstats state.
Changed components
src/index/coinstatsindex.cppCoinStatsIndex::CustomAppendInspect captured patch +2 / −8
diff --git a/src/index/coinstatsindex.cpp b/src/index/coinstatsindex.cpp
index 24b1837b..3d092254 100644
--- a/src/index/coinstatsindex.cpp
+++ b/src/index/coinstatsindex.cpp
@@ -139,15 +139,9 @@ bool CoinStatsIndex::CustomAppend(const interfaces::BlockInfo& block)
if (block.height > 0) {
uint256 expected_block_hash{*Assert(block.prev_hash)};
if (m_current_block_hash != expected_block_hash) {
- LogWarning("previous block header belongs to unexpected block %s; expected %s",
+ LogError("previous block header belongs to unexpected block %s; expected %s",
m_current_block_hash.ToString(), expected_block_hash.ToString());
-
- std::pair<uint256, DBVal> read_out;
- if (!m_db->Read(DBHashKey(expected_block_hash), read_out)) {
- LogError("previous block header not found; expected %s",
- expected_block_hash.ToString());
- return false;
- }
+ return false;
}
// Add the new utxos created from the block
Why this scored 16/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.