diff options
author | Sjors Provoost <sjors@sprovoost.nl> | 2024-02-13 13:25:59 +0100 |
---|---|---|
committer | Sjors Provoost <sjors@sprovoost.nl> | 2024-04-16 17:47:43 +0200 |
commit | 4357158c4712d479522d5cd441ad4dd1693fdd05 (patch) | |
tree | e53759850827b1275022fe3ea96a405dfd7226d6 /src/wallet | |
parent | dc55531087478d01fbde4f5fbb75375b672960c3 (diff) | |
download | bitcoin-4357158c4712d479522d5cd441ad4dd1693fdd05.tar.xz |
wallet: return and display signer error
Both RPC and GUI now render a useful error message instead of (silently) failing.
Replace bool with util::Result<void> to clarify that this either succeeds or returns an error message.
Diffstat (limited to 'src/wallet')
-rw-r--r-- | src/wallet/external_signer_scriptpubkeyman.cpp | 14 | ||||
-rw-r--r-- | src/wallet/external_signer_scriptpubkeyman.h | 8 | ||||
-rw-r--r-- | src/wallet/interfaces.cpp | 2 | ||||
-rw-r--r-- | src/wallet/rpc/addresses.cpp | 5 | ||||
-rw-r--r-- | src/wallet/scriptpubkeyman.h | 1 | ||||
-rw-r--r-- | src/wallet/wallet.cpp | 4 | ||||
-rw-r--r-- | src/wallet/wallet.h | 7 |
7 files changed, 25 insertions, 16 deletions
diff --git a/src/wallet/external_signer_scriptpubkeyman.cpp b/src/wallet/external_signer_scriptpubkeyman.cpp index ce668539e6..b5703fa54a 100644 --- a/src/wallet/external_signer_scriptpubkeyman.cpp +++ b/src/wallet/external_signer_scriptpubkeyman.cpp @@ -13,6 +13,7 @@ #include <memory> #include <stdexcept> #include <string> +#include <univalue.h> #include <utility> #include <vector> @@ -52,7 +53,7 @@ ExternalSigner ExternalSignerScriptPubKeyMan::GetExternalSigner() { return signers[0]; } -bool ExternalSignerScriptPubKeyMan::DisplayAddress(const CTxDestination& dest, const ExternalSigner &signer) const +util::Result<void> ExternalSignerScriptPubKeyMan::DisplayAddress(const CTxDestination& dest, const ExternalSigner &signer) const { // TODO: avoid the need to infer a descriptor from inside a descriptor wallet const CScript& scriptPubKey = GetScriptForDestination(dest); @@ -61,10 +62,17 @@ bool ExternalSignerScriptPubKeyMan::DisplayAddress(const CTxDestination& dest, c const UniValue& result = signer.DisplayAddress(descriptor->ToString()); + const UniValue& error = result.find_value("error"); + if (error.isStr()) return util::Error{strprintf(_("Signer returned error: %s"), error.getValStr())}; + const UniValue& ret_address = result.find_value("address"); - if (!ret_address.isStr()) return false; + if (!ret_address.isStr()) return util::Error{_("Signer did not echo address")}; + + if (ret_address.getValStr() != EncodeDestination(dest)) { + return util::Error{strprintf(_("Signer echoed unexpected address %s"), ret_address.getValStr())}; + } - return ret_address.getValStr() == EncodeDestination(dest); + return util::Result<void>(); } // If sign is true, transaction must previously have been filled diff --git a/src/wallet/external_signer_scriptpubkeyman.h b/src/wallet/external_signer_scriptpubkeyman.h index b249833271..44286456b6 100644 --- a/src/wallet/external_signer_scriptpubkeyman.h +++ b/src/wallet/external_signer_scriptpubkeyman.h @@ -9,6 +9,8 @@ #include <memory> +struct bilingual_str; + namespace wallet { class ExternalSignerScriptPubKeyMan : public DescriptorScriptPubKeyMan { @@ -27,7 +29,11 @@ class ExternalSignerScriptPubKeyMan : public DescriptorScriptPubKeyMan static ExternalSigner GetExternalSigner(); - bool DisplayAddress(const CTxDestination& dest, const ExternalSigner &signer) const; + /** + * Display address on the device and verify that the returned value matches. + * @returns nothing or an error message + */ + util::Result<void> DisplayAddress(const CTxDestination& dest, const ExternalSigner& signer) const; TransactionError FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) const override; }; diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index d33e6f3873..e17e6c332d 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -247,7 +247,7 @@ public: return value.empty() ? m_wallet->EraseAddressReceiveRequest(batch, dest, id) : m_wallet->SetAddressReceiveRequest(batch, dest, id, value); } - bool displayAddress(const CTxDestination& dest) override + util::Result<void> displayAddress(const CTxDestination& dest) override { LOCK(m_wallet->cs_wallet); return m_wallet->DisplayAddress(dest); diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp index 7f068c05ef..bed9ec029a 100644 --- a/src/wallet/rpc/addresses.cpp +++ b/src/wallet/rpc/addresses.cpp @@ -789,9 +789,8 @@ RPCHelpMan walletdisplayaddress() throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid address"); } - if (!pwallet->DisplayAddress(dest)) { - throw JSONRPCError(RPC_MISC_ERROR, "Failed to display address"); - } + util::Result<void> res = pwallet->DisplayAddress(dest); + if (!res) throw JSONRPCError(RPC_MISC_ERROR, util::ErrorString(res).original); UniValue result(UniValue::VOBJ); result.pushKV("address", request.params[0].get_str()); diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 4575881d96..2c1ab8d44a 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -27,7 +27,6 @@ #include <unordered_map> enum class OutputType; -struct bilingual_str; namespace wallet { struct MigrationData; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 675d8f3985..8f4171eb15 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2667,7 +2667,7 @@ void ReserveDestination::ReturnDestination() address = CNoDestination(); } -bool CWallet::DisplayAddress(const CTxDestination& dest) +util::Result<void> CWallet::DisplayAddress(const CTxDestination& dest) { CScript scriptPubKey = GetScriptForDestination(dest); for (const auto& spk_man : GetScriptPubKeyMans(scriptPubKey)) { @@ -2678,7 +2678,7 @@ bool CWallet::DisplayAddress(const CTxDestination& dest) ExternalSigner signer = ExternalSignerScriptPubKeyMan::GetExternalSigner(); return signer_spk_man->DisplayAddress(dest, signer); } - return false; + return util::Error{_("There is no ScriptPubKeyManager for this address")}; } bool CWallet::LockCoin(const COutPoint& output, WalletBatch* batch) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 74c0c5ed08..6a998fa398 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -537,11 +537,8 @@ public: bool IsSpentKey(const CScript& scriptPubKey) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void SetSpentKeyState(WalletBatch& batch, const uint256& hash, unsigned int n, bool used, std::set<CTxDestination>& tx_destinations) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - /** Display address on an external signer. - * Returns false if the signer does not respond with a matching address. - * Returns false if external signer support is not compiled. - */ - bool DisplayAddress(const CTxDestination& dest) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + /** Display address on an external signer. */ + util::Result<void> DisplayAddress(const CTxDestination& dest) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool IsLockedCoin(const COutPoint& output) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool LockCoin(const COutPoint& output, WalletBatch* batch = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); |