HomePhabricator

Fix some chainstate-init-order bugs.

Description

Fix some chainstate-init-order bugs.

Summary:

  1. Fix some LoadChainTip-related init-order bugs.
  • Move the writing of fTxIndex to LoadBlockIndex - this fixes a bug introduced in d6af06d68aae985436cbc942f0d11078041d121b where InitBlockIndex was writing to fTxIndex which had not yet been checked (because LoadChainTip hadn't yet initialized the chainActive, which would otherwise have resulted in InitBlockIndex being a NOP), allowing you to modify -txindex without reindex, potentially corrupting your chainstate!
  • Rename InitBlockIndex to LoadGenesisBlock, which is now a more natural name for it. Also check mapBlockIndex instead of chainActive, fixing a bug where we'd write the genesis block out on every start.
  1. More user-friendly error message if UTXO DB runs ahead of block DB

This gives LoadChainTip a return value - allowing it to indicate that
the UTXO DB ran ahead of the block DB. This just provides a nicer
error message instead of the previous mysterious
assert(!setBlockIndexCandidates.empty()) error.

This also calls ActivateBestChain in case we just loaded the genesis
block in LoadChainTip, avoiding relying on the ActivateBestChain
in ThreadImport before continuing init process.

  1. Call RewindBlockIndex even if we're about to run -reindex-chainstate

RewindBlockIndex works over both chainActive - disconnecting blocks
from the tip that need witness verification - and mapBlockIndex -
requiring redownload of blocks missing witness data.

It should never have been the case that the second half is skipped
if we're about to run -reindex-chainstate.

  1. Order chainstate init more logically.
  • Order chainstate init more logically - first all of the blocktree-related loading, then coinsdb, then pcoinsTip/chainActive. Only create objects as needed.
  • More clearly document exactly what is and isn't called in -reindex and -reindex-chainstate both with comments noting calls as no-ops and by adding if guards.
  • Move LoadGenesisBlock further down in init. This is a more logical location for it, as it is after all of the blockindex-related loading and checking, but before any of the UTXO-related loading and checking.
  • Move all of the VerifyDB()-related stuff into a -reindex + -reindex-chainstate if guard. It couldn't do anything useful as chainActive.Tip() would be null at this point anyway.
  1. Fix segfault when shutting down before fully loading

This was introduced by 3192975f1d177aa9f0bbd823c6387cfbfa943610.
It can be triggered easily when canceling DB upgrade from
pre-per-utxo.

This is a backport of Core PR10758

Depends on D1906 and D1905

Test Plan:

make check

Reviewers: #bitcoin_abc, jasonbcox

Reviewed By: #bitcoin_abc, jasonbcox

Subscribers: jasonbcox, teamcity

Differential Revision: https://reviews.bitcoinabc.org/D1909

Details

Provenance
Matt Corallo <git@bluematt.me>Authored on Jul 6 2017, 23:57
deadalnixCommitted on Oct 14 2018, 18:59
schancelPushed on Oct 14 2018, 22:17
Reviewer
Restricted Project
Differential Revision
D1909: Fix some chainstate-init-order bugs.
Parents
rSTAGINGd90f706195c5: Add wallet backup text to import* and add* RPCs
Branches
Unknown
Tags
Unknown