diff --git a/doc/release-notes-pr13381.md b/doc/release-notes-pr13381.md new file mode 100644 index 00000000000000..75faad99060f59 --- /dev/null +++ b/doc/release-notes-pr13381.md @@ -0,0 +1,29 @@ +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 index c73bfb9d521b4a..9f0e3df8b41a45 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -157,7 +157,10 @@ UniValue importprivkey(const JSONRPCRequest& request) CKeyID vchAddress = pubkey.GetID(); { pwallet->MarkDirty(); - pwallet->SetAddressBook(vchAddress, strLabel, "receive"); + + if (!request.params[1].isNull()) { + pwallet->SetAddressBook(vchAddress, strLabel, "receive"); + } // Don't throw error in case a key is already there if (pwallet->HaveKey(vchAddress)) { diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index c97e001b842670..ffc5cacc48e4db 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -184,6 +184,7 @@ 'mempool_accept.py', 'mempool_expiry.py', 'wallet_import_rescan.py', + 'wallet_import_with_label.py', 'rpc_bind.py --ipv4', 'rpc_bind.py --ipv6', 'rpc_bind.py --nonloopback', diff --git a/test/functional/wallet_import_with_label.py b/test/functional/wallet_import_with_label.py new file mode 100755 index 00000000000000..95acaa752e528e --- /dev/null +++ b/test/functional/wallet_import_with_label.py @@ -0,0 +1,134 @@ +#!/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.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal + + +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." + ) + priv_key4 = self.nodes[0].dumpprivkey(address4) + self.nodes[1].importprivkey(priv_key4) + address_assert4 = self.nodes[1].getaddressinfo(address4) + + assert address_assert4.get("embedded") + + bcaddress_assert = self.nodes[1].getaddressinfo( + address_assert4["embedded"]["address"] + ) + + assert_equal(address_assert4["label"], label4_addr) + assert_equal(bcaddress_assert["label"], "") + + self.stop_nodes() + + +if __name__ == "__main__": + ImportWithLabel().main()