search

WalletWasabi / WalletWasabi

Open-source, non-custodial, privacy preserving Bitcoin wallet for Windows, Linux, and Mac.
https://wasabiwallet.io
MIT License
2.16k stars 500 forks source link

[VDG] Wallet settings gap limit #8585

Open MaxHillebrand opened 2 years ago

MaxHillebrand commented 2 years ago

We keep having troubles with wallet recovery, usually it's a too low gap limit. An easy manual solution is to add this config to the wallet settings.

A more complex and holistic solution is to have a graceful rescan process, which is out of the scope of this issue.

yahiheb commented 2 years ago

We keep having troubles with wallet recovery, usually it's a too low gap limit.

The problem is not the gap limit being too low, either we are not scanning correctly or we are using addresses outside the range of the gap limit. Most likely it is the later.

MaxHillebrand commented 2 years ago

The actual gap limit of my big wallets are all well above 500. Check this in your own wallet statistics.

The default scan with it's 100 [I think?] gap limit does not pick them up. I always have to manually increase the gap limit to work around the issue.

stale[bot] commented 2 years ago

This has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

lontivero commented 2 years ago

I am refactoring the KeyManager class to add support for taproot scripts and before starting to implement anything I decided to clean a bit that class because it smells really bad. Among many changes that I think will be good, these are the ones that come to my mind now:

  1. Remove the gap filler. Time ago we notice a problem with wallets having gaps bigger than 21 and then I implemented a mechanism for filling those gaps instead of hunting the bug and kill it.
  2. Respect key order. Currently internal keys start in 21, what is a bug and keys are generated in the order that is required to fill the gaps so, in some cases the key 1 is generates after key 12, 19 or 35. This will not be the case anymore.
  3. Separation of keys by purpose. Currently the KeyManager uses internal segwit keys for both, change scripts and coinjoin output scripts. By using internal taproot derivation paths for coinjoin scripts (locked) we will end with effectively segregated sets of keys by purpose so, external segwit for receiving, internal segwit for change and internal taproot for locked coins (coinjoin outputs)

There are many other things to do like writing tons of tests for the KeyManager, clean the code (there are 6 methods called ToFile!), minimize the number of moving parts (mutations) and ensure thread safetyness to mention only a few.

In summary, if there is a problem in the KeyManager this is when we will catch it.

stale[bot] commented 1 year ago

This has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Kruwed commented 1 year ago

ACK, I recovered a non-BIP39 seed recently that had an unexpected 300+ address gap, it was a nightmare because no wallets that supported the seed type also supported gap limit adjustment.

MaxHillebrand commented 6 months ago

this should not be in the wallet settings, but an option in the wallet resync button, because after a change to the gap limit, the wallet should be resynced.