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

Displayed miniscript out of sync #4

Closed
sgeisler opened this issue Sep 3, 2020 · 7 comments
Closed

Displayed miniscript out of sync #4

sgeisler opened this issue Sep 3, 2020 · 7 comments

Comments

@sgeisler
Copy link

sgeisler commented Sep 3, 2020

When playing around with different spend policies I noticed that the displayed miniscript/descriptor doesn't change accordingly.

E.g. P2SH-W2WPKH should be displayed as sh(wpkh(key)), but stays pkh(key). Closing and reopening the window doesn't seem to fix the issue.

2020-09-03-235228_5120x1440_scrot

@craigraw
Copy link
Collaborator

craigraw commented Sep 4, 2020

I'm unsure if that's valid miniscript. Certainly sh(wpkh(key)) is used in output descriptors, and (for example) Sparrow will display that in the Receive panel. But I can't seem to find any reference to it in http://bitcoin.sipa.be/miniscript/ or any other document on miniscript that I've seen. It would be good to have clarity on this point!

@sgeisler
Copy link
Author

sgeisler commented Sep 4, 2020

Yeah, I confused the terminology too again. What you display seems to actually be miniscript (maybe missing an underscore), but in the case of p2sh-p2wpkh there just isn't any miniscript involved. What you want to display is probably a miniscript-based descriptor, because while sh(pk_h(key)), sh(wphk(key)) and sh(wsh(pk_h(key))) all encode the same spend policy, they produce different scripts/addresses. And the addresses you display are sh(wphk(key)) ones, so the displayed miniscript is kinda misleading. I think the descriptor concept is easy to see in rust-miniscript.

EDIT: if you want to play around with policies/miniscript/descriptors @afilini built a nice playground based on rust-miniscript.

@craigraw
Copy link
Collaborator

craigraw commented Sep 4, 2020

That seems reasonable. I agree it would be a clearer description of the wallet with less ambiguity. So:

Singlesig:
P2PKH: pkh(keystore)
P2SH-P2PKH: sh(wpkh(keystore))
P2WPKH: wpkh(keystore)

Multisig:
P2SH: sh(multi(m,keystore1,keystore2,...))
P2SH-P2WSH: sh(wsh(multi(m,keystore1,keystore2,...)))
P2WSH: wsh(multi(m,keystore1,keystore2,...))

I agree the underscore seems missing looking at sipa's doc, but it doesn't feel natural to include in the above.

@craigraw
Copy link
Collaborator

craigraw commented Sep 7, 2020

Implemented in sparrowwallet/drongo@3115669

@sgeisler
Copy link
Author

sgeisler commented Sep 7, 2020

Nice! I think it should also be named differently in the ui (something along the lines of Miniscript descriptor) once sparrow actually uses the new version of drogo. Looking forward to the next release!

@craigraw
Copy link
Collaborator

craigraw commented Sep 8, 2020

Agreed - due to label space I think I'm going to go with 'Descriptor', since it is closest to output descriptor language, but with each keystore abbreviated to it's label to make it easier to read. Each keystore will have a a tooltip with the output descriptor representation of the keystore e.g. [d34db33f/44'/0'/0']xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL (no child key derivation). The context menu will allow for copying the full unabbreviated output descriptor.

I'm also moving from multi to sortedmulti as a more accurate representation of multisig wallets in Sparrow.

@craigraw
Copy link
Collaborator

craigraw commented Sep 9, 2020

Implemented in b8c3bf1.

@craigraw craigraw closed this as completed Sep 9, 2020
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

2 participants