diff --git a/src/test/util/chainstate.h b/src/test/util/chainstate.h --- a/src/test/util/chainstate.h +++ b/src/test/util/chainstate.h @@ -121,8 +121,24 @@ return node.chainman->ActiveHeight())); } - return node.chainman->ActivateSnapshot(auto_infile, metadata, - in_memory_chainstate); + auto &new_active = node.chainman->ActiveChainstate(); + auto *tip = new_active.m_chain.Tip(); + + // Disconnect a block so that the snapshot chainstate will be ahead, + // otherwise it will refuse to activate. + // + // TODO this is a unittest-specific hack, and we should probably rethink how + // to better generate/activate snapshots in unittests. + if (tip->pprev) { + new_active.m_chain.SetTip(*(tip->pprev)); + } + + bool res = node.chainman->ActivateSnapshot(auto_infile, metadata, + in_memory_chainstate); + + // Restore the old tip. + new_active.m_chain.SetTip(*tip); + return res; } #endif // BITCOIN_TEST_UTIL_CHAINSTATE_H diff --git a/src/validation.cpp b/src/validation.cpp --- a/src/validation.cpp +++ b/src/validation.cpp @@ -6523,65 +6523,78 @@ current_coinstip_cache_size * SNAPSHOT_CACHE_PERC)); } - bool snapshot_ok = this->PopulateAndValidateSnapshot(*snapshot_chainstate, - coins_file, metadata); + auto cleanup_bad_snapshot = + [&](const char *reason) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { + LogPrintf("[snapshot] activation failed - %s\n", reason); + this->MaybeRebalanceCaches(); + + // PopulateAndValidateSnapshot can return (in error) before the + // leveldb datadir has been created, so only attempt removal if we + // got that far. + if (auto snapshot_datadir = node::FindSnapshotChainstateDir()) { + // We have to destruct leveldb::DB in order to release the db + // lock, otherwise DestroyDB() (in DeleteCoinsDBFromDisk()) will + // fail. See `leveldb::~DBImpl()`. Destructing the chainstate + // (and so resetting the coinsviews object) does this. + snapshot_chainstate.reset(); + bool removed = DeleteCoinsDBFromDisk(*snapshot_datadir, + /*is_snapshot=*/true); + if (!removed) { + AbortNode(strprintf( + "Failed to remove snapshot chainstate dir (%s). " + "Manually remove it before restarting.\n", + fs::PathToString(*snapshot_datadir))); + } + } + return false; + }; + if (!this->PopulateAndValidateSnapshot(*snapshot_chainstate, coins_file, + metadata)) { + LOCK(::cs_main); + return cleanup_bad_snapshot("population failed"); + } + + // cs_main required for rest of snapshot activation. + LOCK(::cs_main); + + // Do a final check to ensure that the snapshot chainstate is actually a + // more work chain than the active chainstate; a user could have loaded a + // snapshot very late in the IBD process, and we wouldn't want to load a + // useless chainstate. + if (!CBlockIndexWorkComparator()(ActiveTip(), + snapshot_chainstate->m_chain.Tip())) { + return cleanup_bad_snapshot("work does not exceed active chainstate"); + } // If not in-memory, persist the base blockhash for use during subsequent // initialization. if (!in_memory) { - LOCK(::cs_main); if (!node::WriteSnapshotBaseBlockhash(*snapshot_chainstate)) { - snapshot_ok = false; - } - } - if (!snapshot_ok) { - LOCK(::cs_main); - this->MaybeRebalanceCaches(); - - // PopulateAndValidateSnapshot can return (in error) before the leveldb - // datadir has been created, so only attempt removal if we got that far. - if (auto snapshot_datadir = node::FindSnapshotChainstateDir()) { - // We have to destruct leveldb::DB in order to release the db lock, - // otherwise DestroyDB() (in DeleteCoinsDBFromDisk()) will fail. See - // `leveldb::~DBImpl()`. Destructing the chainstate (and so - // resetting the coinsviews object) does this. - snapshot_chainstate.reset(); - bool removed = - DeleteCoinsDBFromDisk(*snapshot_datadir, /*is_snapshot=*/true); - if (!removed) { - AbortNode( - strprintf("Failed to remove snapshot chainstate dir (%s). " - "Manually remove it before restarting.\n", - fs::PathToString(*snapshot_datadir))); - } + return cleanup_bad_snapshot("could not write base blockhash"); } - return false; } - { - LOCK(::cs_main); - assert(!m_snapshot_chainstate); - m_snapshot_chainstate.swap(snapshot_chainstate); - const bool chaintip_loaded = m_snapshot_chainstate->LoadChainTip(); - assert(chaintip_loaded); - - // Transfer possession of the mempool to the snapshot chainstate. - // Mempool is empty at this point because we're still in IBD. - Assert(m_active_chainstate->m_mempool->size() == 0); - Assert(!m_snapshot_chainstate->m_mempool); - m_snapshot_chainstate->m_mempool = m_active_chainstate->m_mempool; - m_active_chainstate->m_mempool = nullptr; - m_active_chainstate = m_snapshot_chainstate.get(); - m_blockman.m_snapshot_height = this->GetSnapshotBaseHeight(); - - LogPrintf("[snapshot] successfully activated snapshot %s\n", - base_blockhash.ToString()); - LogPrintf("[snapshot] (%.2f MB)\n", - m_snapshot_chainstate->CoinsTip().DynamicMemoryUsage() / - (1000 * 1000)); + assert(!m_snapshot_chainstate); + m_snapshot_chainstate.swap(snapshot_chainstate); + const bool chaintip_loaded = m_snapshot_chainstate->LoadChainTip(); + assert(chaintip_loaded); - this->MaybeRebalanceCaches(); - } + // Transfer possession of the mempool to the snapshot chainstate. + // Mempool is empty at this point because we're still in IBD. + Assert(m_active_chainstate->m_mempool->size() == 0); + Assert(!m_snapshot_chainstate->m_mempool); + m_snapshot_chainstate->m_mempool = m_active_chainstate->m_mempool; + m_active_chainstate->m_mempool = nullptr; + m_active_chainstate = m_snapshot_chainstate.get(); + m_blockman.m_snapshot_height = this->GetSnapshotBaseHeight(); + + LogPrintf("[snapshot] successfully activated snapshot %s\n", + base_blockhash.ToString()); + LogPrintf("[snapshot] (%.2f MB)\n", + m_snapshot_chainstate->CoinsTip().DynamicMemoryUsage() / + (1000 * 1000)); + + this->MaybeRebalanceCaches(); return true; } @@ -6643,6 +6656,16 @@ const AssumeutxoData &au_data = *maybe_au_data; + // This work comparison is a duplicate check with the one performed later in + // ActivateSnapshot(), but is done so that we avoid doing the long work of + // staging a snapshot that isn't actually usable. + if (WITH_LOCK(::cs_main, return !CBlockIndexWorkComparator()( + ActiveTip(), snapshot_start_block))) { + LogPrintf("[snapshot] activation failed - work does not exceed active " + "chainstate\n"); + return false; + } + COutPoint outpoint; Coin coin; const uint64_t coins_count = metadata.m_coins_count;