mirror of
https://github.com/dashpay/dash.git
synced 2024-12-24 11:32:46 +01:00
Merge #17578: rpc: simplify getaddressinfo labels, deprecate previous behavior
8925df86c4df16b1070343fef8e4d238f3cc3bd1 doc: update release notes (Jon Atack) 8bb405bbadf11391ccba7b334b4cfe66dc85b390 test: getaddressinfo labels purpose deprecation test (Jon Atack) 60aba1f2f11529add115d963d05599130288ae28 rpc: simplify getaddressinfo labels, deprecate previous behavior (Jon Atack) 7851f14ccf2bcd1e9b2ad48e5e08881be06d9d21 rpc: incorporate review feedback from PR 17283 (Jon Atack) Pull request description: This PR builds on #17283 (now merged) and is followed by #17585. It modifies the value returned by rpc getaddressinfo `labels` to an array of label name strings and deprecates the previous behavior of returning an array of JSON hash structures containing label `name` and address `purpose` key/value pairs. before ``` "labels": [ { "name": "DOUBLE SPEND", "purpose": "receive" } ``` after ``` "labels": [ "DOUBLE SPEND" ] ``` The deprecated behavior can be re-enabled by starting bitcoind with `-deprecatedrpc=labelspurpose`. For context, see: - https://github.com/bitcoin/bitcoin/pull/17283#issuecomment-554458001 - http://www.erisian.com.au/bitcoin-core-dev/log-2019-12-13.html#l-425 (lines 425-427) - http://www.erisian.com.au/bitcoin-core-dev/log-2019-11-22.html#l-622 Reviewers: This PR may be tested manually by building, then running bitcoind with and without the `-deprecatedrpc=labelspurpose` flag while verifying the rpc getaddressinfo help text and `labels` output. Next steps: deprecate the rpc getaddressinfo `label` field (EDIT: done in #17585) and add support for multiple labels per address. This PR will unblock those. ACKs for top commit: jnewbery: reACK 8925df8 promag: Code review ACK 8925df86c4df16b1070343fef8e4d238f3cc3bd1. meshcollider: Code review ACK 8925df86c4df16b1070343fef8e4d238f3cc3bd1 Tree-SHA512: c2b717209996da32b6484de7bb8800e7048410f9ce6afdb3e02a6866bd4a8f2c730f905fca27b10b877b91cf407f546e69e8c4feb9cd934325a6c71c166bd438
This commit is contained in:
parent
2ad00d50e0
commit
4fcd7850b0
8
doc/release-notes-17578.md
Normal file
8
doc/release-notes-17578.md
Normal file
@ -0,0 +1,8 @@
|
||||
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
|
||||
`-deprecatedrpc=labelspurpose`.
|
@ -3622,6 +3622,8 @@ static UniValue AddressBookDataToJSON(const CAddressBookData& data, const bool v
|
||||
|
||||
UniValue getaddressinfo(const JSONRPCRequest& request)
|
||||
{
|
||||
const std::string example_address = "\"XwnLY9Tf7Zsef8gMGL2fhWA9ZmMjt4KPwg\"";
|
||||
|
||||
RPCHelpMan{"getaddressinfo",
|
||||
"\nReturn information about the given dash address.\n"
|
||||
"Some of the information will only be present if the address is in the active wallet.\n",
|
||||
@ -3652,7 +3654,7 @@ UniValue getaddressinfo(const JSONRPCRequest& request)
|
||||
{RPCResult::Type::NUM, "sigsrequired", /* optional */ true, "The number of signatures required to spend multisig output (only if script is multisig)."},
|
||||
{RPCResult::Type::STR_HEX, "pubkey", /* optional */ true, "The hex value of the raw public key, for single-key addresses."},
|
||||
{RPCResult::Type::BOOL, "iscompressed", /* optional */ true, "If the pubkey is compressed."},
|
||||
{RPCResult::Type::STR, "label", "The label associated with the address. Defaults to \"\". Equivalent to the name field in the labels array."},
|
||||
{RPCResult::Type::STR, "label", "The label associated with the address. Defaults to \"\". Equivalent to the name label in the labels array below."},
|
||||
{RPCResult::Type::NUM_TIME, "timestamp", /* optional */ true, "The creation time of the key, if available, expressed in " + UNIX_EPOCH_TIME + "."},
|
||||
{RPCResult::Type::STR_HEX, "hdchainid", /* optional */ true, "The ID of the HD chain."},
|
||||
{RPCResult::Type::STR, "hdkeypath", /* optional */ true, "The HD keypath, if the key is HD and available."},
|
||||
@ -3670,8 +3672,8 @@ UniValue getaddressinfo(const JSONRPCRequest& request)
|
||||
}
|
||||
},
|
||||
RPCExamples{
|
||||
HelpExampleCli("getaddressinfo", "\"XwnLY9Tf7Zsef8gMGL2fhWA9ZmMjt4KPwg\"")
|
||||
+ HelpExampleRpc("getaddressinfo", "\"XwnLY9Tf7Zsef8gMGL2fhWA9ZmMjt4KPwg\"")
|
||||
HelpExampleCli("getaddressinfo", example_address)
|
||||
+ HelpExampleRpc("getaddressinfo", example_address)
|
||||
},
|
||||
}.Check(request);
|
||||
|
||||
@ -3683,7 +3685,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");
|
||||
@ -3709,24 +3710,18 @@ UniValue getaddressinfo(const JSONRPCRequest& request)
|
||||
|
||||
ret.pushKV("iswatchonly", bool(mine & ISMINE_WATCH_ONLY));
|
||||
|
||||
// Return DescribeWalletAddress fields.
|
||||
// Always returned: isscript, ischange, iswitness.
|
||||
// Optional: witness_version, witness_program, 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.at(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) {
|
||||
const PKHash *pkhash = std::get_if<PKHash>(&dest);
|
||||
@ -3746,15 +3741,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>::const_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));
|
||||
|
||||
|
48
test/functional/rpc_getaddressinfo_labels_purpose_deprecation.py
Executable file
48
test/functional/rpc_getaddressinfo_labels_purpose_deprecation.py
Executable file
@ -0,0 +1,48 @@
|
||||
#!/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()
|
@ -63,11 +63,6 @@ def get_multisig(node):
|
||||
p2sh_addr=script_to_p2sh(script_code),
|
||||
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)
|
||||
|
@ -265,6 +265,7 @@ BASE_SCRIPTS = [
|
||||
'feature_config_args.py',
|
||||
'feature_settings.py',
|
||||
'rpc_getdescriptorinfo.py',
|
||||
'rpc_getaddressinfo_labels_purpose_deprecation.py',
|
||||
'rpc_help.py',
|
||||
'feature_help.py',
|
||||
# Don't append tests at the end to avoid merge conflicts
|
||||
|
@ -14,10 +14,7 @@ from test_framework.util import (
|
||||
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):
|
||||
@ -397,7 +394,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
|
||||
|
||||
|
@ -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])
|
||||
|
||||
self.log.info(
|
||||
"Import the watch-only address's private key without a "
|
||||
@ -128,10 +115,8 @@ class ImportWithLabel(BitcoinTestFramework):
|
||||
)
|
||||
priv_key4 = self.nodes[0].dumpprivkey(address4)
|
||||
self.nodes[1].importprivkey(priv_key4)
|
||||
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()
|
||||
|
||||
|
@ -29,7 +29,6 @@ from test_framework.util import (
|
||||
from test_framework.wallet_util import (
|
||||
get_key,
|
||||
get_multisig,
|
||||
labels_value,
|
||||
test_address,
|
||||
)
|
||||
|
||||
@ -504,7 +503,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])
|
||||
|
@ -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):
|
||||
@ -159,12 +157,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),
|
||||
|
@ -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)
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user