Skip to content

Commit be3ae51

Browse files
committed
rpc: make importaddress compatible with descriptors wallet
so it's simpler to watch for certain address/hex.
1 parent 3497df4 commit be3ae51

File tree

7 files changed

+90
-47
lines changed

7 files changed

+90
-47
lines changed

src/script/descriptor.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,8 +143,6 @@ std::string DescriptorChecksum(const Span<const char>& span)
143143
return ret;
144144
}
145145

146-
std::string AddChecksum(const std::string& str) { return str + "#" + DescriptorChecksum(str); }
147-
148146
////////////////////////////////////////////////////////////////////////////
149147
// Internal representation //
150148
////////////////////////////////////////////////////////////////////////////
@@ -1746,6 +1744,8 @@ std::string GetDescriptorChecksum(const std::string& descriptor)
17461744
return ret;
17471745
}
17481746

1747+
std::string AddChecksum(const std::string& str) { return str + "#" + DescriptorChecksum(str); }
1748+
17491749
std::unique_ptr<Descriptor> InferDescriptor(const CScript& script, const SigningProvider& provider)
17501750
{
17511751
return InferScript(script, ParseScriptContext::TOP, provider);

src/script/descriptor.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,11 @@ std::unique_ptr<Descriptor> Parse(const std::string& descriptor, FlatSigningProv
166166
*/
167167
std::string GetDescriptorChecksum(const std::string& descriptor);
168168

169+
/**
170+
* Simple wrapper to add the checksum at the end of the descriptor
171+
*/
172+
std::string AddChecksum(const std::string& str);
173+
169174
/** Find a descriptor for the specified `script`, using information from `provider` where possible.
170175
*
171176
* A non-ranged descriptor which only generates the specified script will be returned in all

src/wallet/rpc/backup.cpp

Lines changed: 51 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,8 @@ RPCHelpMan importprivkey()
213213
};
214214
}
215215

216+
static UniValue ProcessDescriptorImport(CWallet& wallet, const UniValue& data, const int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
217+
216218
RPCHelpMan importaddress()
217219
{
218220
return RPCHelpMan{"importaddress",
@@ -226,7 +228,7 @@ RPCHelpMan importaddress()
226228
"\nNote: If you import a non-standard raw script in hex form, outputs sending to it will be treated\n"
227229
"as change, and not show up in many RPCs.\n"
228230
"Note: Use \"getwalletinfo\" to query the scanning progress.\n"
229-
"Note: This command is only compatible with legacy wallets. Use \"importdescriptors\" for descriptor wallets.\n",
231+
"Note: For descriptor wallets, this command will create new descriptor/s, and only works if the wallet has private keys disabled.\n",
230232
{
231233
{"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The Bitcoin address (or hex-encoded script)"},
232234
{"label", RPCArg::Type::STR, RPCArg::Default{""}, "An optional label"},
@@ -247,7 +249,18 @@ RPCHelpMan importaddress()
247249
std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request);
248250
if (!pwallet) return UniValue::VNULL;
249251

250-
EnsureLegacyScriptPubKeyMan(*pwallet, true);
252+
// Use legacy spkm only if the wallet does not support descriptors.
253+
bool use_legacy = !pwallet->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS);
254+
if (!use_legacy) {
255+
// We don't allow mixing watch-only descriptors with spendable ones.
256+
if (!pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) {
257+
throw JSONRPCError(RPC_WALLET_ERROR, "Cannot import address in wallet with private keys enabled. "
258+
"Create wallet with no private keys to watch specific addresses/scripts");
259+
}
260+
} else {
261+
// In case the wallet is blank
262+
EnsureLegacyScriptPubKeyMan(*pwallet, /*also_create=*/true);
263+
}
251264

252265
const std::string strLabel{LabelFromValue(request.params[1])};
253266

@@ -273,33 +286,55 @@ RPCHelpMan importaddress()
273286
if (!request.params[3].isNull())
274287
fP2SH = request.params[3].get_bool();
275288

289+
// Import descriptor helper function
290+
const auto& import_descriptor = [pwallet](const std::string& desc, const std::string label) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) {
291+
UniValue data(UniValue::VType::VOBJ);
292+
data.pushKV("desc", AddChecksum(desc));
293+
if (!label.empty()) data.pushKV("label", label);
294+
const UniValue& ret = ProcessDescriptorImport(*pwallet, data, /*timestamp=*/1);
295+
if (ret.exists("error")) throw ret["error"];
296+
};
297+
276298
{
277299
LOCK(pwallet->cs_wallet);
278300

279-
CTxDestination dest = DecodeDestination(request.params[0].get_str());
301+
const std::string& address = request.params[0].get_str();
302+
CTxDestination dest = DecodeDestination(address);
280303
if (IsValidDestination(dest)) {
281304
if (fP2SH) {
282305
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Cannot use the p2sh flag with an address - use a script instead");
283306
}
284-
if (OutputTypeFromDestination(dest) == OutputType::BECH32M) {
285-
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Bech32m addresses cannot be imported into legacy wallets");
286-
}
287307

288308
pwallet->MarkDirty();
289309

290-
pwallet->ImportScriptPubKeys(strLabel, {GetScriptForDestination(dest)}, /*have_solving_data=*/false, /*apply_label=*/true, /*timestamp=*/1);
310+
if (use_legacy) {
311+
if (OutputTypeFromDestination(dest) == OutputType::BECH32M) {
312+
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Bech32m addresses cannot be imported into legacy wallets");
313+
}
314+
pwallet->ImportScriptPubKeys(strLabel, {GetScriptForDestination(dest)}, /*have_solving_data=*/false, /*apply_label=*/true, /*timestamp=*/1);
315+
} else {
316+
import_descriptor("addr(" + address + ")", strLabel);
317+
}
291318
} else if (IsHex(request.params[0].get_str())) {
292-
std::vector<unsigned char> data(ParseHex(request.params[0].get_str()));
293-
CScript redeem_script(data.begin(), data.end());
294-
295-
std::set<CScript> scripts = {redeem_script};
296-
pwallet->ImportScripts(scripts, /*timestamp=*/0);
319+
const std::string& hex = request.params[0].get_str();
320+
321+
if (use_legacy) {
322+
std::vector<unsigned char> data(ParseHex(hex));
323+
CScript redeem_script(data.begin(), data.end());
324+
std::set<CScript> scripts = {redeem_script};
325+
pwallet->ImportScripts(scripts, /*timestamp=*/0);
326+
if (fP2SH) {
327+
scripts.insert(GetScriptForDestination(ScriptHash(redeem_script)));
328+
}
329+
pwallet->ImportScriptPubKeys(strLabel, scripts, /*have_solving_data=*/false, /*apply_label=*/true, /*timestamp=*/1);
330+
} else {
331+
// P2SH Not allowed. Can't detect inner P2SH function from a raw hex.
332+
if (fP2SH) throw JSONRPCError(RPC_WALLET_ERROR, "P2SH import feature disabled for descriptors' wallet. "
333+
"Use 'importdescriptors' to specify inner P2SH function");
297334

298-
if (fP2SH) {
299-
scripts.insert(GetScriptForDestination(ScriptHash(redeem_script)));
335+
// Import descriptors
336+
import_descriptor("raw(" + hex + ")", strLabel);
300337
}
301-
302-
pwallet->ImportScriptPubKeys(strLabel, scripts, /*have_solving_data=*/false, /*apply_label=*/true, /*timestamp=*/1);
303338
} else {
304339
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid Bitcoin address or script");
305340
}

test/functional/test_framework/test_node.py

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -819,30 +819,3 @@ def importpubkey(self, pubkey, label=None, rescan=None):
819819
import_res = self.importdescriptors(req)
820820
if not import_res[0]['success']:
821821
raise JSONRPCException(import_res[0]['error'])
822-
823-
def importaddress(self, address, label=None, rescan=None, p2sh=None):
824-
wallet_info = self.getwalletinfo()
825-
if 'descriptors' not in wallet_info or ('descriptors' in wallet_info and not wallet_info['descriptors']):
826-
return self.__getattr__('importaddress')(address, label, rescan, p2sh)
827-
is_hex = False
828-
try:
829-
int(address ,16)
830-
is_hex = True
831-
desc = descsum_create('raw(' + address + ')')
832-
except Exception:
833-
desc = descsum_create('addr(' + address + ')')
834-
reqs = [{
835-
'desc': desc,
836-
'timestamp': 0 if rescan else 'now',
837-
'label': label if label else ''
838-
}]
839-
if is_hex and p2sh:
840-
reqs.append({
841-
'desc': descsum_create('p2sh(raw(' + address + '))'),
842-
'timestamp': 0 if rescan else 'now',
843-
'label': label if label else ''
844-
})
845-
import_res = self.importdescriptors(reqs)
846-
for res in import_res:
847-
if not res['success']:
848-
raise JSONRPCException(res['error'])

test/functional/wallet_basic.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,24 @@ def check_fee_amount(self, curr_balance, balance_with_fee, fee_per_byte, tx_size
5555
def get_vsize(self, txn):
5656
return self.nodes[0].decoderawtransaction(txn)['vsize']
5757

58+
def test_legacy_importaddress(self):
59+
if self.options.descriptors:
60+
return
61+
62+
addr = self.nodes[1].getnewaddress()
63+
self.nodes[1].sendtoaddress(addr, 10)
64+
self.sync_mempools(self.nodes[0:2])
65+
66+
self.log.info("Test 'importaddress' on a blank, private keys disabled, wallet with no descriptors support")
67+
self.nodes[0].createwallet(wallet_name="watch-only-legacy", disable_private_keys=False, descriptors=False, blank=True)
68+
wallet_watch_only = self.nodes[0].get_wallet_rpc("watch-only-legacy")
69+
wallet_watch_only.importaddress(addr)
70+
assert_equal(wallet_watch_only.getaddressinfo(addr)['ismine'], False)
71+
assert_equal(wallet_watch_only.getaddressinfo(addr)['iswatchonly'], True)
72+
assert_equal(wallet_watch_only.getaddressinfo(addr)['solvable'], False)
73+
assert_equal(wallet_watch_only.getbalances()["watchonly"]['untrusted_pending'], 10)
74+
self.nodes[0].unloadwallet("watch-only-legacy")
75+
5876
def run_test(self):
5977

6078
# Check that there's no UTXO on none of the nodes
@@ -540,6 +558,9 @@ def run_test(self):
540558
{"address": address_to_import},
541559
{"spendable": True})
542560

561+
# Test importaddress on a blank, private keys disabled, legacy wallet with no descriptors support
562+
self.test_legacy_importaddress()
563+
543564
# Mine a block from node0 to an address from node1
544565
coinbase_addr = self.nodes[1].getnewaddress()
545566
block_hash = self.generatetoaddress(self.nodes[0], 1, coinbase_addr, sync_fun=lambda: self.sync_all(self.nodes[0:3]))[0]

test/functional/wallet_descriptor.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,14 +115,23 @@ def run_test(self):
115115
self.log.info("Test disabled RPCs")
116116
assert_raises_rpc_error(-4, "Only legacy wallets are supported by this command", recv_wrpc.rpc.importprivkey, "cVpF924EspNh8KjYsfhgY96mmxvT6DgdWiTYMtMjuM74hJaU5psW")
117117
assert_raises_rpc_error(-4, "Only legacy wallets are supported by this command", recv_wrpc.rpc.importpubkey, send_wrpc.getaddressinfo(send_wrpc.getnewaddress())["pubkey"])
118-
assert_raises_rpc_error(-4, "Only legacy wallets are supported by this command", recv_wrpc.rpc.importaddress, recv_wrpc.getnewaddress())
119118
assert_raises_rpc_error(-4, "Only legacy wallets are supported by this command", recv_wrpc.rpc.importmulti, [])
120119
assert_raises_rpc_error(-4, "Only legacy wallets are supported by this command", recv_wrpc.rpc.addmultisigaddress, 1, [recv_wrpc.getnewaddress()])
121120
assert_raises_rpc_error(-4, "Only legacy wallets are supported by this command", recv_wrpc.rpc.dumpprivkey, recv_wrpc.getnewaddress())
122121
assert_raises_rpc_error(-4, "Only legacy wallets are supported by this command", recv_wrpc.rpc.dumpwallet, 'wallet.dump')
123122
assert_raises_rpc_error(-4, "Only legacy wallets are supported by this command", recv_wrpc.rpc.importwallet, 'wallet.dump')
124123
assert_raises_rpc_error(-4, "Only legacy wallets are supported by this command", recv_wrpc.rpc.sethdseed)
125124

125+
# Test importaddress
126+
self.log.info("Test watch-only descriptor wallet")
127+
self.nodes[0].createwallet(wallet_name="watch-only-desc", disable_private_keys=True, descriptors=True, blank=True)
128+
wallet_watch_only = self.nodes[0].get_wallet_rpc("watch-only-desc")
129+
wallet_watch_only.importaddress(addr)
130+
assert_equal(wallet_watch_only.getaddressinfo(addr)['ismine'], True)
131+
assert_equal(wallet_watch_only.getaddressinfo(addr)['solvable'], False)
132+
assert_equal(wallet_watch_only.getbalances()["mine"]['untrusted_pending'], 10)
133+
self.nodes[0].unloadwallet("watch-only-desc")
134+
126135
self.log.info("Test encryption")
127136
# Get the master fingerprint before encrypt
128137
info1 = send_wrpc.getaddressinfo(send_wrpc.getnewaddress())

test/functional/wallet_labels.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ def run_test(self):
212212
ad = BECH32_INVALID[l]
213213
assert_raises_rpc_error(
214214
-5,
215-
"Address is not valid" if self.options.descriptors else "Invalid Bitcoin address or script",
215+
"Invalid Bitcoin address or script",
216216
lambda: wallet_watch_only.importaddress(label=l, rescan=False, address=ad),
217217
)
218218

0 commit comments

Comments
 (0)