rpc: have raw transaction decoding infer output descriptors#16795
rpc: have raw transaction decoding infer output descriptors#16795achow101 merged 1 commit intobitcoin:masterfrom
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.
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.
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.
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.
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.
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 |
|
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.