-
Notifications
You must be signed in to change notification settings - Fork 38.6k
rpc: have raw transaction decoding infer output descriptors #16795
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
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsNo conflicts as of last run. |
|
that's cool utACK |
|
forgot to add description in RPC help, added. |
1863168 to
8cd521e
Compare
8cd521e to
c772728
Compare
|
utACK c772728ab4a40bfb9a546eef669b0d7603bd9f89 |
c772728 to
7fcf811
Compare
|
rebased |
|
You need to update the help-text for the RPCs affected by this change (decoderawtransaction, decodescript, getrawtransaction) to include the additional field |
|
Travis found a real problem here (for a change !!! 🎉 ) |
326f322 to
9018d63
Compare
|
fixed rebase error, now all outputs are inferred. added description to |
9018d63 to
9b94596
Compare
Github-Pull: bitcoin#16795 Rebased-From: 9b9459640d2b026448bb40d9462a591e6df5df9f
ariard
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 9b94596 minus gettouxt/rest_getuxtos doc fixs
Tested getrawtransaction, decoderawtransaction, decodescript, work as expected.
src/core_write.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to update help-debug of gettxout and also maybe REST-interface.md on rest_getutxos.
Do you want also to extend feature to decodepsbt ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want also to extend feature to decodepsbt ?
going to take a little more thinking, so for now "no" :)
fixed up other missing documentation to best of knowledge
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see **ScriptToUniv** is only used for PSBT decoding... so yeah I can do that real quick.
eh maybe not im not sure of the reasoning of addresses being included or not in decodepsbt output...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reference, decodepsbt is included in this pull (despite the docs not being updated)
9b94596 to
dcd5c4a
Compare
|
pushed update fixing @ariard nits |
Github-Pull: bitcoin#16795 Rebased-From: dcd5c4a5773bef52bf7c030c4e4b03ad15bc36fe
|
Being pedantic here, but wouldn't it be more logical to put the inferred output descriptor |
|
It would be nice to have this, as it compensates somewhat for the (weird) functionality lost in #20286. There are a bunch of unaddressed comments here though. And also, it'd be good if the |
|
Ah, thought I closed this PR. I'll take a fresh look. |
4372d34 to
3038f94
Compare
|
@sipa Looks like it already does the native segwit inference in |
|
@instagibbs I was confusing |
|
Addressed all concerns, I think. Rebased on master since this is pretty ancient. |
|
Desc for string. Update is good. Need known rebase that. |
Github-Pull: bitcoin#16795 Rebased-From: 3038f944a6d73171ded3dafc9d59a2f7160ba52b
|
Concept ACK, sorry I missed this |
|
kk will rebase |
3038f94 to
5e25688
Compare
|
@meshcollider ready for rereview |
Github-Pull: bitcoin#16795 Rebased-From: 3038f944a6d73171ded3dafc9d59a2f7160ba52b
5e25688 to
30ea22c
Compare
30ea22c to
c90a088
Compare
c90a088 to
6498ba1
Compare
|
rebased and formatting issues fixed |
|
ACK 6498ba1 |
meshcollider
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK 6498ba1
|
I don't think we should be using output descriptors to describe redeem scripts and witness scripts. See #24636 |
Following discussion in #16725 this is complementary data to expose. All outputs are inferred.