From 52f036b316de47e881c5d5dd9cf7be5e4bb496a4 Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 12 Sep 2023 09:30:59 +0100 Subject: [PATCH] Merge bitcoin/bitcoin#28427: index: coinstats reorg, fail when block cannot be reversed c0bf667912064960df194ea94150976b34f7c267 index: add [nodiscard] attribute to functions writing to the db (furszy) eef595560e9ecf3a0d1db4d8ea7ecc33a49d839f index: coinstats reorg, fail when block cannot be reversed (furszy) Pull request description: Found it while reviewing https://github.com/bitcoin/bitcoin/pull/24230#discussion_r1310863359. During a reorg, continuing execution when a block cannot be reversed leaves the coinstats index in an inconsistent state. This was surely overlooked when 'CustomRewind' was implemented. ACKs for top commit: ryanofsky: Code review ACK c0bf667912064960df194ea94150976b34f7c267. Only change since last review is new commit adding [[nodiscard]] Tree-SHA512: f4fc8522508d23e4fff09a29c935971819b1bd3b2a260e08e2e2b72f9340980d74fbec742a58fe216baf61d27de057c7c8300e8fa075f8507cd1227f128af909 --- src/index/blockfilterindex.cpp | 2 +- src/index/coinstatsindex.cpp | 6 ++++-- src/index/coinstatsindex.h | 2 +- src/index/txindex.cpp | 2 +- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/index/blockfilterindex.cpp b/src/index/blockfilterindex.cpp index 2e84934eba..6f20f1ad61 100644 --- a/src/index/blockfilterindex.cpp +++ b/src/index/blockfilterindex.cpp @@ -260,7 +260,7 @@ bool BlockFilterIndex::WriteBlock(const CBlock& block, const CBlockIndex* pindex return true; } -static bool CopyHeightIndexToHashIndex(CDBIterator& db_it, CDBBatch& batch, +[[nodiscard]] static bool CopyHeightIndexToHashIndex(CDBIterator& db_it, CDBBatch& batch, const std::string& index_name, int start_height, int stop_height) { diff --git a/src/index/coinstatsindex.cpp b/src/index/coinstatsindex.cpp index 561f3b38f3..1ee161176f 100644 --- a/src/index/coinstatsindex.cpp +++ b/src/index/coinstatsindex.cpp @@ -228,7 +228,7 @@ bool CoinStatsIndex::WriteBlock(const CBlock& block, const CBlockIndex* pindex) return m_db->Write(DBHeightKey(pindex->nHeight), value); } -static bool CopyHeightIndexToHashIndex(CDBIterator& db_it, CDBBatch& batch, +[[nodiscard]] static bool CopyHeightIndexToHashIndex(CDBIterator& db_it, CDBBatch& batch, const std::string& index_name, int start_height, int stop_height) { @@ -283,7 +283,9 @@ bool CoinStatsIndex::Rewind(const CBlockIndex* current_tip, const CBlockIndex* n __func__, iter_tip->GetBlockHash().ToString()); } - ReverseBlock(block, iter_tip); + if (!ReverseBlock(block, iter_tip)) { + return false; // failure cause logged internally + } iter_tip = iter_tip->GetAncestor(iter_tip->nHeight - 1); } while (new_tip != iter_tip); diff --git a/src/index/coinstatsindex.h b/src/index/coinstatsindex.h index 8cfb87ee8b..e93c2316a4 100644 --- a/src/index/coinstatsindex.h +++ b/src/index/coinstatsindex.h @@ -34,7 +34,7 @@ private: CAmount m_total_unspendables_scripts{0}; CAmount m_total_unspendables_unclaimed_rewards{0}; - bool ReverseBlock(const CBlock& block, const CBlockIndex* pindex); + [[nodiscard]] bool ReverseBlock(const CBlock& block, const CBlockIndex* pindex); bool AllowPrune() const override { return true; } diff --git a/src/index/txindex.cpp b/src/index/txindex.cpp index eebec0c47b..78dbbe5567 100644 --- a/src/index/txindex.cpp +++ b/src/index/txindex.cpp @@ -25,7 +25,7 @@ public: bool ReadTxPos(const uint256& txid, CDiskTxPos& pos) const; /// Write a batch of transaction positions to the DB. - bool WriteTxs(const std::vector>& v_pos); + [[nodiscard]] bool WriteTxs(const std::vector>& v_pos); }; TxIndex::DB::DB(size_t n_cache_size, bool f_memory, bool f_wipe) :