From f69cb37bb3802b7b1990d29ef6b6dd568e5767c4 Mon Sep 17 00:00:00 2001 From: Adam Gibson Date: Wed, 25 Nov 2020 20:28:24 +0000 Subject: [PATCH] Choose maker offers based only on our wallet type Before this commit, the taker would choose offers from the pit based on the setting of `native` in the `[POLICY]` section of the config object; however this could lead to users unwittingly choosing the wrong offer type, i.e. one that is incompatible with their own wallets, which could result in coinjoins with mixed address types. This commit fixes that error by only selecting offers that are compatible with the return value of `BaseWallet.get_txtype()`. Also fixes up tests for this wallet type enforcement. --- jmclient/jmclient/taker.py | 18 ++++++++++++------ jmclient/test/test_coinjoin.py | 10 +++++++--- jmclient/test/test_taker.py | 27 +++++++++++++++------------ 3 files changed, 34 insertions(+), 21 deletions(-) diff --git a/jmclient/jmclient/taker.py b/jmclient/jmclient/taker.py index 6858dc4..16021dc 100644 --- a/jmclient/jmclient/taker.py +++ b/jmclient/jmclient/taker.py @@ -250,12 +250,15 @@ class Taker(object): if sweep: self.orderbook = orderbook #offers choosing deferred to next step else: - if jm_single().config.get("POLICY", "segwit") == "false": + if self.wallet_service.get_txtype() == "p2pkh": allowed_types = ["reloffer", "absoffer"] - elif jm_single().config.get("POLICY", "native") == "false": + elif self.wallet_service.get_txtype() == "p2sh-p2wpkh": allowed_types = ["swreloffer", "swabsoffer"] - else: + elif self.wallet_service.get_txtype() == "p2wpkh": allowed_types = ["sw0reloffer", "sw0absoffer"] + else: + jlog.error("Unrecognized wallet type, taker cannot continue.") + return False self.orderbook, self.total_cj_fee = choose_orders( orderbook, self.cjamount, self.n_counterparties, self.order_chooser, self.ignored_makers, allowed_types=allowed_types, @@ -324,12 +327,15 @@ class Taker(object): txtype=self.wallet_service.get_txtype()) jlog.debug("We have a fee estimate: "+str(self.total_txfee)) total_value = sum([va['value'] for va in self.input_utxos.values()]) - if jm_single().config.get("POLICY", "segwit") == "false": + if self.wallet_service.get_txtype() == "p2pkh": allowed_types = ["reloffer", "absoffer"] - elif jm_single().config.get("POLICY", "native") == "false": + elif self.wallet_service.get_txtype() == "p2sh-p2wpkh": allowed_types = ["swreloffer", "swabsoffer"] - else: + elif self.wallet_service.get_txtype() == "p2wpkh": allowed_types = ["sw0reloffer", "sw0absoffer"] + else: + jlog.error("Unrecognized wallet type, taker cannot continue.") + return False self.orderbook, self.cjamount, self.total_cj_fee = choose_sweep_orders( self.orderbook, total_value, self.total_txfee, self.n_counterparties, self.order_chooser, diff --git a/jmclient/test/test_coinjoin.py b/jmclient/test/test_coinjoin.py index 54edb84..11e499c 100644 --- a/jmclient/test/test_coinjoin.py +++ b/jmclient/test/test_coinjoin.py @@ -21,6 +21,10 @@ import jmbitcoin as btc testdir = os.path.dirname(os.path.realpath(__file__)) log = get_log() +# map wallet types to offer types: +absoffer_type_map = {LegacyWallet: "absoffer", + SegwitLegacyWallet: "swabsoffer", + SegwitWallet: "sw0absoffer"} def make_wallets_to_list(make_wallets_data): wallets = [None for x in range(len(make_wallets_data))] @@ -137,7 +141,7 @@ def test_simple_coinjoin(monkeypatch, tmpdir, setup_cj, wallet_cls): makers = [YieldGeneratorBasic( wallet_services[i], - [0, 2000, 0, 'sw0absoffer', 10**7]) for i in range(MAKER_NUM)] + [0, 2000, 0, absoffer_type_map[wallet_cls], 10**7]) for i in range(MAKER_NUM)] create_orders(makers) orderbook = create_orderbook(makers) @@ -182,7 +186,7 @@ def test_coinjoin_mixdepth_wrap_taker(monkeypatch, tmpdir, setup_cj): cj_fee = 2000 makers = [YieldGeneratorBasic( wallet_services[i], - [0, cj_fee, 0, 'sw0absoffer', 10**7]) for i in range(MAKER_NUM)] + [0, cj_fee, 0, absoffer_type_map[SegwitWallet], 10**7]) for i in range(MAKER_NUM)] create_orders(makers) orderbook = create_orderbook(makers) @@ -238,7 +242,7 @@ def test_coinjoin_mixdepth_wrap_maker(monkeypatch, tmpdir, setup_cj): cj_fee = 2000 makers = [YieldGeneratorBasic( wallet_services[i], - [0, cj_fee, 0, 'sw0absoffer', 10**7]) for i in range(MAKER_NUM)] + [0, cj_fee, 0, absoffer_type_map[SegwitWallet], 10**7]) for i in range(MAKER_NUM)] create_orders(makers) orderbook = create_orderbook(makers) assert len(orderbook) == MAKER_NUM diff --git a/jmclient/test/test_taker.py b/jmclient/test/test_taker.py index 43078f0..a338084 100644 --- a/jmclient/test/test_taker.py +++ b/jmclient/test/test_taker.py @@ -12,7 +12,7 @@ import struct from base64 import b64encode from jmbase import utxostr_to_utxo, hextobin from jmclient import load_test_config, jm_single, set_commitment_file,\ - get_commitment_file, SegwitLegacyWallet, Taker, VolatileStorage,\ + get_commitment_file, SegwitWallet, Taker, VolatileStorage,\ get_network, WalletService, NO_ROUNDING, BTC_P2PKH,\ NotEnoughFundsException from taker_test_data import t_utxos_by_mixdepth, t_orderbook,\ @@ -25,7 +25,7 @@ def convert_utxos(utxodict): return_dict[utxostr_to_utxo(uk)[1]] = val return return_dict -class DummyWallet(SegwitLegacyWallet): +class DummyWallet(SegwitWallet): def __init__(self): storage = VolatileStorage() super().initialize(storage, get_network(), max_mixdepth=5) @@ -88,7 +88,7 @@ class DummyWallet(SegwitLegacyWallet): """Return string defining wallet type for purposes of transaction size estimates """ - return 'p2sh-p2wpkh' + return 'p2wpkh' def get_key_from_addr(self, addr): """usable addresses: privkey all 1s, 2s, 3s, ... :""" @@ -226,10 +226,10 @@ def test_auth_pub_not_found(setup_taker): #edge case triggers that don't fail ([(0, 0, 4, "mxeLuX8PP7qLkcM8uarHmdZyvP1b5e1Ynf", 0, NO_ROUNDING)], False, False, 2, False, None, None), #sweep rounding error case - ([(0, 199850001, 3, "mnsquzxrHXpFsZeL42qwbKdCP2y1esN3qw", 0, NO_ROUNDING)], False, False, + ([(0, 199856001, 3, "mnsquzxrHXpFsZeL42qwbKdCP2y1esN3qw", 0, NO_ROUNDING)], False, False, 2, False, None, None), #trigger sub dust change for taker #edge case triggers that do fail - ([(0, 199851000, 3, "mnsquzxrHXpFsZeL42qwbKdCP2y1esN3qw", 0, NO_ROUNDING)], False, False, + ([(0, 199857000, 3, "mnsquzxrHXpFsZeL42qwbKdCP2y1esN3qw", 0, NO_ROUNDING)], False, False, 2, False, None, None), #trigger negative change ([(0, 199599800, 3, "mnsquzxrHXpFsZeL42qwbKdCP2y1esN3qw", 0, NO_ROUNDING)], False, False, 2, False, None, None), #trigger sub dust change for maker @@ -251,11 +251,14 @@ def test_taker_init(setup_taker, schedule, highfee, toomuchcoins, minmakers, #these tests do not trigger utxo_retries oldtakerutxoretries = jm_single().config.get("POLICY", "taker_utxo_retries") oldtakerutxoamtpercent = jm_single().config.get("POLICY", "taker_utxo_amtpercent") + oldtxfees = jm_single().config.get("POLICY", "tx_fees") jm_single().config.set("POLICY", "taker_utxo_retries", "20") + jm_single().config.set("POLICY", "tx_fees", "30000") def clean_up(): jm_single().config.set("POLICY", "minimum_makers", oldminmakers) jm_single().config.set("POLICY", "taker_utxo_retries", oldtakerutxoretries) jm_single().config.set("POLICY", "taker_utxo_amtpercent", oldtakerutxoamtpercent) + jm_single().config.set("POLICY", "tx_fees", oldtxfees) oldminmakers = jm_single().config.get("POLICY", "minimum_makers") jm_single().config.set("POLICY", "minimum_makers", str(minmakers)) taker = get_taker(schedule) @@ -288,19 +291,19 @@ def test_taker_init(setup_taker, schedule, highfee, toomuchcoins, minmakers, if notauthed: #Doctor one of the maker response data fields maker_response["J659UPUSLLjHJpaB"][1] = "xx" #the auth pub - if schedule[0][1] == 199851000: + if schedule[0][1] == 199857000: #triggers negative change - #((109 + 4*64)*ins + 34 * outs + 8)/4. plug in 9 ins and 8 outs gives - #tx size estimate = 1101 bytes. Times 30 ~= 33030. - #makers offer 3000 txfee, so we pay 30030, plus maker fees = 3*0.0002*200000000 - #roughly, gives required selected = amt + 120k+30k, hence the above = - #2btc - 140k sats = 199851000 (tweaked because of aggressive coin selection) + # ((10 + 31 * outs + 41 * ins)*4 + 109 * ins)/4. plug in 9 ins and 8 outs gives + #tx size estimate = 872.25 bytes. Times 30 ~= 26167.5. + #makers offer 3000 txfee, so we pay 23168, plus maker fees = 3*0.0002*200000000 + #roughly, gives required selected = amt + 120k+23k, hence the above = + #2btc - 143k sats = 199857000 (tweaked because of aggressive coin selection) #simulate the effect of a maker giving us a lot more utxos taker.utxos["dummy_for_negative_change"] = [(struct.pack(b"B", a) *32, a+1) for a in range(7,12)] with pytest.raises(ValueError) as e_info: res = taker.receive_utxos(maker_response) return clean_up() - if schedule[0][1] == 199850001: + if schedule[0][1] == 199856001: #our own change is greater than zero but less than dust #use the same edge case as for negative change, don't add dummy inputs #(because we need tx creation to complete), but trigger case by