Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wallet restore misses transactions due to dense transaction activity: (lookahead / BIP44 gap limit) #407

Closed
mslipper opened this issue Mar 27, 2020 · 4 comments · Fixed by #506

Comments

@mslipper
Copy link
Contributor

When you have lots of names (>100), rescanning the wallet causes names to be lost. Checkout the following screenshot:

Screen Shot 2020-03-26 at 5 48 03 PM

Here, I'm executing a wallet rescan then getting the list of names from the wallet once the rescan completes. (I look at the logs in another terminal to determine when the rescan is done.) As you can see, the number of names in the wallet changes even though the wallet itself hasn't made any new transactions. The number of names increases randomly whenever I rescan then get the names in the wallet.

@turbomaze
Copy link
Contributor

Have you tried modifying the lookahead parameter to be very large? 1000 instead of the default 10 for instance. Can edit the max lookahead value in lib/wallet/account.js

@mslipper
Copy link
Contributor Author

Yes, @pinheadmz mentioned this workaround to me and it fixed the issue. However, I still think that the real fix for this is within HSD itself given that it seems to be triggered by some kind of race condition.

@pinheadmz
Copy link
Member

pinheadmz commented Mar 27, 2020

This issue is being tracked upstream in bcoin:

bcoin-org/bcoin#835

@pinheadmz pinheadmz changed the title Wallet rescan loses name state when you have lots of names Wallet rescan misses transactions due to dense transaction activity Mar 27, 2020
@pinheadmz pinheadmz changed the title Wallet rescan misses transactions due to dense transaction activity Wallet restore misses transactions due to dense transaction activity Apr 3, 2020
@pinheadmz pinheadmz changed the title Wallet restore misses transactions due to dense transaction activity Wallet restore misses transactions due to dense transaction activity: (lookahead / BIP44 gap limit) Aug 4, 2020
@pinheadmz
Copy link
Member

pinheadmz commented Aug 4, 2020

The hsd wallet gap limit issue discussion has bled into Bob-wallet: kyokan/bob-wallet#165

Here are some references:

It seems to be a common work-around for users to just set the value to 1000 and in fact, Bob I think already does this. This will create a problem if users try to restore their balance from a different version of Bob or hsd however. So I think a standard is important.

There are better solutions described in bcoin-org/bcoin#835 and in fact one unmerged patch in bcoin avoids the issue by rescanning blocks every time a TX is found in a block.

The main issue is that bcoin/hsd uses bloom filters for rescanning (even against a full node) and these filters are updated after each block is scanned. We could just remove that optimization and probably a gap limit of 10 or 20 would be fine, but would slow down rescans.

Note that this is only an issue during rescan. During a regular "block connect" event, every tx in the block is checked one at a time for a wallet-match.

Solutions

  1. Band-aid: bump lookahead to 1000 in hsd, make sure that wallets upgrading to latest version will generate those addresses as well, maybe include note about rescanning in the changelog after updating

  2. Research the performance hit to rescanning if the bloom filter is removed and the "block connect" path is used instead

  3. Research the performance hit (or other tradeoffs) of repeatedly rescanning a block once a wallet TX is found, adding to the address depth each time and adding new addresses to the bloom filter, until new TXs stop turning up

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants