fix: bitcoin address generation util

[edgarkhanzadian]

This PR showcases how we can derive a set of addresses that we can use to check if any other address is part of user's account or not.

deriveAddressSetForAccount - derive addresses synchronously, simple but blocks the main thread
asyncDeriveAddressSetForAccount - a little more complex version but doesn't block the main thread as it periodically awaits for a setTimeout(0).
useDeriveAddressSetForCurrentAccount - optional, hook that preconfigures the functions with trAccount and nsAccount

[leather-bot]

Leather Extension build 0f5a207 — Extension build, Extension test report, Storybook, Chromatic

[codecov]

Codecov Report

❌ Patch coverage is 0% with 72 lines in your changes missing coverage. Please review.
✅ Project coverage is 12.18%. Comparing base (f2878b4) to head (0f5a207).

Files with missing lines	Patch %	Lines
...nsion/src/app/common/bitcoin-address-generation.ts	0.00%	71 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #2171      +/-   ##
==========================================
- Coverage   12.19%   12.18%   -0.01%     
==========================================
  Files        2380     2381       +1     
  Lines       97173    97245      +72     
  Branches     3988     3989       +1     
==========================================
  Hits        11848    11848              
- Misses      84318    84389      +71     
- Partials     1007     1008       +1     

Files with missing lines	Coverage Δ
...nsion/src/app/common/bitcoin-address-generation.ts	0.00% <0.00%> (ø)

Components	Coverage Δ
bitcoin	62.80% <ø> (ø)
query	24.49% <ø> (ø)
utils	87.10% <ø> (ø)
crypto	70.62% <ø> (ø)
stacks	49.42% <ø> (ø)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[kyranjamie]

Thanks @edgarkhanzadian, let's jam on this together when multi-wallet is in.

Some initial thoughts, I don't see any caching/memoization in this approach. Even if there were, each wallet session would have to derive the same data.

I think it would be a good idea to make this a job for the service worker, communicated via messaging. Beneficial because it 1) doesn't slow the main thread, 2) cache will persist between UI sessions. (could go further with adding data to a store, but I think that's overkill/complication)

Much of this logic is actually quite similar to some of the "payer" derivation going on in multi-wallet PR. We could better derive and cache all the payer info, rather than just the address (we have to do the work anyway to get the address).

Last, I think addresses themselves are not actually the most thorough way to do ownership detection (only works for simply tx types). We need to check the scriptPubKey, for outputs this means looking up the input being spent which contains this data.

[edgarkhanzadian]

@kyranjamie sure this doesn't have the caching, just deriving addresses. imo it would be better to keep this in redux-persist but i wanted to hold off coding that part until we get multiwallet in as it contains many changes to redux.
It is a good idea to have it on a worker but tbh running this async is pretty non-blocking by itself, i haven't noticed any visual lag when deriving 4000 addresses. I'm not 100% sure using worker is worth the complexity.

Keeping that many payer info objects might be a little too heavy to cache? In case of addresses, it's just a set of strings.

Wouldn't it be quite hard to check scriptPubKey ourselves? As far as i understand we would need to understand how user's wallet is related to the script which isn't always trivial. Moreover, will apis actually return the transactions that are not standard just using user's xpub?

[kyranjamie]

This is something expensive enough to be cautious about running all the time (e.g. every time a user presses the action button) but not so expensive we need to write to a store imo.

Caching in the service worker is somewhere in the middle, executing only once per service worker lifecycle. There is little over head doing this, we already have the messaging structure in place. We should be using the service worker more imo, there's too much heavy lifting going on in the web frames of the extension. We're running pretty high spec Macbooks, try again with 10x CPU throttling, must be noticeably slower.

Reluctant to put in redux. IMO we should really be keeping this as minimal as possible, anything that can be derived shouldn't be in the store (unless it really is so critically slow there is no other option, but then we could just write to storage outside redux).

Whatever solution we come to, I think it should compliment the existing logic we already have to derive child key details, so that we're not deriving the same information twice, once for our address derivation stucture, and again in our utxo ownership logic.

My least favourite part of the multi-wallet code is where I have some funky cache logic going on to prevent rerender loops. Hopefully we can address that as well.

Wouldn't it be quite hard to check scriptPubKey ourselves? As far as i understand we would need to understand how user's wallet is related to the script which isn't always trivial. Moreover, will apis actually return the transactions that are not standard just using user's xpub?

For outputs, the scriptPubKey can be read from the previous transaction's input. From that, we can see if we have the key eligible to spend it. This works also for complex tx types, multi-sig etc. As far I understand, this is the only complete way to establish whether a user's wallet can spend a utxo.

The "easiest" way would be to just compare a tx's outputs against our known list of spendable utxos, I was half-way writing a comment suggesting this earlier. The problem is that a tx can be in the past, where you did receive a utxo, but it has already been spent.

[edgarkhanzadian]

We should be using the service worker more imo, there's too much heavy lifting going on in the web frames of the extension

yeap agree let's use workers for this.

anything that can be derived shouldn't be in the store

+1, we can think about the caching solution after we get the multiwallet in

For outputs, the scriptPubKey can be read from the previous transaction's input. From that, we can see if we have the key eligible to spend it. This works also for complex tx types, multi-sig etc. As far I understand, this is the only complete way to establish whether a user's wallet can spend a utxo.

The "easiest" way would be to just compare a tx's outputs against our known list of spendable utxos, I was half-way writing a comment suggesting this earlier. The problem is that a tx can be in the past, where you did receive a utxo, but it has already been spent.

These are good solutions, but these are needed to fix a very niche problem on bitcoin IMO.

I think checking input/output address against a set of known addresses has much less complexity than using solutions mentioned earlier to check scriptpubkeys.

Currently we only support zero index ns and tr addresses, we can approach this issue incrementally. First make an adjustment to keep a set of strings and check against a set of strings, and then consider adding scriptpubkey check for non-standard bitcoin transactions.