descriptor: handle listdescriptors(private=true) for descriptors having partial keys #32186
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32186. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
|
cc: @furszy |
|
🚧 At least one of the CI tasks failed. HintsTry to run the tests locally, according to the documentation. However, a CI failure may still
Leave a comment here, if you need help tracking down a confusing failure. |
|
I haven't checked the approach, but based on the title, I assume it is linked to #32078. |
rkrux
changed the title
rkrux
force-pushed
the
taproot-list-desc
branch
2 times, most recently
from
1adc7e7 to
92d9340
Compare
|
Concept ACK. |
src/script/descriptor.cpp
Outdated
There was a problem hiding this comment.
nit: 'Atleast' should be 'At least'
|
Code Review ACK 3d3d8491948c2825c43db7fae69c32aae343742f |
|
Approach NACK. There is no reason why this needs to be special cased for taproot.
#24361 would have fixed this. Feel free to pick it up. |
|
I see, it makes sense to solve it for all the cases; I'll rework this PR. |
When parsing descriptors with multiple keys (tr, wsh, sh, miniscript), we might not have all the private keys but only few of them (rest being public keys). `listdescriptors(private=true)` RPC should not fail in such scenario and instead return those partial private keys, using public keys for the rest.
rkrux
force-pushed
the
taproot-list-desc
branch
from
92d9340 to
60f432e
Compare
rkrux
changed the title
rkrux
force-pushed
the
taproot-list-desc
branch
from
60f432e to
a0c7384
Compare
|
🚧 At least one of the CI tasks failed. HintsTry to run the tests locally, according to the documentation. However, a CI failure may still
Leave a comment here, if you need help tracking down a confusing failure. |
|
I don't like the way this diff has turned out, seems repetitive. I will take inspiration from #24361 and spend some time figuring out how to make it work for Miniscript expressions as well. The branch name is outdated too now. Most likely will close this and open a new PR. |
|
@rkrux still working on this? |
|
@Eunovo I have not gotten around to trying the other approach yet as I was working on few other PRs. Please feel free to give it a shot if you want else I will give it another try some time later. Closing this PR either way. |
9 participants
When parsing descriptors with multiple keys (tr, wsh, sh, miniscript),
we might not have all the private keys but only few of them
(rest being public keys).
listdescriptors(private=true)RPC should notfail in such scenario and instead return those partial private keys,
using public keys for the rest.
Fixes #32078