Page MenuHomePhabricator
Files F10615073

D6185.id20422.diff
No OneTemporary
Actions

D6185.id20422.diff
View Options

diff --git a/doc/release-notes.md b/doc/release-notes.md
--- a/doc/release-notes.md
+++ b/doc/release-notes.md
@@ -22,3 +22,33 @@
- The `importmulti` RPC will now contain a new per-request `warnings`
field with strings that explain when fields are being ignored or
inconsistent, if any.
+
+RPC importprivkey: new label behavior
+-------------------------------------
+
+Previously, `importprivkey` automatically added the default empty label
+("") to all addresses associated with the imported private key. Now it
+defaults to using any existing label for those addresses. For example:
+
+- Old behavior: you import a watch-only address with the label "cold
+ wallet". Later, you import the corresponding private key using the
+ default settings. The address's label is changed from "cold wallet"
+ to "".
+
+- New behavior: you import a watch-only address with the label "cold
+ wallet". Later, you import the corresponding private key using the
+ default settings. The address's label remains "cold wallet".
+
+In both the previous and current case, if you directly specify a label
+during the import, that label will override whatever previous label the
+addresses may have had. Also in both cases, if none of the addresses
+previously had a label, they will still receive the default empty label
+(""). Examples:
+
+- You import a watch-only address with the label "temporary". Later you
+ import the corresponding private key with the label "final". The
+ address's label will be changed to "final".
+
+- You use the default settings to import a private key for an address that
+ was not previously in the wallet. Its addresses will receive the default
+ empty label ("").
diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp
--- a/src/wallet/rpcdump.cpp
+++ b/src/wallet/rpcdump.cpp
@@ -116,7 +116,8 @@
/* default_val */ "", "The private key (see dumpprivkey)"},
{"label", RPCArg::Type::STR, /* opt */ true,
/* default_val */
- "\"\"", "An optional label"},
+ "current label if address exists, otherwise \"\"",
+ "An optional label"},
{"rescan", RPCArg::Type::BOOL, /* opt */ true,
/* default_val */ "true",
"Rescan the wallet for transactions"},
@@ -185,10 +186,15 @@
CKeyID vchAddress = pubkey.GetID();
{
pwallet->MarkDirty();
- // We don't know which corresponding address will be used; label
- // them all
+
+ // We don't know which corresponding address will be used;
+ // label all new addresses, and label existing addresses if a
+ // label was passed.
for (const auto &dest : GetAllDestinationsForKey(pubkey)) {
- pwallet->SetAddressBook(dest, strLabel, "receive");
+ if (!request.params[1].isNull() ||
+ pwallet->mapAddressBook.count(dest) == 0) {
+ pwallet->SetAddressBook(dest, strLabel, "receive");
+ }
}
// Don't throw error in case a key is already there
diff --git a/test/functional/wallet_import_with_label.py b/test/functional/wallet_import_with_label.py
new file mode 100755
--- /dev/null
+++ b/test/functional/wallet_import_with_label.py
@@ -0,0 +1,164 @@
+#!/usr/bin/env python3
+# Copyright (c) 2018 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""Test the behavior of RPC importprivkey on set and unset labels of
+addresses.
+
+It tests different cases in which an address is imported with importaddress
+with or without a label and then its private key is imported with importprivkey
+with and without a label.
+"""
+
+from test_framework.address import script_to_p2sh
+from test_framework.script import (
+ CScript,
+ OP_CHECKSIG,
+ OP_DUP,
+ OP_EQUALVERIFY,
+ OP_HASH160,
+ hash160,
+)
+
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import (
+ assert_equal,
+ hex_str_to_bytes,
+)
+
+
+class ImportWithLabel(BitcoinTestFramework):
+ def set_test_params(self):
+ self.num_nodes = 2
+ self.setup_clean_chain = True
+
+ def skip_test_if_missing_module(self):
+ self.skip_if_no_wallet()
+
+ def run_test(self):
+ """Main test logic"""
+
+ self.log.info(
+ "Test importaddress with label and importprivkey without label."
+ )
+ self.log.info("Import a watch-only address with a label.")
+ address = self.nodes[0].getnewaddress()
+ label = "Test Label"
+ self.nodes[1].importaddress(address, label)
+ address_assert = self.nodes[1].getaddressinfo(address)
+
+ assert_equal(address_assert["iswatchonly"], True)
+ assert_equal(address_assert["ismine"], False)
+ assert_equal(address_assert["label"], label)
+
+ self.log.info(
+ "Import the watch-only address's private key without a "
+ "label and the address should keep its label."
+ )
+ priv_key = self.nodes[0].dumpprivkey(address)
+ self.nodes[1].importprivkey(priv_key)
+
+ assert_equal(label, self.nodes[1].getaddressinfo(address)["label"])
+
+ self.log.info(
+ "Test importaddress without label and importprivkey with label."
+ )
+ self.log.info("Import a watch-only address without a label.")
+ address2 = self.nodes[0].getnewaddress()
+ self.nodes[1].importaddress(address2)
+ address_assert2 = self.nodes[1].getaddressinfo(address2)
+
+ assert_equal(address_assert2["iswatchonly"], True)
+ assert_equal(address_assert2["ismine"], False)
+ assert_equal(address_assert2["label"], "")
+
+ self.log.info(
+ "Import the watch-only address's private key with a "
+ "label and the address should have its label updated."
+ )
+ priv_key2 = self.nodes[0].dumpprivkey(address2)
+ label2 = "Test Label 2"
+ self.nodes[1].importprivkey(priv_key2, label2)
+
+ assert_equal(label2, self.nodes[1].getaddressinfo(address2)["label"])
+
+ self.log.info(
+ "Test importaddress with label and importprivkey with label.")
+ self.log.info("Import a watch-only address with a label.")
+ address3 = self.nodes[0].getnewaddress()
+ label3_addr = "Test Label 3 for importaddress"
+ self.nodes[1].importaddress(address3, label3_addr)
+ address_assert3 = self.nodes[1].getaddressinfo(address3)
+
+ assert_equal(address_assert3["iswatchonly"], True)
+ assert_equal(address_assert3["ismine"], False)
+ assert_equal(address_assert3["label"], label3_addr)
+
+ self.log.info(
+ "Import the watch-only address's private key with a "
+ "label and the address should have its label updated."
+ )
+ priv_key3 = self.nodes[0].dumpprivkey(address3)
+ label3_priv = "Test Label 3 for importprivkey"
+ self.nodes[1].importprivkey(priv_key3, label3_priv)
+
+ assert_equal(
+ label3_priv,
+ self.nodes[1].getaddressinfo(address3)["label"])
+
+ self.log.info(
+ "Test importprivkey won't label new dests with the same "
+ "label as others labeled dests for the same key."
+ )
+ self.log.info("Import a watch-only legacy address with a label.")
+ address4 = self.nodes[0].getnewaddress()
+ label4_addr = "Test Label 4 for importaddress"
+ self.nodes[1].importaddress(address4, label4_addr)
+ address_assert4 = self.nodes[1].getaddressinfo(address4)
+
+ assert_equal(address_assert4["iswatchonly"], True)
+ assert_equal(address_assert4["ismine"], False)
+ assert_equal(address_assert4["label"], label4_addr)
+
+ self.log.info("Asserts address has no embedded field with dests.")
+
+ assert_equal(address_assert4.get("embedded"), None)
+
+ self.log.info(
+ "Import the watch-only address's private key without a "
+ "label and new destinations for the key should have an "
+ "empty label while the 'old' destination should keep "
+ "its label."
+ )
+
+ # Build a P2SH manually for this test.
+ priv_key4 = self.nodes[0].dumpprivkey(address4)
+ pubkey4 = self.nodes[0].getaddressinfo(address4)['pubkey']
+ pkh4 = hash160(hex_str_to_bytes(pubkey4))
+ script4 = CScript(
+ [OP_DUP, OP_HASH160, pkh4, OP_EQUALVERIFY, OP_CHECKSIG])
+ p2shaddr4 = script_to_p2sh(script4)
+
+ self.nodes[1].importmulti([{
+ "scriptPubKey": {"address": p2shaddr4},
+ "timestamp": "now",
+ "redeemscript": script4.hex(),
+ "keys": [priv_key4],
+ }])
+
+ p2shaddrinfo = self.nodes[1].getaddressinfo(p2shaddr4)
+
+ assert p2shaddrinfo.get("embedded")
+
+ embedded_addrinfo = self.nodes[1].getaddressinfo(
+ p2shaddrinfo["embedded"]["address"]
+ )
+
+ assert_equal(p2shaddrinfo["label"], "")
+ assert_equal(embedded_addrinfo["label"], label4_addr)
+
+ self.stop_nodes()
+
+
+if __name__ == "__main__":
+ ImportWithLabel().main()

File Metadata

Mime Type
text/plain
Expires
Fri, Nov 22, 09:55 (11 h, 3 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
4559197
Default Alt Text
D6185.id20422.diff (9 KB)

Event Timeline