Skip to content

Commit

Permalink
Fix bug in detailed wallet sync relating to gap addrs.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
AdamISZ committed Jun 15, 2019
1 parent 641c956 commit 8240655
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 8 deletions.
13 changes: 12 additions & 1 deletion jmclient/jmclient/blockchaininterface.py
Original file line number Diff line number Diff line change
Expand Up @@ -504,10 +504,20 @@ def get_address_usages(self, wallet):
# overall performance gain is probably negligible
used_indices = self._get_used_indices(wallet, used_addresses)
self._rewind_wallet_indices(wallet, used_indices, saved_indices)
# Now that we have finalized the wallet indices, update:
wallet.save()
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)
Expand All @@ -529,7 +539,6 @@ def sync_addresses(self, wallet, restart_cb=None):
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)
Expand All @@ -547,6 +556,8 @@ def sync_addresses(self, wallet, restart_cb=None):
else:
log.debug("Wallet successfully synced")
self._rewind_wallet_indices(wallet, used_indices, saved_indices)
# Now that we have finalized the wallet indices, update:
wallet.save()
self.wallet_synced = True

@staticmethod
Expand Down
18 changes: 11 additions & 7 deletions jmclient/jmclient/wallet.py
Original file line number Diff line number Diff line change
Expand Up @@ -426,12 +426,18 @@ def get_key_from_addr(self, addr):
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):
Expand All @@ -443,8 +449,7 @@ def get_external_addr(self, mixdepth, bci=None):
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):
"""
Expand All @@ -454,8 +459,7 @@ def get_internal_addr(self, mixdepth, bci=None):
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)
Expand Down

0 comments on commit 8240655

Please sign in to comment.