From 57e6ecda5a33115e3481ee542e642b19d3dd4667 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 21 Jun 2017 23:53:01 +1200 Subject: [PATCH 1/2] Implement roll-back limit for reorganisation Closes #713. --- qa/pull-tester/rpc-tests.sh | 1 + qa/rpc-tests/reorg_limit.py | 86 +++++++++++++++++++++++++++++ qa/rpc-tests/test_framework/util.py | 4 ++ src/main.cpp | 25 +++++++++ src/main.h | 2 + src/wallet/wallet.h | 4 +- 6 files changed, 120 insertions(+), 2 deletions(-) create mode 100755 qa/rpc-tests/reorg_limit.py diff --git a/qa/pull-tester/rpc-tests.sh b/qa/pull-tester/rpc-tests.sh index dfbd78f9a..a4f3f91d8 100755 --- a/qa/pull-tester/rpc-tests.sh +++ b/qa/pull-tester/rpc-tests.sh @@ -44,6 +44,7 @@ testScripts=( 'zcjoinsplit.py' 'zcjoinsplitdoublespend.py' 'zkey_import_export.py' + 'reorg_limit.py' 'getblocktemplate.py' 'bip65-cltv-p2p.py' 'bipdersig-p2p.py' diff --git a/qa/rpc-tests/reorg_limit.py b/qa/rpc-tests/reorg_limit.py new file mode 100755 index 000000000..3857498ee --- /dev/null +++ b/qa/rpc-tests/reorg_limit.py @@ -0,0 +1,86 @@ +#!/usr/bin/env python2 +# Copyright (c) 2017 The Zcash developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +# +# Test reorg limit +# + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + check_node, + connect_nodes_bi, + initialize_chain_clean, + start_node, + sync_blocks, +) +from time import sleep + +def check_stopped(i, timeout=10): + stopped = False + for x in xrange(1, timeout): + ret = check_node(i) + if ret is None: + sleep(1) + else: + stopped = True + break + return stopped + +class ReorgLimitTest(BitcoinTestFramework): + + def run_test(self): + assert(self.nodes[0].getblockcount() == 200) + assert(self.nodes[2].getblockcount() == 200) + + self.split_network() + + print "Test the maximum-allowed reorg:" + print "Mine 99 blocks on Node 0" + self.nodes[0].generate(99) + assert(self.nodes[0].getblockcount() == 299) + assert(self.nodes[2].getblockcount() == 200) + + print "Mine competing 100 blocks on Node 2" + self.nodes[2].generate(100) + assert(self.nodes[0].getblockcount() == 299) + assert(self.nodes[2].getblockcount() == 300) + + print "Connect nodes to force a reorg" + connect_nodes_bi(self.nodes, 0, 2) + self.is_network_split = False + sync_blocks(self.nodes) + + print "Check Node 0 is still running and on the correct chain" + assert(self.nodes[0].getblockcount() == 300) + + self.split_network() + + print "Test the minimum-rejected reorg:" + print "Mine 100 blocks on Node 0" + self.nodes[0].generate(100) + assert(self.nodes[0].getblockcount() == 400) + assert(self.nodes[2].getblockcount() == 300) + + print "Mine competing 101 blocks on Node 2" + self.nodes[2].generate(101) + assert(self.nodes[0].getblockcount() == 400) + assert(self.nodes[2].getblockcount() == 401) + + print "Sync nodes to force a reorg" + connect_nodes_bi(self.nodes, 0, 2) + self.is_network_split = False + # sync_blocks uses RPC calls to wait for nodes to be synced, so don't + # call it here, because it will have a non-specific connection error + # when Node 0 stops. Instead, we explicitly check for the process itself + # to stop. + + print "Check Node 0 is no longer running" + assert(check_stopped(0)) + + # Dummy stop to enable the test to tear down + self.nodes[0].stop = lambda: True + +if __name__ == '__main__': + ReorgLimitTest().main() diff --git a/qa/rpc-tests/test_framework/util.py b/qa/rpc-tests/test_framework/util.py index 20267521f..f3ea481ee 100644 --- a/qa/rpc-tests/test_framework/util.py +++ b/qa/rpc-tests/test_framework/util.py @@ -213,6 +213,10 @@ def start_nodes(num_nodes, dirname, extra_args=None, rpchost=None, binary=None): def log_filename(dirname, n_node, logname): return os.path.join(dirname, "node"+str(n_node), "regtest", logname) +def check_node(i): + bitcoind_processes[i].poll() + return bitcoind_processes[i].returncode + def stop_node(node, i): node.stop() bitcoind_processes[i].wait() diff --git a/src/main.cpp b/src/main.cpp index 062507d0e..755b58b8a 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -2631,6 +2631,31 @@ static bool ActivateBestChainStep(CValidationState &state, CBlockIndex *pindexMo const CBlockIndex *pindexOldTip = chainActive.Tip(); const CBlockIndex *pindexFork = chainActive.FindFork(pindexMostWork); + // - On ChainDB initialization, pindexOldTip will be null, so there are no removable blocks. + // - If pindexMostWork is in a chain that doesn't have the same genesis block as our chain, + // then pindexFork will be null, and we would need to remove the entire chain including + // our genesis block. In practice this (probably) won't happen because of checks elsewhere. + auto reorgLength = pindexOldTip ? pindexOldTip->nHeight - (pindexFork ? pindexFork->nHeight : -1) : 0; + static_assert(MAX_REORG_LENGTH > 0, "We must be able to reorg some distance"); + if (reorgLength > MAX_REORG_LENGTH) { + auto msg = strprintf(_( + "A block chain reorganization has been detected that would roll back %d blocks! " + "This is larger than the maximum of %d blocks, and so the node is shutting down for your safety." + ), reorgLength, MAX_REORG_LENGTH) + "\n\n" + + _("Reorganization details") + ":\n" + + "- " + strprintf(_("Current tip: %s, height %d, work %s"), + pindexOldTip->phashBlock->GetHex(), pindexOldTip->nHeight, pindexOldTip->nChainWork.GetHex()) + "\n" + + "- " + strprintf(_("New tip: %s, height %d, work %s"), + pindexMostWork->phashBlock->GetHex(), pindexMostWork->nHeight, pindexMostWork->nChainWork.GetHex()) + "\n" + + "- " + strprintf(_("Fork point: %s, height %d"), + pindexFork->phashBlock->GetHex(), pindexFork->nHeight) + "\n\n" + + _("Please help, human!"); + LogPrintf("*** %s\n", msg); + uiInterface.ThreadSafeMessageBox(msg, "", CClientUIInterface::MSG_ERROR); + StartShutdown(); + return false; + } + // Disconnect active blocks which are no longer in the best chain. while (chainActive.Tip() && chainActive.Tip() != pindexFork) { if (!DisconnectTip(state)) diff --git a/src/main.h b/src/main.h index 03b0464eb..534034b91 100644 --- a/src/main.h +++ b/src/main.h @@ -56,6 +56,8 @@ static const unsigned int DEFAULT_BLOCK_PRIORITY_SIZE = DEFAULT_BLOCK_MAX_SIZE / static const bool DEFAULT_ALERTS = true; /** Minimum alert priority for enabling safe mode. */ static const int ALERT_PRIORITY_SAFE_MODE = 4000; +/** Maximum reorg length we will accept before we shut down and alert the user. */ +static const unsigned int MAX_REORG_LENGTH = COINBASE_MATURITY - 1; /** Maximum number of signature check operations in an IsStandard() P2SH script */ static const unsigned int MAX_P2SH_SIGOPS = 15; /** The maximum number of sigops we're willing to relay/mine in a single tx */ diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index eaaf6bbdf..dfe7b6a64 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -8,9 +8,9 @@ #include "amount.h" #include "coins.h" -#include "consensus/consensus.h" #include "key.h" #include "keystore.h" +#include "main.h" #include "primitives/block.h" #include "primitives/transaction.h" #include "tinyformat.h" @@ -58,7 +58,7 @@ static const unsigned int MAX_FREE_TRANSACTION_CREATE_SIZE = 1000; //! Size of witness cache // Should be large enough that we can expect not to reorg beyond our cache // unless there is some exceptional network disruption. -static const unsigned int WITNESS_CACHE_SIZE = COINBASE_MATURITY; +static const unsigned int WITNESS_CACHE_SIZE = MAX_REORG_LENGTH + 1; class CBlockIndex; class CCoinControl; From cb580c72413c42ee26398f98fd553a8b4cd7f64e Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sun, 4 Feb 2018 00:35:41 +0000 Subject: [PATCH 2/2] Add rollback limit to block index rewinding This will prevent users from starting their nodes if they switch between software versions that implement different network upgrades. It will also prevent users from using the testnet if they have more than MAX_REORG_LIMIT post-upgrade blocks, and the upgrade point is shifted in a newer software version. --- src/main.cpp | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/main.cpp b/src/main.cpp index 755b58b8a..43aac0d18 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3826,6 +3826,26 @@ bool RewindBlockIndex(const CChainParams& params) } // nHeight is now the height of the first insufficiently-validated block, or tipheight + 1 + auto rewindLength = chainActive.Height() - nHeight; + if (rewindLength > 0 && rewindLength > MAX_REORG_LENGTH) { + auto pindexOldTip = chainActive.Tip(); + auto pindexRewind = chainActive[nHeight - 1]; + auto msg = strprintf(_( + "A block chain rewind has been detected that would roll back %d blocks! " + "This is larger than the maximum of %d blocks, and so the node is shutting down for your safety." + ), rewindLength, MAX_REORG_LENGTH) + "\n\n" + + _("Rewind details") + ":\n" + + "- " + strprintf(_("Current tip: %s, height %d"), + pindexOldTip->phashBlock->GetHex(), pindexOldTip->nHeight) + "\n" + + "- " + strprintf(_("Rewinding to: %s, height %d"), + pindexRewind->phashBlock->GetHex(), pindexRewind->nHeight) + "\n\n" + + _("Please help, human!"); + LogPrintf("*** %s\n", msg); + uiInterface.ThreadSafeMessageBox(msg, "", CClientUIInterface::MSG_ERROR); + StartShutdown(); + return false; + } + CValidationState state; CBlockIndex* pindex = chainActive.Tip(); while (chainActive.Height() >= nHeight) {