-
Notifications
You must be signed in to change notification settings - Fork 37.7k
Miscellaneous external signer changes #21666
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
Rather than 3 different messages that are confusing / leak implementation details, use a single message, that is similar to other wallet related messages. i.e: "Compiled without sqlite support (required for descriptor wallets)".
This is undocumented and unused.
It's not clear why this need it's own exception class, as opposed to just throwing std::runtime_error().
tACK c8f469c Tested with a locally rebased https://github.com/bitcoin-core/gui/pull/4/files (with minimal changes). 06a0673 was added so that the GUI doesn't throw an error in the create wallet menu if HWI is configured and there's a problem. But it looks like I'm not even using that. I'll bring it back if I need it. How did you find the includes that were needed for With regards to c8f469c it may be useful to catch HWI specific errors in the GUI in order to present more actionable advise, but we can revert if needed. For now the GUI just throws rather useless "Can't list signers" / "Sign failed" / "Can't display address" messages anyway. |
utACK c8f469c straight forward cleanups |
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. 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. |
Just by reading through the code and looking at I agree that other code can be (re-)added if/when it's actually used. |
These are a few followups after #21467.