RPC: Return external_signer in getwalletinfo #24307

pull kristapsk wants to merge 1 commits into bitcoin:master from kristapsk:getwalletinfo-external_signer changing 2 files +4 −0

kristapsk commented at 1:27 AM on February 10, 2022: contributor

Add external_signer to the result object of getwalletinfo RPC which indicates whether WALLET_FLAG_EXTERNAL_SIGNER flag is set for the wallet.
RPC: Return external_signer in getwalletinfo b75f4c89ec
DrahtBot added the label RPC/REST/ZMQ on Feb 10, 2022
DrahtBot added the label Wallet on Feb 10, 2022
unknown approved
unknown commented at 3:28 AM on February 10, 2022: none

utACK https://github.com/bitcoin/bitcoin/pull/24307/commits/b75f4c89ec4d33a3014ccd5151964881b5e0aa1c
S3RK commented at 7:12 AM on February 10, 2022: member

utACK b75f4c89ec4d33a3014ccd5151964881b5e0aa1c
Sjors commented at 6:09 PM on February 10, 2022: member

#21928 contains this change as well, though it's valid on its own.
ghost commented at 6:19 PM on February 10, 2022: none

#21928 contains this change as well, though it's valid on its own. @Sjors why not build on top of this PR?
Sjors commented at 6:33 PM on February 10, 2022: member

@prayank23 my PR is almost a year old. But if this gets merged first I'll rebase it of course.
kristapsk commented at 9:22 PM on February 10, 2022: contributor

@Sjors Didn't know about your PR. Was playing around with external signer stuff and at one moment wanted to check is that flag really set for a specific wallet and noticed I can't do that. Anyway, this one is simple change, should be easy to review, test and merge sooner.
ghost commented at 9:28 PM on February 10, 2022: none

@prayank23 my PR is almost a year old. But if this gets merged first I'll rebase it of course.

TBH I can't review all the things in #21928

This looks ready for review and merge :)
fanquake requested review from achow101 on Feb 11, 2022
DrahtBot commented at 11:36 AM on February 11, 2022: member


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #21928 (wallet: allow toggling external_signer flag by Sjors)
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.
brunoerg commented at 1:30 PM on February 11, 2022: member

utACK b75f4c89ec4d33a3014ccd5151964881b5e0aa1c
achow101 commented at 5:19 PM on February 11, 2022: member

ACK b75f4c89ec4d33a3014ccd5151964881b5e0aa1c
achow101 merged this on Feb 11, 2022
achow101 closed this on Feb 11, 2022
kristapsk deleted the branch on Feb 11, 2022
sidhujag referenced this in commit e80e1e1a33 on Feb 12, 2022
luke-jr commented at 11:37 PM on February 12, 2022: member

Seems like this should probably be a string indicating the external signer program... but I guess changing it later is okay?
Sjors commented at 7:17 PM on February 15, 2022: member

Fine by me :-) I'll rebase. @luke-jr that would just echo -signer though
luke-jr commented at 8:01 PM on February 15, 2022: member

For now, yes - but eventually it'd be ideal to have wallets with different external signers.
kristapsk commented at 8:33 PM on February 15, 2022: contributor

I agree with @luke-jr, different external signers for different wallets in the future makes sense.
Sjors commented at 9:11 PM on February 15, 2022: member

different external signers

To clarify: different external signer programs, e.g. alternatives to HWI. You can already have multiple wallets each using a different (type of) physical device, all via HWI.
w0xlt commented at 11:19 PM on February 16, 2022: contributor

ACK b75f4c8
DrahtBot locked this on Feb 16, 2023