From fa8fffebe8ac126f31143619843dd6578a2f4e3c Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Sun, 4 Apr 2021 07:47:14 +0200 Subject: [PATCH 1/2] refactor: Prefer clean assert over UB in coinstats --- src/node/coinstats.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/node/coinstats.cpp b/src/node/coinstats.cpp index 268580c6e60..f8f0fff43f5 100644 --- a/src/node/coinstats.cpp +++ b/src/node/coinstats.cpp @@ -94,7 +94,8 @@ static bool GetUTXOStats(CCoinsView* view, BlockManager& blockman, CCoinsStats& { LOCK(cs_main); assert(std::addressof(g_chainman.m_blockman) == std::addressof(blockman)); - stats.nHeight = blockman.LookupBlockIndex(stats.hashBlock)->nHeight; + const CBlockIndex* block = blockman.LookupBlockIndex(stats.hashBlock); + stats.nHeight = Assert(block)->nHeight; } PrepareHash(hash_obj, stats); From fa9b74f5ea89624e052934c48391b5076a87ffef Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Sat, 3 Apr 2021 15:30:04 +0200 Subject: [PATCH 2/2] Fix assumeutxo crash due to missing base_blockhash --- .../validation_chainstatemanager_tests.cpp | 5 +++ src/validation.cpp | 34 +++++-------------- 2 files changed, 14 insertions(+), 25 deletions(-) diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 94d42770194..35e087c899a 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -259,6 +259,11 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_activate_snapshot, TestChain100Determi // Coins count is smaller than coins in file metadata.m_coins_count -= 1; })); + BOOST_REQUIRE(!CreateAndActivateUTXOSnapshot( + m_node, m_path_root, [](CAutoFile& auto_infile, SnapshotMetadata& metadata) { + // Wrong hash + metadata.m_base_blockhash = uint256::ONE; + })); BOOST_REQUIRE(CreateAndActivateUTXOSnapshot(m_node, m_path_root)); diff --git a/src/validation.cpp b/src/validation.cpp index 19363c0efbf..619d3cea98b 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -5423,6 +5423,15 @@ bool ChainstateManager::PopulateAndValidateSnapshot( assert(coins_cache.GetBestBlock() == base_blockhash); + CBlockIndex* snapshot_start_block = WITH_LOCK(::cs_main, return m_blockman.LookupBlockIndex(base_blockhash)); + + if (!snapshot_start_block) { + // Needed for GetUTXOStats to determine the height + LogPrintf("[snapshot] Did not find snapshot start blockheader %s\n", + base_blockhash.ToString()); + return false; + } + CCoinsStats stats; auto breakpoint_fnc = [] { /* TODO insert breakpoint here? */ }; @@ -5435,31 +5444,6 @@ bool ChainstateManager::PopulateAndValidateSnapshot( return false; } - // Ensure that the base blockhash appears in the known chain of valid headers. We're willing to - // wait a bit here because the snapshot may have been loaded on startup, before we've - // received headers from the network. - - int max_secs_to_wait_for_headers = 60 * 10; - CBlockIndex* snapshot_start_block = nullptr; - - while (max_secs_to_wait_for_headers > 0) { - snapshot_start_block = WITH_LOCK(::cs_main, - return m_blockman.LookupBlockIndex(base_blockhash)); - --max_secs_to_wait_for_headers; - - if (!snapshot_start_block) { - std::this_thread::sleep_for(std::chrono::seconds(1)); - } else { - break; - } - } - - if (snapshot_start_block == nullptr) { - LogPrintf("[snapshot] timed out waiting for snapshot start blockheader %s\n", - base_blockhash.ToString()); - return false; - } - // Assert that the deserialized chainstate contents match the expected assumeutxo value. int base_height = snapshot_start_block->nHeight;