From 8ba2f9b7c8a6c6a91cc718d256354f7a73083b68 Mon Sep 17 00:00:00 2001
From: Sjors Provoost <sjors@sprovoost.nl>
Date: Thu, 5 Jun 2025 11:32:23 +0200
Subject: [PATCH] refactor: use util::Result for GetExternalSigner()

This commit does not change behavior, except that the error message no longer contains the function name.
---
 src/wallet/external_signer_scriptpubkeyman.cpp | 17 ++++++++++-------
 src/wallet/external_signer_scriptpubkeyman.h   |  3 ++-
 src/wallet/wallet.cpp                          | 10 ++++++----
 test/functional/wallet_signer.py               |  2 +-
 4 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/src/wallet/external_signer_scriptpubkeyman.cpp b/src/wallet/external_signer_scriptpubkeyman.cpp
index 7268354f1a9..5ffc802ec14 100644
--- a/src/wallet/external_signer_scriptpubkeyman.cpp
+++ b/src/wallet/external_signer_scriptpubkeyman.cpp
@@ -45,14 +45,14 @@ bool ExternalSignerScriptPubKeyMan::SetupDescriptor(WalletBatch& batch, std::uni
     return true;
 }
 
-ExternalSigner ExternalSignerScriptPubKeyMan::GetExternalSigner() {
+ util::Result<ExternalSigner> ExternalSignerScriptPubKeyMan::GetExternalSigner() {
     const std::string command = gArgs.GetArg("-signer", "");
-    if (command == "") throw std::runtime_error(std::string(__func__) + ": restart bitcoind with -signer=<cmd>");
+    if (command == "") return util::Error{Untranslated("restart bitcoind with -signer=<cmd>")};
     std::vector<ExternalSigner> signers;
     ExternalSigner::Enumerate(command, signers, Params().GetChainTypeString());
-    if (signers.empty()) throw std::runtime_error(std::string(__func__) + ": No external signers found");
+    if (signers.empty()) return util::Error{Untranslated("No external signers found")};
     // TODO: add fingerprint argument instead of failing in case of multiple signers.
-    if (signers.size() > 1) throw std::runtime_error(std::string(__func__) + ": More than one external signer found. Please connect only one at a time.");
+    if (signers.size() > 1) return util::Error{Untranslated("More than one external signer found. Please connect only one at a time.")};
     return signers[0];
 }
 
@@ -93,9 +93,12 @@ std::optional<PSBTError> ExternalSignerScriptPubKeyMan::FillPSBT(PartiallySigned
     }
     if (complete) return {};
 
-    std::string strFailReason;
-    if(!GetExternalSigner().SignTransaction(psbt, strFailReason)) {
-        tfm::format(std::cerr, "Failed to sign: %s\n", strFailReason);
+    auto signer{GetExternalSigner()};
+    if (!signer) throw std::runtime_error(util::ErrorString(signer).original);
+
+    std::string failure_reason;
+    if(!signer->SignTransaction(psbt, failure_reason)) {
+        tfm::format(std::cerr, "Failed to sign: %s\n", failure_reason);
         return PSBTError::EXTERNAL_SIGNER_FAILED;
     }
     if (finalize) FinalizePSBT(psbt); // This won't work in a multisig setup
diff --git a/src/wallet/external_signer_scriptpubkeyman.h b/src/wallet/external_signer_scriptpubkeyman.h
index 66c2dd9d170..27c0258ea51 100644
--- a/src/wallet/external_signer_scriptpubkeyman.h
+++ b/src/wallet/external_signer_scriptpubkeyman.h
@@ -8,6 +8,7 @@
 #include <wallet/scriptpubkeyman.h>
 
 #include <memory>
+#include <util/result.h>
 
 struct bilingual_str;
 
@@ -27,7 +28,7 @@ class ExternalSignerScriptPubKeyMan : public DescriptorScriptPubKeyMan
   */
   bool SetupDescriptor(WalletBatch& batch, std::unique_ptr<Descriptor>desc);
 
-  static ExternalSigner GetExternalSigner();
+  static util::Result<ExternalSigner> GetExternalSigner();
 
   /**
   * Display address on the device and verify that the returned value matches.
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 6542abc23df..62a5fba7c40 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -2602,8 +2602,9 @@ util::Result<void> CWallet::DisplayAddress(const CTxDestination& dest)
         if (signer_spk_man == nullptr) {
             continue;
         }
-        ExternalSigner signer = ExternalSignerScriptPubKeyMan::GetExternalSigner();
-        return signer_spk_man->DisplayAddress(dest, signer);
+        auto signer{ExternalSignerScriptPubKeyMan::GetExternalSigner()};
+        if (!signer) throw std::runtime_error(util::ErrorString(signer).original);
+        return signer_spk_man->DisplayAddress(dest, *signer);
     }
     return util::Error{_("There is no ScriptPubKeyManager for this address")};
 }
@@ -3586,11 +3587,12 @@ void CWallet::SetupDescriptorScriptPubKeyMans()
             return true;
         })) throw std::runtime_error("Error: cannot process db transaction for descriptors setup");
     } else {
-        ExternalSigner signer = ExternalSignerScriptPubKeyMan::GetExternalSigner();
+        auto signer = ExternalSignerScriptPubKeyMan::GetExternalSigner();
+        if (!signer) throw std::runtime_error(util::ErrorString(signer).original);
 
         // TODO: add account parameter
         int account = 0;
-        UniValue signer_res = signer.GetDescriptors(account);
+        UniValue signer_res = signer->GetDescriptors(account);
 
         if (!signer_res.isObject()) throw std::runtime_error(std::string(__func__) + ": Unexpected result");
 
diff --git a/test/functional/wallet_signer.py b/test/functional/wallet_signer.py
index 3159f66641e..c9ac876eb71 100755
--- a/test/functional/wallet_signer.py
+++ b/test/functional/wallet_signer.py
@@ -243,7 +243,7 @@ class WalletSignerTest(BitcoinTestFramework):
         self.log.debug(f"-signer={self.mock_multi_signers_path()}")
         self.log.info('Test multiple external signers')
 
-        assert_raises_rpc_error(-1, "GetExternalSigner: More than one external signer found", self.nodes[1].createwallet, wallet_name='multi_hww', disable_private_keys=True, external_signer=True)
+        assert_raises_rpc_error(-1, "More than one external signer found", self.nodes[1].createwallet, wallet_name='multi_hww', disable_private_keys=True, external_signer=True)
 
 if __name__ == '__main__':
     WalletSignerTest(__file__).main()