aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--doc/external-signer.md3
-rw-r--r--src/interfaces/wallet.h2
-rw-r--r--src/qt/walletmodel.cpp9
-rw-r--r--src/qt/walletmodel.h2
-rw-r--r--src/wallet/external_signer_scriptpubkeyman.cpp21
-rw-r--r--src/wallet/external_signer_scriptpubkeyman.h8
-rw-r--r--src/wallet/interfaces.cpp2
-rw-r--r--src/wallet/rpc/addresses.cpp5
-rw-r--r--src/wallet/scriptpubkeyman.h1
-rw-r--r--src/wallet/wallet.cpp6
-rw-r--r--src/wallet/wallet.h4
-rwxr-xr-xtest/functional/mocks/signer.py31
-rwxr-xr-xtest/functional/wallet_signer.py12
13 files changed, 68 insertions, 38 deletions
diff --git a/doc/external-signer.md b/doc/external-signer.md
index de44cdd880..1468e852ef 100644
--- a/doc/external-signer.md
+++ b/doc/external-signer.md
@@ -150,6 +150,9 @@ Example, display the first native SegWit receive address on Testnet:
The command MUST be able to figure out the address type from the descriptor.
+The command MUST return an object containing `{"address": "[the address]"}`.
+As a sanity check, for devices that support this, it SHOULD ask the device to derive the address.
+
If <descriptor> contains a master key fingerprint, the command MUST fail if it does not match the fingerprint known by the device.
If <descriptor> contains an xpub, the command MUST fail if it does not match the xpub known by the device.
diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h
index 6114236623..c41f35829d 100644
--- a/src/interfaces/wallet.h
+++ b/src/interfaces/wallet.h
@@ -127,7 +127,7 @@ public:
virtual bool setAddressReceiveRequest(const CTxDestination& dest, const std::string& id, const std::string& value) = 0;
//! Display address on external signer
- virtual bool displayAddress(const CTxDestination& dest) = 0;
+ virtual util::Result<void> displayAddress(const CTxDestination& dest) = 0;
//! Lock coin.
virtual bool lockCoin(const COutPoint& output, const bool write_to_db) = 0;
diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp
index 1bdf94d3b5..fe000bcbb8 100644
--- a/src/qt/walletmodel.cpp
+++ b/src/qt/walletmodel.cpp
@@ -569,16 +569,17 @@ bool WalletModel::bumpFee(uint256 hash, uint256& new_hash)
return true;
}
-bool WalletModel::displayAddress(std::string sAddress) const
+void WalletModel::displayAddress(std::string sAddress) const
{
CTxDestination dest = DecodeDestination(sAddress);
- bool res = false;
try {
- res = m_wallet->displayAddress(dest);
+ util::Result<void> result = m_wallet->displayAddress(dest);
+ if (!result) {
+ QMessageBox::warning(nullptr, tr("Signer error"), QString::fromStdString(util::ErrorString(result).translated));
+ }
} catch (const std::runtime_error& e) {
QMessageBox::critical(nullptr, tr("Can't display address"), e.what());
}
- return res;
}
bool WalletModel::isWalletEnabled()
diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h
index 503ee16823..ab2096c1fe 100644
--- a/src/qt/walletmodel.h
+++ b/src/qt/walletmodel.h
@@ -130,7 +130,7 @@ public:
UnlockContext requestUnlock();
bool bumpFee(uint256 hash, uint256& new_hash);
- bool displayAddress(std::string sAddress) const;
+ void displayAddress(std::string sAddress) const;
static bool isWalletEnabled();
diff --git a/src/wallet/external_signer_scriptpubkeyman.cpp b/src/wallet/external_signer_scriptpubkeyman.cpp
index a71f8f9fbc..b5703fa54a 100644
--- a/src/wallet/external_signer_scriptpubkeyman.cpp
+++ b/src/wallet/external_signer_scriptpubkeyman.cpp
@@ -9,9 +9,11 @@
#include <wallet/external_signer_scriptpubkeyman.h>
#include <iostream>
+#include <key_io.h>
#include <memory>
#include <stdexcept>
#include <string>
+#include <univalue.h>
#include <utility>
#include <vector>
@@ -51,15 +53,26 @@ ExternalSigner ExternalSignerScriptPubKeyMan::GetExternalSigner() {
return signers[0];
}
-bool ExternalSignerScriptPubKeyMan::DisplayAddress(const CScript scriptPubKey, 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);
auto provider = GetSolvingProvider(scriptPubKey);
auto descriptor = InferDescriptor(scriptPubKey, *provider);
- signer.DisplayAddress(descriptor->ToString());
- // TODO inspect result
- return true;
+ 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 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 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 c052ce6129..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 CScript scriptPubKey, 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 a866666f64..0c1cae7253 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 96c4397504..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)) {
@@ -2676,9 +2676,9 @@ bool CWallet::DisplayAddress(const CTxDestination& dest)
continue;
}
ExternalSigner signer = ExternalSignerScriptPubKeyMan::GetExternalSigner();
- return signer_spk_man->DisplayAddress(scriptPubKey, signer);
+ 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 b49b5a7d0d..6a998fa398 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -537,8 +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 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);
diff --git a/test/functional/mocks/signer.py b/test/functional/mocks/signer.py
index 5f4fad6380..23d163aac3 100755
--- a/test/functional/mocks/signer.py
+++ b/test/functional/mocks/signer.py
@@ -25,35 +25,36 @@ def getdescriptors(args):
sys.stdout.write(json.dumps({
"receive": [
- "pkh([00000001/44'/1'/" + args.account + "']" + xpub + "/0/*)#vt6w3l3j",
- "sh(wpkh([00000001/49'/1'/" + args.account + "']" + xpub + "/0/*))#r0grqw5x",
- "wpkh([00000001/84'/1'/" + args.account + "']" + xpub + "/0/*)#x30uthjs",
- "tr([00000001/86'/1'/" + args.account + "']" + xpub + "/0/*)#sng9rd4t"
+ "pkh([00000001/44h/1h/" + args.account + "']" + xpub + "/0/*)#aqllu46s",
+ "sh(wpkh([00000001/49h/1h/" + args.account + "']" + xpub + "/0/*))#5dh56mgg",
+ "wpkh([00000001/84h/1h/" + args.account + "']" + xpub + "/0/*)#h62dxaej",
+ "tr([00000001/86h/1h/" + args.account + "']" + xpub + "/0/*)#pcd5w87f"
],
"internal": [
- "pkh([00000001/44'/1'/" + args.account + "']" + xpub + "/1/*)#all0v2p2",
- "sh(wpkh([00000001/49'/1'/" + args.account + "']" + xpub + "/1/*))#kwx4c3pe",
- "wpkh([00000001/84'/1'/" + args.account + "']" + xpub + "/1/*)#h92akzzg",
- "tr([00000001/86'/1'/" + args.account + "']" + xpub + "/1/*)#p8dy7c9n"
+ "pkh([00000001/44h/1h/" + args.account + "']" + xpub + "/1/*)#v567pq2g",
+ "sh(wpkh([00000001/49h/1h/" + args.account + "']" + xpub + "/1/*))#pvezzyah",
+ "wpkh([00000001/84h/1h/" + args.account + "']" + xpub + "/1/*)#xw0vmgf2",
+ "tr([00000001/86h/1h/" + args.account + "']" + xpub + "/1/*)#svg4njw3"
]
}))
def displayaddress(args):
- # Several descriptor formats are acceptable, so allowing for potential
- # changes to InferDescriptor:
if args.fingerprint != "00000001":
return sys.stdout.write(json.dumps({"error": "Unexpected fingerprint", "fingerprint": args.fingerprint}))
- expected_desc = [
- "wpkh([00000001/84'/1'/0'/0/0]02c97dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7)#0yneg42r",
- "tr([00000001/86'/1'/0'/0/0]c97dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7)#4vdj9jqk",
- ]
+ expected_desc = {
+ "wpkh([00000001/84h/1h/0h/0/0]02c97dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7)#3te6hhy7": "bcrt1qm90ugl4d48jv8n6e5t9ln6t9zlpm5th68x4f8g",
+ "sh(wpkh([00000001/49h/1h/0h/0/0]02c97dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7))#kz9y5w82": "2N2gQKzjUe47gM8p1JZxaAkTcoHPXV6YyVp",
+ "pkh([00000001/44h/1h/0h/0/0]02c97dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7)#q3pqd8wh": "n1LKejAadN6hg2FrBXoU1KrwX4uK16mco9",
+ "tr([00000001/86h/1h/0h/0/0]c97dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7)#puqqa90m": "tb1phw4cgpt6cd30kz9k4wkpwm872cdvhss29jga2xpmftelhqll62mscq0k4g",
+ "wpkh([00000001/84h/1h/0h/0/1]03a20a46308be0b8ded6dff0a22b10b4245c587ccf23f3b4a303885be3a524f172)#aqpjv5xr": "wrong_address",
+ }
if args.desc not in expected_desc:
return sys.stdout.write(json.dumps({"error": "Unexpected descriptor", "desc": args.desc}))
- return sys.stdout.write(json.dumps({"address": "bcrt1qm90ugl4d48jv8n6e5t9ln6t9zlpm5th68x4f8g"}))
+ return sys.stdout.write(json.dumps({"address": expected_desc[args.desc]}))
def signtx(args):
if args.fingerprint != "00000001":
diff --git a/test/functional/wallet_signer.py b/test/functional/wallet_signer.py
index 32a1887153..abfc3c1ba1 100755
--- a/test/functional/wallet_signer.py
+++ b/test/functional/wallet_signer.py
@@ -130,8 +130,9 @@ class WalletSignerTest(BitcoinTestFramework):
assert_equal(address_info['hdkeypath'], "m/86h/1h/0h/0/0")
self.log.info('Test walletdisplayaddress')
- result = hww.walletdisplayaddress(address1)
- assert_equal(result, {"address": address1})
+ for address in [address1, address2, address3]:
+ result = hww.walletdisplayaddress(address)
+ assert_equal(result, {"address": address})
# Handle error thrown by script
self.set_mock_result(self.nodes[1], "2")
@@ -140,6 +141,13 @@ class WalletSignerTest(BitcoinTestFramework):
)
self.clear_mock_result(self.nodes[1])
+ # Returned address MUST match:
+ address_fail = hww.getnewaddress(address_type="bech32")
+ assert_equal(address_fail, "bcrt1ql7zg7ukh3dwr25ex2zn9jse926f27xy2jz58tm")
+ assert_raises_rpc_error(-1, 'Signer echoed unexpected address wrong_address',
+ hww.walletdisplayaddress, address_fail
+ )
+
self.log.info('Prepare mock PSBT')
self.nodes[0].sendtoaddress(address4, 1)
self.generate(self.nodes[0], 1)
generated by cgit v1.2.3 (git 2.26.2) at 2024-08-28 02:42:08 +0000