interlay / interbtc-clients

interBTC Clients | Vault, Oracle, Faucet
Apache License 2.0
49 stars 30 forks source link

Switch to descriptor wallets #457

Open nud3l opened 1 year ago

nud3l commented 1 year ago

Is your feature request related to a problem? Please describe. The vault client uses Bitcoin legacy wallets by default.

For a more detailed overview, see here: https://achow101.com/2020/10/0.21-wallets and https://bitcoinmagazine.com/technical/bitcoin-core-23-0-released-whats-new

Describe the solution you'd like Instead of using legacy wallets, the vault client should use descriptor wallets.

Context Bitcoin core 24 supports migrating legacy wallets to descriptor wallets: https://github.com/bitcoin/bitcoin/blob/master/doc/managing-wallets.md#migrating-legacy-wallets-to-descriptor-wallets

gregdhill commented 1 year ago

This might not be feasible due to the On-chain Key Derivation (OKD) scheme we use, but worth investigating!

nud3l commented 1 year ago

Yep, I think we need to check how this would work with OKD and if it's compatible/possible.

Descriptor wallets would making setting vaults up as multi-sigs a lot easier as well though.

sander2 commented 1 year ago

Note that bitcoin core intentionally made it difficult to get the private key associated with an address. Apparently this is because users assumed that the private keys are random, and that sharing them only risks the address itself, while in reality leaking one private key risks all other derived keys as well

Also, the fact that we have to iterate over all of the descriptors to find the derivation key is a point in favor of keeping a separate -master wallet (there has been talk recently to possibly merge wallets)

nud3l commented 1 year ago

Summarizing my understanding:

Questions:

I find it quite worthwhile to make the switch as we will make it easier for vaults to backup/restore wallets, make it possible to use hardware wallets eventually and help with multisig vaults.

sander2 commented 1 year ago

For finding the descriptor/UTXO, does it add a lot of complexity when e.g., a vault needs to restart?

Well we need to do the the iteration over all descriptors that's mentioned. This iteration needs to be done once per vault start-up if we just keep it in memory.

Is there anything blocking us from switching to descriptor wallets?

No hard blockers that I'm aware of, but the bitcoin rpc library is missing some rpcs. The maintenance is not very active at all so we'll probably have to implement this ourselves on a fork.

we will make it easier for vaults to backup/restore wallets

I'm not trying to argue against this point, but why is this the case? With legacy wallets you can just copy the wallet file and that's it, right?

nud3l commented 1 year ago

There's a blog post I've read somewhere that describes a bit the issues with legacy wallet recovery. IIRC, legacy wallet are not compatible with wallets other than core and core will also stop supporting them in late 2024. So while you can back them up, at some point you can only use them with outdated software. There were also some other issues where reconstructing addresses was complex but not sure if that isn't an issue if you restore just all files.