From 6c15bd7b2a4c7b845dc7a2799ec7b3deaae8a27f Mon Sep 17 00:00:00 2001 From: AdamISZ Date: Thu, 13 Jun 2019 16:15:16 +0200 Subject: [PATCH 1/2] Make address imports with address requests in wallet Prior to this commit, there was duplicated code in maker and taker modules to import addresses, now all calls to the wallet for fresh addresses can optionally pass a blockchaininterface instance and if this is done, the new address will be imported to the BCI at the same time. --- jmclient/jmclient/maker.py | 8 -------- jmclient/jmclient/taker.py | 22 ++++++++-------------- jmclient/jmclient/taker_utils.py | 13 ++----------- jmclient/jmclient/wallet.py | 26 ++++++++++++++++++++------ jmclient/jmclient/yieldgenerator.py | 7 ++++--- jmclient/test/test_taker.py | 2 +- 6 files changed, 35 insertions(+), 43 deletions(-) diff --git a/jmclient/jmclient/maker.py b/jmclient/jmclient/maker.py index 43adc83..6241e8b 100644 --- a/jmclient/jmclient/maker.py +++ b/jmclient/jmclient/maker.py @@ -253,14 +253,6 @@ class Maker(object): self.offerlist.remove(oldorder_s[0]) self.offerlist += to_announce - def import_new_addresses(self, addr_list): - # FIXME: same code as in taker.py - bci = jm_single().bc_interface - if not hasattr(bci, 'import_addresses'): - return - assert hasattr(bci, 'get_wallet_name') - bci.import_addresses(addr_list, bci.get_wallet_name(self.wallet)) - @abc.abstractmethod def create_my_orders(self): """Must generate a set of orders to be displayed diff --git a/jmclient/jmclient/taker.py b/jmclient/jmclient/taker.py index 3a941f5..956105d 100644 --- a/jmclient/jmclient/taker.py +++ b/jmclient/jmclient/taker.py @@ -186,11 +186,13 @@ class Taker(object): #if destination is flagged "INTERNAL", choose a destination #from the next mixdepth modulo the maxmixdepth if self.my_cj_addr == "INTERNAL": - next_mixdepth = (self.mixdepth + 1) % (self.wallet.mixdepth + 1) - jlog.info("Choosing a destination from mixdepth: " + str(next_mixdepth)) - self.my_cj_addr = self.wallet.get_internal_addr(next_mixdepth) + next_mixdepth = (self.mixdepth + 1) % ( + self.wallet.mixdepth + 1) + jlog.info("Choosing a destination from mixdepth: " + str( + next_mixdepth)) + self.my_cj_addr = self.wallet.get_internal_addr(next_mixdepth, + bci=jm_single().bc_interface) jlog.info("Chose destination address: " + self.my_cj_addr) - self.import_new_addresses([self.my_cj_addr]) self.outputs = [] self.cjfee_total = 0 self.maker_txfee_contributions = 0 @@ -275,8 +277,8 @@ class Taker(object): self.my_change_addr = None if self.cjamount != 0: try: - self.my_change_addr = self.wallet.get_internal_addr(self.mixdepth) - self.import_new_addresses([self.my_change_addr]) + self.my_change_addr = self.wallet.get_internal_addr(self.mixdepth, + bci=jm_single().bc_interface) except: self.taker_info_callback("ABORT", "Failed to get a change address") return False @@ -835,14 +837,6 @@ class Taker(object): self.on_finished_callback(True, fromtx=fromtx, waittime=waittime, txdetails=(txd, txid)) - def import_new_addresses(self, addr_list): - # FIXME: same code as in maker.py - bci = jm_single().bc_interface - if not hasattr(bci, 'import_addresses'): - return - assert hasattr(bci, 'get_wallet_name') - bci.import_addresses(addr_list, bci.get_wallet_name(self.wallet)) - class P2EPTaker(Taker): """ The P2EP Taker will initialize its protocol directly with the prescribed counterparty (see -T argument to diff --git a/jmclient/jmclient/taker_utils.py b/jmclient/jmclient/taker_utils.py index 57b4e40..b75eb85 100644 --- a/jmclient/jmclient/taker_utils.py +++ b/jmclient/jmclient/taker_utils.py @@ -70,8 +70,8 @@ def direct_send(wallet, amount, mixdepth, destaddr, answeryes=False, total_inputs_val = sum([va['value'] for u, va in iteritems(utxos)]) changeval = total_inputs_val - fee_est - amount outs = [{"value": amount, "address": destaddr}] - change_addr = wallet.get_internal_addr(mixdepth) - import_new_addresses(wallet, [change_addr]) + change_addr = wallet.get_internal_addr(mixdepth, + jm_single().bc_interface) outs.append({"value": changeval, "address": change_addr}) #Now ready to construct transaction @@ -114,15 +114,6 @@ def sign_tx(wallet, tx, utxos): our_inputs[index] = (script, amount) return wallet.sign_tx(stx, our_inputs) -def import_new_addresses(wallet, addr_list): - # FIXME: same code as in maker.py and taker.py - bci = jm_single().bc_interface - if not hasattr(bci, 'import_addresses'): - return - assert hasattr(bci, 'get_wallet_name') - bci.import_addresses(addr_list, bci.get_wallet_name(wallet)) - - def get_tumble_log(logsdir): tumble_log = logging.getLogger('tumbler') tumble_log.setLevel(logging.DEBUG) diff --git a/jmclient/jmclient/wallet.py b/jmclient/jmclient/wallet.py index 8c8fa2d..d6dddbc 100644 --- a/jmclient/jmclient/wallet.py +++ b/jmclient/jmclient/wallet.py @@ -426,22 +426,36 @@ class BaseWallet(object): privkey = self._get_priv_from_path(path)[0] return hexlify(privkey).decode('ascii') - def get_external_addr(self, mixdepth): + def _get_addr_int_ext(self, get_script_func, mixdepth, bci=None): + script = get_script_func(mixdepth) + addr = self.script_to_addr(script) + if bci is not None and hasattr(bci, 'import_addresses'): + assert hasattr(bci, 'get_wallet_name') + bci.import_addresses([addr], bci.get_wallet_name(self)) + return addr + + def get_external_addr(self, mixdepth, bci=None): """ Return an address suitable for external distribution, including funding the wallet from other sources, or receiving payments or donations. JoinMarket will never generate these addresses for internal use. + If the argument bci is non-null, we attempt to import the new + address into this blockchaininterface instance + (based on Bitcoin Core's model). """ - script = self.get_external_script(mixdepth) - return self.script_to_addr(script) + return self._get_addr_int_ext(self.get_external_script, mixdepth, + bci=bci) - def get_internal_addr(self, mixdepth): + def get_internal_addr(self, mixdepth, bci=None): """ Return an address for internal usage, as change addresses and when participating in transactions initiated by other parties. + If the argument bci is non-null, we attempt to import the new + address into this blockchaininterface instance + (based on Bitcoin Core's model). """ - script = self.get_internal_script(mixdepth) - return self.script_to_addr(script) + return self._get_addr_int_ext(self.get_internal_script, mixdepth, + bci=bci) def get_external_script(self, mixdepth): return self.get_new_script(mixdepth, False) diff --git a/jmclient/jmclient/yieldgenerator.py b/jmclient/jmclient/yieldgenerator.py index 347d67f..8a4fd11 100644 --- a/jmclient/jmclient/yieldgenerator.py +++ b/jmclient/jmclient/yieldgenerator.py @@ -108,9 +108,10 @@ class YieldGeneratorBasic(YieldGenerator): # mixdepth is the chosen depth we'll be spending from cj_addr = self.wallet.get_internal_addr( - (mixdepth + 1) % (self.wallet.mixdepth + 1)) - change_addr = self.wallet.get_internal_addr(mixdepth) - self.import_new_addresses([cj_addr, change_addr]) + (mixdepth + 1) % (self.wallet.mixdepth + 1), + jm_single().bc_interface) + change_addr = self.wallet.get_internal_addr(mixdepth, + jm_single().bc_interface) utxos = self.wallet.select_utxos(mixdepth, total_amount) my_total_in = sum([va['value'] for va in utxos.values()]) diff --git a/jmclient/test/test_taker.py b/jmclient/test/test_taker.py index 1a6a69f..d2bbe2c 100644 --- a/jmclient/test/test_taker.py +++ b/jmclient/test/test_taker.py @@ -64,7 +64,7 @@ class DummyWallet(SegwitLegacyWallet): raise Exception("Not enough funds") return t_utxos_by_mixdepth[mixdepth] - def get_internal_addr(self, mixing_depth): + def get_internal_addr(self, mixing_depth, bci=None): if self.inject_addr_get_failure: raise Exception("address get failure") return "mxeLuX8PP7qLkcM8uarHmdZyvP1b5e1Ynf" From 4b4f8c97f40e232cf25315a3a2568a93d4f67456 Mon Sep 17 00:00:00 2001 From: AdamISZ Date: Sun, 16 Jun 2019 15:48:01 +0200 Subject: [PATCH 2/2] Fix bug in detailed wallet sync relating to gap addrs. Prior to this commit, addresses imported on tx creation were at the tip of the index only, but detailed sync requires all address gap-forwards to be imported, else it requires a restart. The particular fix approach taken her is aggression because import is cheap; we import wallet.gaplimit forwards on all branches, taking advantage of the bci's function _collect_addresses_gap, thus ensuring that unless the gap limit changes, we will definitely not need a restart. Fast sync did not suffer from this and is unchanged. --- jmclient/jmclient/blockchaininterface.py | 9 ++++++++- jmclient/jmclient/wallet.py | 18 +++++++++++------- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/jmclient/jmclient/blockchaininterface.py b/jmclient/jmclient/blockchaininterface.py index b219f9f..b1435ba 100644 --- a/jmclient/jmclient/blockchaininterface.py +++ b/jmclient/jmclient/blockchaininterface.py @@ -512,7 +512,15 @@ class BitcoinCoreInterface(BlockchainInterface): self.wallet_synced = True def sync_addresses(self, wallet, restart_cb=None): + # This more detailed version of wallet syncing must cover situations + # where the user has used the wallet on other Bitcoin Core instances + # and therefore the wallet label either does not have addresses + # imported, and/or, after the addresses are imported, they may not + # have the full history, since a rescan is required to discover + # the history of addresses before they were imported. + log.debug("requesting detailed wallet history") + wallet_name = self.get_wallet_name(wallet) addresses, saved_indices = self._collect_addresses_init(wallet) @@ -534,7 +542,6 @@ class BitcoinCoreInterface(BlockchainInterface): used_addresses_gen = (tx['address'] for tx in self._yield_transactions(wallet_name) if tx['category'] == 'receive') - used_indices = self._get_used_indices(wallet, used_addresses_gen) log.debug("got used indices: {}".format(used_indices)) gap_limit_used = not self._check_gap_indices(wallet, used_indices) diff --git a/jmclient/jmclient/wallet.py b/jmclient/jmclient/wallet.py index d6dddbc..1fdf633 100644 --- a/jmclient/jmclient/wallet.py +++ b/jmclient/jmclient/wallet.py @@ -426,12 +426,18 @@ class BaseWallet(object): privkey = self._get_priv_from_path(path)[0] return hexlify(privkey).decode('ascii') - def _get_addr_int_ext(self, get_script_func, mixdepth, bci=None): - script = get_script_func(mixdepth) + def _get_addr_int_ext(self, internal, mixdepth, bci=None): + script = self.get_internal_script(mixdepth) if internal else \ + self.get_external_script(mixdepth) addr = self.script_to_addr(script) if bci is not None and hasattr(bci, 'import_addresses'): assert hasattr(bci, 'get_wallet_name') - bci.import_addresses([addr], bci.get_wallet_name(self)) + # we aggressively import ahead of our index, so that when + # detailed sync is needed in future, it will not find + # imports missing (and this operation costs nothing). + addrs_to_import = list(bci._collect_addresses_gap( + self, self.gaplimit)) + bci.import_addresses(addrs_to_import, bci.get_wallet_name(self)) return addr def get_external_addr(self, mixdepth, bci=None): @@ -443,8 +449,7 @@ class BaseWallet(object): address into this blockchaininterface instance (based on Bitcoin Core's model). """ - return self._get_addr_int_ext(self.get_external_script, mixdepth, - bci=bci) + return self._get_addr_int_ext(False, mixdepth, bci=bci) def get_internal_addr(self, mixdepth, bci=None): """ @@ -454,8 +459,7 @@ class BaseWallet(object): address into this blockchaininterface instance (based on Bitcoin Core's model). """ - return self._get_addr_int_ext(self.get_internal_script, mixdepth, - bci=bci) + return self._get_addr_int_ext(True, mixdepth, bci=bci) def get_external_script(self, mixdepth): return self.get_new_script(mixdepth, False)