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

a47df13471e3168e2e02023fb20cdf2414141b36 [qa] Test disconnect block failure -> shutdown (Suhas Daftuar)
4433ed0f730cfd60eeba3694ff3c283ce2c0c8ee [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 a47df13471e3168e2e02023fb20cdf2414141b36
  TheBlueMatt:
    utACK a47df13471e3168e2e02023fb20cdf2414141b36. 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 a47df13471e3168e2e02023fb20cdf2414141b36. 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 a47df13471e3168e2e02023fb20cdf2414141b36

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

View File

@ -2694,7 +2694,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();
{
@ -2968,6 +2968,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;

View File

@ -0,0 +1,48 @@
#!/usr/bin/env python3
# Copyright (c) 2019 The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
"""Test bitcoind aborts if can't disconnect a block.
- Start a single node and generate 3 blocks.
- Delete the undo data.
- Mine a fork that requires disconnecting the tip.
- Verify that bitcoind AbortNode's.
"""
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import wait_until, get_datadir_path, connect_nodes
import os
class AbortNodeTest(BitcoinTestFramework):
def set_test_params(self):
self.setup_clean_chain = True
self.num_nodes = 2
def setup_network(self):
self.setup_nodes()
# We'll connect the nodes later
def run_test(self):
self.nodes[0].generate(3)
datadir = get_datadir_path(self.options.tmpdir, 0)
# Deleting the undo file will result in reorg failure
os.unlink(os.path.join(datadir, 'regtest', 'blocks', 'rev00000.dat'))
# Connecting to a node with a more work chain will trigger a reorg
# attempt.
self.nodes[1].generate(3)
with self.nodes[0].assert_debug_log(["Failed to disconnect block"]):
connect_nodes(self.nodes[0], 1)
self.nodes[1].generate(1)
# Check that node0 aborted
self.log.info("Waiting for crash")
wait_until(lambda: self.nodes[0].is_node_stopped(), timeout=60)
self.log.info("Node crashed - now verifying restart fails")
self.nodes[0].assert_start_raises_init_error()
if __name__ == '__main__':
AbortNodeTest().main()

View File

@ -101,6 +101,7 @@ BASE_SCRIPTS = [
'feature_csv_activation.py',
'rpc_rawtransaction.py',
'feature_reindex.py',
'feature_abortnode.py',
# vv Tests less than 30s vv
'wallet_keypool_topup.py',
'interface_zmq_dash.py',