Make LoadChainstate return an explicit error when snapshot validation succeeds,
but there is an error trying to replace the background chainstate with the
snapshot chainstate. Previously in this case LoadChainstate would trigger a
shutdown and return INTERRUPTED, now it will return an actual error code.
There's no real change to behavior other than error message being formatted a
little differently.
Motivation for this change is to replace error handling via callbacks with
error handling via return value ahead of
https://github.com/bitcoin/bitcoin/pull/27861
The rpc command verifychain now fails if the dbcache was not sufficient
to complete the verification at the specified level and depth.
In the same situation, the VerifyDB check during Init will now fail (and lead to
an early shutdown) if the user has explicitly specified -checkblocks or
-checklevel but the check couldn't be executed because of the limited
cache. If the user didn't change any of the two and is using the defaults, log a warning
but don't prevent the node from starting up.
...instead pass in a std::function<int64_t()>
Note that the static_cast is needed (apparently) for the compiler to
know which overloaded GetTime to choose.
...instead allow the caller to optionally pass in callbacks which are
triggered for certain events.
Behaviour change: The string "Verifying blocks..." was previously
printed for each chainstate in chainman which did not have an
effectively empty coinsview, now it will be printed once unconditionally
before we call VerifyLoadedChain.
This allows us to separate the initialization code from translations and
error reporting.
This change changes the caller semantics of LoadChainstate quite
drastically.
To see that this change doesn't change behaviour, observe that:
1. Prior to this change, LoadChainstate returned false only in the "bad
genesis block" failure case (by returning InitError()), indicating
that the caller should immediately bail. After this change, the
corresponding ERROR_BAD_GENESIS_BLOCK handler in src/init.cpp
maintains behavioue by also bailing immediately.
2. The failed_* temporary booleans were only used to break out of the
outer do/while(false) loop. They can therefore be safely removed.
I strongly recommend reviewing with the following git-diff flags:
--color-moved=dimmed_zebra --color-moved-ws=allow-indentation-change
[META] This commit is intended to be as close to a move-only commit as
possible, and lingering ugliness will be resolved in subsequent
commits.
A few variables that are passed in by value instead of by reference
deserve explanation:
- fReset and fReindexChainstate are both local variables in AppInitMain
and are not modified in the sequence
- fPruneMode, despite being a global, is only modified in
AppInitParameterInteraction, long before LoadChainstate is called
----
[META] This semantic will change in a future commit named
"node/chainstate: Decouple from stringy errors"
