Merge #15305: [validation] Crash if disconnecting a block fails

a47df13471 [qa] Test disconnect block failure -> shutdown (Suhas Daftuar)
4433ed0f73 [validation] Crash if disconnecting a block fails (Suhas Daftuar)

Pull request description:

  If we're unable to disconnect a block during normal operation, then that is a
  failure of our local system (such as disk failure) or the chain that we are on
  (eg CVE-2018-17144), but cannot be due to failure of the (more work) chain that
  we're trying to validate.

  We should abort rather than stay on a less work chain.

  Fixes #14341.

ACKs for top commit:
  practicalswift:
    utACK a47df13471
  TheBlueMatt:
    utACK a47df13471. Didn't bother to review the test in detail, it looked fine. Debated whether invalidateblock should ever crash the node, but *not* crashing in the case of hitting a pruned block (which is the only change here) is clearly better, even if there are other cases I'd argue we should crash in.
  ryanofsky:
    utACK a47df13471. Only change since last review is new comment.
  promag:
    ACK a47df1347, it takes awhile to quit (RPC connection timeouts) but that's unrelated - hope to fix that soon.
  fanquake:
    ACK a47df13471

Tree-SHA512: 4dec8cef6e7dbbe513c138fc5821a7ceab855e603ece3c16185b51a3830ab7ebbc844a28827bf64e75326f45325991dcb672f13bd7baede53304f27289c4af8d
This commit is contained in:
fanquake
2019-07-25 08:30:18 +08:00
3 changed files with 55 additions and 1 deletions

View File

@@ -2185,7 +2185,7 @@ bool CChainState::DisconnectTip(CValidationState& state, const CChainParams& cha
std::shared_ptr<CBlock> pblock = std::make_shared<CBlock>();
CBlock& block = *pblock;
if (!ReadBlockFromDisk(block, pindexDelete, chainparams.GetConsensus()))
return AbortNode(state, "Failed to read block");
return error("DisconnectTip(): Failed to read block");
// Apply the block atomically to the chain state.
int64_t nStart = GetTimeMicros();
{
@@ -2442,6 +2442,11 @@ bool CChainState::ActivateBestChainStep(CValidationState& state, const CChainPar
// This is likely a fatal error, but keep the mempool consistent,
// just in case. Only remove from the mempool in this case.
UpdateMempoolForReorg(disconnectpool, false);
// If we're unable to disconnect a block during normal operation,
// then that is a failure of our local system -- we should abort
// rather than stay on a less work chain.
AbortNode(state, "Failed to disconnect block; see debug.log for details");
return false;
}
fBlocksDisconnected = true;