From 4433ed0f730cfd60eeba3694ff3c283ce2c0c8ee Mon Sep 17 00:00:00 2001
From: Suhas Daftuar <sdaftuar@gmail.com>
Date: Thu, 31 Jan 2019 15:47:25 -0500
Subject: [PATCH 1/2] [validation] Crash if disconnecting a block fails

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.
---
 src/validation.cpp | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/validation.cpp b/src/validation.cpp
index de9c0d96d..83a17de09 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2295,7 +2295,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();
     {
@@ -2551,6 +2551,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;

From a47df13471e3168e2e02023fb20cdf2414141b36 Mon Sep 17 00:00:00 2001
From: Suhas Daftuar <sdaftuar@gmail.com>
Date: Thu, 31 Jan 2019 16:06:07 -0500
Subject: [PATCH 2/2] [qa] Test disconnect block failure -> shutdown

---
 test/functional/feature_abortnode.py | 48 ++++++++++++++++++++++++++++
 test/functional/test_runner.py       |  1 +
 2 files changed, 49 insertions(+)
 create mode 100755 test/functional/feature_abortnode.py

diff --git a/test/functional/feature_abortnode.py b/test/functional/feature_abortnode.py
new file mode 100755
index 000000000..62c3eca07
--- /dev/null
+++ b/test/functional/feature_abortnode.py
@@ -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()
diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
index 5c92370b8..2a6691a66 100755
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -102,6 +102,7 @@ BASE_SCRIPTS = [
     'feature_bip68_sequence.py',
     'p2p_feefilter.py',
     'feature_reindex.py',
+    'feature_abortnode.py',
     # vv Tests less than 30s vv
     'wallet_keypool_topup.py',
     'interface_zmq.py',