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, 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

    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?
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

Contributors
Sjors DrahtBot brunoerg achow101

Labels
Wallet