Merge #585: Quit gracefully if Bitcoin RPC connection lost

52b9ba8 Initialization of BCI raises Exception not sys.exit (Adam Gibson)
cc219cc Handles RPC failure in Qt with message box and quit. (Adam Gibson)
5ce4b55 Quit gracefully if Bitcoin RPC connection lost (Adam Gibson)

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):

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

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

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

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