Skip to content

Commit a6b5ec1

Browse files
marcohextormarcohextor
committed
rpc: creates possibility to preserve labels on importprivkey
- changes importprivkey behavior to overwrite existent label if one is passed and keep existing ones if no label is passed - tests behavior of importprivkey on existing address labels and different same key destination
1 parent f504a14 commit a6b5ec1

File tree

4 files changed

+172
-3
lines changed

4 files changed

+172
-3
lines changed

‎doc/release-notes-pr13381.md

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
RPC importprivkey: new label behavior
2+
-------------------------------------
3+
4+
Previously, `importprivkey` automatically added the default empty label
5+
("") to all addresses associated with the imported private key. Now it
6+
defaults to using any existing label for those addresses. For example:
7+
8+
- Old behavior: you import a watch-only address with the label "cold
9+
wallet". Later, you import the corresponding private key using the
10+
default settings. The address's label is changed from "cold wallet"
11+
to "".
12+
13+
- New behavior: you import a watch-only address with the label "cold
14+
wallet". Later, you import the corresponding private key using the
15+
default settings. The address's label remains "cold wallet".
16+
17+
In both the previous and current case, if you directly specify a label
18+
during the import, that label will override whatever previous label the
19+
addresses may have had. Also in both cases, if none of the addresses
20+
previously had a label, they will still receive the default empty label
21+
(""). Examples:
22+
23+
- You import a watch-only address with the label "temporary". Later you
24+
import the corresponding private key with the label "final". The
25+
address's label will be changed to "final".
26+
27+
- You use the default settings to import a private key for an address that
28+
was not previously in the wallet. Its addresses will receive the default
29+
empty label ("").

‎src/wallet/rpcdump.cpp

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ UniValue importprivkey(const JSONRPCRequest& request)
113113
"Hint: use importmulti to import more than one private key.\n"
114114
"\nArguments:\n"
115115
"1. \"privkey\" (string, required) The private key (see dumpprivkey)\n"
116-
"2. \"label\" (string, optional, default=\"\") An optional label\n"
116+
"2. \"label\" (string, optional, default=current label if address exists, otherwise \"\") An optional label\n"
117117
"3. rescan (boolean, optional, default=true) Rescan the wallet for transactions\n"
118118
"\nNote: This call can take over an hour to complete if rescan is true, during that time, other rpc calls\n"
119119
"may report that the imported key exists but related transactions are still missing, leading to temporarily incorrect/bogus balances and unspent outputs until rescan completes.\n"
@@ -162,9 +162,14 @@ UniValue importprivkey(const JSONRPCRequest& request)
162162
CKeyID vchAddress = pubkey.GetID();
163163
{
164164
pwallet->MarkDirty();
165-
// We don't know which corresponding address will be used; label them all
165+
166+
// We don't know which corresponding address will be used;
167+
// label all new addresses, and label existing addresses if a
168+
// label was passed.
166169
for (const auto& dest : GetAllDestinationsForKey(pubkey)) {
167-
pwallet->SetAddressBook(dest, strLabel, "receive");
170+
if (!request.params[1].isNull() || pwallet->mapAddressBook.count(dest) == 0) {
171+
pwallet->SetAddressBook(dest, strLabel, "receive");
172+
}
168173
}
169174

170175
// Don't throw error in case a key is already there

‎test/functional/test_runner.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@
157157
'feature_nulldummy.py',
158158
'mempool_accept.py',
159159
'wallet_import_rescan.py',
160+
'wallet_import_with_label.py',
160161
'rpc_bind.py --ipv4',
161162
'rpc_bind.py --ipv6',
162163
'rpc_bind.py --nonloopback',

‎test/functional/wallet_import_with_label.py

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
#!/usr/bin/env python3
2+
# Copyright (c) 2018 The Bitcoin Core developers
3+
# Distributed under the MIT software license, see the accompanying
4+
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
5+
"""Test the behavior of RPC importprivkey on set and unset labels of
6+
addresses.
7+
8+
It tests different cases in which an address is imported with importaddress
9+
with or without a label and then its private key is imported with importprivkey
10+
with and without a label.
11+
"""
12+
13+
from test_framework.test_framework import BitcoinTestFramework
14+
from test_framework.util import assert_equal
15+
16+
17+
class ImportWithLabel(BitcoinTestFramework):
18+
def set_test_params(self):
19+
self.num_nodes = 2
20+
self.setup_clean_chain = True
21+
22+
def skip_test_if_missing_module(self):
23+
self.skip_if_no_wallet()
24+
25+
def run_test(self):
26+
"""Main test logic"""
27+
28+
self.log.info(
29+
"Test importaddress with label and importprivkey without label."
30+
)
31+
self.log.info("Import a watch-only address with a label.")
32+
address = self.nodes[0].getnewaddress()
33+
label = "Test Label"
34+
self.nodes[1].importaddress(address, label)
35+
address_assert = self.nodes[1].getaddressinfo(address)
36+
37+
assert_equal(address_assert["iswatchonly"], True)
38+
assert_equal(address_assert["ismine"], False)
39+
assert_equal(address_assert["label"], label)
40+
41+
self.log.info(
42+
"Import the watch-only address's private key without a "
43+
"label and the address should keep its label."
44+
)
45+
priv_key = self.nodes[0].dumpprivkey(address)
46+
self.nodes[1].importprivkey(priv_key)
47+
48+
assert_equal(label, self.nodes[1].getaddressinfo(address)["label"])
49+
50+
self.log.info(
51+
"Test importaddress without label and importprivkey with label."
52+
)
53+
self.log.info("Import a watch-only address without a label.")
54+
address2 = self.nodes[0].getnewaddress()
55+
self.nodes[1].importaddress(address2)
56+
address_assert2 = self.nodes[1].getaddressinfo(address2)
57+
58+
assert_equal(address_assert2["iswatchonly"], True)
59+
assert_equal(address_assert2["ismine"], False)
60+
assert_equal(address_assert2["label"], "")
61+
62+
self.log.info(
63+
"Import the watch-only address's private key with a "
64+
"label and the address should have its label updated."
65+
)
66+
priv_key2 = self.nodes[0].dumpprivkey(address2)
67+
label2 = "Test Label 2"
68+
self.nodes[1].importprivkey(priv_key2, label2)
69+
70+
assert_equal(label2, self.nodes[1].getaddressinfo(address2)["label"])
71+
72+
self.log.info("Test importaddress with label and importprivkey with label.")
73+
self.log.info("Import a watch-only address with a label.")
74+
address3 = self.nodes[0].getnewaddress()
75+
label3_addr = "Test Label 3 for importaddress"
76+
self.nodes[1].importaddress(address3, label3_addr)
77+
address_assert3 = self.nodes[1].getaddressinfo(address3)
78+
79+
assert_equal(address_assert3["iswatchonly"], True)
80+
assert_equal(address_assert3["ismine"], False)
81+
assert_equal(address_assert3["label"], label3_addr)
82+
83+
self.log.info(
84+
"Import the watch-only address's private key with a "
85+
"label and the address should have its label updated."
86+
)
87+
priv_key3 = self.nodes[0].dumpprivkey(address3)
88+
label3_priv = "Test Label 3 for importprivkey"
89+
self.nodes[1].importprivkey(priv_key3, label3_priv)
90+
91+
assert_equal(label3_priv, self.nodes[1].getaddressinfo(address3)["label"])
92+
93+
self.log.info(
94+
"Test importprivkey won't label new dests with the same "
95+
"label as others labeled dests for the same key."
96+
)
97+
self.log.info("Import a watch-only legacy address with a label.")
98+
address4 = self.nodes[0].getnewaddress()
99+
label4_addr = "Test Label 4 for importaddress"
100+
self.nodes[1].importaddress(address4, label4_addr)
101+
address_assert4 = self.nodes[1].getaddressinfo(address4)
102+
103+
assert_equal(address_assert4["iswatchonly"], True)
104+
assert_equal(address_assert4["ismine"], False)
105+
assert_equal(address_assert4["label"], label4_addr)
106+
107+
self.log.info("Asserts address has no embedded field with dests.")
108+
109+
assert_equal(address_assert4.get("embedded"), None)
110+
111+
self.log.info(
112+
"Import the watch-only address's private key without a "
113+
"label and new destinations for the key should have an "
114+
"empty label while the 'old' destination should keep "
115+
"its label."
116+
)
117+
priv_key4 = self.nodes[0].dumpprivkey(address4)
118+
self.nodes[1].importprivkey(priv_key4)
119+
address_assert4 = self.nodes[1].getaddressinfo(address4)
120+
121+
assert address_assert4.get("embedded")
122+
123+
bcaddress_assert = self.nodes[1].getaddressinfo(
124+
address_assert4["embedded"]["address"]
125+
)
126+
127+
assert_equal(address_assert4["label"], label4_addr)
128+
assert_equal(bcaddress_assert["label"], "")
129+
130+
self.stop_nodes()
131+
132+
133+
if __name__ == "__main__":
134+
ImportWithLabel().main()

0 commit comments

Comments
 (0)