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

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

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,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

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