From 5ce4b55d53cb5f22ebed6a422a7c79f6326d5a03 Mon Sep 17 00:00:00 2001 From: Adam Gibson Date: Mon, 25 May 2020 12:16:08 +0100 Subject: [PATCH] Quit gracefully if Bitcoin RPC connection lost Fixes #442. First, the CONNREFUSED socket error is handled in jsonrpc. Second, we respond to this (but *not* to resets) with a reactor shutdown in BitcoinCoreInterface.rpc(). This also necessitates early-quitting in the calling function (WalletService.transaction_monitor) since the reactor stop will only stop future deferred calls, not the currently running one. The obvious sys.exit approach is only used in startup, because the reactor is not currently running at that point. Also minor change to DummyBlockchainInterface for test. --- jmclient/jmclient/blockchaininterface.py | 30 ++++++++++++++++++++-- jmclient/jmclient/jsonrpc.py | 11 ++++++-- jmclient/jmclient/wallet_service.py | 32 +++++++++++++++++++++--- jmclient/test/commontest.py | 5 ++++ 4 files changed, 71 insertions(+), 7 deletions(-) diff --git a/jmclient/jmclient/blockchaininterface.py b/jmclient/jmclient/blockchaininterface.py index 71e43ae..d9e272a 100644 --- a/jmclient/jmclient/blockchaininterface.py +++ b/jmclient/jmclient/blockchaininterface.py @@ -164,7 +164,12 @@ class BitcoinCoreInterface(BlockchainInterface): def __init__(self, jsonRpc, network): super(BitcoinCoreInterface, self).__init__() self.jsonRpc = jsonRpc - blockchainInfo = self.jsonRpc.call("getblockchaininfo", []) + blockchainInfo = self.rpc("getblockchaininfo", []) + if not blockchainInfo: + # see note in BitcoinCoreInterface.rpc - here + # we have to create this object before reactor start, + # so sys.exit *is* the right call: + sys.exit(EXIT_FAILURE) actualNet = blockchainInfo['chain'] netmap = {'main': 'mainnet', 'test': 'testnet', 'regtest': 'regtest'} @@ -183,12 +188,33 @@ class BitcoinCoreInterface(BlockchainInterface): return block def rpc(self, method, args): + """ Returns the result of an rpc call to the Bitcoin Core RPC API. + If the connection is permanently or unrecognizably broken, None + is returned *and the reactor is shutdown* (because we consider this + condition unsafe - TODO possibly create a "freeze" mode that could + restart when the connection is healed, but that is tricky). + """ if method not in ['importaddress', 'walletpassphrase', 'getaccount', 'gettransaction', 'getrawtransaction', 'gettxout', 'importmulti', 'listtransactions', 'getblockcount', 'scantxoutset']: log.debug('rpc: ' + method + " " + str(args)) - res = self.jsonRpc.call(method, args) + try: + res = self.jsonRpc.call(method, args) + except JsonRpcConnectionError as e: + # note that we only raise this in case the connection error is + # a refusal, or is unrecognized/unknown by our code. So this does + # NOT happen in a reset or broken pipe scenario. + # It is safest to simply shut down. + # Why not sys.exit? sys.exit calls do *not* work inside delayedCalls + # or deferreds in twisted, since a bare exception catch prevents + # an actual system exit (i.e. SystemExit is caught as a + # BareException type). + log.error("Failure of RPC connection to Bitcoin Core. " + "Application cannot continue, shutting down.") + if reactor.running: + reactor.stop() + return None return res def is_address_labeled(self, utxo, walletname): diff --git a/jmclient/jmclient/jsonrpc.py b/jmclient/jmclient/jsonrpc.py index ad8b1d2..436f1dc 100644 --- a/jmclient/jmclient/jsonrpc.py +++ b/jmclient/jmclient/jsonrpc.py @@ -110,15 +110,22 @@ class JsonRpc(object): self.conn.connect() continue elif e.errno == errno.EPIPE: - jlog.warn('Connection had broken pipe, attempting reconnect.') + jlog.warn('Connection had broken pipe, attempting ' + 'reconnect.') self.conn.close() self.conn.connect() continue elif e.errno == errno.EPROTOTYPE: - jlog.warn('Connection had protocol wrong type for socket error, attempting reconnect.') + jlog.warn('Connection had protocol wrong type for socket ' + 'error, attempting reconnect.') self.conn.close() self.conn.connect() continue + elif e.errno == errno.ECONNREFUSED: + # Will not reattempt in this case: + jlog.error("Connection refused.") + self.conn.close() + raise JsonRpcConnectionError("JSON-RPC connection refused.") else: jlog.error('Unhandled connection error ' + str(e)) raise e diff --git a/jmclient/jmclient/wallet_service.py b/jmclient/jmclient/wallet_service.py index c5d9206..ae19f9b 100644 --- a/jmclient/jmclient/wallet_service.py +++ b/jmclient/jmclient/wallet_service.py @@ -40,8 +40,10 @@ class WalletService(Service): # could be more flexible in future, and # the JM wallet object. self.bci = jm_single().bc_interface + # keep track of the quasi-real-time blockheight # (updated in main monitor loop) + self.current_blockheight = None if self.bci is not None: self.update_blockheight() else: @@ -73,14 +75,34 @@ class WalletService(Service): but will be called as part of main monitoring loop to ensure new transactions are added at the right height. + Any failure of the RPC call must result in this returning + False, otherwise return True (means self.current_blockheight + has been correctly updated). """ + if self.current_blockheight: + old_blockheight = self.current_blockheight + else: + old_blockheight = -1 try: self.current_blockheight = self.bci.get_current_block_height() - assert isinstance(self.current_blockheight, Integral) except Exception as e: + # This should never happen now, as we are catching every + # possible Exception in jsonrpc or bci.rpc: jlog.error("Failure to get blockheight from Bitcoin Core:") jlog.error(repr(e)) - return + if reactor.running: + reactor.stop() + return False + if not self.current_blockheight: + return False + + # We have received a new blockheight from Core, sanity check it: + assert isinstance(self.current_blockheight, Integral) + assert self.current_blockheight >= 0 + if self.current_blockheight < old_blockheight: + jlog.warn("Bitcoin Core is reporting a lower blockheight, " + "possibly a reorg.") + return True def startService(self): """ Encapsulates start up actions. @@ -240,9 +262,13 @@ class WalletService(Service): Service is constantly in near-realtime sync with the blockchain. """ - self.update_blockheight() + if not self.update_blockheight(): + return txlist = self.bci.list_transactions(100) + if not txlist: + return + new_txs = [] for x in txlist: # process either (a) a completely new tx or diff --git a/jmclient/test/commontest.py b/jmclient/test/commontest.py index 55395e0..01ce37a 100644 --- a/jmclient/test/commontest.py +++ b/jmclient/test/commontest.py @@ -29,6 +29,11 @@ class DummyBlockchainInterface(BlockchainInterface): def __init__(self): self.fake_query_results = None self.qusfail = False + self.cbh = 1 + + def get_current_block_height(self): + self.cbh += 1 + return self.cbh def rpc(self, a, b): return None