Page MenuHomePhabricator

D7724.diff
No OneTemporary

D7724.diff

diff --git a/doc/release-notes.md b/doc/release-notes.md
--- a/doc/release-notes.md
+++ b/doc/release-notes.md
@@ -12,9 +12,17 @@
- The `-debug=db` logging category has been renamed to `-debug=walletdb`, to distinguish it from `coindb`.
`-debug=db` has been deprecated and will be removed in a next release.
-
Low-level RPC Changes
---------------------
- The RPC gettransaction, listtransactions and listsinceblock responses now also
includes the height of the block that contains the wallet transaction, if any.
+
+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 a future
+ release. To re-enable the previous behavior, launch bitcoind with
+ `-deprecatedrpc=labelspurpose`.
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -4147,6 +4147,9 @@
return NullUniValue;
}
+ const std::string example_address =
+ "\"qrmzys48glkpevp2l4t24jtcltc9hyzx9cep2qffm4\"";
+
RPCHelpMan{
"getaddressinfo",
"\nReturn information about the given bitcoin address.\n"
@@ -4208,7 +4211,7 @@
"the pubkey is compressed.\n"
" \"label\" : \"label\" (string) The label "
"associated with the address. Defaults to \"\". Equivalent to the "
- "name field in the labels array.\n"
+ "label name in the labels array below.\n"
" \"timestamp\" : timestamp, (number, optional) The "
"creation time of the key if available, expressed in seconds since "
"Epoch Time (Jan 1 1970 GMT).\n"
@@ -4218,26 +4221,27 @@
"Hash160 of the HD seed.\n"
" \"hdmasterfingerprint\" : \"<hash160>\" (string, optional) The "
"fingerprint of the master key.\n"
- " \"labels\" (object) An array of "
+ " \"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"
" [\n"
+ " \"label name\" (string) The label name. Defaults to \"\". "
+ "Equivalent to the label field above.\n\n"
+ " DEPRECATED, will be removed in a future version. To "
+ "re-enable, launch bitcoind with `-deprecatedrpc=labelspurpose`:\n"
" { (json object of label data)\n"
- " \"name\": \"label name\" (string) The label name. "
+ " \"name\" : \"label name\" (string) The label name. "
"Defaults to \"\". Equivalent to the label field above.\n"
- " \"purpose\": \"purpose\" (string) The purpose of the "
+ " \"purpose\" : \"purpose\" (string) The purpose of the "
"associated address (send or receive).\n"
" },...\n"
" ]\n"
"}\n"},
- RPCExamples{
- HelpExampleCli("getaddressinfo",
- "\"qrmzys48glkpevp2l4t24jtcltc9hyzx9cep2qffm4\"") +
- HelpExampleRpc("getaddressinfo",
- "\"qrmzys48glkpevp2l4t24jtcltc9hyzx9cep2qffm4\"")},
+ RPCExamples{HelpExampleCli("getaddressinfo", example_address) +
+ HelpExampleRpc("getaddressinfo", example_address)},
}
.Check(request);
@@ -4246,7 +4250,6 @@
UniValue ret(UniValue::VOBJ);
CTxDestination dest =
DecodeDestination(request.params[0].get_str(), config.GetChainParams());
-
// Make sure the destination is valid
if (!IsValidDestination(dest)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid address");
@@ -4274,24 +4277,18 @@
ret.pushKV("iswatchonly", bool(mine & ISMINE_WATCH_ONLY));
- // Return DescribeWalletAddress fields.
- // Always returned: isscript, ischange.
- // Optional: script, hex, pubkeys (array), sigsrequired, pubkey, embedded,
- // iscompressed.
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
- // value of the name key/value pair in the labels hash array below.
+ // value of the name key/value pair in the labels array below.
if (pwallet->mapAddressBook.count(dest)) {
ret.pushKV("label", pwallet->mapAddressBook[dest].name);
}
ret.pushKV("ischange", pwallet->IsChange(scriptPubKey));
- // Fetch KeyMetadata, if present, for the timestamp, hdkeypath, hdseedid,
- // and hdmasterfingerprint fields.
ScriptPubKeyMan *spk_man = pwallet->GetScriptPubKeyMan(scriptPubKey);
if (spk_man) {
if (const CKeyMetadata *meta = spk_man->GetMetadata(dest)) {
@@ -4306,16 +4303,23 @@
}
}
- // Return a labels array containing a hash of key/value pairs for the label
- // name and address purpose. The name value is equivalent to the label field
- // above. Currently only one label can be 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.
+ // Return a `labels` array containing the label associated with the address,
+ // equivalent to the `label` field above. Currently only one label can be
+ // 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<CTxDestination, CAddressBookData>::iterator mi =
pwallet->mapAddressBook.find(dest);
if (mi != pwallet->mapAddressBook.end()) {
- labels.push_back(AddressBookDataToJSON(mi->second, true));
+ if (pwallet->chain().rpcEnableDeprecated("labelspurpose")) {
+ labels.push_back(AddressBookDataToJSON(mi->second, true));
+ } else {
+ labels.push_back(mi->second.name);
+ }
}
ret.pushKV("labels", std::move(labels));
diff --git a/test/functional/rpc_getaddressinfo_labels_purpose_deprecation.py b/test/functional/rpc_getaddressinfo_labels_purpose_deprecation.py
new file mode 100755
--- /dev/null
+++ b/test/functional/rpc_getaddressinfo_labels_purpose_deprecation.py
@@ -0,0 +1,53 @@
+#!/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 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.
+
+"""
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import assert_equal
+
+LABELS_TO_TEST = frozenset({"", "New 𝅘𝅥𝅯 $<#>&!рыба Label"})
+
+
+class GetAddressInfoLabelsPurposeDeprecationTest(BitcoinTestFramework):
+ def set_test_params(self):
+ self.num_nodes = 2
+ self.setup_clean_chain = False
+ # Start node[0] with -deprecatedrpc=labelspurpose and node[1] without.
+ self.extra_args = [["-deprecatedrpc=labelspurpose"], []]
+
+ def skip_test_if_missing_module(self):
+ self.skip_if_no_wallet()
+
+ def test_labels(self, node_num, label_name, expected_value):
+ node = self.nodes[node_num]
+ address = node.getnewaddress()
+ if label_name != "":
+ node.setlabel(address, label_name)
+ self.log.info(" set label to {}".format(label_name))
+ labels = node.getaddressinfo(address)["labels"]
+ self.log.info(" labels = {}".format(labels))
+ assert_equal(labels, expected_value)
+
+ def run_test(self):
+ """Test getaddressinfo labels with and without -deprecatedrpc flag."""
+ self.log.info("Test getaddressinfo labels with -deprecatedrpc flag")
+ for label in LABELS_TO_TEST:
+ self.test_labels(node_num=0, label_name=label, expected_value=[
+ {"name": label, "purpose": "receive"}])
+
+ self.log.info("Test getaddressinfo labels without -deprecatedrpc flag")
+ for label in LABELS_TO_TEST:
+ self.test_labels(
+ node_num=1,
+ label_name=label,
+ expected_value=[label])
+
+
+if __name__ == '__main__':
+ GetAddressInfoLabelsPurposeDeprecationTest().main()
diff --git a/test/functional/test_framework/wallet_util.py b/test/functional/test_framework/wallet_util.py
--- a/test/functional/test_framework/wallet_util.py
+++ b/test/functional/test_framework/wallet_util.py
@@ -69,12 +69,6 @@
redeem_script=script_code.hex())
-def labels_value(name="", purpose="receive"):
- """Generate a getaddressinfo labels array from a name and purpose.
- Often used as the value of a labels kwarg for calling test_address below."""
- return [{"name": name, "purpose": purpose}]
-
-
def test_address(node, address, **kwargs):
"""Get address info for `address` and test whether the returned values are as expected."""
addr_info = node.getaddressinfo(address)
diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py
--- a/test/functional/wallet_basic.py
+++ b/test/functional/wallet_basic.py
@@ -17,10 +17,7 @@
count_bytes,
wait_until,
)
-from test_framework.wallet_util import (
- labels_value,
- test_address,
-)
+from test_framework.wallet_util import test_address
class WalletTest(BitcoinTestFramework):
@@ -464,12 +461,7 @@
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=labels_value(
- name=label))
+ test_address(self.nodes[0], addr, label=label, labels=[label])
assert label in self.nodes[0].listlabels()
# restore to default
self.nodes[0].rpc.ensure_ascii = True
diff --git a/test/functional/wallet_import_with_label.py b/test/functional/wallet_import_with_label.py
--- a/test/functional/wallet_import_with_label.py
+++ b/test/functional/wallet_import_with_label.py
@@ -22,10 +22,7 @@
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import hex_str_to_bytes
-from test_framework.wallet_util import (
- labels_value,
- test_address,
-)
+from test_framework.wallet_util import test_address
class ImportWithLabel(BitcoinTestFramework):
@@ -51,7 +48,7 @@
iswatchonly=True,
ismine=False,
label=label,
- labels=labels_value(name=label))
+ labels=[label])
self.log.info(
"Import the watch-only address's private key without a "
@@ -59,11 +56,7 @@
)
priv_key = self.nodes[0].dumpprivkey(address)
self.nodes[1].importprivkey(priv_key)
-
- test_address(self.nodes[1],
- address,
- label=label,
- labels=labels_value(name=label))
+ test_address(self.nodes[1], address, label=label, labels=[label])
self.log.info(
"Test importaddress without label and importprivkey with label."
@@ -76,7 +69,7 @@
iswatchonly=True,
ismine=False,
label="",
- labels=labels_value())
+ labels=[""])
self.log.info(
"Import the watch-only address's private key with a "
@@ -86,10 +79,7 @@
label2 = "Test Label 2"
self.nodes[1].importprivkey(priv_key2, label2)
- test_address(self.nodes[1],
- address2,
- label=label2,
- labels=labels_value(name=label2))
+ test_address(self.nodes[1], address2, label=label2, labels=[label2])
self.log.info(
"Test importaddress with label and importprivkey with label.")
@@ -102,7 +92,7 @@
iswatchonly=True,
ismine=False,
label=label3_addr,
- labels=labels_value(name=label3_addr))
+ labels=[label3_addr])
self.log.info(
"Import the watch-only address's private key with a "
@@ -112,10 +102,11 @@
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=labels_value(name=label3_priv))
+ test_address(
+ self.nodes[1],
+ address3,
+ label=label3_priv,
+ labels=[label3_priv])
self.log.info(
"Test importprivkey won't label new dests with the same "
@@ -130,7 +121,7 @@
iswatchonly=True,
ismine=False,
label=label4_addr,
- labels=labels_value(name=label4_addr),
+ labels=[label4_addr],
embedded=None)
self.log.info(
@@ -158,14 +149,13 @@
test_address(self.nodes[1],
p2shaddr4,
label="",
- labels=labels_value())
+ labels=[""])
embedded_addr = self.nodes[1].getaddressinfo(
p2shaddr4)['embedded']['address']
test_address(self.nodes[1],
embedded_addr,
- label=label4_addr,
- labels=labels_value(name=label4_addr))
+ label=label4_addr, labels=[label4_addr])
self.stop_nodes()
diff --git a/test/functional/wallet_importmulti.py b/test/functional/wallet_importmulti.py
--- a/test/functional/wallet_importmulti.py
+++ b/test/functional/wallet_importmulti.py
@@ -29,7 +29,6 @@
from test_framework.wallet_util import (
get_key,
get_multisig,
- labels_value,
test_address,
)
@@ -515,7 +514,7 @@
solvable=True,
ismine=False,
label=p2pkh_label,
- labels=labels_value(name=p2pkh_label))
+ labels=[p2pkh_label])
# Test import fails if both desc and scriptPubKey are provided
key = get_key(self.nodes[0])
diff --git a/test/functional/wallet_labels.py b/test/functional/wallet_labels.py
--- a/test/functional/wallet_labels.py
+++ b/test/functional/wallet_labels.py
@@ -13,10 +13,7 @@
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import assert_equal, assert_raises_rpc_error
-from test_framework.wallet_util import (
- labels_value,
- test_address,
-)
+from test_framework.wallet_util import test_address
class WalletLabelsTest(BitcoinTestFramework):
@@ -170,13 +167,7 @@
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=labels_value(
- name=self.name, purpose=self.purpose[address])
- )
+ test_address(node, address, label=self.name, 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
--- a/test/functional/wallet_listreceivedby.py
+++ b/test/functional/wallet_listreceivedby.py
@@ -12,10 +12,7 @@
assert_equal,
assert_raises_rpc_error,
)
-from test_framework.wallet_util import (
- labels_value,
- test_address,
-)
+from test_framework.wallet_util import test_address
class ReceivedByTest(BitcoinTestFramework):
@@ -149,12 +146,7 @@
# set pre-state
label = ''
address = self.nodes[1].getnewaddress()
- test_address(
- self.nodes[1],
- address,
- label=label,
- labels=labels_value(
- name=label))
+ test_address(self.nodes[1], address, label=label, 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)

File Metadata

Mime Type
text/plain
Expires
Sat, Mar 1, 11:59 (3 h, 38 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
5187754
Default Alt Text
D7724.diff (17 KB)

Event Timeline