From 6ec27a667342acf80b368d8cfe0eb71429c1f691 Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Mon, 25 Mar 2024 14:11:03 -0400 Subject: [PATCH 1/2] init, validation: Fix -reindex option with an existing snapshot This didn't work for two reasons: 1.) GetSnapshotCoinsDBPath() was used to retrieve the path. This requires coins_views to exist, but the initialisation only happens later (in CompleteChainstateInitialization) so the node hits an assert in CCoinsViewDB& CoinsDB() and crashes. 2.) The snapshot was already activated, so it has the mempool attached. Therefore, the mempool needs to be transferred back to the ibd chainstate before deleting the snapshot chainstate. Github-Pull: #29726 Rebased-From: e57f951805b429534c75ec1e6b2a1f16ae24efb5 --- src/validation.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/validation.cpp b/src/validation.cpp index f8e1de55e9..8022f7f3ee 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -5994,13 +5994,14 @@ bool ChainstateManager::DeleteSnapshotChainstate() Assert(m_snapshot_chainstate); Assert(m_ibd_chainstate); - fs::path snapshot_datadir = GetSnapshotCoinsDBPath(*m_snapshot_chainstate); + fs::path snapshot_datadir = Assert(node::FindSnapshotChainstateDir(m_options.datadir)).value(); if (!DeleteCoinsDBFromDisk(snapshot_datadir, /*is_snapshot=*/ true)) { LogPrintf("Deletion of %s failed. Please remove it manually to continue reindexing.\n", fs::PathToString(snapshot_datadir)); return false; } m_active_chainstate = m_ibd_chainstate.get(); + m_active_chainstate->m_mempool = m_snapshot_chainstate->m_mempool; m_snapshot_chainstate.reset(); return true; } From acf242b0629faaed3fa4151800baabaf8d013f2a Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Mon, 25 Mar 2024 12:51:55 -0400 Subject: [PATCH 2/2] test: add coverage for -reindex and assumeutxo Co-authored-by: Fabian Jahr Github-Pull: #29726 Rebased-From: b7ba60f81a33db876f88b5f9af1e5025d679b5be --- test/functional/feature_assumeutxo.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/test/functional/feature_assumeutxo.py b/test/functional/feature_assumeutxo.py index 60dd751ff8..345f6f1227 100755 --- a/test/functional/feature_assumeutxo.py +++ b/test/functional/feature_assumeutxo.py @@ -11,9 +11,6 @@ The assumeutxo value generated and used here is committed to in ## Possible test improvements -- TODO: test what happens with -reindex and -reindex-chainstate before the - snapshot is validated, and make sure it's deleted successfully. - Interesting test cases could be loading an assumeutxo snapshot file with: - TODO: Valid hash but invalid snapshot file (bad coin height or @@ -309,6 +306,17 @@ class AssumeutxoTest(BitcoinTestFramework): assert_equal(loaded['coins_loaded'], SNAPSHOT_BASE_HEIGHT) assert_equal(loaded['base_height'], SNAPSHOT_BASE_HEIGHT) + for reindex_arg in ['-reindex=1', '-reindex-chainstate=1']: + self.log.info(f"Check that restarting with {reindex_arg} will delete the snapshot chainstate") + self.restart_node(2, extra_args=[reindex_arg, *self.extra_args[2]]) + assert_equal(1, len(n2.getchainstates()["chainstates"])) + for i in range(1, 300): + block = n0.getblock(n0.getblockhash(i), 0) + n2.submitheader(block) + loaded = n2.loadtxoutset(dump_output['path']) + assert_equal(loaded['coins_loaded'], SNAPSHOT_BASE_HEIGHT) + assert_equal(loaded['base_height'], SNAPSHOT_BASE_HEIGHT) + normal, snapshot = n2.getchainstates()['chainstates'] assert_equal(normal['blocks'], START_HEIGHT) assert_equal(normal.get('snapshot_blockhash'), None)