Default 3rd argument of gettxout should be True

Fixes #1294.
Before this commit, calls to query_utxo_set with default arguments
would ignore the mempool and thus return utxos which were spent in
unconfirmed transactions. Thus, takers would continue negotiation of
coinjoins with makers who sent them already-spent utxos, leading to
failure at broadcast time. This was not intended behaviour; we want
takers to reject utxos that are double spent in the mempool.
This commit changes that default argument to True so that utxo set
changes in the mempool are accounted for. It also switches the name of
the includeunconf argument, which was misleading, to include_mempool,
with appropriately updated docstring.
Finally, in this commit we also ensure that callers of this function
check, where necessary, the returned confirmations field to disallow
unconfirmed utxos where that is necessary.

Author: AdamISZ

jmclient/jmclient/blockchaininterface.py

@@ -39,7 +39,7 @@ def pushtx(self, txhex):
         """pushes tx to the network, returns False if failed"""
 
     @abc.abstractmethod
-    def query_utxo_set(self, txouts, includeconf=False):
+    def query_utxo_set(self, txouts, includeconfs=False):
         """
         takes a utxo or a list of utxos
         returns None if they are spend or unconfirmed
@@ -109,7 +109,7 @@ def pushtx(self, txhex, timeout=10):
         log.debug("Pushed via Electrum successfully, hash: " + tx_hash)
         return True
 
-    def query_utxo_set(self, txout, includeconf=False):
+    def query_utxo_set(self, txout, includeconfs=False):
         """Behaves as for Core; TODO make it faster if possible.
         Note in particular a failed connection should result in
         a result list containing at least one "None" which the
@@ -141,7 +141,7 @@ def query_utxo_set(self, txout, includeconf=False):
                 'address': address,
                 'script': btc.address_to_script(address)
             }
-            if includeconf:
+            if includeconfs:
                 if int(u['height']) in [0, -1]:
                     #-1 means unconfirmed inputs
                     r['confirms'] = 0
@@ -389,15 +389,17 @@ def pushtx(self, txbin):
             return False
         return True
 
-    def query_utxo_set(self, txout, includeconf=False, includeunconf=False):
+    def query_utxo_set(self, txout, includeconfs=False, include_mempool=True):
         """If txout is either (a) a single utxo in (txidbin, n) form,
         or a list of the same, returns, as a list for each txout item,
         the result of gettxout from the bitcoind rpc for those utxos;
         if any utxo is invalid, None is returned.
-        includeconf: if this is True, the current number of confirmations
+        includeconfs: if this is True, the current number of confirmations
         of the prescribed utxo is included in the returned result dict.
-        includeunconf: if True, utxos which currently have zero confirmations
-        are included in the result set.
+        include_mempool: if True, the contents of the mempool are included;
+        this *both* means that utxos that are spent in in-mempool transactions
+        are *not* returned, *and* means that utxos that are created in the
+        mempool but have zero confirmations *are* returned.
         If the utxo is of a non-standard type such that there is no address,
         the address field in the dict is None.
         """
@@ -416,14 +418,14 @@ def query_utxo_set(self, txout, includeconf=False, includeunconf=False):
                 log.warn("Invalid utxo format, ignoring: {}".format(txo))
                 result.append(None)
                 continue
-            ret = self._rpc('gettxout', [txo_hex, txo_idx, includeunconf])
+            ret = self._rpc('gettxout', [txo_hex, txo_idx, include_mempool])
             if ret is None:
                 result.append(None)
             else:
                 result_dict = {'value': int(Decimal(str(ret['value'])) *
                                             Decimal('1e8')),
                                'script': hextobin(ret['scriptPubKey']['hex'])}
-                if includeconf:
+                if includeconfs:
                     result_dict['confirms'] = int(ret['confirmations'])
                 result.append(result_dict)
         return result

jmclient/jmclient/maker.py

@@ -82,7 +82,7 @@ def reject(msg):
         #finally, check that the proffered utxo is real, old enough, large enough,
         #and corresponds to the pubkey
         res = jm_single().bc_interface.query_utxo_set([cr_dict['utxo']],
-                                                      includeconf=True)
+                                                      includeconfs=True)
         if len(res) != 1 or not res[0]:
             reason = "authorizing utxo is not valid"
             return reject(reason)

jmclient/jmclient/taker.py

@@ -558,7 +558,8 @@ def _verify_ioauth_data(self, ioauth_data):
         return verified_data
 
     def _verify_ioauth_inputs(self, nick, utxo_list, auth_pub):
-        utxo_data = jm_single().bc_interface.query_utxo_set(utxo_list)
+        utxo_data = jm_single().bc_interface.query_utxo_set(utxo_list,
+                                                            includeconfs=True)
         if None in utxo_data:
             raise IoauthInputVerificationError([
                 "ERROR: outputs unconfirmed or already spent. utxo_data="
@@ -570,6 +571,10 @@ def _verify_ioauth_inputs(self, nick, utxo_list, auth_pub):
         # Construct the Bitcoin address for the auth_pub field
         # Ensure that at least one address from utxos corresponds.
         for inp in utxo_data:
+            if inp["confirms"] <= 0:
+                raise IoauthInputVerificationError([
+                    f"maker's ({nick}) proposed utxo is not confirmed, "
+                    "rejecting."])
             try:
                 if self.wallet_service.pubkey_has_script(
                         auth_pub, inp['script']):
@@ -756,7 +761,7 @@ def make_commitment(self):
 
         def filter_by_coin_age_amt(utxos, age, amt):
             results = jm_single().bc_interface.query_utxo_set(utxos,
-                                                    includeconf=True)
+                                                    includeconfs=True)
             newresults = []
             too_new = []
             too_small = []

jmclient/jmclient/wallet.py

@@ -2564,7 +2564,7 @@ def calculate_timelocked_fidelity_bond_value(cls, utxo_value, confirmation_time,
     def get_validated_timelocked_fidelity_bond_utxo(cls, utxo, utxo_pubkey, locktime,
             cert_expiry, current_block_height):
 
-        utxo_data = jm_single().bc_interface.query_utxo_set(utxo, includeconf=True)
+        utxo_data = jm_single().bc_interface.query_utxo_set(utxo, includeconfs=True)
         if utxo_data[0] == None:
             return None
         if utxo_data[0]["confirms"] <= 0:

jmclient/test/commontest.py

@@ -72,7 +72,7 @@ def set_confs(self, confs_utxos):
     def reset_confs(self):
         self.confs_for_qus = {}
 
-    def query_utxo_set(self, txouts, includeconf=False):
+    def query_utxo_set(self, txouts, includeconfs=False):
         if self.qusfail:
             #simulate failure to find the utxo
             return [None]
@@ -97,8 +97,8 @@ def query_utxo_set(self, txouts, includeconf=False):
                        'fd9711a2ef340750db21efb761f5f7d665d94b312332dc354e252c77e9c48349:0': [50000000, 6]}
         wallet_outs = dictchanger(wallet_outs)
 
-        if includeconf and set(txouts).issubset(set(wallet_outs)):
-            #includeconf used as a trigger for a podle check;
+        if includeconfs and set(txouts).issubset(set(wallet_outs)):
+            #includeconfs used as a trigger for a podle check;
             #here we simulate a variety of amount/age returns
             results = []
             for to in txouts:
@@ -116,7 +116,7 @@ def query_utxo_set(self, txouts, includeconf=False):
             result_dict = {'value': 200000000,
                            'address': "mrcNu71ztWjAQA6ww9kHiW3zBWSQidHXTQ",
                            'script': hextobin('76a91479b000887626b294a914501a4cd226b58b23598388ac')}
-            if includeconf:
+            if includeconfs:
                 if t in self.confs_for_qus:
                     confs = self.confs_for_qus[t]
                 else:

jmclient/test/test_wallets.py

@@ -44,10 +44,10 @@ def test_query_utxo_set(setup_wallets):
     txid2 = do_tx(wallet_service, 20000000)
     print("Got txs: ", txid, txid2)
     res1 = jm_single().bc_interface.query_utxo_set(
-        (txid, 0), includeunconf=True)
+        (txid, 0), include_mempool=True)
     res2 = jm_single().bc_interface.query_utxo_set(
         [(txid, 0), (txid2, 1)],
-        includeconf=True, includeunconf=True)
+        includeconfs=True, include_mempool=True)
     assert len(res1) == 1
     assert len(res2) == 2
     assert all([x in res1[0] for x in ['script', 'value']])