From 6fd4341c10b319399c58d71c4ddeae4417e337d7 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Thu, 9 Sep 2021 17:17:39 +0200 Subject: Require CBlockIndex::GetBlockPos() to hold mutex cs_main --- src/chain.h | 6 +++++- src/index/txindex.cpp | 4 +++- src/test/fuzz/chain.cpp | 21 ++++++++++++--------- src/test/util/blockfilter.cpp | 2 ++ src/wallet/test/wallet_tests.cpp | 17 +++++++++++------ 5 files changed, 33 insertions(+), 17 deletions(-) diff --git a/src/chain.h b/src/chain.h index 55bdf4cd56..534f226f28 100644 --- a/src/chain.h +++ b/src/chain.h @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -37,6 +38,8 @@ static constexpr int64_t TIMESTAMP_WINDOW = MAX_FUTURE_BLOCK_TIME; */ static constexpr int64_t MAX_BLOCK_TIME_GAP = 90 * 60; +extern RecursiveMutex cs_main; + class CBlockFileInfo { public: @@ -223,8 +226,9 @@ public: { } - FlatFilePos GetBlockPos() const + FlatFilePos GetBlockPos() const EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { + AssertLockHeld(::cs_main); FlatFilePos ret; if (nStatus & BLOCK_HAVE_DATA) { ret.nFile = nFile; diff --git a/src/index/txindex.cpp b/src/index/txindex.cpp index e9aeb58194..e1d807f39a 100644 --- a/src/index/txindex.cpp +++ b/src/index/txindex.cpp @@ -59,7 +59,9 @@ bool TxIndex::WriteBlock(const CBlock& block, const CBlockIndex* pindex) // Exclude genesis block transaction because outputs are not spendable. if (pindex->nHeight == 0) return true; - CDiskTxPos pos(pindex->GetBlockPos(), GetSizeOfCompactSize(block.vtx.size())); + CDiskTxPos pos{ + WITH_LOCK(::cs_main, return pindex->GetBlockPos()), + GetSizeOfCompactSize(block.vtx.size())}; std::vector> vPos; vPos.reserve(block.vtx.size()); for (const auto& tx : block.vtx) { diff --git a/src/test/fuzz/chain.cpp b/src/test/fuzz/chain.cpp index 326904a811..4f97e1ebb1 100644 --- a/src/test/fuzz/chain.cpp +++ b/src/test/fuzz/chain.cpp @@ -21,15 +21,18 @@ FUZZ_TARGET(chain) const uint256 zero{}; disk_block_index->phashBlock = &zero; - (void)disk_block_index->GetBlockHash(); - (void)disk_block_index->GetBlockPos(); - (void)disk_block_index->GetBlockTime(); - (void)disk_block_index->GetBlockTimeMax(); - (void)disk_block_index->GetMedianTimePast(); - (void)disk_block_index->GetUndoPos(); - (void)disk_block_index->HaveTxsDownloaded(); - (void)disk_block_index->IsValid(); - (void)disk_block_index->ToString(); + { + LOCK(::cs_main); + (void)disk_block_index->GetBlockHash(); + (void)disk_block_index->GetBlockPos(); + (void)disk_block_index->GetBlockTime(); + (void)disk_block_index->GetBlockTimeMax(); + (void)disk_block_index->GetMedianTimePast(); + (void)disk_block_index->GetUndoPos(); + (void)disk_block_index->HaveTxsDownloaded(); + (void)disk_block_index->IsValid(); + (void)disk_block_index->ToString(); + } const CBlockHeader block_header = disk_block_index->GetBlockHeader(); (void)CDiskBlockIndex{*disk_block_index}; diff --git a/src/test/util/blockfilter.cpp b/src/test/util/blockfilter.cpp index 538981ce36..3ae22921b9 100644 --- a/src/test/util/blockfilter.cpp +++ b/src/test/util/blockfilter.cpp @@ -13,6 +13,8 @@ using node::UndoReadFromDisk; bool ComputeFilter(BlockFilterType filter_type, const CBlockIndex* block_index, BlockFilter& filter) { + LOCK(::cs_main); + CBlock block; if (!ReadBlockFromDisk(block, block_index->GetBlockPos(), Params().GetConsensus())) { return false; diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index bb6021b857..9a74545fb5 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -140,11 +140,13 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) } // Prune the older block file. + int file_number; { LOCK(cs_main); - Assert(m_node.chainman)->m_blockman.PruneOneBlockFile(oldTip->GetBlockPos().nFile); + file_number = oldTip->GetBlockPos().nFile; + Assert(m_node.chainman)->m_blockman.PruneOneBlockFile(file_number); } - UnlinkPrunedFiles({oldTip->GetBlockPos().nFile}); + UnlinkPrunedFiles({file_number}); // Verify ScanForWalletTransactions only picks transactions in the new block // file. @@ -169,9 +171,10 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) // Prune the remaining block file. { LOCK(cs_main); - Assert(m_node.chainman)->m_blockman.PruneOneBlockFile(newTip->GetBlockPos().nFile); + file_number = newTip->GetBlockPos().nFile; + Assert(m_node.chainman)->m_blockman.PruneOneBlockFile(file_number); } - UnlinkPrunedFiles({newTip->GetBlockPos().nFile}); + UnlinkPrunedFiles({file_number}); // Verify ScanForWalletTransactions scans no blocks. { @@ -202,11 +205,13 @@ BOOST_FIXTURE_TEST_CASE(importmulti_rescan, TestChain100Setup) CBlockIndex* newTip = m_node.chainman->ActiveChain().Tip(); // Prune the older block file. + int file_number; { LOCK(cs_main); - Assert(m_node.chainman)->m_blockman.PruneOneBlockFile(oldTip->GetBlockPos().nFile); + file_number = oldTip->GetBlockPos().nFile; + Assert(m_node.chainman)->m_blockman.PruneOneBlockFile(file_number); } - UnlinkPrunedFiles({oldTip->GetBlockPos().nFile}); + UnlinkPrunedFiles({file_number}); // Verify importmulti RPC returns failure for a key whose creation time is // before the missing block, and success for a key whose creation time is -- cgit v1.2.3