From a3e1ba3c2572da5545c3a87d4d9b38919d7b828a Mon Sep 17 00:00:00 2001 From: Adam Gibson Date: Sun, 29 May 2022 15:42:08 -0500 Subject: [PATCH] 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. --- jmclient/jmclient/blockchaininterface.py | 20 +++++++++++--------- jmclient/jmclient/maker.py | 2 +- jmclient/jmclient/taker.py | 9 +++++++-- jmclient/jmclient/wallet.py | 2 +- jmclient/test/commontest.py | 8 ++++---- jmclient/test/test_wallets.py | 4 ++-- 6 files changed, 26 insertions(+), 19 deletions(-) diff --git a/jmclient/jmclient/blockchaininterface.py b/jmclient/jmclient/blockchaininterface.py index 46627b4..c506459 100644 --- a/jmclient/jmclient/blockchaininterface.py +++ b/jmclient/jmclient/blockchaininterface.py @@ -39,7 +39,7 @@ class BlockchainInterface(object): """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 @@ class ElectrumWalletInterface(BlockchainInterface): #pragma: no cover 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 @@ class ElectrumWalletInterface(BlockchainInterface): #pragma: no cover '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 @@ class BitcoinCoreInterface(BlockchainInterface): 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 @@ class BitcoinCoreInterface(BlockchainInterface): 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 diff --git a/jmclient/jmclient/maker.py b/jmclient/jmclient/maker.py index 89ec9df..43c0ddc 100644 --- a/jmclient/jmclient/maker.py +++ b/jmclient/jmclient/maker.py @@ -82,7 +82,7 @@ class Maker(object): #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) diff --git a/jmclient/jmclient/taker.py b/jmclient/jmclient/taker.py index 67af570..228bd33 100644 --- a/jmclient/jmclient/taker.py +++ b/jmclient/jmclient/taker.py @@ -558,7 +558,8 @@ class Taker(object): 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 @@ class Taker(object): # 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 @@ class Taker(object): 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 = [] diff --git a/jmclient/jmclient/wallet.py b/jmclient/jmclient/wallet.py index 1938f14..2f373fb 100644 --- a/jmclient/jmclient/wallet.py +++ b/jmclient/jmclient/wallet.py @@ -2564,7 +2564,7 @@ class FidelityBondMixin(object): 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: diff --git a/jmclient/test/commontest.py b/jmclient/test/commontest.py index b06b618..a6689ac 100644 --- a/jmclient/test/commontest.py +++ b/jmclient/test/commontest.py @@ -72,7 +72,7 @@ class DummyBlockchainInterface(BlockchainInterface): 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 @@ class DummyBlockchainInterface(BlockchainInterface): '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 @@ class DummyBlockchainInterface(BlockchainInterface): 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: diff --git a/jmclient/test/test_wallets.py b/jmclient/test/test_wallets.py index dec25b4..f085469 100644 --- a/jmclient/test/test_wallets.py +++ b/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']])