-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).
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.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For detailed information about the code coverage, see the test coverage report.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | fjahr, BrandonOdiwuor, hernanmarino, byaye |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
274@@ -278,6 +275,18 @@ def check_tx_counts(final: bool) -> None:
275
276 assert_equal(n1.getblockchaininfo()["blocks"], SNAPSHOT_BASE_HEIGHT)
277
278+ self.log.info(f"Check that restarting with -reindex will delete the snapshot chainstate")
279+ self.restart_node(1, extra_args=['-reindex=1', *self.extra_args[1]])
-reindex but not -reindex-chainstate. It’s easily possible when using node 2 for the test instead which is not pruned: https://github.com/fjahr/bitcoin/commit/38c5d80f73c7058f17ba8e89a3802ef981080864
tACK cfdef5c5d7a2d2e0bc78aa2cde534e541af61d1c
Good catch! I could reproduce the crash and that the change here fixes this.
My suggested test extension can be pulled in here if you retouch or I can add it to one of my outstanding assumeutxo PRs as a small follow-up.
Co-authored-by: Fabian Jahr <fjahr@protonmail.com>
Tested ACK b7ba60f81a33db876f88b5f9af1e5025d679b5be
Test result before the fix
Test result after the fix