justinmoon / junction

UI For Using Hardware Wallets With Bitcoin Core
MIT License
60 stars 13 forks source link

Native descriptor wallets #21

Open Sjors opened 5 years ago

Sjors commented 5 years ago

I just tried Junction with @achow101's native descriptor wallet (in testnet!) PR: https://github.com/bitcoin/bitcoin/pull/16528

Works like a charm. In particular, try getaddressinfo on the multisig address and notice Bitcoin Core knows the full descriptor!

If you like, you can also try my PR which adds better signer support to the RPC: https://github.com/bitcoin/bitcoin/pull/16546

It involves launching bitcoind with -signer. You then create a wallet with the externalsigner flag true and it will Just Works(tm). Send also just works(tm). Except not with multisig... Suggestions are welcome, what should the RPC look like?

Sjors commented 5 years ago

Mmm, I just realized I didn't actually test the native descriptor wallet, because I didn't edit the script to use the new flag.

Sjors commented 5 years ago

Two changes are needed:

Set the desciptor flag: default_wallet_rpc.createwallet(watch_only_name, True, True, None, True, True) (support for named arguments doesn't work?)

Use importdescriptors instead of importmulti, add active": True and drop keypool. See #12 though, it's better to import two descriptors, one for receive and one for change (internal: true).

I believe you can also swap derviveaddress for getnewaddress label legacy.

If you launch QT with -addresstype=legacy you can just use the UI to generate addresses.

justinmoon commented 5 years ago

Use importdescriptors instead of importmulti, add active": True and drop keypool. See #12 though, it's better to import two descriptors, one for receive and one for change (internal: true).

AWESOME! I've been trying to figure out how to do this for a whlie

I will try your PR and Andrew's ASAP.

Sjors commented 5 years ago

I also tried native segwit (wsh instead of sh) with a Trezor T and a Ledger Nano S.

I complained "No confirmed UTXOs to spend" even though bitcoin-cli -testnet -rpcwallet=... listunspent 0 9999999 '[]' true does return a UTXO with 1 confirmation. It's probably because getbalances is different:'

{
  "mine": {
    "trusted": 0.0020000,
    "untrusted_pending": 0.00000000,
    "immature": 0.00000000,
    "used": 0.00000000
  }

I hacked around that. Although the devices display weird stuff, they do produce a valid transaction!

justinmoon commented 5 years ago

Re ^^, This line is the problem

Previously, outputs would show up under the watchonly key. Now it appears that with descriptor wallets they show up under the mine key.

This is the description of the mine key in the docs: balances from outputs that the wallet can sign. I'm a little confused why Bitcoin Core thinks it can sign these watch-only outputs. Does this have to do with your "add signer" PR?

achow101 commented 5 years ago

This is the description of the mine key in the docs: balances from outputs that the wallet can sign. I'm a little confused why Bitcoin Core thinks it can sign these watch-only outputs.

Native descriptor wallets redefine IsMine to completely different semantics. Basically, native descriptor wallets completely gets rid of the distinction between watchonly and mine within a single wallet. Instead, any scriptPubKey tracked by the wallet is considered mine, regardless of whether it can sign or not. There won't be any mixed watchonly and non-watchonly things. Either something is part of the wallet (IsMine is true) or it is not (IsMine is false). This greatly simplifies the behavior of the wallet and gets rid of having to say "IncludeWatching" everywhere.

justinmoon commented 5 years ago

Thanks @achow101, that's extremely helpful. You PR seems like a gamechanger for projects like Junction.