-
Notifications
You must be signed in to change notification settings - Fork 178
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
Basic output descriptor functions (generate only) #1270
Conversation
cb9a4b0
to
2acbefd
Compare
Anyone plans to review / test this or I can just merge it as is (as it does not affect any existing JM functionality)? |
I was planning to take a look at it, but I keep getting delayed by other things. Can you give a link to something I should read from the Core repo to compare? |
OK after further review I can't see a reason not to merge this. It's just a first step with no client side impact yet. A few things struck me:
I guess there will be more to think about re: whether we use these as inputs to I guess some of this stuff you've already thought about. |
OK I was about to merge this but then I suddenly thought: have you checked which aspects of this are currently in |
Guess could add tests for p2pkh and p2sh-p2wpkh xpub descriptors. Was initially looking for existing tests in Core, didn't find ones.
Didn't get this one. Checksums are required for it to be a valid descriptor. Previously in #1064 I used
Didn't check before, but searching for "descriptor" and "descriptors" gives no results in their github. This code is actually taken mostly from the stuff I am slowly working on (not yet even in draft PR state) to write external signer HWI driver of JM for Bitcoin Core, which would allow to use stuff writen against Core, like my bitcoin-scripts, with JM wallet, so that JM wallet would be like a hardware wallet from Core's perspective. That required this basic descriptor functionality. Currently only useful stuff for this I see is adding "copy output descriptor to clipboard" in addition to "copy extended public key to clipboard" in Qt GUI. |
2acbefd
to
fc5bda4
Compare
Added tests for p2pkh and p2sh-p2wpkh xpub descriptors. |
Yeah, that makes sense. Doubtless we will add some later when we get to a more concrete implementation.
OK I guess that's just my misunderstanding; I read "The top level expression is either a SCRIPT, or SCRIPT#CHECKSUM where CHECKSUM is an 8-character alphanumeric descriptor checksum." in the descriptors doc you linked, and took it to mean that checksums are optional. Anyway no issue here.
OK, thanks for the check. Confirmed same here.
Good to hear you're moving forward with this very desirable feature, I know it's not going to be a simple task. So given that, merging. |
Adds two functions for generating output descriptors (not yet used anywhere in code):
get_address_descriptor(addr)
to derive output descriptor from Bitcoin address;get_xpub_descriptor(xpub_key, address_type)
to derive output descriptor from an extended public key.This is very basic, taken from some unfinished stuff I'm doing (slowly). More tests could be added, also there is no support for fidelity bond
wsh()
descriptors currently. But should be already useful in context of #1267.Validness of checksums can be tested using
bitcoin-cli getdescriptorinfo
.