Merge #693: Shutdown on blockcount RPC failure and other cases cleanly

5604857 quit scripts gracefully on walletservice rpc startup failure (Adam Gibson)
5af2d49 handle Qt wallet load failure (Adam Gibson)
202f8ee Add clarifying comments for delayed order creation. (Adam Gibson)
a2aafd2 Fixes #673. Shutdown cleanly on failure to access blockheight (Adam Gibson)

jmbase/jmbase/__init__.py

@@ -7,6 +7,7 @@ from .support import (get_log, chunks, debug_silence, jmprint,
                       utxo_to_utxostr, EXIT_ARGERROR, EXIT_FAILURE,
                       EXIT_SUCCESS, hexbin, dictchanger, listchanger,
                       JM_WALLET_NAME_PREFIX, JM_APP_NAME)
+from .twisted_utils import stop_reactor
 from .bytesprod import BytesProducer
 from .commands import *
 

jmbase/jmbase/twisted_utils.py

@@ -0,0 +1,16 @@
+
+from twisted.internet.error import ReactorNotRunning, AlreadyCancelled
+from twisted.internet import reactor
+
+def stop_reactor():
+    """ The value of the bool `reactor.running`
+    does not reliably tell us whether the
+    reactor is running (!). There are startup
+    and shutdown phases not reported externally
+    by IReactorCore. So we must catch Exceptions
+    raised by trying to stop the reactor.
+    """
+    try:
+        reactor.stop()
+    except ReactorNotRunning:
+        pass

jmclient/jmclient/blockchaininterface.py

@@ -6,7 +6,7 @@ import time
 from decimal import Decimal
 import binascii
 from twisted.internet import reactor, task
-from jmbase import bintohex, hextobin
+from jmbase import bintohex, hextobin, stop_reactor
 import jmbitcoin as btc
 
 from jmclient.jsonrpc import JsonRpcConnectionError, JsonRpcError
@@ -214,9 +214,11 @@ class BitcoinCoreInterface(BlockchainInterface):
             # BareException type).
             log.error("Failure of RPC connection to Bitcoin Core. "
                       "Application cannot continue, shutting down.")
-            if reactor.running:
+            stop_reactor()
-                reactor.stop()
             return None
+        # note that JsonRpcError is not caught here; for some calls, we
+        # have specific behaviour requirements depending on these errors,
+        # so this is handled elsewhere in BitcoinCoreInterface.
         return res
 
     def is_address_labeled(self, utxo, walletname):
@@ -430,7 +432,13 @@ class BitcoinCoreInterface(BlockchainInterface):
         return retval
 
     def get_current_block_height(self):
-        return self.rpc("getblockcount", [])
+        try:
+            res = self.rpc("getblockcount", [])
+        except JsonRpcError as e:
+            log.error("Getblockcount RPC failed with: %i, %s" % (
+                e.code, e.message))
+            res = None
+        return res
 
     def get_best_block_hash(self):
         return self.rpc('getbestblockhash', [])

jmclient/jmclient/maker.py

@@ -6,7 +6,7 @@ import sys
 import abc
 
 import jmbitcoin as btc
-from jmbase import bintohex, hexbin, get_log, EXIT_SUCCESS, EXIT_FAILURE
+from jmbase import bintohex, hexbin, get_log, EXIT_SUCCESS, EXIT_FAILURE, stop_reactor
 from jmclient.wallet import estimate_tx_fee, compute_tx_locktime
 from jmclient.wallet_service import WalletService
 from jmclient.configure import jm_single
@@ -25,7 +25,10 @@ class Maker(object):
         self.nextoid = -1
         self.offerlist = None
         self.sync_wait_loop = task.LoopingCall(self.try_to_create_my_orders)
-        self.sync_wait_loop.start(2.0)
+        # don't fire on the first tick since reactor is still starting up
+        # and may not shutdown appropriately if we immediately recognize
+        # not-enough-coins:
+        self.sync_wait_loop.start(2.0, now=False)
         self.aborted = False
 
     def try_to_create_my_orders(self):
@@ -41,7 +44,7 @@ class Maker(object):
         self.sync_wait_loop.stop()
         if not self.offerlist:
             jlog.info("Failed to create offers, giving up.")
-            sys.exit(EXIT_FAILURE)
+            stop_reactor()
         jlog.info('offerlist={}'.format(self.offerlist))
 
     @hexbin

jmclient/jmclient/wallet_service.py

@@ -15,6 +15,7 @@ from jmclient.output import fmt_tx_data
 from jmclient.blockchaininterface import (INF_HEIGHT, BitcoinCoreInterface,
     BitcoinCoreNoHistoryInterface)
 from jmclient.wallet import FidelityBondMixin
+from jmbase import stop_reactor
 from jmbase.support import jmprint, EXIT_SUCCESS, utxo_to_utxostr, hextobin
 
 
@@ -46,9 +47,13 @@ class WalletService(Service):
         self.wallet = wallet
         self.synced = False
 
+        # used to flag RPC failure at construction of object:
+        self.rpc_error = False
+
         # keep track of the quasi-real-time blockheight
         # (updated in main monitor loop)
         self.current_blockheight = None
+
         if self.bci is not None:
             if not self.update_blockheight():
                 # this accounts for the unusual case
@@ -56,8 +61,13 @@ class WalletService(Service):
                 # a functioning blockchain interface, but
                 # that bci is now failing when we are starting
                 # the wallet service.
-                raise Exception("WalletService failed to start "
+                jlog.error("Failure of RPC connection to Bitcoin Core in "
-                                "due to inability to query block height.")
+                          "wallet service startup. Application cannot "
+                          "continue, shutting down.")
+                self.rpc_error = ("Failure of RPC connection to Bitcoin "
+                                  "Core in wallet service startup.")
+                # no need to call stopService as it has not yet been started.
+                stop_reactor()
         else:
             jlog.warning("No blockchain source available, " +
                 "wallet tools will not show correct balances.")
@@ -91,8 +101,13 @@ class WalletService(Service):
         """
 
         def critical_error():
-            jlog.error("Failure to get blockheight from Bitcoin Core.")
+            jlog.error("Critical error updating blockheight.")
+            # this cleanup (a) closes the wallet, removing the lock
+            # and (b) signals to clients that the service is no longer
+            # in a running state, both of which can be useful
+            # post reactor shutdown.
             self.stopService()
+            stop_reactor()
             return False
 
         if self.current_blockheight:
@@ -707,6 +722,10 @@ class WalletService(Service):
         st = time.time()
         # block height needs to be real time for addition to our utxos:
         current_blockheight = self.bci.get_current_block_height()
+        if not current_blockheight:
+            # this failure will shut down the application elsewhere, here
+            # just give up:
+            return
         wallet_name = self.get_wallet_name()
         self.reset_utxos()
 

jmclient/jmclient/wallet_utils.py

@@ -1432,6 +1432,8 @@ def wallet_tool_main(wallet_root_path):
         # this object is only to respect the layering,
         # the service will not be started since this is a synchronous script:
         wallet_service = WalletService(wallet)
+        if wallet_service.rpc_error:
+            sys.exit(EXIT_FAILURE)
 
         if method not in noscan_methods and jm_single().bc_interface is not None:
             # if nothing was configured, we override bitcoind's options so that

jmclient/test/test_coinjoin.py

@@ -69,6 +69,11 @@ def create_taker(wallet, schedule, monkeypatch):
     monkeypatch.setattr(taker, 'auth_counterparty', lambda *args: True)
     return taker
 
+def create_orders(makers):
+    # fire the order creation immediately (delayed 2s in prod,
+    # but this is too slow for test):
+    for maker in makers:
+        maker.try_to_create_my_orders()
 
 def init_coinjoin(taker, makers, orderbook, cj_amount):
     init_data = taker.initialize(orderbook)
@@ -133,6 +138,7 @@ def test_simple_coinjoin(monkeypatch, tmpdir, setup_cj, wallet_cls):
     makers = [YieldGeneratorBasic(
         wallet_services[i],
         [0, 2000, 0, 'swabsoffer', 10**7]) for i in range(MAKER_NUM)]
+    create_orders(makers)
 
     orderbook = create_orderbook(makers)
     assert len(orderbook) == MAKER_NUM
@@ -177,6 +183,7 @@ def test_coinjoin_mixdepth_wrap_taker(monkeypatch, tmpdir, setup_cj):
     makers = [YieldGeneratorBasic(
         wallet_services[i],
         [0, cj_fee, 0, 'swabsoffer', 10**7]) for i in range(MAKER_NUM)]
+    create_orders(makers)
 
     orderbook = create_orderbook(makers)
     assert len(orderbook) == MAKER_NUM
@@ -232,7 +239,7 @@ def test_coinjoin_mixdepth_wrap_maker(monkeypatch, tmpdir, setup_cj):
     makers = [YieldGeneratorBasic(
         wallet_services[i],
         [0, cj_fee, 0, 'swabsoffer', 10**7]) for i in range(MAKER_NUM)]
-
+    create_orders(makers)
     orderbook = create_orderbook(makers)
     assert len(orderbook) == MAKER_NUM
 

scripts/add-utxo.py

@@ -172,6 +172,8 @@ def main():
         wallet_path = get_wallet_path(options.loadwallet)
         wallet = open_wallet(wallet_path, gap_limit=options.gaplimit)
         wallet_service = WalletService(wallet)
+        if wallet_service.rpc_error:
+            sys.exit(EXIT_FAILURE)
         while True:
             if wallet_service.sync_wallet(fast=not options.recoversync):
                 break

scripts/joinmarket-qt.py

@@ -63,7 +63,7 @@ donation_address_url = "https://bitcoinprivacy.me/joinmarket-donations"
 #Version of this Qt script specifically
 JM_GUI_VERSION = '16dev'
 
-from jmbase import get_log
+from jmbase import get_log, stop_reactor
 from jmbase.support import DUST_THRESHOLD, EXIT_FAILURE, utxo_to_utxostr,\
     bintohex, hextobin, JM_CORE_VERSION
 from jmclient import load_program_config, get_network, update_persist_config,\
@@ -1489,8 +1489,7 @@ class JMMainWindow(QMainWindow):
             event.accept()
             if self.reactor.threadpool is not None:
                 self.reactor.threadpool.stop()
-            if reactor.running:
+            stop_reactor()
-                self.reactor.stop()
         else:
             event.ignore()
 
@@ -1845,6 +1844,10 @@ class JMMainWindow(QMainWindow):
                                mbtype='warn',
                                title="Error")
                     return
+                if decrypted == "error":
+                    # special case, not a failure to decrypt the file but
+                    # a failure of wallet loading, give up:
+                    self.close()
         else:
             if not testnet_seed:
                 testnet_seed, ok = QInputDialog.getText(self,
@@ -1888,6 +1891,11 @@ class JMMainWindow(QMainWindow):
             self.walletRefresh.stop()
 
         self.wallet_service = WalletService(wallet)
+        # in case an RPC error occurs in the constructor:
+        if self.wallet_service.rpc_error:
+            JMQtMessageBox(self,self.wallet_service.rpc_error,
+                           mbtype='warn',title="Error")
+            return "error"
 
         if jm_single().bc_interface is None:
             self.centralWidget().widget(0).updateWalletInfo(

scripts/sendpayment.py

@@ -170,6 +170,8 @@ def main():
         wallet_password_stdin=options.wallet_password_stdin,
         gap_limit=options.gaplimit)
     wallet_service = WalletService(wallet)
+    if wallet_service.rpc_error:
+        sys.exit(EXIT_FAILURE)
     # in this script, we need the wallet synced before
     # logic processing for some paths, so do it now:
     while not wallet_service.synced:

scripts/tumbler.py

@@ -45,6 +45,8 @@ def main():
     wallet_path = get_wallet_path(wallet_name, None)
     wallet = open_test_wallet_maybe(wallet_path, wallet_name, max_mix_depth, wallet_password_stdin=options_org.wallet_password_stdin)
     wallet_service = WalletService(wallet)
+    if wallet_service.rpc_error:
+        sys.exit(EXIT_FAILURE)
     # in this script, we need the wallet synced before
     # logic processing for some paths, so do it now:
     while not wallet_service.synced: