rpc: simplify getaddressinfo labels, deprecate previous behavior

- change the value returned in the RPC getaddressinfo `labels` field to an array
  of label name strings

- deprecate the previous behavior of returning a JSON hash structure containing
  label `name` and address `purpose` key/value pairs

- update the relevant tests
This commit is contained in:
Jon Atack 2019-11-24 12:05:38 +01:00
parent 7851f14ccf
commit 60aba1f2f1
No known key found for this signature in database
GPG Key ID: 4F5721B3D0E3921D
7 changed files with 40 additions and 70 deletions

View File

@ -3744,18 +3744,20 @@ 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 name field in the labels array.\n"
" \"label\" : \"label\" (string) The label associated with the address. Defaults to \"\". Equivalent to the label name in 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\" : \"<hash160>\" (string, optional) The Hash160 of the HD seed.\n"
" \"hdmasterfingerprint\" : \"<hash160>\" (string, optional) The fingerprint of the master key.\n"
" \"labels\" (object) An array of labels associated with the address. Currently limited to one label but returned\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"
" [\n"
" \"label name\" (string) The label name. Defaults to \"\". Equivalent to the label field above.\n\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"
" \"purpose\": \"purpose\" (string) The purpose of the associated address (send or receive).\n"
" },...\n"
" \"name\" : \"label name\" (string) The label name. Defaults to \"\". Equivalent to the label field above.\n"
" \"purpose\" : \"purpose\" (string) The purpose of the associated address (send or receive).\n"
" }\n"
" ]\n"
"}\n"
},
@ -3769,7 +3771,6 @@ UniValue getaddressinfo(const JSONRPCRequest& request)
UniValue ret(UniValue::VOBJ);
CTxDestination dest = DecodeDestination(request.params[0].get_str());
// Make sure the destination is valid
if (!IsValidDestination(dest)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid address");
@ -3800,7 +3801,7 @@ UniValue getaddressinfo(const JSONRPCRequest& request)
// 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);
}
@ -3819,15 +3820,22 @@ UniValue getaddressinfo(const JSONRPCRequest& request)
}
}
// 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));

View File

@ -88,11 +88,6 @@ def get_multisig(node):
p2sh_p2wsh_script=CScript([OP_HASH160, witness_script, OP_EQUAL]).hex(),
p2sh_p2wsh_addr=script_to_p2sh_p2wsh(script_code))
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)

View File

@ -15,10 +15,7 @@ from test_framework.util import (
connect_nodes,
wait_until,
)
from test_framework.wallet_util import (
labels_value,
test_address,
)
from test_framework.wallet_util import test_address
class WalletTest(BitcoinTestFramework):
@ -395,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=labels_value(name=label))
test_address(self.nodes[0], addr, label=label, labels=[label])
assert label in self.nodes[0].listlabels()
self.nodes[0].rpc.ensure_ascii = True # restore to default

View File

@ -11,10 +11,7 @@ with and without a label.
"""
from test_framework.test_framework import BitcoinTestFramework
from test_framework.wallet_util import (
labels_value,
test_address,
)
from test_framework.wallet_util import test_address
class ImportWithLabel(BitcoinTestFramework):
@ -40,7 +37,7 @@ class ImportWithLabel(BitcoinTestFramework):
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 "
@ -48,11 +45,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=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."
@ -65,7 +58,7 @@ class ImportWithLabel(BitcoinTestFramework):
iswatchonly=True,
ismine=False,
label="",
labels=labels_value())
labels=[""])
self.log.info(
"Import the watch-only address's private key with a "
@ -75,10 +68,7 @@ class ImportWithLabel(BitcoinTestFramework):
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.")
self.log.info("Import a watch-only address with a label.")
@ -90,7 +80,7 @@ class ImportWithLabel(BitcoinTestFramework):
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 "
@ -100,10 +90,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=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 "
@ -118,7 +105,7 @@ class ImportWithLabel(BitcoinTestFramework):
iswatchonly=True,
ismine=False,
label=label4_addr,
labels=labels_value(name=label4_addr),
labels=[label4_addr],
embedded=None)
self.log.info(
@ -131,15 +118,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=labels_value())
test_address(self.nodes[1], embedded_addr, label="", labels=[""])
test_address(self.nodes[1],
address4,
label=label4_addr,
labels=labels_value(name=label4_addr))
test_address(self.nodes[1], address4, label=label4_addr, labels=[label4_addr])
self.stop_nodes()

View File

@ -29,7 +29,6 @@ from test_framework.util import (
from test_framework.wallet_util import (
get_key,
get_multisig,
labels_value,
test_address,
)
@ -571,7 +570,7 @@ class ImportMultiTest(BitcoinTestFramework):
solvable=True,
ismine=True,
label=p2sh_p2wpkh_label,
labels=labels_value(name=p2sh_p2wpkh_label))
labels=[p2sh_p2wpkh_label])
# Test ranged descriptor fails if range is not specified
xpriv = "tprv8ZgxMBicQKsPeuVhWwi6wuMQGfPKi9Li5GtX35jVNknACgqe3CY4g5xgkfDDJcmtF7o1QnxWDRYw4H5P26PXq7sbcUkEqeR4fg3Kxp2tigg"
@ -643,7 +642,7 @@ class ImportMultiTest(BitcoinTestFramework):
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])

View File

@ -13,10 +13,8 @@ from collections import defaultdict
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):
def set_test_params(self):
@ -157,12 +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=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),

View File

@ -11,10 +11,7 @@ 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 ReceivedByTest(BitcoinTestFramework):
@ -131,7 +128,7 @@ class ReceivedByTest(BitcoinTestFramework):
# 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)