Merge #454: Make Qt check config fee filters as well as checktx

c571613 Make Qt check config fee filters as well as checktx (Adam Gibson)

jmclient/jmclient/taker.py

@@ -35,14 +35,17 @@ class Taker(object):
     def __init__(self,
                  wallet_service,
                  schedule,
+                 max_cj_fee,
                  order_chooser=weighted_order_choose,
                  callbacks=None,
                  tdestaddrs=None,
-                 ignored_makers=None,
-                 max_cj_fee=(1, float('inf'))):
-        """Schedule must be a list of tuples: (see sample_schedule_for_testnet
+                 ignored_makers=None):
+        """`schedule`` must be a list of tuples: (see sample_schedule_for_testnet
         for explanation of syntax, also schedule.py module in this directory),
         which will be a sequence of joins to do.
+        `max_cj_fee` must be a tuple of form: (float, int or float) where the first
+        is the maximum relative fee as a decimal and the second is the maximum
+        absolute fee in satoshis.
         Callbacks:
         External callers set the 3 callbacks for filtering orders,
         sending info messages to client, and action on completion.
@@ -883,7 +886,7 @@ class P2EPTaker(Taker):
     """
 
     def __init__(self, counterparty, wallet_service, schedule, callbacks):
-        super(P2EPTaker, self).__init__(wallet_service, schedule, callbacks=callbacks)
+        super(P2EPTaker, self).__init__(wallet_service, schedule, (1, float('inf')), callbacks=callbacks)
         self.p2ep_receiver_nick = counterparty
         # Callback to request user permission (for e.g. GUI)
         # args: (1) message, as string

jmclient/jmclient/taker_utils.py

@@ -328,7 +328,7 @@ def tumbler_taker_finished_update(taker, schedulefile, tumble_log, options,
                 f.write(schedule_to_text(taker.schedule))
 
 
-def tumbler_filter_orders_callback(orders_fees, cjamount, taker, options):
+def tumbler_filter_orders_callback(orders_fees, cjamount, taker):
     """Since the tumbler does not use interactive fee checking,
     we use the -x values from the command line instead.
     """

jmclient/test/commontest.py

@@ -26,6 +26,8 @@ import platform
 OS = platform.system()
 PINL = '\r\n' if OS == 'Windows' else '\n'
 
+default_max_cj_fee = (1, float('inf'))
+
 class DummyBlockchainInterface(BlockchainInterface):
     def __init__(self):
         self.fake_query_results = None

jmclient/test/test_client_protocol.py

@@ -19,6 +19,7 @@ from twisted.trial import unittest
 from twisted.test import proto_helpers
 from jmbase.commands import *
 from taker_test_data import t_raw_signed_tx
+from commontest import default_max_cj_fee
 import json
 import jmbitcoin as bitcoin
 
@@ -282,7 +283,7 @@ class TrialTestJMClientProto(unittest.TestCase):
             self.addCleanup(self.client.transport.loseConnection)
         clientfactories = []
         takers = [DummyTaker(
-            WalletService(DummyWallet()), ["a", "b"], callbacks=(
+            WalletService(DummyWallet()), ["a", "b"], default_max_cj_fee, callbacks=(
                 None, None, dummy_taker_finished)) for _ in range(len(params))]
         for i, p in enumerate(params):
             takers[i].set_fail_init(p[0])

jmclient/test/test_coinjoin.py

@@ -16,7 +16,7 @@ from jmclient import load_program_config, jm_single,\
     YieldGeneratorBasic, Taker, LegacyWallet, SegwitLegacyWallet,\
     NO_ROUNDING
 from jmclient.podle import set_commitment_file
-from commontest import make_wallets, binarize_tx
+from commontest import make_wallets, binarize_tx, default_max_cj_fee
 from test_taker import dummy_filter_orderbook
 import jmbitcoin as btc
 
@@ -64,8 +64,8 @@ def create_taker(wallet, schedule, monkeypatch):
         on_finished_callback.called = True
     on_finished_callback.called = False
     on_finished_callback.status = None
-    taker = Taker(wallet, schedule, callbacks=(dummy_filter_orderbook, None,
-                                               on_finished_callback))
+    taker = Taker(wallet, schedule, default_max_cj_fee,
+                callbacks=(dummy_filter_orderbook, None, on_finished_callback))
 
     # we have skipped irc key setup and key exchange, handled by jmdaemon
     monkeypatch.setattr(taker, 'auth_counterparty', lambda *args: True)

jmclient/test/test_taker.py

@@ -18,7 +18,7 @@ from jmclient import load_program_config, jm_single, set_commitment_file,\
     get_network, WalletService, NO_ROUNDING
 from taker_test_data import t_utxos_by_mixdepth, t_orderbook,\
     t_maker_response, t_chosen_orders, t_dummy_ext
-
+from commontest import default_max_cj_fee
 
 class DummyWallet(SegwitLegacyWallet):
     def __init__(self):
@@ -128,7 +128,7 @@ def get_taker(schedule=None, schedule_len=0, on_finished=None,
     print("Using schedule: " + str(schedule))
     on_finished_callback = on_finished if on_finished else taker_finished
     filter_orders_callback = filter_orders if filter_orders else dummy_filter_orderbook
-    taker = Taker(WalletService(DummyWallet()), schedule,
+    taker = Taker(WalletService(DummyWallet()), schedule, default_max_cj_fee,
                   callbacks=[filter_orders_callback, None, on_finished_callback])
     taker.wallet_service.current_blockheight = 10**6
     return taker

scripts/cli_options.py

@@ -105,30 +105,19 @@ def get_order_choose_algorithm(option, opt_str, value, parser, value_kw=None):
     setattr(parser.values, option.dest, fn)
 
 
-def get_max_cj_fee_values(config, parser_options):
-    CONFIG_SECTION = 'POLICY'
-    CONFIG_OPTION = 'max_cj_fee_'
-    # rel, abs
-    fee_values = [None, None]
-    fee_types = [float, int]
-
-    for i, option in enumerate(('rel', 'abs')):
-        value = getattr(parser_options, CONFIG_OPTION + option, None)
-        if value is not None:
-            fee_values[i] = fee_types[i](value)
-            continue
-
-        try:
-            fee_values[i] = config.get(CONFIG_SECTION, CONFIG_OPTION + option)
-        except NoOptionError:
-            pass
+"""
+The following defaults are maintained as accessed via functions for
+flexibility.
+TODO This should be moved from this module."""
+MAX_DEFAULT_REL_FEE = 0.001
+MIN_MAX_DEFAULT_ABS_FEE = 1000
+MAX_MAX_DEFAULT_ABS_FEE = 10000
 
-    if any(x is None for x in fee_values):
-        fee_values = prompt_user_for_cj_fee(*fee_values)
-
-    return tuple(map(lambda j: fee_types[j](fee_values[j]),
-                     range(len(fee_values))))
+def get_default_max_relative_fee():
+    return MAX_DEFAULT_REL_FEE
 
+def get_default_max_absolute_fee():
+    return random.randint(MIN_MAX_DEFAULT_ABS_FEE, MAX_MAX_DEFAULT_ABS_FEE)
 
 def prompt_user_for_cj_fee(rel_val, abs_val):
     msg = """Joinmarket will choose market makers randomly as long as their
@@ -163,12 +152,12 @@ counterparties are selected."""
     rel_prompt = False
     if rel_val is None:
         rel_prompt = True
-        rel_val = 0.001
+        rel_val = get_default_max_relative_fee()
 
     abs_prompt = False
     if abs_val is None:
         abs_prompt = True
-        abs_val = random.randint(1000, 10000)
+        abs_val = get_default_max_absolute_fee()
 
     print(msg.format(rel_val=rel_val, abs_val=abs_val))
     if rel_prompt:
@@ -206,6 +195,40 @@ max_cj_fee_rel = {rel_val}\n""".format(rel_val=rel_val, abs_val=abs_val))
 
     return rel_val, abs_val
 
+def get_max_cj_fee_values(config, parser_options,
+                          user_callback=prompt_user_for_cj_fee):
+    """ Given a config object, retrieve the chosen maximum absolute
+    and relative coinjoin fees chosen by the user, or prompt
+    the user via the user_callback function, if not present in
+    the config.
+
+    user_callback:
+    Arguments: relative value(default None), absolute value (default None)
+    Returns: relative value (float), absolute value (int, satoshis)
+    """
+
+    CONFIG_SECTION = 'POLICY'
+    CONFIG_OPTION = 'max_cj_fee_'
+    # rel, abs
+    fee_values = [None, None]
+    fee_types = [float, int]
+
+    for i, option in enumerate(('rel', 'abs')):
+        if parser_options is not None:
+            value = getattr(parser_options, CONFIG_OPTION + option, None)
+            if value is not None:
+                fee_values[i] = fee_types[i](value)
+                continue
+        try:
+            fee_values[i] = config.get(CONFIG_SECTION, CONFIG_OPTION + option)
+        except NoOptionError:
+            pass
+
+    if any(x is None for x in fee_values):
+        fee_values = user_callback(*fee_values)
+
+    return tuple(map(lambda j: fee_types[j](fee_values[j]),
+                     range(len(fee_values))))
 
 def check_regtest(blockchain_start=True):
     """ Applies any regtest-specific configuration

scripts/joinmarket-qt.py

@@ -80,7 +80,9 @@ from qtsupport import ScheduleWizard, TumbleRestartWizard, config_tips,\
     config_types, QtHandler, XStream, Buttons, OkButton, CancelButton,\
     PasswordDialog, MyTreeWidget, JMQtMessageBox, BLUE_FG,\
     donation_more_message
-
+# TODO refactor; these functions do not belong in cli_options:
+from cli_options import get_max_cj_fee_values, get_default_max_absolute_fee, \
+     get_default_max_relative_fee
 
 from twisted.internet import task
 
@@ -649,6 +651,27 @@ class SpendTab(QWidget):
         self.spendstate.updateRun('running')
         self.startJoin()
 
+    def getMaxCJFees(self, relfee, absfee):
+        """ Used as a callback to decide relative and absolute
+        maximum fees for coinjoins, in cases where the user has not
+        set these values in the config (which is the default)."""
+        if relfee is None:
+            relfee = get_default_max_relative_fee()
+        if absfee is None:
+            absfee = get_default_max_absolute_fee()
+        msg = ("Your maximum absolute fee in from one counterparty has been "
+              "set to: " + str(absfee) + " satoshis.\n"
+              "Your maximum relative fee from one counterparty has been set "
+              "to: " + str(relfee) + ".\n"
+              "To change these, please edit the config file and change the "
+              "settings:\n"
+              "max_cj_fee_abs = your-value-in-satoshis\n"
+              "max_cj_fee_rel = your-value-as-decimal\n"
+              "in the [POLICY] section.\n"
+              "Note: If you don't do this, this dialog will interrupt the tumbler.")
+        JMQtMessageBox(self, msg, mbtype="info", title="Setting fee limits.")
+        return relfee, absfee
+
     def startJoin(self):
         if not mainWindow.wallet_service:
             JMQtMessageBox(self, "Cannot start without a loaded wallet.",
@@ -664,8 +687,13 @@ class SpendTab(QWidget):
             check_offers_callback = None
 
         destaddrs = self.tumbler_destaddrs if self.tumbler_options else []
+        maxcjfee = get_max_cj_fee_values(jm_single().config, None,
+                                         user_callback=self.getMaxCJFees)
+        log.info("Using maximum coinjoin fee limits per maker of {:.4%}, {} "
+                     "".format(maxcjfee[0], btc.amount_to_str(maxcjfee[1])))
         self.taker = Taker(mainWindow.wallet_service,
                            self.spendstate.loaded_schedule,
+                           maxcjfee,
                            order_chooser=weighted_order_choose,
                            callbacks=[check_offers_callback,
                                       self.takerInfo,
@@ -707,7 +735,7 @@ class SpendTab(QWidget):
 
     def checkOffersTumbler(self, offers_fees, cjamount):
         return tumbler_filter_orders_callback(offers_fees, cjamount,
-                                              self.taker, self.tumbler_options)
+                                              self.taker)
 
     def checkOffers(self, offers_fee, cjamount):
         """Parse offers and total fee from client protocol,

scripts/qtsupport.py

@@ -531,9 +531,7 @@ class SchDynamicPage1(QWizardPage):
         sN = ['Starting mixdepth', 'Average number of counterparties',
               'How many mixdepths to tumble through',
               'Average wait time between transactions, in minutes',
-              'Average number of transactions per mixdepth',
-              'Max relative fee per counterparty (e.g. 0.005)',
-              'Max fee per counterparty, satoshis (e.g. 10000)']
+              'Average number of transactions per mixdepth']
         #Tooltips
         sH = ["The starting mixdepth can be decided from the Wallet tab; it must\n"
         "have coins in it, but it's OK if some coins are in other mixdepths.",
@@ -544,16 +542,13 @@ class SchDynamicPage1(QWizardPage):
         "will move coins from mixdepth 1 to mixdepth 5",
         "This is the time waited *after* 1 confirmation has occurred, and is\n"
         "varied randomly.",
-        "Will be varied randomly, see advanced settings next page",
-        "A decimal fraction (e.g. 0.001 = 0.1%) (this AND next must be violated to reject",
-        "Integer number of satoshis (this AND previous must be violated to reject)"]
+        "Will be varied randomly, see advanced settings next page"]
         #types
-        sT = [int, int, int, float, int, float, int]
+        sT = [int, int, int, float, int]
         #constraints
         sMM = [(0, jm_single().config.getint("GUI", "max_mix_depth") - 1), (3, 20),
-               (2, 7), (0.00000001, 100.0, 8), (2, 10), (0.000001, 0.25, 6),
-               (0, 10000000)]
-        sD = ['0', '9', '4', '60.0', '2', '0.005', '10000']
+               (2, 7), (0.00000001, 100.0, 8), (2, 10)]
+        sD = ['0', '9', '4', '60.0', '2']
         for x in zip(sN, sH, sT, sD, sMM):
             ql = QLabel(x[0])
             ql.setToolTip(x[1])
@@ -574,8 +569,6 @@ class SchDynamicPage1(QWizardPage):
         self.registerField("mixdepthcount", results[2][1])
         self.registerField("timelambda", results[3][1])
         self.registerField("txcountparams", results[4][1])
-        self.registerField("maxrelfee", results[5][1])
-        self.registerField("maxabsfee", results[6][1])
 
 class SchDynamicPage2(QWizardPage):
 
@@ -753,9 +746,6 @@ class ScheduleWizard(QWizard):
         self.opts['waittime'] = float(self.field("waittime"))
         self.opts["stage1_timelambda_increase"] = float(self.field("stage1_timelambda_increase"))
         self.opts['mincjamount'] = int(self.field("mincjamount"))
-        relfeeval = float(self.field("maxrelfee"))
-        absfeeval = int(self.field("maxabsfee"))
-        self.opts['maxcjfee'] = (relfeeval, absfeeval)
         #needed for Taker to check:
         self.opts['rounding_chance'] = float(self.field("rounding_chance"))
         self.opts['rounding_sigfig_weights'] = tuple([int(self.field("rounding_sigfig_weight_" + str(i+1))) for i in range(5)])

scripts/tumbler.py

@@ -158,7 +158,7 @@ def main():
     def filter_orders_callback(orders_fees, cjamount):
         """Decide whether to accept fees
         """
-        return tumbler_filter_orders_callback(orders_fees, cjamount, taker, options)
+        return tumbler_filter_orders_callback(orders_fees, cjamount, taker)
 
     def taker_finished(res, fromtx=False, waittime=0.0, txdetails=None):
         """on_finished_callback for tumbler; processing is almost entirely
@@ -176,8 +176,8 @@ def main():
     #instantiate Taker with given schedule and run
     taker = Taker(wallet_service,
                   schedule,
+                  maxcjfee,
                   order_chooser=options['order_choose_fn'],
-                  max_cj_fee=maxcjfee,
                   callbacks=(filter_orders_callback, None, taker_finished),
                   tdestaddrs=destaddrs)
     clientfactory = JMClientProtocolFactory(taker)