add more detailed comments to maker and daemon code

jmclient/jmclient/maker.py

@@ -29,6 +29,12 @@ class Maker(object):
         self.aborted = False
 
     def try_to_create_my_orders(self):
+        """Because wallet syncing is not synchronous(!),
+        we cannot calculate our offers until we know the wallet
+        contents, so poll until BlockchainInterface.wallet_synced
+        is flagged as True. TODO: Use a deferred, probably.
+        Note that create_my_orders() is defined by subclasses.
+        """
         if not jm_single().bc_interface.wallet_synced:
             return
         self.offerlist = self.create_my_orders()
@@ -91,6 +97,11 @@ class Maker(object):
         return (True, utxos, auth_pub, cj_addr, change_addr, btc_sig)
 
     def on_tx_received(self, nick, txhex, offerinfo):
+        """Called when the counterparty has sent an unsigned
+        transaction. Sigs are created and returned if and only
+        if the transaction passes verification checks (see
+        verify_unsigned_tx()).
+        """
         try:
             tx = btc.deserialize(txhex)
         except IndexError as e:
@@ -123,6 +134,13 @@ class Maker(object):
         return (True, sigs)
 
     def verify_unsigned_tx(self, txd, offerinfo):
+        """This code is security-critical.
+        Before signing the transaction the Maker must ensure
+        that all details are as expected, and most importantly
+        that it receives the exact number of coins to expected
+        in total. The data is taken from the offerinfo dict and
+        compared with the serialized txhex.
+        """
         tx_utxo_set = set(ins['outpoint']['hash'] + ':' + str(
                 ins['outpoint']['index']) for ins in txd['ins'])
 
@@ -131,6 +149,8 @@ class Maker(object):
         cjaddr_script = btc.address_to_script(cjaddr)
         changeaddr = offerinfo["changeaddr"]
         changeaddr_script = btc.address_to_script(changeaddr)
+        #Note: this value is under the control of the Taker,
+        #see comment below.
         amount = offerinfo["amount"]
         cjfee = offerinfo["offer"]["cjfee"]
         txfee = offerinfo["offer"]["txfee"]
@@ -139,12 +159,25 @@ class Maker(object):
         if not tx_utxo_set.issuperset(my_utxo_set):
             return (False, 'my utxos are not contained')
 
+        #The three lines below ensure that the Maker receives
+        #back what he puts in, minus his bitcointxfee contribution,
+        #plus his expected fee. These values are fully under
+        #Maker control so no combination of messages from the Taker
+        #can change them.
+        #(mathematically: amount + expected_change_value is independent
+        #of amount); there is not a (known) way for an attacker to
+        #alter the amount (note: !fill resubmissions *overwrite*
+        #the active_orders[dict] entry in daemon), but this is an
+        #extra layer of safety.
         my_total_in = sum([va['value'] for va in utxos.values()])
         real_cjfee = calc_cj_fee(ordertype, cjfee, amount)
         expected_change_value = (my_total_in - amount - txfee + real_cjfee)
         jlog.info('potentially earned = {}'.format(real_cjfee - txfee))
         jlog.info('mycjaddr, mychange = {}, {}'.format(cjaddr, changeaddr))
 
+        #The remaining checks are needed to ensure
+        #that the coinjoin and change addresses occur
+        #exactly once with the required amts, in the output.
         times_seen_cj_addr = 0
         times_seen_change_addr = 0
         for outs in txd['outs']:
@@ -164,6 +197,10 @@ class Maker(object):
         return (True, None)
 
     def modify_orders(self, to_cancel, to_announce):
+        """This code is called on unconfirm and confirm callbacks,
+        and replaces existing orders with new ones, or just cancels
+        old ones.
+        """
         jlog.info('modifying orders. to_cancel={}\nto_announce={}'.format(
                 to_cancel, to_announce))
         for oid in to_cancel:

jmdaemon/jmdaemon/daemon_protocol.py

@@ -227,6 +227,9 @@ class JMDaemonServerProtocol(amp.AMP, OrderbookWatch):
 
     @JMFill.responder
     def on_JM_FILL(self, amount, commitment, revelation, filled_offers):
+        """Takes the necessary data from the Taker and initiates the Stage 1
+	interaction with the Makers.
+	"""
         if not (self.jm_state == 1 and isinstance(amount, int) and amount >=0):
             return {'accepted': False}
         self.cjamount = amount
@@ -245,6 +248,9 @@ class JMDaemonServerProtocol(amp.AMP, OrderbookWatch):
 
     @JMMakeTx.responder
     def on_JM_MAKE_TX(self, nick_list, txhex):
+        """Taker sends the prepared unsigned transaction
+	to all the Makers in nick_list
+	"""
         if not self.jm_state == 4:
             log.msg("Make tx was called in wrong state, rejecting")
             return {'accepted': False}
@@ -262,6 +268,10 @@ class JMDaemonServerProtocol(amp.AMP, OrderbookWatch):
 
     @JMAnnounceOffers.responder
     def on_JM_ANNOUNCE_OFFERS(self, to_announce, to_cancel, offerlist):
+        """Called by Maker to reset his current offerlist;
+	Daemon decides what messages (cancel, announce) to
+	send to the message channel.
+	"""
         if self.role != "MAKER":
             return
         to_announce = json.loads(to_announce)
@@ -275,6 +285,10 @@ class JMDaemonServerProtocol(amp.AMP, OrderbookWatch):
 
     @JMIOAuth.responder
     def on_JM_IOAUTH(self, nick, utxolist, pubkey, cjaddr, changeaddr, pubkeysig):
+        """Daemon constructs full !ioauth message to be sent on message
+	channel based on data from Maker. Relevant data (utxos, addresses)
+	are stored in the active_orders dict keyed by the nick of the Taker.
+	"""
         nick, utxolist, pubkey, cjaddr, changeaddr, pubkeysig = [_byteify(
             x) for x in nick, utxolist, pubkey, cjaddr, changeaddr, pubkeysig]
         if not self.role == "MAKER":
@@ -301,6 +315,11 @@ class JMDaemonServerProtocol(amp.AMP, OrderbookWatch):
 
     @JMTXSigs.responder
     def on_JM_TX_SIGS(self, nick, sigs):
+        """Signatures that the Maker has produced
+	are passed here to the daemon as a list and
+	broadcast one by one. TODO: could shorten this,
+	have more than one sig per message.
+	"""
         sigs = _byteify(json.loads(sigs))
         for sig in sigs:
             self.mcc.prepare_privmsg(nick, "sig", sig)
@@ -323,7 +342,17 @@ class JMDaemonServerProtocol(amp.AMP, OrderbookWatch):
 
     @maker_only
     def on_order_fill(self, nick, oid, amount, taker_pk, commit):
-        """Handled locally in daemon.
+        """Handled locally in daemon. This is the start of
+        communication with the Taker. Does the following:
+
+        * Immediately rejects if commitment is invalid or already used.
+        * Checks that the fill is against a valid offer.
+        * Establishes encryption with a new ephemeral keypair
+        * Creates the amount, commitment and keypair fields in
+          active_orders[nick] (or resets if already existing).
+
+        Processing will only return to the Maker once the conversation
+        up to !ioauth is complete.
         """
         if nick in self.active_orders:
             log.msg("Restarting transaction for nick: " + nick)
@@ -353,6 +382,10 @@ class JMDaemonServerProtocol(amp.AMP, OrderbookWatch):
         except NaclError as e:
             log.msg("Unable to set up cryptobox with counterparty: " + repr(e))
             self.mcc.send_error(nick, "Invalid nacl pubkey: " + taker_pk)
+            return
+        #Note this sets the *whole* dict, old entries (e.g. changeaddr)
+        #are removed, so we can't have a conflict between old and new
+        #versions of active_orders[nick]
         self.active_orders[nick] = {"crypto_box": crypto_box,
                                         "kp": kp,
                                         "offer": offer,
@@ -362,6 +395,11 @@ class JMDaemonServerProtocol(amp.AMP, OrderbookWatch):
 
     @maker_only
     def on_seen_auth(self, nick, commitment_revelation):
+        """Passes to Maker the !auth message from the Taker,
+	for processing. This will include validating the PoDLE
+	commitment revelation against the existing commitment,
+	which was already stored in active_orders[nick].
+	"""
         if not nick in self.active_orders:
             return
         ao =self.active_orders[nick]
@@ -406,10 +444,15 @@ class JMDaemonServerProtocol(amp.AMP, OrderbookWatch):
 
     @maker_only
     def on_push_tx(self, nick, txhex):
+        """Not yet implemented; ignore rather than raise.
+	"""
         log.msg('received pushtx message, ignoring, TODO')
 
     @maker_only
     def on_seen_tx(self, nick, txhex):
+        """Passes the txhex to the Maker for verification
+	and signing. Note the security checks occur in Maker.
+	"""
         if nick not in self.active_orders:
             return
         #we send a copy of the entire "active_orders" entry except the cryptobox,
@@ -535,6 +578,16 @@ class JMDaemonServerProtocol(amp.AMP, OrderbookWatch):
         self.mcc.prepare_privmsg(counterparty, 'hp2', commit)
 
     def respondToIoauths(self, accepted):
+        """Sends the full set of data from the Makers to the
+        Taker after processing of first stage is completed,
+        using the JMFillResponse command. But if the responses
+        were not accepted (including, not sufficient number
+        of responses), we send the list of Makers who did not
+        respond to the Taker, instead of the ioauth data,
+        so that the Taker can keep track of non-responders
+        (although note this code is not yet quite ideal, see
+        comments below).
+        """
         if self.jm_state != 2:
             #this can be called a second time on timeout, in which case we
             #do nothing