Shuffle lowest amount - Fix #68 (#87)

- Version=200 of the protocol.  We now set the shuffle amount based on the lowest player in the pool we are shuffling with.

This has a lot of benefits as discussed in issue #68.

- Also got rid of the wallet.storage value for "Spend mode" -- it defaults to "Spend Shuffled" on each wallet restart.

Also some refactoring and fixups.

* Made the "Too small" and "Too big" coin statuses take precedence over 'Unconfirmed' in the coins tab. This way you see right away as dust is created that it will not participate in future shuffles.

* nits

lib/version.py

@@ -1,4 +1,4 @@
-PACKAGE_VERSION  = '3.9.1ShufBeta'  # version of the client package
+PACKAGE_VERSION  = '3.9.2ShufBeta'  # version of the client package
 PROTOCOL_VERSION = '1.4'     # protocol version requested
 
 # The hash of the mnemonic seed must begin with this

plugins/shuffle/client.py

@@ -31,10 +31,11 @@ class ProtocolThread(threading.Thread, PrintErrorThread):
     in class 'Round' in coin_shuffle.py which this class wraps and calls into).
     """
     def __init__(self, *, host, port, coin,
-                 amount, fee, sk, sks, inputs, pubk,
-                 addr_new_addr, change_addr, version, logger=None, ssl=False,
-                 comm_timeout=60.0, ctimeout=5.0, total_amount=0,
-                 fake_change=False,
+                 scale, fee, sk, sks, inputs, pubk,
+                 addr_new_addr, change_addr, version, coin_value,
+                 logger=None, ssl=False,
+                 comm_timeout=60.0, ctimeout=5.0,
+                 reserved_change=True,
                  typ=Messages.DEFAULT  # NB: For now only 'DEFAULT' type is supported
                  ):
 
@@ -43,7 +44,7 @@ def __init__(self, *, host, port, coin,
         self.version = version
         self.type = typ
         self.messages = Messages()
-        self.comm = Comm(host, port, ssl=ssl, timeout = comm_timeout, infoText = "Scale: {}".format(amount))
+        self.comm = Comm(host, port, ssl=ssl, timeout = comm_timeout, infoText = "Scale: {}".format(scale))
         self.ctimeout = ctimeout
         if not logger:
             self.logger = ChannelWithPrint()
@@ -54,19 +55,20 @@ def __init__(self, *, host, port, coin,
         self.number = None
         self.number_of_players = None
         self.players = {}
-        self.amount = amount
+        self.scale = scale
         self.coin = coin
         self.fee = fee
         self.sk = sk
         self.sks = sks
         self.inputs = inputs
-        self.total_amount = total_amount
+        assert coin_value > 0, "Coin value must be > 0!"
+        self.coin_value = coin_value
         self.all_inputs = {}
         self.addr_new_addr = addr_new_addr # used by outside code
         self.addr_new = addr_new_addr.to_storage_string() # used by internal protocol code
         self.change_addr = change_addr #outside
         self.change = change_addr.to_storage_string() #inside
-        self.fake_change = fake_change
+        self.reserved_change = reserved_change
         self.protocol = None
         self.tx = None
         self.ts = time.time()
@@ -84,7 +86,7 @@ def wrapper(self):
     @not_time_to_die
     def register_on_the_pool(self):
         "Register the player on the pool"
-        self.messages.make_greeting(self.vk, int(self.amount), self.type, self.version)
+        self.messages.make_greeting(self.vk, int(self.scale), self.type, self.version)
         msg = self.messages.packets.SerializeToString()
         self.comm.send(msg)
         req = self.comm.recv()
@@ -164,10 +166,10 @@ def start_protocol(self):
         self.protocol = Round(
             coin_utils, crypto, self.messages,
             self.comm, self.comm, self.logger,
-            self.session, begin_phase, self.amount, self.fee,
+            self.session, begin_phase, self.scale, self.fee,
             self.sk, self.sks, self.all_inputs, self.vk,
             self.players, self.addr_new, self.change, self.coin,
-            total_amount = self.total_amount, fake_change = self.fake_change
+            coin_value = self.coin_value
         )
         if not self.done.is_set():
             self.protocol.start_protocol()
@@ -196,7 +198,7 @@ def run(self):
                 return
             self.start_protocol()
         finally:
-            self.logger.send("Exit: Scale '{}' Coin '{}'".format(self.amount, self.coin))
+            self.logger.send("Exit: Scale '{}' Coin '{}'".format(self.scale, self.coin))
             self.comm.close()  # simply force socket close if exiting thread for any reason
 
     def stop(self):
@@ -220,7 +222,7 @@ def join(self, timeout_ignored=None):
 
     def diagnostic_name(self):
         n = super().diagnostic_name()
-        return "{} <Scale: {}> ".format(n, self.amount)
+        return "{} <Scale: {}> ".format(n, self.scale)
 
 
 def keys_from_priv(priv_key):
@@ -240,7 +242,7 @@ def generate_random_sk():
 class BackgroundShufflingThread(threading.Thread, PrintErrorThread):
 
     scales = (
-        1000000000, # 10.0    BCH ➡➡
+        1000000000, # 10.0    BCH ➡
         100000000,  #  1.0    BCH ➡
         10000000,   #  0.1    BCH ➝
         1000000,    #  0.01   BCH ➟
@@ -249,10 +251,10 @@ class BackgroundShufflingThread(threading.Thread, PrintErrorThread):
     )
 
     # The below defaults control coin selection and which pools (scales) we use
-    PROTOCOL_VERSION = 100  # protocol version. old clients use 0. Must be an int. Version=100 is for the new fee-270. In the future this may be a dynamic quantity but for now it's always this value.
+    PROTOCOL_VERSION = 200  # protocol version. old clients use 0. Must be an int. Version=100 is for the new fee-270. Version=200 is for new dynamic amounts. In the future this may be a dynamic quantity but for now it's always this value.
     FEE = 270  # Fee formula should be roughly 270 for first input + 200 for each additional input. Right now we support only 1 input per shuffler.
     SORTED_SCALES = sorted(scales)
-    SCALE_ARROWS = ('→','⇢','➟','➝','➡','➡➡')  # if you add a scale above, add an arrow here, in reverse order from above
+    SCALE_ARROWS = ('→','⇢','➟','➝','➡','➡')  # if you add a scale above, add an arrow here, in reverse order from above
     assert len(SORTED_SCALES) == len(SCALE_ARROWS), "Please add a scale arrow if you modify the scales!"
     SCALE_ARROW_DICT = dict(zip(SORTED_SCALES, SCALE_ARROWS))
     SCALE_0 = SORTED_SCALES[0]
@@ -444,7 +446,7 @@ def _loopCondition(t0):
                     ''' Got a message from the ProtocolThread '''
 
                     killme, thr, message = tup
-                    scale, sender = thr.amount, thr.coin
+                    scale, sender = thr.scale, thr.coin
                     if killme:
                         res = self.stop_protocol_thread(thr, scale, sender, message) # implicitly forwards message to gui thread
                         if res:
@@ -518,7 +520,7 @@ def stop_protocol_thread(self, thr, scale, sender, message):
                 if message.startswith("Error"):
                     # unreserve addresses that were previously reserved iff error
                     self.wallet._addresses_cashshuffle_reserved.discard(thr.addr_new_addr)
-                    if not thr.fake_change:
+                    if thr.reserved_change:
                         self.wallet._addresses_cashshuffle_reserved.discard(thr.change_addr)
                     #self.print_error("Unreserving", thr.addr_new_addr, thr.change_addr)
             self.tell_gui_to_refresh()
@@ -541,12 +543,12 @@ def protocol_thread_callback(self, thr, message):
         ''' This callback runs in the ProtocolThread's thread context '''
         def signal_stop_thread(thr, message):
             ''' Sends the stop request to our run() thread, which will join on this thread context '''
-            self.print_error("Signalling stop for scale: {}".format(thr.amount))
+            self.print_error("Signalling stop for scale: {}".format(thr.scale))
             self.shared_chan.send((True, thr, message))
         def fwd_message(thr, message):
-            #self.print_error("Fwd msg for: Scale='{}' Msg='{}'".format(thr.amount, message))
+            #self.print_error("Fwd msg for: Scale='{}' Msg='{}'".format(thr.scale, message))
             self.shared_chan.send((False, thr, message))
-        scale = thr.amount
+        scale = thr.scale
         thr.ts = time.time()
         self.print_error("Scale: {} Message: '{}'".format(scale, message.strip()))
         if message.startswith("Error") or message.startswith("Exit"):
@@ -629,24 +631,34 @@ def get_coin_for_shuffling(scale, coins):
                 output = address
         # Reserve the output address so other threads don't use it
         self.wallet._addresses_cashshuffle_reserved.add(output)   # NB: only modify this when holding wallet locks
-        # Check if we will really use the change address. We won't be receving to it if the change is below dust threshold (see #67)
-        will_receive_change = coin['value'] - scale - self.FEE >= dust_threshold(Network.get_instance())
-        if will_receive_change:
+        # Check if we will really use the change address. We definitely won't
+        # be receving to it if the change is below dust threshold (see #67).
+        # Furthermore, we may not receive change even if this check predicts we
+        # will due to #68.
+        may_receive_change = coin['value'] - scale - self.FEE >= dust_threshold(Network.get_instance())
+        if may_receive_change:
             change = self.wallet.cashshuffle_get_new_change_address(for_shufflethread=True)
-            # We anticipate using the change address in the shuffle tx, so reserve this address
+            # We anticipate (maybe) using the change address in the shuffle tx,
+            # so reserve this address. Note that due to "smallest player raises
+            # shuffle amount" rules in version=200+ (#68) we WON'T necessarily
+            # USE this change address. (In that case it will be freed up later
+            # after shuffling anyway so no address leaking occurs).
+            # We just reserve it if we think we MAY need it.
             self.wallet._addresses_cashshuffle_reserved.add(change)
         else:
-            # We still have to specify a change address to the protocol even if it won't be used. :/
-            # We'll just take whatever address. The leftover dust amount will go to fee.
+            # (The leftover dust amount will go to fee.)
+            # We still have to specify a change address to the protocol even if
+            # it definitely won't be used. :/
+            # We'll just take 'whatever' address.
             change = self.wallet.get_change_addresses()[0]
-        self.print_error("Scale {} Coin {} OutAddr {} {} {} make_protocol_thread".format(scale, utxo_name, output.to_storage_string(), "Change" if will_receive_change else "FakeChange",change.to_storage_string()))
+        self.print_error("Scale {} Coin {} OutAddr {} {} {} make_protocol_thread".format(scale, utxo_name, output.to_storage_string(), "Change" if may_receive_change else "FakeChange", change.to_storage_string()))
         #self.print_error("Reserved addresses:", self.wallet._addresses_cashshuffle_reserved)
         ctimeout = 12.5 if (Network.get_instance() and Network.get_instance().get_proxies()) else 5.0 # allow for 12.5 second connection timeouts if using a proxy server
         thr = ProtocolThread(host=self.host, port=self.port, ssl=self.ssl,
                              comm_timeout=self.timeout, ctimeout=ctimeout,  # comm timeout and connect timeout
                              coin=utxo_name,
-                             amount=scale, fee=self.FEE, total_amount=coin['value'],
-                             addr_new_addr=output, change_addr=change, fake_change=not will_receive_change,
+                             scale=scale, fee=self.FEE, coin_value=coin['value'],
+                             addr_new_addr=output, change_addr=change, reserved_change=may_receive_change,
                              sk=id_sk, sks=sks, inputs=inputs, pubk=id_pub,
                              logger=None, version=self.version, typ=self.type)
         thr.logger = ChannelSendLambda(lambda msg: self.protocol_thread_callback(thr, msg))

plugins/shuffle/coin_shuffle.py

@@ -13,9 +13,9 @@ class Round(PrintErrorThread):
 
     def __init__(self, coin_utils, crypto, messages,
                  inchan, outchan, logchan,
-                 session, phase, amount, fee,
+                 session, phase, scale, fee,
                  sk, sks, inputs, pubkey, players, addr_new, change, utxo,
-                 total_amount = 0, fake_change = False):
+                 coin_value):
         self.coin_utils = coin_utils
         self.crypto = crypto
         self.inchan = inchan
@@ -24,17 +24,19 @@ def __init__(self, coin_utils, crypto, messages,
         self.session = session
         self.messages = messages
         self.phase = phase
-        self.total_amount = total_amount
-        assert (amount > 0), 'Wrong amount value'
-        self.amount = amount
-        assert (fee > 0), 'Wrong fee value'
+        assert coin_value > 0, 'Coin value must be > 0!'
+        self.coin_value = coin_value
+        assert scale > 0, 'Scale value must be > 0!'
+        self.scale = scale
+        self.shuffle_amount = scale # this will grow to be the largest amount possible based on the smallest player participating
+        assert fee > 0, 'Fee value must be > 0!'
         self.fee = fee
         self.sk = sk
         self.sks = sks
         self.inputs = inputs
         self.me = None
         self.number_of_players = None
-        assert(isinstance(players, dict)), "Players should be stored in dict"
+        assert isinstance(players, dict), "Players should be stored in a dict"
         self.players = players
         self.number_of_players = len(players)
         self.vk = pubkey
@@ -49,7 +51,7 @@ def __init__(self, coin_utils, crypto, messages,
         self.debug = False
         self.transaction = None
         self.tx = None
-        self.fake_change = fake_change
+        self.did_use_change = True  # This will get recomputed later as the shuffle proceeds based on actual amounts in shuffle (#68)
         self.done = None
         if self.number_of_players == len(set(players.values())):
             if self.vk in players.values():
@@ -77,21 +79,21 @@ def start_protocol(self):
         3. Broadcasts the new key for other players
         4. Starts the main protocol loop
         """
-        assert (self.amount > 0), "Wrong amount for transction"
-        self.log_message("begins CoinShuffle protocol " + " with " +
-                         str(self.number_of_players) + " players.")
+        assert self.scale > 0, "Wrong scale for transaction"
+        self.log_message("begins CoinShuffle protocol with {} players."
+                         .format(self.number_of_players))
         try:
-            if self.blame_insufficient_funds():
+            if self.check_and_blame_insufficient_funds():  # NB: this may raise AssertionError. If it does, we want the crash reporter.
                 self.broadcast_new_key()
             self.protocol_loop()
         except OSError as e: # Socket closed or timed out
-            self.print_error(str(e))
+            self.print_error(repr(e))
             self.logchan.send("Error: Socket closed or timed out")
         except BadServerPacketError as e:
-            self.print_error(str(e))
-            self.logchan.send(ERR_BAD_SERVER_PREFIX + (" {}".format(str(e))))
+            self.print_error(repr(e))
+            self.logchan.send("{} {}".format(ERR_BAD_SERVER_PREFIX, str(e)))
         except ImplementationMissing as e:
-            self.print_error(str(e))
+            self.print_error(repr(e))
             self.logchan.send("Error: ImplementationMissing -- original programmer's implentation is incomplete. FIXME!")
         finally:
             self.done = True
@@ -301,7 +303,7 @@ def process_equivocation_check(self):
                     return
             self.phase = 'VerificationAndSubmission'
             self.log_message("reaches phase 5")
-            self.transaction = self.coin_utils.make_unsigned_transaction(self.amount,
+            self.transaction = self.coin_utils.make_unsigned_transaction(self.shuffle_amount,
                                                                          self.fee,
                                                                          self.inputs,
                                                                          self.new_addresses,
@@ -374,11 +376,11 @@ def _get_total_scale_change_fee_str(self):
         "total_input scale change fee" used in the shuffle. Useful for the
         shuffle_txid: internal message '''
         fee = self.fee
-        chg = self.total_amount - self.amount - self.fee
-        if self.fake_change:
+        chg = self.coin_value - self.shuffle_amount - self.fee
+        if not self.did_use_change:
             fee += chg
             chg = 0
-        return "{} {} {} {}".format(self.total_amount, self.amount, chg, fee)
+        return "{} {} {} {} {}".format(self.coin_value, self.shuffle_amount, chg, fee, self.scale)
 
     def _get_tentative_shuffle_string(self):
         return "{} {} {}".format(self.utxo, self.addr_new, self._get_total_scale_change_fee_str())
@@ -675,23 +677,33 @@ def log_message(self, message):
         self.logchan.send("Player " + str(self.me) + " " + message)
 
 # Miscellaneous functions
-    def blame_insufficient_funds(self):
+    def check_and_blame_insufficient_funds(self):
         """
         Checks for all players to have a sufficient funds to do the shuffling
         Enter the Blame phase if someone have no funds for shuffling
         """
         offenders = list()
-
-        for player,inp in self.inputs.items():
-            is_funds_sufficient = self.coin_utils.check_inputs_for_sufficient_funds(inp, self.amount + self.fee)
+        totals = set()
+        self.shuffle_amount = self.scale
+        for player, inp in self.inputs.items():
+            is_funds_sufficient, tot = self.coin_utils.check_inputs_for_sufficient_funds_and_return_total(inp, self.scale + self.fee)
             if is_funds_sufficient is None:
                 self.logchan.send("Error: Check inputs for sufficient funds failed!")
                 self.done = True
                 return None
             elif not is_funds_sufficient:
                 offenders.append(player)
+            else:
+                assert tot is not None
+                totals.add(tot)
         if len(offenders) == 0:
             self.log_message("finds sufficient funds")
+            assert totals
+            self.shuffle_amount = min(totals) - self.fee
+            self.log_message("adjusts shuffle amount to {} BCH".format(self.shuffle_amount / 1e8))
+            assert self.shuffle_amount >= self.scale
+            # recompute did_use_change here.
+            self.did_use_change = self.coin_value - self.shuffle_amount - self.fee >= self.coin_utils.dust_threshold()
             return True
         else:
             self.phase = "Blame"