Merge bitcoin/bitcoin#29726: assumeutxo: Fix -reindex before snapshot was validated

b7ba60f81a test: add coverage for -reindex and assumeutxo (Martin Zumsande)
e57f951805 init, validation: Fix -reindex option with an existing snapshot (Martin Zumsande)

Pull request description:

  In c711ca186f logic was introduced that `-reindex` and `-reindex-chainstate` will delete the snapshot chainstate.
  This doesn't work currently, instead of deleting the snapshot chainstate the node crashes with an assert (this can be triggered by applying the added test commit on master).
  Fix this, and another bug that would prevent the new active chainstate from having a mempool after `-reindex` has deleted the snapshot (also covered by the test).

ACKs for top commit:
  fjahr:
    re-ACK b7ba60f81a
  hernanmarino:
    crACK b7ba60f81a . Good fix
  BrandonOdiwuor:
    re-ACK b7ba60f81a
  byaye:
    Tested ACK b7ba60f81a

Tree-SHA512: c168f36997d7677d590af37b10427870f5d30123abf1c76032a16661e486735373bfa7e049e6aca439526fbcb6d619f970bf9d042196c851bf058a75a32fafdc
This commit is contained in:
Ryan Ofsky 2024-04-16 12:20:16 -04:00
commit 312f54278f
No known key found for this signature in database
GPG Key ID: 46800E30FC748A66
2 changed files with 13 additions and 4 deletions

View File

@ -6142,13 +6142,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;
}

View File

@ -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
@ -379,6 +376,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)