Merge #840: Change taker's commitments logic

4e4b15b add tests cases for commitment sourcing (Adam Gibson)
f0cd0a9 Update SOURCING-COMMITMENTS.md (PulpCattel)
40768cf Change taker's commitments logic (PulpCattel)

docs/SOURCING-COMMITMENTS.md

@@ -10,7 +10,7 @@ The gory crypto details don't matter of course, what matters is that for each ut
 
 ## Source of commitment utxos
 
-Usually they will be sourced from your Joinmarket wallet, and this will require no intervention. The main purpose of this page is (a) to make that happen as much as possible and (b) to tell you what to do if it goes wrong.
+Usually they will be sourced from your spending mixdepth, and this will require no intervention. The main purpose of this page is (a) to make that happen as much as possible and (b) to tell you what to do if it goes wrong.
 
 ### Wait for at least 5 confirmations
 
@@ -31,7 +31,7 @@ In most cases this will be enough.
 In cases where you don't have enough utxos with valid commitments left, or you're not prepared to wait 5 blocks, there are alternatives made available. If you go into the `scripts` directory you'll find a tool `add-utxo.py`. Run `python add-utxo.py --help` to see an explanation. In short, you can:
 
 * Add external utxos from a non-JM wallet, like Electrum or Core, one at a time, or from a prepared file (-r).
-* Add external utxos from another Joinmarket wallet (-w).
+* Add external utxos from a Joinmarket wallet (-w).
 * Delete existing external utxos (-d)
 * Validate utxos (-v, -o)
 
@@ -45,38 +45,41 @@ Be aware that Makers will see this utxo *only if* your usage is successful (they
 
 The commitments you've already used (just hash values) are stored in the `used` section of the file `~/.joinmarket/cmtdata/commitments.json` - **don't delete this file**. Although you won't need to read this file, it represents your memory of which commitments you've already used, so if you lose that record, your Taker scripts will find themselves spending a lot of time retrying commitments that the rest of the Joinmarket "network" knows are already used, and so will be rejected. This is only an inconvenience, but it could be pretty annoying. Any external commitments you sourced according to the previous section are also added here, and deleted automatically once they're used up. You can add/delete the contents of that `external` section using the previously mentioned `scripts/add-utxo.py` script (see previous section for a brief overview of options).
 
-If in a run of `sendpayment.py` no commitment can be sourced (in tumbler it just waits), either within the internal wallet, or external as described above, a file `commitments_debug.txt` is created that will show exactly which utxos have been tried and why they failed - and gives brief instructions on what to do depending on the state of each of those. A sample file is shown at the end of this page.
+If in a run of `sendpayment.py` no commitment can be sourced (in tumbler it just waits), either within the spending mixdepth, or external as described above, a file `commitments_debug.txt` is created that will show exactly which utxos have been tried and why they failed - and gives brief instructions on what to do depending on the state of each of those. A sample file is shown at the end of this page.
 
 ### Minor additional tools
 
-An extra method for `wallet-tool.py` is added: `showutxos` - this will pretty print the all the available utxos in any Joinmarket wallet. This is only potentially useful for people who run both as Maker and Taker and want to consider transferring utxos for commitments from one wallet to another (using the `-w` option to `scripts/add-utxo.py`).
+An extra method for `wallet-tool.py` is added: `showutxos` - this will pretty print all the available utxos in any Joinmarket wallet. This is only potentially useful for people who run both as Maker and Taker and want to consider transferring utxos for commitments from one mixdepth/wallet to another (using the `-w` option to `scripts/add-utxo.py`).
 
 Similarly a `sendtomany` function is available in `cd scripts; python sendtomany.py --help`; read the help for details but it's as simple as it sounds, specifically creating equal sized outputs for each of the destination addresses you specify; it requires one utxo and its private key as inputs. You're prompted before broadcast, so you can check its validity.
 
 
 Sample commitments_debug.txt:
+
 ```
 THIS IS A TEMPORARY FILE FOR DEBUGGING; IT CAN BE SAFELY DELETED ANY TIME.
 ***
 1: Utxos that passed age and size limits, but have been used too many times (see taker_utxo_retries in the config):
-None
+dba822a36ad524b775af63db4e25b3558c75ac90d8566a74b2d08b9f63adff97:1
+cde0608bf5426c7ed705d87553718e8c74fa75b9428f14e0759f9e95b03d25f2:1
 2: Utxos that have less than 5 confirmations:
-f8f0256f70ed3b60c2d933a697a8462ccf7d165bbf3d5a33fd4a4ff57eb8cc27:0
-ed00d570efc763706bbca3cf008afe9ab50da730f0a428ea0115f7642186ff54:0
-ac947720cabab156c41cb9d6ade90c260107dd08584522b703b79433aa877767:1
-71b527f7802d2a6694d474b0e5532dfa0abd349a64f163809c93ed6324c9230f:0
-aee068c78fa9d576378828f1539fba6027ddb00c2bdb3747b970d21ecbd64f39:1
-3: Utxos that were not at least 20% of the size of the coinjoin amount 199164661
 None
+3: Utxos that were not at least 20% of the size of the coinjoin amount 499948833
+8ddaee775825a6f89f48e17b47177d3858044cdc633ced09c7c84cc75f609522:0
 ***
 Utxos that appeared in item 1 cannot be used again.
 Utxos only in item 2 can be used by waiting for more confirmations, (set by the value of taker_utxo_age).
 Utxos only in item 3 are not big enough for this coinjoin transaction, set by the value of taker_utxo_amtpercent.
-If you cannot source a utxo from your wallet according to these rules, use the tool add-utxo.py to source a utxo external to your joinmarket wallet. Read the help with 'python add-utxo.py --help'
+If you cannot source a utxo from your spending mixdepth according to these rules, use the tool add-utxo.py to source a utxo from another mixdepth or a utxo external to your joinmarket wallet. Read the help with 'python add-utxo.py --help'
 
-You can also reset the rules in the joinmarket.cfg file, but this is generally inadvisable.
 ***
 For reference, here are the utxos in your wallet:
 
-{u'f8f0256f70ed3b60c2d933a697a8462ccf7d165bbf3d5a33fd4a4ff57eb8cc27:0': {'value': 100000000, 'address': u'mfv6e3fjmTbBRgTMoLSmcGoykocuLYoctZ'}, u'ed00d570efc763706bbca3cf008afe9ab50da730f0a428ea0115f7642186ff54:0': {'value': 100000000, 'address': u'n3k7HrKj7wA3HZLjUnHyWjHWpkFhzPvodv'}, u'ac947720cabab156c41cb9d6ade90c260107dd08584522b703b79433aa877767:1': {'value': 100000000, 'address': u'muzRPUFFo5LdVF51gR4PeapNrYLikM2JkN'}, u'71b527f7802d2a6694d474b0e5532dfa0abd349a64f163809c93ed6324c9230f:0': {'value': 100000000, 'address': u'muqTeLpF8ANJBXUK3SZbQU9WdyJ4LUVktm'}, u'aee068c78fa9d576378828f1539fba6027ddb00c2bdb3747b970d21ecbd64f39:1': {'value': 100000000, 'address': u'n45s5NXoAo7Qrq1YctdpoBHpAoeaBunF6i'}}
+mixdepth 1:
+    dba822a36ad524b775af63db4e25b3558c75ac90d8566a74b2d08b9f63adff97:1 - path: m/84'/1'/1'/1/1, address: bcrt1qwdf43llnn8t7h38vxsqswadyelh4trz3ne6syu, value: 200000000
+    cde0608bf5426c7ed705d87553718e8c74fa75b9428f14e0759f9e95b03d25f2:1 - path: m/84'/1'/1'/1/0, address: bcrt1ql6akvae73rf433d7ga0muqcrh4y0nuxer5zczd, value: 200000000
+    8ddaee775825a6f89f48e17b47177d3858044cdc633ced09c7c84cc75f609522:0 - path: m/84'/1'/1'/1/3, address: bcrt1ql0z4wcjapmrkehgtcjc8c0gvv59hls30h0qhwq, value: 99988990
+mixdepth 0:
+    57fcec6f770e3b2ac35e9cb17b1ee88ae5b0dc84b941f7418b04a4edecc46d7f:1 - path: m/84'/1'/0'/1/0, address: bcrt1qctllzvz5jcyfa08q0psfyx6p2luss8llldh06u, value: 200000000
+    8ddaee775825a6f89f48e17b47177d3858044cdc633ced09c7c84cc75f609522:4 - path: m/84'/1'/0'/0/4, address: bcrt1qv5utq4crzwpq4gw3mvfmaw7q63ukpjf3098f6k, value: 100000000
 ```

jmclient/jmclient/output.py

@@ -55,8 +55,9 @@ def generate_podle_error_string(priv_utxo_pairs, to, ts, wallet_service, cjamoun
     errmsg += ("Utxos only in item 3 are not big enough for this "
                "coinjoin transaction, set by the value "
                "of taker_utxo_amtpercent.\n")
-    errmsg += ("If you cannot source a utxo from your wallet according "
+    errmsg += ("If you cannot source a utxo from your spending mixdepth according "
                "to these rules, use the tool add-utxo.py to source a "
+               "utxo from another mixdepth or a "
                "utxo external to your joinmarket wallet. Read the help "
                "with 'python add-utxo.py --help'\n\n")
     errmsg += ("***\nFor reference, here are the utxos in your wallet:\n")

jmclient/jmclient/taker.py

@@ -742,7 +742,6 @@ class Taker(object):
             return priv_utxo_pairs, too_old, too_small
 
         commit_type_byte = "P"
-        podle_data = None
         tries = jm_single().config.getint("POLICY", "taker_utxo_retries")
         age = jm_single().config.getint("POLICY", "taker_utxo_age")
         #Minor rounding errors don't matter here
@@ -751,24 +750,11 @@ class Taker(object):
                                             "taker_utxo_amtpercent") / 100.0)
         priv_utxo_pairs, to, ts = priv_utxo_pairs_from_utxos(self.input_utxos,
                                                              age, amt)
-        #Note that we ignore the "too old" and "too small" lists in the first
-        #pass through, because the same utxos appear in the whole-wallet check.
 
         #For podle data format see: podle.PoDLE.reveal()
         #In first round try, don't use external commitments
         podle_data = generate_podle(priv_utxo_pairs, tries)
         if not podle_data:
-            #We defer to a second round to try *all* utxos in wallet;
-            #this is because it's much cleaner to use the utxos involved
-            #in the transaction, about to be consumed, rather than use
-            #random utxos that will persist after. At this step we also
-            #allow use of external utxos in the json file.
-            if any(self.wallet_service.get_utxos_by_mixdepth().values()):
-                utxos = {}
-                for mdutxo in self.wallet_service.get_utxos_by_mixdepth().values():
-                    utxos.update(mdutxo)
-                priv_utxo_pairs, to, ts = priv_utxo_pairs_from_utxos(
-                    utxos, age, amt)
             #Pre-filter the set of external commitments that work for this
             #transaction according to its size and age.
             dummy, extdict = get_podle_commitments()
@@ -777,7 +763,18 @@ class Taker(object):
                     list(extdict.keys()), age, amt)
             else:
                 ext_valid = None
-            podle_data = generate_podle(priv_utxo_pairs, tries, ext_valid)
+            #We defer to a second round to try *all* utxos in spending mixdepth;
+            #this is because it's much cleaner to use the utxos involved
+            #in the transaction, about to be consumed, rather than use
+            #random utxos that will persist after. At this step we also
+            #allow use of external utxos in the json file.
+            mixdepth_utxos = self.wallet_service.get_utxos_by_mixdepth()[self.mixdepth]
+            if len(self.input_utxos) == len(mixdepth_utxos):
+                # Already tried the whole mixdepth
+                podle_data = generate_podle([], tries, ext_valid)
+            else:
+                priv_utxo_pairs, to, ts = priv_utxo_pairs_from_utxos(mixdepth_utxos, age, amt)
+                podle_data = generate_podle(priv_utxo_pairs, tries, ext_valid)
         if podle_data:
             jlog.debug("Generated PoDLE: " + repr(podle_data))
             return (commit_type_byte + bintohex(podle_data.commitment),

jmclient/test/commontest.py

@@ -36,6 +36,8 @@ class DummyBlockchainInterface(BlockchainInterface):
         self.fake_query_results = None
         self.qusfail = False
         self.cbh = 1
+        self.default_confs = 20
+        self.confs_for_qus = {}
 
     def rpc(self, a, b):
         return None
@@ -59,7 +61,16 @@ class DummyBlockchainInterface(BlockchainInterface):
 
     def setQUSFail(self, state):
         self.qusfail = state
-    
+
+    def set_confs(self, confs_utxos):
+        # we hook specific confirmation results
+        # for specific utxos so that query_utxo_set
+        # can return a non-constant fake value.
+        self.confs_for_qus.update(confs_utxos)
+
+    def reset_confs(self):
+        self.confs_for_qus = {}
+
     def query_utxo_set(self, txouts, includeconf=False):
         if self.qusfail:
             #simulate failure to find the utxo
@@ -99,13 +110,17 @@ class DummyBlockchainInterface(BlockchainInterface):
             return [{'value': 200000000,
                      'address': addr,
                      'script': scr,
-                     'confirms': 20}]
+                     'confirms': self.default_confs}]
         for t in txouts:
-            result_dict = {'value': 10000000000,
+            result_dict = {'value': 200000000,
                            'address': "mrcNu71ztWjAQA6ww9kHiW3zBWSQidHXTQ",
                            'script': hextobin('76a91479b000887626b294a914501a4cd226b58b23598388ac')}
             if includeconf:
-                result_dict['confirms'] = 20
+                if t in self.confs_for_qus:
+                    confs = self.confs_for_qus[t]
+                else:
+                    confs = self.default_confs
+                result_dict['confirms'] = confs
             result.append(result_dict)        
         return result
 

jmclient/test/test_taker.py

@@ -31,6 +31,7 @@ class DummyWallet(SegwitWallet):
         super().initialize(storage, get_network(), max_mixdepth=5)
         super().__init__(storage)
         self._add_utxos()
+        self.ex_utxos = {}
         self.inject_addr_get_failure = False
 
     def _add_utxos(self):
@@ -43,6 +44,26 @@ class DummyWallet(SegwitWallet):
                 script = self._ENGINE.address_to_script(data['address'])
                 self._script_map[script] = path
 
+    def add_extra_utxo(self, txid, index, value, md,
+                       address="mrcNu71ztWjAQA6ww9kHiW3zBWSQidHXTQ",
+                       i=0):
+        # note branch and index, path will be ignored in these test cases,
+        # the tree is not real.
+        # if we have extra utxos that have been added for some test,
+        # we will need to return a script and an address, although it
+        # won't be used; note we can't use base class get_utxos_by_mixdepth
+        # because the paths are fake.
+        if md not in self.ex_utxos:
+            self.ex_utxos[md] = {}
+        self.ex_utxos[md].update({(txid, index): {"mixdepth": md,
+                            "address": address,
+                            "value": value,
+                            "script": self._ENGINE.address_to_script(address),
+                            "path": (b'dummy', md, i)}})
+
+    def remove_extra_utxo(self, txid, index, md):
+        del self.ex_utxos[(txid, index)]
+
     def get_utxos_by_mixdepth(self, include_disabled=False, verbose=True,
                               includeheight=False):
         # utxostr conversion routines because taker_test_data uses hex:
@@ -53,6 +74,8 @@ class DummyWallet(SegwitWallet):
                 retval[mixdepth][utxostr_to_utxo(utxo)[1]] = val
                 val["script"] = self._ENGINE.address_to_script(val['address'])
                 val["path"] = (b'dummy', mixdepth, i)
+        for md, u in self.ex_utxos.items():
+            retval[md].update(u)
         return retval
 
     def select_utxos(self, mixdepth, amount, utxo_filter=None, select_fn=None,
@@ -149,38 +172,88 @@ def test_filter_rejection(setup_taker):
     assert not res[0]
 
 @pytest.mark.parametrize(
-    "failquery, external",
+    "mixdepth, cjamt, failquery, external, expected_success, amtpercent, age, mixdepth_extras",
     [
-        (False, False),
-        (True, False),
-        (False, True),
+        (0, 110000000, False, False, True, 0, 0, {}),
+        (0, 110000000, True, False, True, 0, 0, {}),
+        (0, 110000000, False, True, True, 0, 0, {}),
+        # this will fail to source from mixdepth 1 just because 2 < 50% of 5.5:
+        (1, 550000000, False, False, False, 50, 5, {}),
+        # this must fail to source even though the size in mixdepth 0 is enough:
+        (1, 550000000, False, False, False, 50, 5, {0: [600000000]}),
+        # this should succeed in sourcing because even though there are 9 utxos
+        # in mixdepth 0, one of them is more than 20% (the original 2BTC):
+        (0, 900000000, False, False, True, 20, 5, {0:[100000000]*8}),
+        # this case must fail since the utxos are all at 20 confs and too new:
+        (0, 110000000, False, False, False, 20, 25, {}),
+        # make the confs in the spending mixdepth insufficient, while those
+        # in another mixdepth are OK; must fail:
+        (0, 110000000, False, False, False, 20, 5, {"confchange": {0: 1}}),
     ])
-def test_make_commitment(setup_taker, failquery, external):
+def test_make_commitment(setup_taker, mixdepth, cjamt, failquery, external,
+                         expected_success, amtpercent, age, mixdepth_extras):
     def clean_up():
         jm_single().config.set("POLICY", "taker_utxo_age", old_taker_utxo_age)
         jm_single().config.set("POLICY", "taker_utxo_amtpercent", old_taker_utxo_amtpercent)
         set_commitment_file(old_commitment_file)
         jm_single().bc_interface.setQUSFail(False)
+        jm_single().bc_interface.reset_confs()
         os.remove('dummyext')
     old_commitment_file = get_commitment_file()
     with open('dummyext', 'wb') as f:
         f.write(json.dumps(t_dummy_ext, indent=4).encode('utf-8'))
     if external:
         set_commitment_file('dummyext')
+
+    # define the appropriate podle acceptance parameters in the global config:
     old_taker_utxo_age = jm_single().config.get("POLICY", "taker_utxo_age")
     old_taker_utxo_amtpercent = jm_single().config.get("POLICY", "taker_utxo_amtpercent")
-    jm_single().config.set("POLICY", "taker_utxo_age", "5")
-    jm_single().config.set("POLICY", "taker_utxo_amtpercent", "20")
-    mixdepth = 0
-    amount = 110000000
-    taker = get_taker([(mixdepth, amount, 3, "mnsquzxrHXpFsZeL42qwbKdCP2y1esN3qw", NO_ROUNDING)])
-    taker.cjamount = amount
-    taker.input_utxos = convert_utxos(t_utxos_by_mixdepth[0])
+    if expected_success:
+        # set to defaults for mainnet
+        newtua = "5"
+        newtuap = "20"
+    else:
+        newtua = str(age)
+        newtuap = str(amtpercent)
+        jm_single().config.set("POLICY", "taker_utxo_age", newtua)
+        jm_single().config.set("POLICY", "taker_utxo_amtpercent", newtuap)
+
+    taker = get_taker([(mixdepth, cjamt, 3, "mnsquzxrHXpFsZeL42qwbKdCP2y1esN3qw", NO_ROUNDING)])
+
+    # modify or add any extra utxos for this run:
+    for k, v in mixdepth_extras.items():
+        if k == "confchange":
+            for k2, v2 in v.items():
+                # set the utxos in mixdepth k2 to have confs v2:
+                cdict = taker.wallet_service.get_utxos_by_mixdepth()[k2]
+                jm_single().bc_interface.set_confs({utxo: v2 for utxo in cdict.keys()})
+        else:
+            for value in v:
+                taker.wallet_service.add_extra_utxo(
+                    os.urandom(32), 0, value, k)
+
+    taker.cjamount = cjamt
+    taker.input_utxos = taker.wallet_service.get_utxos_by_mixdepth()[mixdepth]
+    taker.mixdepth = mixdepth
     if failquery:
         jm_single().bc_interface.setQUSFail(True)
-    taker.make_commitment()
+    comm, revelation, msg = taker.make_commitment()
+    if expected_success and failquery:
+        # for manual tests, show the error message:
+        print("Failure case due to QUS fail: ")
+        print("Erromsg: ", msg)
+        assert not comm
+    elif expected_success:
+        assert comm, "podle was not generated but should have been."
+    else:
+        # in these cases we have set the podle acceptance
+        # parameters such that our in-mixdepth utxos are not good
+        # enough.
+        # for manual tests, show the errormsg:
+        print("Failure case, errormsg: ", msg)
+        assert not comm, "podle was generated but should not have been."
     clean_up()
-    
+
 def test_not_found_maker_utxos(setup_taker):
     taker = get_taker([(0, 20000000, 3, "mnsquzxrHXpFsZeL42qwbKdCP2y1esN3qw", 0, NO_ROUNDING)])
     orderbook = copy.deepcopy(t_orderbook)