refactor: use util::Result for GetExternalSigner()

This commit does not change behavior, except that the error message no longer contains the function name.
This commit is contained in:
Sjors Provoost
2025-06-05 11:32:23 +02:00
parent fd4399cb9c
commit 8ba2f9b7c8
4 changed files with 19 additions and 13 deletions

View File

@@ -45,14 +45,14 @@ bool ExternalSignerScriptPubKeyMan::SetupDescriptor(WalletBatch& batch, std::uni
return true; return true;
} }
ExternalSigner ExternalSignerScriptPubKeyMan::GetExternalSigner() { util::Result<ExternalSigner> ExternalSignerScriptPubKeyMan::GetExternalSigner() {
const std::string command = gArgs.GetArg("-signer", ""); 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; std::vector<ExternalSigner> signers;
ExternalSigner::Enumerate(command, signers, Params().GetChainTypeString()); 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. // 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]; return signers[0];
} }
@@ -93,9 +93,12 @@ std::optional<PSBTError> ExternalSignerScriptPubKeyMan::FillPSBT(PartiallySigned
} }
if (complete) return {}; if (complete) return {};
std::string strFailReason; auto signer{GetExternalSigner()};
if(!GetExternalSigner().SignTransaction(psbt, strFailReason)) { if (!signer) throw std::runtime_error(util::ErrorString(signer).original);
tfm::format(std::cerr, "Failed to sign: %s\n", strFailReason);
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; return PSBTError::EXTERNAL_SIGNER_FAILED;
} }
if (finalize) FinalizePSBT(psbt); // This won't work in a multisig setup if (finalize) FinalizePSBT(psbt); // This won't work in a multisig setup

View File

@@ -8,6 +8,7 @@
#include <wallet/scriptpubkeyman.h> #include <wallet/scriptpubkeyman.h>
#include <memory> #include <memory>
#include <util/result.h>
struct bilingual_str; struct bilingual_str;
@@ -27,7 +28,7 @@ class ExternalSignerScriptPubKeyMan : public DescriptorScriptPubKeyMan
*/ */
bool SetupDescriptor(WalletBatch& batch, std::unique_ptr<Descriptor>desc); 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. * Display address on the device and verify that the returned value matches.

View File

@@ -2602,8 +2602,9 @@ util::Result<void> CWallet::DisplayAddress(const CTxDestination& dest)
if (signer_spk_man == nullptr) { if (signer_spk_man == nullptr) {
continue; continue;
} }
ExternalSigner signer = ExternalSignerScriptPubKeyMan::GetExternalSigner(); auto signer{ExternalSignerScriptPubKeyMan::GetExternalSigner()};
return signer_spk_man->DisplayAddress(dest, signer); 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")}; return util::Error{_("There is no ScriptPubKeyManager for this address")};
} }
@@ -3586,11 +3587,12 @@ void CWallet::SetupDescriptorScriptPubKeyMans()
return true; return true;
})) throw std::runtime_error("Error: cannot process db transaction for descriptors setup"); })) throw std::runtime_error("Error: cannot process db transaction for descriptors setup");
} else { } else {
ExternalSigner signer = ExternalSignerScriptPubKeyMan::GetExternalSigner(); auto signer = ExternalSignerScriptPubKeyMan::GetExternalSigner();
if (!signer) throw std::runtime_error(util::ErrorString(signer).original);
// TODO: add account parameter // TODO: add account parameter
int account = 0; 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"); if (!signer_res.isObject()) throw std::runtime_error(std::string(__func__) + ": Unexpected result");

View File

@@ -243,7 +243,7 @@ class WalletSignerTest(BitcoinTestFramework):
self.log.debug(f"-signer={self.mock_multi_signers_path()}") self.log.debug(f"-signer={self.mock_multi_signers_path()}")
self.log.info('Test multiple external signers') 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__': if __name__ == '__main__':
WalletSignerTest(__file__).main() WalletSignerTest(__file__).main()