From 202f8ee04774804af05edda6e95a14b85d260cda Mon Sep 17 00:00:00 2001 From: Adam Gibson Date: Thu, 1 Oct 2020 16:07:40 +0100 Subject: [PATCH] Add clarifying comments for delayed order creation. Also manually fire order creation in coinjoin tests. This clarification and test change is required due to the fact that LoopingCalls are designed to fire immediately by default, before the reactor is initialized (and therefore in a `running` state), making it not possible to shutdown the reactor as a result of events happening in that first call; so we delay the first call of the maker's orderbook populating code, so that if a no-coins error occurs, it will actually shut down the reactor and hence the whole yield generator program, as intended. --- jmbase/jmbase/twisted_utils.py | 7 +++---- jmclient/jmclient/blockchaininterface.py | 2 +- jmclient/jmclient/maker.py | 3 +++ jmclient/test/test_coinjoin.py | 9 ++++++++- 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/jmbase/jmbase/twisted_utils.py b/jmbase/jmbase/twisted_utils.py index 57a6584..6d35029 100644 --- a/jmbase/jmbase/twisted_utils.py +++ b/jmbase/jmbase/twisted_utils.py @@ -3,13 +3,12 @@ from twisted.internet.error import ReactorNotRunning, AlreadyCancelled from twisted.internet import reactor def stop_reactor(): - """ Both in startup and shutdown, - the value of the bool `reactor.running` + """ 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. - Hence the Exception catch is needed here. + by IReactorCore. So we must catch Exceptions + raised by trying to stop the reactor. """ try: if reactor.running: diff --git a/jmclient/jmclient/blockchaininterface.py b/jmclient/jmclient/blockchaininterface.py index dcf5274..24b23e7 100644 --- a/jmclient/jmclient/blockchaininterface.py +++ b/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 diff --git a/jmclient/jmclient/maker.py b/jmclient/jmclient/maker.py index 598eaa1..631a511 100644 --- a/jmclient/jmclient/maker.py +++ b/jmclient/jmclient/maker.py @@ -25,6 +25,9 @@ class Maker(object): self.nextoid = -1 self.offerlist = None self.sync_wait_loop = task.LoopingCall(self.try_to_create_my_orders) + # 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 diff --git a/jmclient/test/test_coinjoin.py b/jmclient/test/test_coinjoin.py index 26e8305..0c851b8 100644 --- a/jmclient/test/test_coinjoin.py +++ b/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