wallet: have external signer use PSBT error code EXTERNAL_SIGNER_NOT_FOUND #32682

1. 
   Sjors commented at 10:06 am on June 5, 2025: member

   When attempting to sign a transaction involving an external signer, if the device isn’t connected we throw an std::runtime_error. This prevents the (mainly GUI) code that’s actually supposed to handle this case from running.

   This PR returns a PSBTError::EXTERNAL_SIGNER_NOT_FOUND instead of throwing.

   The first commit is a refactor to have GetExternalSigner() return a util::Result<ExternalSigner> so the caller can decide how to handle the error. There are two other places where call GetExternalSigner() which this PR doesn’t change (which I think is fine there).

   Before:

   After (the translation already exist):

   Fixes #32426

   Additionally use LogWarning instead of std::cerr for both a missing signer and failure to sign.

2. refactor: use util::Result for GetExternalSigner()

   This commit does not change behavior, except that the error message no longer contains the function name.

   8ba2f9b7c8
3. wallet: use PSBTError::EXTERNAL_SIGNER_NOT_FOUND

   Instead of throwing a runtime error, let the caller decide how to handle a missing signer.
   
   GUI code was already in place to handle this, but it was unused until this commit.
   
   Fixes #32426
   
   Additionally use LogWarning instead of std::cerr.

   0a4ee93529
4. 
   DrahtBot commented at 10:06 am on June 5, 2025: contributor

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

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32682.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	achow101
   Stale ACK	brunoerg

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

5. DrahtBot added the label Wallet on Jun 5, 2025
6. 
   brunoerg commented at 2:26 pm on June 9, 2025: contributor

   Concept ACK
7. 
   achow101 commented at 9:45 pm on June 9, 2025: member

   ACK 0a4ee93529d68a31f3ba6c7c6009954be47bbbd6
8. DrahtBot requested review from brunoerg on Jun 9, 2025
9. in src/wallet/external_signer_scriptpubkeyman.cpp:99 in 0a4ee93529 outdated

    93@@ -94,11 +94,14 @@ std::optional<PSBTError> ExternalSignerScriptPubKeyMan::FillPSBT(PartiallySigned
    94     if (complete) return {};
    95 
    96     auto signer{GetExternalSigner()};
    97-    if (!signer) throw std::runtime_error(util::ErrorString(signer).original);
    98+    if (!signer) {
    99+        LogWarning("%s", util::ErrorString(signer).original);
   100+        return PSBTError::EXTERNAL_SIGNER_NOT_FOUND;
   

   ---

   ---

   

   brunoerg commented at 9:47 pm on June 9, 2025:

   Perhaps we could have a test for it since it doesn’t throw a runtime error anymore?

   ---

   

   Sjors commented at 7:44 am on June 13, 2025:

   Added a functional test.

   Note that this test would pass on the original code as well, with just a slightly different error:

   0diff --git a/test/functional/wallet_signer.py b/test/functional/wallet_signer.py
   1index 58705a23a3..07f18124c9 100755
   2--- a/test/functional/wallet_signer.py
   3+++ b/test/functional/wallet_signer.py
   4@@ -259,7 +259,7 @@ class WalletSignerTest(BitcoinTestFramework):
   5
   6         # Try to spend
   7         dest = hww.getrawchangeaddress()
   8-        assert_raises_rpc_error(-25, "External signer not found", hww.send, outputs=[{dest:0.5}])
   9+        assert_raises_rpc_error(-1, "GetExternalSigner: No external signers found", hww.send, outputs=[{dest:0.5}])
   

   A unit test might be able to cover PSBTError::EXTERNAL_SIGNER_NOT_FOUND directly.

10. DrahtBot requested review from brunoerg on Jun 9, 2025
11. brunoerg approved
12. 
    brunoerg commented at 9:48 pm on June 9, 2025: contributor

    code review ACK 0a4ee93529d68a31f3ba6c7c6009954be47bbbd6
13. test: detect no external signer connected 9dfc61d95f
14. 
    achow101 commented at 0:12 am on June 14, 2025: member

    ACK 9dfc61d95f0082672a9b90528386e6bcd7014a78
15. DrahtBot requested review from brunoerg on Jun 14, 2025

Contributors
Sjors DrahtBot brunoerg achow101

Review Requested
brunoerg

Labels
Wallet