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

Native descriptor wallets #21

Open
Sjors opened this issue Sep 4, 2019 · 7 comments
Open

Native descriptor wallets #21

Sjors opened this issue Sep 4, 2019 · 7 comments

Comments

@Sjors
Copy link

Sjors commented Sep 4, 2019

I just tried Junction with @achow101's native descriptor wallet (in testnet!) PR: bitcoin/bitcoin#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: bitcoin/bitcoin#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
Copy link
Author

Sjors commented Sep 4, 2019

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
Copy link
Author

Sjors commented Sep 4, 2019

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
Copy link
Owner

justinmoon commented Sep 4, 2019

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
Copy link
Author

Sjors commented Sep 4, 2019

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
Copy link
Owner

justinmoon commented Sep 5, 2019

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
Copy link

achow101 commented Sep 5, 2019

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
Copy link
Owner

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

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

No branches or pull requests

3 participants