Change tumbler algo to cycle and add tests

Prior to this commit, the tumbler algorithm assumed that destination
mixdepths of INTERNAL transactions were incremented by 1, but the
underlying taker code uses (mod maxmixdepth) logic always. This commit
takes the decision to make the usage of the wallet "purely" cyclic, that
is, not only the Taker object but also the tumbler algorithm now always
treat the wallet as a cycle. This is not problematic in a tumbler
algorith (or any other schedule generation algorithm), as long as we use
the strict rule of "always exit each mixdepth with a sweep", which the
tumbler always did and this commit does not change.
Also, and importantly, several much more detailed tests of the tumbler
schedule generation have been added.

jmclient/jmclient/schedule.py

@@ -83,7 +83,8 @@ def get_amount_fractions(count):
             break
     return y
 
-def get_tumble_schedule(options, destaddrs, mixdepth_balance_dict):
+def get_tumble_schedule(options, destaddrs, mixdepth_balance_dict,
+                        max_mixdepth_in_wallet=4):
     """for the general intent and design of the tumbler algo, see the docs in
     joinmarket-org/joinmarket.
     Alterations:
@@ -93,10 +94,14 @@ def get_tumble_schedule(options, destaddrs, mixdepth_balance_dict):
     any other taker algo; it interprets floats as fractions and integers as satoshis,
     and zero as sweep (as before).
     This is a modified version of tumbler.py/generate_tumbler_tx()
+    Args:
+    * options - as specified in scripts/tumbler.py and taken from cli_options.py
+    * destaddrs - a list of valid address strings for the destination of funds
+    * mixdepth_balance_dict - a dict mapping mixdepths to balances, note zero balances are not included.
+    * max_mixdepth_in_wallet - this is actually the highest mixdepth in this JM wallet (
+      so 4 actually means 5 mixdepths in wallet; we do not consider the possibility of a wallet not
+      having a 0 mixdepth). The default value 4 is almost always used.
     """
-    #if options['mixdepthsrc'] != 0:
-    #    raise NotImplementedError("Non-zero mixdepth source not supported; "
-    #                              "restart the tumbler with --restart instead")
 
     def lower_bounded_int(thelist, lowerbound):
         return [int(l) if int(l) >= lowerbound else lowerbound for l in thelist]
@@ -154,7 +159,8 @@ def get_tumble_schedule(options, destaddrs, mixdepth_balance_dict):
                 rounding = rand_weighted_choice(len(weight_prob), weight_prob) + 1
             tx = {'amount_fraction': amount_fraction,
                   'wait': round(wait, 2),
-                  'srcmixdepth': lowest_initial_filled_mixdepth + m + options['mixdepthsrc'] + 1,
+                  'srcmixdepth': (lowest_initial_filled_mixdepth + m + options[
+                      'mixdepthsrc'] + 1) % (max_mixdepth_in_wallet + 1),
                   'makercount': makercount,
                   'destination': 'INTERNAL',
                   'rounding': rounding
@@ -168,7 +174,7 @@ def get_tumble_schedule(options, destaddrs, mixdepth_balance_dict):
     external_dest_addrs = ['addrask'] * addrask + destaddrs[::-1]
     for mix_offset in range(options['addrcount']):
         srcmix = (lowest_initial_filled_mixdepth + options['mixdepthsrc']
-            + options['mixdepthcount'] - mix_offset)
+            + options['mixdepthcount'] - mix_offset) % (max_mixdepth_in_wallet + 1)
         for tx in reversed(tx_list):
             if tx['srcmixdepth'] == srcmix:
                 tx['destination'] = external_dest_addrs[mix_offset]
@@ -176,8 +182,10 @@ def get_tumble_schedule(options, destaddrs, mixdepth_balance_dict):
         if mix_offset == 0:
             # setting last mixdepth to send all to dest
             tx_list_remove = []
-            for tx in tx_list:
-                if tx['srcmixdepth'] == srcmix:
+            ending_mixdepth = tx_list[-1]['srcmixdepth']
+            for tx in tx_list[::-1]:
+                if tx['srcmixdepth'] != ending_mixdepth:
+                    break
                 if tx['destination'] == 'INTERNAL':
                     tx_list_remove.append(tx)
                 else:

jmclient/test/test_schedule.py

@@ -73,23 +73,70 @@ def get_options():
     return options
 
 @pytest.mark.parametrize(
-    "destaddrs, txcparams, mixdepthcount",
+    "destaddrs, txcparams, mixdepthcount, mixdepthbal",
     [
+        # very simple case
         (["mzzAYbtPpANxpNVGCVBAhZYzrxyZtoix7i",
           "mifCWfmygxKhsP3qM3HZi3ZjBEJu7m39h8",
-          "mnTn9KVQQT9zy9R4E2ZGzWPK4EfcEcV9Y5"], (18,3), 4),
+          "mnTn9KVQQT9zy9R4E2ZGzWPK4EfcEcV9Y5"], (3,0), 3,
+        {0:1}),
+        # with 2 non-empty mixdepths
+        (["mzzAYbtPpANxpNVGCVBAhZYzrxyZtoix7i",
+          "mifCWfmygxKhsP3qM3HZi3ZjBEJu7m39h8",
+          "mnTn9KVQQT9zy9R4E2ZGzWPK4EfcEcV9Y5"], (7,0), 3,
+         {2:1, 3: 1}),
         #intended to trigger txcount=1 bump to 2
         (["mzzAYbtPpANxpNVGCVBAhZYzrxyZtoix7i",
           "mifCWfmygxKhsP3qM3HZi3ZjBEJu7m39h8",
-          "mnTn9KVQQT9zy9R4E2ZGzWPK4EfcEcV9Y5"], (3,2), 80),
+          "mnTn9KVQQT9zy9R4E2ZGzWPK4EfcEcV9Y5"], (3,2), 8,
+         {2:1, 3: 1}),
     ])
-def test_tumble_schedule(destaddrs, txcparams, mixdepthcount):
+def test_tumble_schedule(destaddrs, txcparams, mixdepthcount, mixdepthbal):
+    # note that these tests are currently only leaving the default
+    # value for the final argument to get_tumble_schedule, i.e. 4,
+    # and will fail if this is changed:
+    wallet_total_mixdepths = 5
     options = get_options()
     options['mixdepthcount'] = mixdepthcount
     options['txcountparams'] = txcparams
-    schedule = get_tumble_schedule(options, destaddrs, {0:1})
+    schedule = get_tumble_schedule(options, destaddrs, mixdepthbal)
+    # first, examine the destination addresses; all the requested
+    # ones should be in the list, and all the others should be one
+    # of the two standard 'code' alternatives.
     dests = [x[3] for x in schedule]
     assert set(destaddrs).issubset(set(dests))
+    nondestaddrs = [x[3] for x in schedule if x[3] not in destaddrs]
+    assert all([x in ["INTERNAL", "addrask"] for x in nondestaddrs])
+    # second: check that the total number of transactions is larger
+    # than the minimum it could be and smaller than the max;
+    # the last term accounts for the Phase 1 sweeps
+    assert len(schedule) >= (mixdepthcount - 1) * (
+        txcparams[0] - txcparams[1]) + 1 + len(mixdepthbal.items())
+    assert len(schedule) <= (mixdepthcount - 1) * (
+        txcparams[0] + txcparams[1]) + 1 + len(mixdepthbal.items())
+    # check that the source mixdepths for the phase 1 transactions are the
+    # expected, and that they are all sweeps:
+    for i, s in enumerate(schedule[:len(mixdepthbal)]):
+        assert s[1] == 0
+        assert s[0] in mixdepthbal.keys()
+    # check that the list of created transactions in Phase 2 only
+    # progresses forward, one mixdepth at a time:
+    for first, second in zip(schedule[len(mixdepthbal):-1],
+                             schedule[len(mixdepthbal) + 1:]):
+        assert (second[0] - first[0]) % wallet_total_mixdepths in [1, 0]
+    # check that the amount fractions are always total < 1
+    last_s = []
+    for s in schedule:
+        if last_s == []:
+            last_s = s
+            total_amt = 0
+            continue
+        if s[0] == last_s[0]:
+            total_amt += s[1]
+        else:
+            assert total_amt < 1
+            total_amt = 0
+        last_s = s
 
 @pytest.mark.parametrize(
     "destaddrs, txcparams, mixdepthcount, lastcompleted, makercountrange",

scripts/tumbler.py

@@ -110,7 +110,7 @@ def main():
     else:
         #Create a new schedule from scratch
         schedule = get_tumble_schedule(options, destaddrs,
-            wallet.get_balance_by_mixdepth())
+            wallet.get_balance_by_mixdepth(), wallet_service.mixdepth)
         tumble_log.info("TUMBLE STARTING")
         with open(os.path.join(logsdir, options['schedulefile']), "wb") as f:
             f.write(schedule_to_text(schedule))