Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

rpc: improve getaddressinfo test coverage, help, code docs #17283

Merged
merged 7 commits into from
Nov 26, 2019
Merged

rpc: improve getaddressinfo test coverage, help, code docs #17283

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
70cda34
rpc: improve getaddressinfo RPCHelpman formatting
jonatack Oct 26, 2019
5a0ed85
rpc: improve getaddressinfo RPCHelpman content
jonatack Oct 26, 2019
8d1ed0c
rpc: clarify label vs labels in getaddressinfo RPCHelpman
jonatack Oct 28, 2019
2ee0cb3
rpc: update getaddressinfo RPCExamples to bech32
jonatack Oct 28, 2019
1388de8
rpc: add getaddressinfo code documentation
jonatack Oct 27, 2019
0f3539a
test: add listlabels test in wallet_labels.py
jonatack Oct 28, 2019
33f5fc3
test: add rpc getaddressinfo labels test coverage
jonatack Oct 28, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 59 additions & 36 deletions src/wallet/rpcwallet.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -951,7 +951,7 @@ static UniValue addmultisigaddress(const JSONRPCRequest& request)
}

RPCHelpMan{"addmultisigaddress",
"\nAdd a nrequired-to-sign multisignature address to the wallet. Requires a new wallet backup.\n"
"\nAdd an nrequired-to-sign multisignature address to the wallet. Requires a new wallet backup.\n"
"Each key is a Bitcoin address or hex-encoded public key.\n"
"This functionality is only intended for use with non-watchonly addresses.\n"
"See `importaddress` for watchonly p2sh address support.\n"
Expand Down Expand Up @@ -3709,52 +3709,57 @@ UniValue getaddressinfo(const JSONRPCRequest& request)
}

RPCHelpMan{"getaddressinfo",
"\nReturn information about the given bitcoin address. Some information requires the address\n"
"to be in the wallet.\n",
"\nReturn information about the given bitcoin address.\n"
"Some of the information will only be present if the address is in the active wallet.\n",
{
{"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The bitcoin address to get the information of."},
{"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The bitcoin address for which to get information."},
},
RPCResult{
"{\n"
" \"address\" : \"address\", (string) The bitcoin address validated\n"
jonatack marked this conversation as resolved.
Show resolved Hide resolved
" \"scriptPubKey\" : \"hex\", (string) The hex-encoded scriptPubKey generated by the address\n"
" \"ismine\" : true|false, (boolean) If the address is yours or not\n"
" \"iswatchonly\" : true|false, (boolean) If the address is watchonly\n"
" \"solvable\" : true|false, (boolean) Whether we know how to spend coins sent to this address, ignoring the possible lack of private keys\n"
" \"desc\" : \"desc\", (string, optional) A descriptor for spending coins sent to this address (only when solvable)\n"
" \"isscript\" : true|false, (boolean) If the key is a script\n"
" \"ischange\" : true|false, (boolean) If the address was used for change output\n"
" \"iswitness\" : true|false, (boolean) If the address is a witness address\n"
" \"witness_version\" : version (numeric, optional) The version number of the witness program\n"
" \"witness_program\" : \"hex\" (string, optional) The hex value of the witness program\n"
" \"script\" : \"type\" (string, optional) The output script type. Only if \"isscript\" is true and the redeemscript is known. Possible types: nonstandard, pubkey, pubkeyhash, scripthash, multisig, nulldata, witness_v0_keyhash, witness_v0_scripthash, witness_unknown\n"
" \"hex\" : \"hex\", (string, optional) The redeemscript for the p2sh address\n"
" \"pubkeys\" (string, optional) Array of pubkeys associated with the known redeemscript (only if \"script\" is \"multisig\")\n"
" \"address\" : \"address\", (string) The bitcoin address validated.\n"
" \"scriptPubKey\" : \"hex\", (string) The hex-encoded scriptPubKey generated by the address.\n"
" \"ismine\" : true|false, (boolean) If the address is yours.\n"
" \"iswatchonly\" : true|false, (boolean) If the address is watchonly.\n"
" \"solvable\" : true|false, (boolean) If we know how to spend coins sent to this address, ignoring the possible lack of private keys.\n"
" \"desc\" : \"desc\", (string, optional) A descriptor for spending coins sent to this address (only when solvable).\n"
" \"isscript\" : true|false, (boolean) If the key is a script.\n"
" \"ischange\" : true|false, (boolean) If the address was used for change output.\n"
" \"iswitness\" : true|false, (boolean) If the address is a witness address.\n"
" \"witness_version\" : version (numeric, optional) The version number of the witness program.\n"
" \"witness_program\" : \"hex\" (string, optional) The hex value of the witness program.\n"
" \"script\" : \"type\" (string, optional) The output script type. Only if isscript is true and the redeemscript is known. Possible\n"
" types: nonstandard, pubkey, pubkeyhash, scripthash, multisig, nulldata, witness_v0_keyhash,\n"
" witness_v0_scripthash, witness_unknown.\n"
" \"hex\" : \"hex\", (string, optional) The redeemscript for the p2sh address.\n"
" \"pubkeys\" (array, optional) Array of pubkeys associated with the known redeemscript (only if script is multisig).\n"
" [\n"
" \"pubkey\"\n"
" \"pubkey\" (string)\n"
" ,...\n"
" ]\n"
" \"sigsrequired\" : xxxxx (numeric, optional) Number of signatures required to spend multisig output (only if \"script\" is \"multisig\")\n"
" \"pubkey\" : \"publickeyhex\", (string, optional) The hex value of the raw public key, for single-key addresses (possibly embedded in P2SH or P2WSH)\n"
" \"embedded\" : {...}, (object, optional) Information about the address embedded in P2SH or P2WSH, if relevant and known. It includes all getaddressinfo output fields for the embedded address, excluding metadata (\"timestamp\", \"hdkeypath\", \"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, \"\" is the default label\n"
" \"timestamp\" : timestamp, (number, optional) The creation time of the key if available in seconds since epoch (Jan 1 1970 GMT)\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 fingperint of the master key.\n"
" \"labels\" (object) Array of labels associated with the address.\n"
" \"sigsrequired\" : xxxxx (numeric, optional) The number of signatures required to spend multisig output (only if script is multisig).\n"
" \"pubkey\" : \"publickeyhex\", (string, optional) The hex value of the raw public key for single-key addresses (possibly embedded in P2SH or P2WSH).\n"
" \"embedded\" : {...}, (object, optional) Information about the address embedded in P2SH or P2WSH, if relevant and known. Includes all\n"
" 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"
" \"timestamp\" : timestamp, (number, optional) The creation time of the key if available, expressed in seconds since Epoch Time (Jan 1 1970 GMT).\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"
" as an array to keep the API stable if multiple labels are enabled in the future.\n"
" [\n"
" { (json object of label data)\n"
" \"name\": \"labelname\" (string) The label\n"
" \"purpose\": \"string\" (string) Purpose of address (\"send\" for sending address, \"receive\" for receiving address)\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"
},
RPCExamples{
HelpExampleCli("getaddressinfo", "\"1PSSGeFHDnKNxiEyFrD1wcEaHr9hrQDDWc\"")
+ HelpExampleRpc("getaddressinfo", "\"1PSSGeFHDnKNxiEyFrD1wcEaHr9hrQDDWc\"")
HelpExampleCli("getaddressinfo", "\"bc1q09vm5lfy0j5reeulh4x5752q25uqqvz34hufdl\"") +
jonatack marked this conversation as resolved.
Show resolved Hide resolved
HelpExampleRpc("getaddressinfo", "\"bc1q09vm5lfy0j5reeulh4x5752q25uqqvz34hufdl\"")
},
}.Check(request);

Expand All @@ -3773,23 +3778,39 @@ UniValue getaddressinfo(const JSONRPCRequest& request)

CScript scriptPubKey = GetScriptForDestination(dest);
ret.pushKV("scriptPubKey", HexStr(scriptPubKey.begin(), scriptPubKey.end()));

const SigningProvider* provider = pwallet->GetSigningProvider(scriptPubKey);

isminetype mine = pwallet->IsMine(dest);
ret.pushKV("ismine", bool(mine & ISMINE_SPENDABLE));

bool solvable = provider && IsSolvable(*provider, scriptPubKey);
ret.pushKV("solvable", solvable);

if (solvable) {
ret.pushKV("desc", InferDescriptor(scriptPubKey, *provider)->ToString());
}

ret.pushKV("iswatchonly", bool(mine & ISMINE_WATCH_ONLY));

// Return DescribeWalletAddress fields.
// Always returned: isscript, ischange, iswitness.
jonatack marked this conversation as resolved.
Show resolved Hide resolved
// 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.
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,
jonatack marked this conversation as resolved.
Show resolved Hide resolved
// and hdmasterfingerprint fields.
ScriptPubKeyMan* spk_man = pwallet->GetScriptPubKeyMan(scriptPubKey);
if (spk_man) {
if (const CKeyMetadata* meta = spk_man->GetMetadata(dest)) {
Expand All @@ -3802,9 +3823,11 @@ UniValue getaddressinfo(const JSONRPCRequest& request)
}
}

// Currently only one label can be associated with an address, return an array
// so the API remains stable if we allow multiple labels to be associated with
// an address.
// 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.
UniValue labels(UniValue::VARR);
std::map<CTxDestination, CAddressBookData>::iterator mi = pwallet->mapAddressBook.find(dest);
if (mi != pwallet->mapAddressBook.end()) {
Expand Down
5 changes: 5 additions & 0 deletions test/functional/test_framework/wallet_util.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,11 @@ 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"):
jonatack marked this conversation as resolved.
Show resolved Hide resolved
"""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)
Expand Down
6 changes: 5 additions & 1 deletion test/functional/wallet_basic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@
connect_nodes,
wait_until,
)
from test_framework.wallet_util import (
labels_value,
test_address,
)


class WalletTest(BitcoinTestFramework):
Expand Down Expand Up @@ -390,7 +394,7 @@ def run_test(self):
for label in [u'рыба', u'𝅘𝅥𝅯']:
addr = self.nodes[0].getnewaddress()
self.nodes[0].setlabel(addr, label)
assert_equal(self.nodes[0].getaddressinfo(addr)['label'], label)
test_address(self.nodes[0], addr, label=label, labels=labels_value(name=label))
assert label in self.nodes[0].listlabels()
self.nodes[0].rpc.ensure_ascii = True # restore to default

Expand Down
31 changes: 22 additions & 9 deletions test/functional/wallet_import_with_label.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@
"""

from test_framework.test_framework import BitcoinTestFramework
from test_framework.wallet_util import test_address
from test_framework.wallet_util import (
labels_value,
test_address,
)


class ImportWithLabel(BitcoinTestFramework):
Expand All @@ -36,7 +39,8 @@ def run_test(self):
address,
iswatchonly=True,
ismine=False,
label=label)
label=label,
labels=labels_value(name=label))

self.log.info(
"Import the watch-only address's private key without a "
Expand All @@ -47,7 +51,8 @@ def run_test(self):

test_address(self.nodes[1],
address,
label=label)
label=label,
labels=labels_value(name=label))

self.log.info(
"Test importaddress without label and importprivkey with label."
Expand All @@ -59,7 +64,8 @@ def run_test(self):
address2,
iswatchonly=True,
ismine=False,
label="")
label="",
labels=labels_value())

self.log.info(
"Import the watch-only address's private key with a "
Expand All @@ -71,7 +77,8 @@ def run_test(self):

test_address(self.nodes[1],
address2,
label=label2)
label=label2,
labels=labels_value(name=label2))

self.log.info("Test importaddress with label and importprivkey with label.")
self.log.info("Import a watch-only address with a label.")
Expand All @@ -82,7 +89,8 @@ def run_test(self):
address3,
iswatchonly=True,
ismine=False,
label=label3_addr)
label=label3_addr,
labels=labels_value(name=label3_addr))

self.log.info(
"Import the watch-only address's private key with a "
Expand All @@ -94,7 +102,8 @@ def run_test(self):

test_address(self.nodes[1],
address3,
label=label3_priv)
label=label3_priv,
labels=labels_value(name=label3_priv))

self.log.info(
"Test importprivkey won't label new dests with the same "
Expand All @@ -109,6 +118,7 @@ def run_test(self):
iswatchonly=True,
ismine=False,
label=label4_addr,
labels=labels_value(name=label4_addr),
embedded=None)

self.log.info(
Expand All @@ -123,10 +133,13 @@ def run_test(self):

test_address(self.nodes[1],
embedded_addr,
label="")
label="",
labels=labels_value())

test_address(self.nodes[1],
address4,
label=label4_addr)
label=label4_addr,
labels=labels_value(name=label4_addr))

self.stop_nodes()

Expand Down
Loading