diff options
author | MarcoFalke <falke.marco@gmail.com> | 2022-03-09 11:43:03 +0100 |
---|---|---|
committer | MarcoFalke <falke.marco@gmail.com> | 2022-03-09 11:43:13 +0100 |
commit | 7003b6ab24f6adfffd71d7b7d4182afde52ff859 (patch) | |
tree | 1bc63e98f5da491fa2449a9c07794400725a402a /src/test/util | |
parent | aa83bbb1fe4807e3cd018ca3185f14ae2fd5120f (diff) | |
parent | 691d45fdc83ec14f87a400f548553168ac70263f (diff) | |
download | bitcoin-7003b6ab24f6adfffd71d7b7d4182afde52ff859.tar.xz |
Merge bitcoin/bitcoin#24138: index: Commit MuHash and best block together for coinstatsindex
691d45fdc83ec14f87a400f548553168ac70263f Add coinstatsindex_unclean_shutdown test (Ryan Ofsky)
eb6cc05da32c5bde122725a0bc907d3767a791cd index: Commit DB_MUHASH and DB_BEST_BLOCK to disk together (Martin Zumsande)
Pull request description:
Fixes #24076
Coinstatsindex currently writes the MuHash (`DB_MUHASH`) to disk in `CoinStatsIndex::WriteBlock()` and `CoinStatsIndex::ReverseBlock()`, but the best synced block is written in `BaseIndex::Commit()`. These are called at different points in time, both during the ThreadSync phase, and also after the initial sync is finished and validation callbacks (`BlockConnected()` vs `ChainStateFlushed()`) perform the syncing.
As a result, the index DB is temporarily in an inconsistent state, and if bitcoind is terminated uncleanly (so that there is no time to call `Commit()` by receiving an interrupt or by flushing the chainstate) this leads to problems:
On the next startup, `Init()` will read the best block and a MuHash that corresponds to a different (higher) block. Indexing will be picked up at the the best block processing some blocks again, but since MuHash is a rolling hash, it will process some utxos twice and the muhashes for all future blocks will be wrong, as was observed in #24076.
Fix this by always committing `DB_MUHASH` together with `DB_BEST_BLOCK`.
Note that the block data for the index is still written at different times, but this does not corrupt the index - at worst, these entries will be processed another time and overwritten after an unclean shutdown and restart.
ACKs for top commit:
ryanofsky:
Code review ACK 691d45fdc83ec14f87a400f548553168ac70263f. Only change since last review is adding test
fjahr:
ACK 691d45fdc83ec14f87a400f548553168ac70263f
Tree-SHA512: e1c3b5f06fa4baacd1b070abb0f8111fe2ea4a001ca8b8bf892e96597cf8b5d5ea10fa8fb837cfbf46648f052c742d912add4ce26d4406294fc5fc20809a0e1b
Diffstat (limited to 'src/test/util')
-rw-r--r-- | src/test/util/validation.cpp | 6 | ||||
-rw-r--r-- | src/test/util/validation.h | 8 |
2 files changed, 14 insertions, 0 deletions
diff --git a/src/test/util/validation.cpp b/src/test/util/validation.cpp index 1aed492c3c..49535855f9 100644 --- a/src/test/util/validation.cpp +++ b/src/test/util/validation.cpp @@ -7,6 +7,7 @@ #include <util/check.h> #include <util/time.h> #include <validation.h> +#include <validationinterface.h> void TestChainState::ResetIbd() { @@ -20,3 +21,8 @@ void TestChainState::JumpOutOfIbd() m_cached_finished_ibd = true; Assert(!IsInitialBlockDownload()); } + +void ValidationInterfaceTest::BlockConnected(CValidationInterface& obj, const std::shared_ptr<const CBlock>& block, const CBlockIndex* pindex) +{ + obj.BlockConnected(block, pindex); +} diff --git a/src/test/util/validation.h b/src/test/util/validation.h index b13aa0be60..b0bc717b6c 100644 --- a/src/test/util/validation.h +++ b/src/test/util/validation.h @@ -7,6 +7,8 @@ #include <validation.h> +class CValidationInterface; + struct TestChainState : public CChainState { /** Reset the ibd cache to its initial state */ void ResetIbd(); @@ -14,4 +16,10 @@ struct TestChainState : public CChainState { void JumpOutOfIbd(); }; +class ValidationInterfaceTest +{ +public: + static void BlockConnected(CValidationInterface& obj, const std::shared_ptr<const CBlock>& block, const CBlockIndex* pindex); +}; + #endif // BITCOIN_TEST_UTIL_VALIDATION_H |