diff --git a/jmclient/jmclient/blockchaininterface.py b/jmclient/jmclient/blockchaininterface.py index 71e43ae..777530a 100644 --- a/jmclient/jmclient/blockchaininterface.py +++ b/jmclient/jmclient/blockchaininterface.py @@ -164,7 +164,14 @@ 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 reactor is not stopped, so we override the 'swallowing' + # of the Exception that happened in self.rpc(): + raise JsonRpcConnectionError("RPC connection to Bitcoin Core " + "was not established successfully.") actualNet = blockchainInfo['chain'] netmap = {'main': 'mainnet', 'test': 'testnet', 'regtest': 'regtest'} @@ -183,12 +190,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..25118af 100644 --- a/jmclient/jmclient/wallet_service.py +++ b/jmclient/jmclient/wallet_service.py @@ -40,15 +40,28 @@ class WalletService(Service): # could be more flexible in future, and # the JM wallet object. self.bci = jm_single().bc_interface + + # main loop used to check for transactions, instantiated + # after wallet is synced: + self.monitor_loop = None + self.wallet = wallet + self.synced = False + # 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() + if not self.update_blockheight(): + # this accounts for the unusual case + # where the application started up with + # a functioning blockchain interface, but + # that bci is now failing when we are starting + # the wallet service. + raise Exception("WalletService failed to start " + "due to inability to query block height.") else: jlog.warning("No blockchain source available, " + "wallet tools will not show correct balances.") - self.wallet = wallet - self.synced = False # Dicts of registered callbacks, by type # and then by txinfo, for events @@ -73,14 +86,36 @@ 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). """ + + def critical_error(): + jlog.error("Failure to get blockheight from Bitcoin Core.") + self.stopService() + return False + + 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: - jlog.error("Failure to get blockheight from Bitcoin Core:") - jlog.error(repr(e)) - return + # This should never happen now, as we are catching every + # possible Exception in jsonrpc or bci.rpc: + return critical_error() + if not self.current_blockheight: + return critical_error() + + # 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. @@ -95,7 +130,8 @@ class WalletService(Service): should *not* be restarted, instead a new WalletService instance should be created. """ - self.monitor_loop.stop() + if self.monitor_loop: + self.monitor_loop.stop() self.wallet.close() super(WalletService, self).stopService() @@ -169,7 +205,9 @@ class WalletService(Service): """ if not syncresult: jlog.error("Failed to sync the bitcoin wallet. Shutting down.") - reactor.stop() + self.stopService() + if reactor.running: + reactor.stop() return jlog.info("Starting transaction monitor in walletservice") self.monitor_loop = task.LoopingCall( @@ -240,9 +278,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 diff --git a/scripts/joinmarket-qt.py b/scripts/joinmarket-qt.py index 8513bd1..7e42e48 100755 --- a/scripts/joinmarket-qt.py +++ b/scripts/joinmarket-qt.py @@ -1403,7 +1403,8 @@ class JMMainWindow(QMainWindow): event.accept() if self.reactor.threadpool is not None: self.reactor.threadpool.stop() - self.reactor.stop() + if reactor.running: + self.reactor.stop() else: event.ignore() @@ -1752,7 +1753,6 @@ class JMMainWindow(QMainWindow): # add information callbacks: self.wallet_service.add_restart_callback(self.restartWithMsg) self.wallet_service.autofreeze_warning_cb = self.autofreeze_warning_cb - self.wallet_service.startService() self.syncmsg = "" self.walletRefresh = task.LoopingCall(self.updateWalletInfo) @@ -1765,10 +1765,25 @@ class JMMainWindow(QMainWindow): t = self.centralWidget().widget(0) if not self.wallet_service: #failure to sync in constructor means object is not created newsyncmsg = "Unable to sync wallet - see error in console." + elif not self.wallet_service.isRunning(): + JMQtMessageBox(self, + "The Joinmarket wallet service has stopped; this is usually caused " + "by a Bitcoin Core RPC connection failure. Is your node running?", + mbtype='crit', + title="Error") + qApp.exit(EXIT_FAILURE) + return elif not self.wallet_service.synced: return else: - t.updateWalletInfo(get_wallet_printout(self.wallet_service)) + try: + t.updateWalletInfo(get_wallet_printout(self.wallet_service)) + except Exception: + # this is very likely to happen in case Core RPC connection goes + # down (but, order of events means it is not deterministic). + log.debug("Failed to get wallet information, is there a problem with " + "the blockchain interface?") + return newsyncmsg = "Wallet synced successfully." if newsyncmsg != self.syncmsg: self.syncmsg = newsyncmsg