From c7654af6f830577a54df12b5d65df93532db0dc2 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Thu, 9 Jan 2020 17:11:54 +0100 Subject: doc: address pr17578 review feedback - https://github.com/bitcoin/bitcoin/pull/17578#discussion_r363975411 - https://github.com/bitcoin/bitcoin/pull/17578#discussion_r363969721 - https://github.com/bitcoin/bitcoin/pull/17578#discussion_r362703553 --- doc/release-notes-17578.md | 2 +- src/wallet/rpcwallet.cpp | 5 ++--- test/functional/rpc_getaddressinfo_labels_purpose_deprecation.py | 4 ++-- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/doc/release-notes-17578.md b/doc/release-notes-17578.md index 1b07436bb1..90156d6081 100644 --- a/doc/release-notes-17578.md +++ b/doc/release-notes-17578.md @@ -4,5 +4,5 @@ Deprecated or removed RPCs - The `getaddressinfo` RPC `labels` field now returns an array of label name strings. Previously, it returned an array of JSON objects containing `name` and `purpose` key/value pairs, which is now deprecated and will be removed in - 0.21. To re-enable the previous behavior, launch bitcoind with + 0.21. To re-enable the previous behavior, launch with `-deprecatedrpc=labelspurpose`. diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 85793ef180..e14e9157ca 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3843,12 +3843,11 @@ UniValue getaddressinfo(const JSONRPCRequest& request) // associated with an address, but we return an array so the API remains // stable if we allow multiple labels to be associated with an address in // the future. - // - // DEPRECATED: The previous behavior of returning an array containing a JSON - // object of `name` and `purpose` key/value pairs has been deprecated. UniValue labels(UniValue::VARR); std::map::iterator mi = pwallet->mapAddressBook.find(dest); if (mi != pwallet->mapAddressBook.end()) { + // DEPRECATED: The previous behavior of returning an array containing a + // JSON object of `name` and `purpose` key/value pairs is deprecated. if (pwallet->chain().rpcEnableDeprecated("labelspurpose")) { labels.push_back(AddressBookDataToJSON(mi->second, true)); } else { diff --git a/test/functional/rpc_getaddressinfo_labels_purpose_deprecation.py b/test/functional/rpc_getaddressinfo_labels_purpose_deprecation.py index 1049440d49..903f5536b9 100755 --- a/test/functional/rpc_getaddressinfo_labels_purpose_deprecation.py +++ b/test/functional/rpc_getaddressinfo_labels_purpose_deprecation.py @@ -4,8 +4,8 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """ Test deprecation of RPC getaddressinfo `labels` returning an array -containing a JSON hash of `name` and purpose` key-value pairs. It now -returns an array of label names. +containing a JSON object of `name` and purpose` key-value pairs. It now +returns an array containing only the label name. """ from test_framework.test_framework import BitcoinTestFramework -- cgit v1.2.3 From dc0cabeda49a7edbfa71df22846721b6f6224aea Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Mon, 25 Nov 2019 01:31:38 +0100 Subject: test: remove getaddressinfo label tests --- test/functional/wallet_basic.py | 2 +- test/functional/wallet_import_with_label.py | 14 +++++--------- test/functional/wallet_importmulti.py | 2 -- test/functional/wallet_labels.py | 2 +- test/functional/wallet_listreceivedby.py | 2 +- 5 files changed, 8 insertions(+), 14 deletions(-) diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index 4780e9263e..15746d312c 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -392,7 +392,7 @@ class WalletTest(BitcoinTestFramework): for label in [u'рыба', u'𝅘𝅥𝅯']: addr = self.nodes[0].getnewaddress() self.nodes[0].setlabel(addr, label) - test_address(self.nodes[0], addr, label=label, labels=[label]) + test_address(self.nodes[0], addr, labels=[label]) assert label in self.nodes[0].listlabels() self.nodes[0].rpc.ensure_ascii = True # restore to default diff --git a/test/functional/wallet_import_with_label.py b/test/functional/wallet_import_with_label.py index 64d227b30f..afb740b858 100755 --- a/test/functional/wallet_import_with_label.py +++ b/test/functional/wallet_import_with_label.py @@ -36,7 +36,6 @@ class ImportWithLabel(BitcoinTestFramework): address, iswatchonly=True, ismine=False, - label=label, labels=[label]) self.log.info( @@ -45,7 +44,7 @@ class ImportWithLabel(BitcoinTestFramework): ) priv_key = self.nodes[0].dumpprivkey(address) self.nodes[1].importprivkey(priv_key) - test_address(self.nodes[1], address, label=label, labels=[label]) + test_address(self.nodes[1], address, labels=[label]) self.log.info( "Test importaddress without label and importprivkey with label." @@ -57,7 +56,6 @@ class ImportWithLabel(BitcoinTestFramework): address2, iswatchonly=True, ismine=False, - label="", labels=[""]) self.log.info( @@ -68,7 +66,7 @@ class ImportWithLabel(BitcoinTestFramework): label2 = "Test Label 2" self.nodes[1].importprivkey(priv_key2, label2) - test_address(self.nodes[1], address2, label=label2, labels=[label2]) + test_address(self.nodes[1], address2, labels=[label2]) self.log.info("Test importaddress with label and importprivkey with label.") self.log.info("Import a watch-only address with a label.") @@ -79,7 +77,6 @@ class ImportWithLabel(BitcoinTestFramework): address3, iswatchonly=True, ismine=False, - label=label3_addr, labels=[label3_addr]) self.log.info( @@ -90,7 +87,7 @@ class ImportWithLabel(BitcoinTestFramework): label3_priv = "Test Label 3 for importprivkey" self.nodes[1].importprivkey(priv_key3, label3_priv) - test_address(self.nodes[1], address3, label=label3_priv, labels=[label3_priv]) + test_address(self.nodes[1], address3, labels=[label3_priv]) self.log.info( "Test importprivkey won't label new dests with the same " @@ -104,7 +101,6 @@ class ImportWithLabel(BitcoinTestFramework): address4, iswatchonly=True, ismine=False, - label=label4_addr, labels=[label4_addr], embedded=None) @@ -118,9 +114,9 @@ class ImportWithLabel(BitcoinTestFramework): self.nodes[1].importprivkey(priv_key4) embedded_addr = self.nodes[1].getaddressinfo(address4)['embedded']['address'] - test_address(self.nodes[1], embedded_addr, label="", labels=[""]) + test_address(self.nodes[1], embedded_addr, labels=[""]) - test_address(self.nodes[1], address4, label=label4_addr, labels=[label4_addr]) + test_address(self.nodes[1], address4, labels=[label4_addr]) self.stop_nodes() diff --git a/test/functional/wallet_importmulti.py b/test/functional/wallet_importmulti.py index eb55578bfd..f152fcd1a4 100755 --- a/test/functional/wallet_importmulti.py +++ b/test/functional/wallet_importmulti.py @@ -569,7 +569,6 @@ class ImportMultiTest(BitcoinTestFramework): key.p2sh_p2wpkh_addr, solvable=True, ismine=True, - label=p2sh_p2wpkh_label, labels=[p2sh_p2wpkh_label]) # Test ranged descriptor fails if range is not specified @@ -641,7 +640,6 @@ class ImportMultiTest(BitcoinTestFramework): key.p2pkh_addr, solvable=True, ismine=False, - label=p2pkh_label, labels=[p2pkh_label]) # Test import fails if both desc and scriptPubKey are provided diff --git a/test/functional/wallet_labels.py b/test/functional/wallet_labels.py index f795202d72..95a180ad27 100755 --- a/test/functional/wallet_labels.py +++ b/test/functional/wallet_labels.py @@ -155,7 +155,7 @@ class Label: if self.receive_address is not None: assert self.receive_address in self.addresses for address in self.addresses: - test_address(node, address, label=self.name, labels=[self.name]) + test_address(node, address, labels=[self.name]) assert self.name in node.listlabels() assert_equal( node.getaddressesbylabel(self.name), diff --git a/test/functional/wallet_listreceivedby.py b/test/functional/wallet_listreceivedby.py index 4b83e1613f..b0590b149a 100755 --- a/test/functional/wallet_listreceivedby.py +++ b/test/functional/wallet_listreceivedby.py @@ -128,7 +128,7 @@ class ReceivedByTest(BitcoinTestFramework): # set pre-state label = '' address = self.nodes[1].getnewaddress() - test_address(self.nodes[1], address, label=label, labels=[label]) + test_address(self.nodes[1], address, labels=[label]) received_by_label_json = [r for r in self.nodes[1].listreceivedbylabel() if r["label"] == label][0] balance_by_label = self.nodes[1].getreceivedbylabel(label) -- cgit v1.2.3 From d48875fa20d0b71b978cb3d1f85dd9ec14e664cc Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Mon, 25 Nov 2019 01:20:59 +0100 Subject: rpc: deprecate getaddressinfo label field --- src/wallet/rpcwallet.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index e14e9157ca..581590e755 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3762,18 +3762,18 @@ UniValue getaddressinfo(const JSONRPCRequest& request) " getaddressinfo output fields for the embedded address, excluding metadata (timestamp, hdkeypath,\n" " hdseedid) and relation to the wallet (ismine, iswatchonly).\n" " \"iscompressed\" : true|false, (boolean, optional) If the pubkey is compressed.\n" - " \"label\" : \"label\" (string) The label associated with the address. Defaults to \"\". Equivalent to the label name in the labels array below.\n" + " \"label\" : \"label\" (string) DEPRECATED. The label associated with the address. Defaults to \"\". Replaced by the labels array below.\n" " \"timestamp\" : timestamp, (number, optional) The creation time of the key, if available, expressed in " + UNIX_EPOCH_TIME + ".\n" " \"hdkeypath\" : \"keypath\" (string, optional) The HD keypath, if the key is HD and available.\n" " \"hdseedid\" : \"\" (string, optional) The Hash160 of the HD seed.\n" " \"hdmasterfingerprint\" : \"\" (string, optional) The fingerprint of the master key.\n" - " \"labels\" (json object) An array of labels associated with the address. Currently limited to one label but returned\n" - " as an array to keep the API stable if multiple labels are enabled in the future.\n" + " \"labels\" (array) Array of labels associated with the address. Currently limited to one label but returned\n" + " as an array to keep the API stable if multiple labels are enabled in the future.\n" " [\n" - " \"label name\" (string) The label name. Defaults to \"\". Equivalent to the label field above.\n\n" + " \"label name\" (string) The label name. Defaults to \"\".\n" " DEPRECATED, will be removed in 0.21. To re-enable, launch bitcoind with `-deprecatedrpc=labelspurpose`:\n" - " { (json object of label data)\n" - " \"name\" : \"label name\" (string) The label name. Defaults to \"\". Equivalent to the label field above.\n" + " {\n" + " \"name\" : \"label name\" (string) The label name. Defaults to \"\".\n" " \"purpose\" : \"purpose\" (string) The purpose of the associated address (send or receive).\n" " }\n" " ]\n" @@ -3817,10 +3817,10 @@ UniValue getaddressinfo(const JSONRPCRequest& request) UniValue detail = DescribeWalletAddress(pwallet, dest); ret.pushKVs(detail); - // Return label field if existing. Currently only one label can be - // associated with an address, so the label should be equivalent to the + // DEPRECATED: Return label field if existing. Currently only one label can + // be associated with an address, so the label should be equivalent to the // value of the name key/value pair in the labels array below. - if (pwallet->mapAddressBook.count(dest)) { + if ((pwallet->chain().rpcEnableDeprecated("label")) && (pwallet->mapAddressBook.count(dest))) { ret.pushKV("label", pwallet->mapAddressBook[dest].name); } -- cgit v1.2.3 From 72af93f36479dc12d795f1d05fa3d8fbd9b293bd Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Mon, 25 Nov 2019 02:04:14 +0100 Subject: test: getaddressinfo label deprecation test --- .../rpc_getaddressinfo_label_deprecation.py | 43 ++++++++++++++++++++++ test/functional/test_runner.py | 1 + 2 files changed, 44 insertions(+) create mode 100755 test/functional/rpc_getaddressinfo_label_deprecation.py diff --git a/test/functional/rpc_getaddressinfo_label_deprecation.py b/test/functional/rpc_getaddressinfo_label_deprecation.py new file mode 100755 index 0000000000..5e739ebede --- /dev/null +++ b/test/functional/rpc_getaddressinfo_label_deprecation.py @@ -0,0 +1,43 @@ +#!/usr/bin/env python3 +# Copyright (c) 2020 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 deprecation of the RPC getaddressinfo `label` field. It has been +superceded by the `labels` field. + +""" +from test_framework.test_framework import BitcoinTestFramework + +class GetAddressInfoLabelDeprecationTest(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 2 + self.setup_clean_chain = False + # Start node[0] with -deprecatedrpc=label, and node[1] without. + self.extra_args = [["-deprecatedrpc=label"], []] + + def skip_test_if_missing_module(self): + self.skip_if_no_wallet() + + def test_label_with_deprecatedrpc_flag(self): + self.log.info("Test getaddressinfo label with -deprecatedrpc flag") + node = self.nodes[0] + address = node.getnewaddress() + info = node.getaddressinfo(address) + assert "label" in info + + def test_label_without_deprecatedrpc_flag(self): + self.log.info("Test getaddressinfo label without -deprecatedrpc flag") + node = self.nodes[1] + address = node.getnewaddress() + info = node.getaddressinfo(address) + assert "label" not in info + + def run_test(self): + """Test getaddressinfo label with and without -deprecatedrpc flag.""" + self.test_label_with_deprecatedrpc_flag() + self.test_label_without_deprecatedrpc_flag() + + +if __name__ == '__main__': + GetAddressInfoLabelDeprecationTest().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index acb559911b..8b527bcff0 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -213,6 +213,7 @@ BASE_SCRIPTS = [ 'feature_blocksdir.py', 'feature_config_args.py', 'rpc_getaddressinfo_labels_purpose_deprecation.py', + 'rpc_getaddressinfo_label_deprecation.py', 'rpc_help.py', 'feature_help.py', 'feature_shutdown.py', -- cgit v1.2.3 From d3bc18408146e91b3836f72360ff6fa2420b6887 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Fri, 13 Dec 2019 15:15:24 +0100 Subject: doc: update release notes with getaddressinfo label deprecation --- doc/release-notes-17578.md | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/doc/release-notes-17578.md b/doc/release-notes-17578.md index 90156d6081..664d17fd78 100644 --- a/doc/release-notes-17578.md +++ b/doc/release-notes-17578.md @@ -1,8 +1,13 @@ Deprecated or removed RPCs -------------------------- -- The `getaddressinfo` RPC `labels` field now returns an array of label name - strings. Previously, it returned an array of JSON objects containing `name` and - `purpose` key/value pairs, which is now deprecated and will be removed in - 0.21. To re-enable the previous behavior, launch with - `-deprecatedrpc=labelspurpose`. +- RPC `getaddressinfo` changes: + + - the `label` field has been deprecated in favor of the `labels` field and + will be removed in 0.21. It can be re-enabled in the interim by launching + with `-deprecatedrpc=label`. + + - the `labels` behavior of returning an array of JSON objects containing name + and purpose key/value pairs has been deprecated in favor of an array of + label names and will be removed in 0.21. The previous behavior can be + re-enabled in the interim by launching with `-deprecatedrpc=labelspurpose`. -- cgit v1.2.3