rpc: introduce 'label' API for wallet #7729
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -187,6 +187,11 @@ UniValue getnewaddress(const JSONRPCRequest& request) | |
| return EncodeDestination(dest); | ||
| } | ||
|
|
||
| void DeleteLabel(CWallet& wallet, std::string label) | ||
| { | ||
| CWalletDB walletdb(wallet.GetDBHandle()); | ||
| walletdb.EraseAccount(label); | ||
| } | ||
|
|
||
| CTxDestination GetLabelDestination(CWallet* const pwallet, const std::string& label, bool bForceNew=false) | ||
| { | ||
|
|
@@ -208,11 +213,11 @@ UniValue getlabeladdress(const JSONRPCRequest& request) | |
| if (request.fHelp || request.params.size() != 1) | ||
| throw std::runtime_error( | ||
| "getlabeladdress \"label\"\n" | ||
| "\nReturns the current 'label address' for this label.\n" | ||
| "\nArguments:\n" | ||
|
There was a problem hiding this comment. I'm fine with removing this remark, but note that the behavior of creating a new address still exists (though without an address type param). It might make sense to deprecate that behavior in a followup PR, and require |
||
| "1. \"label\" (string, required) The label for the address. It can also be set to the empty string \"\" to represent the default label.\n" | ||
| "\nResult:\n" | ||
| "\"bitcoinaddress\" (string) The 'label address' for the label\n" | ||
| "\nExamples:\n" | ||
| + HelpExampleCli("getlabeladdress", "") | ||
| + HelpExampleCli("getlabeladdress", "\"\"") | ||
|
|
@@ -290,14 +295,14 @@ UniValue setlabel(const JSONRPCRequest& request) | |
|
|
||
| if (request.fHelp || request.params.size() < 1 || request.params.size() > 2) | ||
| throw std::runtime_error( | ||
| "setlabel \"bitcoinaddress\" \"label\"\n" | ||
|
There was a problem hiding this comment. Why rename the arguments/return values to bitcoinaddress everywhere? Seems like a gratuitous API break.
There was a problem hiding this comment. |
||
| "\nSets the label associated with the given address.\n" | ||
| "\nArguments:\n" | ||
| "1. \"bitcoinaddress\" (string, required) The bitcoin address to be associated with a label.\n" | ||
| "2. \"label\" (string, required) The label to assign to the address.\n" | ||
| "\nExamples:\n" | ||
| + HelpExampleCli("setlabel", "\"1D1ZrZNe3JUo7ZycKEYQQiQAWd9y54F4XZ\" \"tabby\"") | ||
|
There was a problem hiding this comment. Unrelated change? If you really wanted to change that |
||
| + HelpExampleRpc("setlabel", "\"1D1ZrZNe3JUo7ZycKEYQQiQAWd9y54F4XZ\", \"tabby\"") | ||
| ); | ||
|
|
||
| LOCK2(cs_main, pwallet->cs_wallet); | ||
|
|
@@ -308,22 +313,24 @@ UniValue setlabel(const JSONRPCRequest& request) | |
| } | ||
|
|
||
| std::string label; | ||
| if (!request.params[1].isNull()) { | ||
| label = LabelFromValue(request.params[1]); | ||
| } | ||
|
|
||
| if (IsMine(*pwallet, dest)) { | ||
| // Detect when changing the label of an address that is the 'label address' of another label: | ||
| // If so, delete the account record for it. Labels, unlike addresses, can be deleted, | ||
| // and if we wouldn't do this, the record would stick around forever. | ||
| if (pwallet->mapAddressBook.count(dest)) { | ||
| std::string old_label = pwallet->mapAddressBook[dest].name; | ||
| if (old_label != label && dest == GetLabelDestination(pwallet, old_label)) { | ||
| DeleteLabel(*pwallet, old_label); | ||
| } | ||
| } | ||
| pwallet->SetAddressBook(dest, label, "receive"); | ||
| } else { | ||
| pwallet->SetAddressBook(dest, label, "send"); | ||
| } | ||
|
|
||
| return NullUniValue; | ||
| } | ||
|
|
@@ -3809,6 +3816,152 @@ UniValue getaddressinfo(const JSONRPCRequest& request) | |
| return ret; | ||
| } | ||
|
|
||
| /** Convert CAddressBookData to JSON record. | ||
| * The verbosity of the output is configurable based on the command. | ||
| */ | ||
| static UniValue AddressBookDataToJSON(const CAddressBookData& data, bool verbose) | ||
| { | ||
| UniValue ret(UniValue::VOBJ); | ||
| if (verbose) { | ||
| ret.push_back(Pair("name", data.name)); | ||
| } | ||
| ret.push_back(Pair("purpose", data.purpose)); | ||
| if (verbose) { | ||
| UniValue ddata(UniValue::VOBJ); | ||
|
|
||
| for (const std::pair<std::string, std::string>& item : data.destdata) { | ||
| ddata.push_back(Pair(item.first, item.second)); | ||
| } | ||
| ret.push_back(Pair("destdata", ddata)); | ||
| } | ||
| return ret; | ||
| } | ||
|
|
||
| UniValue getlabel(const JSONRPCRequest& request) | ||
| { | ||
| CWallet * const pwallet = GetWalletForJSONRPCRequest(request); | ||
| if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { | ||
| return NullUniValue; | ||
| } | ||
|
|
||
| if (request.fHelp || request.params.size() != 1) | ||
| throw std::runtime_error( | ||
| "getlabel \"bitcoinaddress\"\n" | ||
| "\nReturns the label associated with the given address.\n" | ||
| "\nArguments:\n" | ||
| "1. \"bitcoinaddress\" (string, required) The bitcoin address for label lookup.\n" | ||
| "\nResult:\n" | ||
| " { (json object with information about address)\n" | ||
|
There was a problem hiding this comment. Can we make this RPC return an array? We may want to be able to attach multiple labels to addresses in the future (#7729 (comment)), and making this RPC return an array will allow us to do that without a breaking API change There was a problem hiding this comment.
It might be better to not add a |
||
| " \"name\": \"labelname\" (string) The label\n" | ||
| " \"purpose\": \"string\" (string) Purpose of address (\"send\" for sending address, \"receive\" for receiving address)\n" | ||
|
There was a problem hiding this comment. If I'm understanding There was a problem hiding this comment. Comment: https://github.com/bitcoin/bitcoin/pull/7729/files#r121986789 not addressed. Result also includes an array of destdata. Please update help text. EDIT: I think we should just drop the We can always add destdata to the response in a later PR if required. |
||
| " },...\n" | ||
| " Result is null if there is no record for this address.\n" | ||
| "\nExamples:\n" | ||
| + HelpExampleCli("getlabel", "\"1D1ZrZNe3JUo7ZycKEYQQiQAWd9y54F4XZ\"") | ||
| + HelpExampleRpc("getlabel", "\"1D1ZrZNe3JUo7ZycKEYQQiQAWd9y54F4XZ\"") | ||
| ); | ||
|
|
||
| LOCK2(cs_main, pwallet->cs_wallet); | ||
|
|
||
| CTxDestination dest = DecodeDestination(request.params[0].get_str()); | ||
| if (!IsValidDestination(dest)) { | ||
| throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid Bitcoin address"); | ||
| } | ||
|
|
||
| std::map<CTxDestination, CAddressBookData>::iterator mi = pwallet->mapAddressBook.find(dest); | ||
| if (mi != pwallet->mapAddressBook.end()) { | ||
| return AddressBookDataToJSON(mi->second, true); | ||
| } | ||
| return NullUniValue; | ||
| } | ||
|
|
||
| UniValue getaddressesbylabel(const JSONRPCRequest& request) | ||
| { | ||
| CWallet * const pwallet = GetWalletForJSONRPCRequest(request); | ||
| if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { | ||
| return NullUniValue; | ||
| } | ||
|
|
||
| if (request.fHelp || request.params.size() != 1) | ||
| throw std::runtime_error( | ||
| "getaddressesbylabel \"label\"\n" | ||
| "\nReturns the list of addresses assigned the specified label.\n" | ||
| "\nArguments:\n" | ||
| "1. \"label\" (string, required) The label.\n" | ||
| "\nResult:\n" | ||
| "{ (json object with addresses as keys)\n" | ||
| " \"bitcoinaddress\": { (json object with information about address)\n" | ||
| " \"purpose\": \"string\" (string) Purpose of address (\"send\" for sending address, \"receive\" for receiving address)\n" | ||
| " },...\n" | ||
| "}\n" | ||
| "\nExamples:\n" | ||
| + HelpExampleCli("getaddressesbylabel", "\"tabby\"") | ||
| + HelpExampleRpc("getaddressesbylabel", "\"tabby\"") | ||
| ); | ||
|
|
||
| LOCK2(cs_main, pwallet->cs_wallet); | ||
|
|
||
| std::string strLabel = LabelFromValue(request.params[0]); | ||
|
|
||
| // Find all addresses that have the given label | ||
| UniValue ret(UniValue::VOBJ); | ||
| for (const std::pair<CTxDestination, CAddressBookData>& item : pwallet->mapAddressBook) { | ||
| if (item.second.name == strLabel) { | ||
| ret.push_back(Pair(EncodeDestination(item.first), AddressBookDataToJSON(item.second, false))); | ||
| } | ||
| } | ||
|
There was a problem hiding this comment. Should this throw an error if the label doesn't exist? Currently it returns an empty object. |
||
| return ret; | ||
| } | ||
|
|
||
| UniValue listlabels(const JSONRPCRequest& request) | ||
| { | ||
| CWallet * const pwallet = GetWalletForJSONRPCRequest(request); | ||
| if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { | ||
| return NullUniValue; | ||
| } | ||
|
|
||
| if (request.fHelp || request.params.size() > 1) | ||
| throw std::runtime_error( | ||
| "listlabels ( \"purpose\" )\n" | ||
| "\nReturns the list of all labels, or labels that are assigned to addresses with a specific purpose.\n" | ||
| "\nArguments:\n" | ||
| "1. \"purpose\" (string, optional) Address purpose to list labels for ('send','receive'). An empty string is the same as not providing this argument.\n" | ||
|
There was a problem hiding this comment. nit: I don't think There was a problem hiding this comment. Agree with @jnewbery, empty value should be an error then. There was a problem hiding this comment. BTW, why is this argument needed? I mean, the client can filter the result. IMO pagination is more interesting. |
||
| "\nResult:\n" | ||
| "[ (json array of string)\n" | ||
| " \"label\", (string) Label name\n" | ||
| " ...\n" | ||
| "]\n" | ||
| "\nExamples:\n" | ||
| "\nList all labels\n" | ||
| + HelpExampleCli("listlabels", "") + | ||
| "\nList labels that have receiving addresses\n" | ||
| + HelpExampleCli("listlabels", "receive") + | ||
| "\nList labels that have sending addresses\n" | ||
| + HelpExampleCli("listlabels", "send") + | ||
| "\nAs json rpc call\n" | ||
| + HelpExampleRpc("listlabels", "receive") | ||
| ); | ||
|
|
||
| LOCK2(cs_main, pwallet->cs_wallet); | ||
|
|
||
| std::string purpose; | ||
| if (request.params.size() > 0) { | ||
| purpose = request.params[0].get_str(); | ||
| } | ||
|
|
||
| std::set<std::string> setLabels; | ||
| for (const std::pair<CTxDestination, CAddressBookData>& entry : pwallet->mapAddressBook) { | ||
| if (purpose.empty() || entry.second.purpose == purpose){ | ||
| setLabels.insert(entry.second.name); | ||
| } | ||
| } | ||
| UniValue ret(UniValue::VARR); | ||
| for (const std::string &name : setLabels) { | ||
|
There was a problem hiding this comment. Should we sort the label names before returning? |
||
| ret.push_back(name); | ||
| } | ||
|
|
||
| return ret; | ||
| } | ||
|
|
||
| extern UniValue abortrescan(const JSONRPCRequest& request); // in rpcdump.cpp | ||
| extern UniValue dumpprivkey(const JSONRPCRequest& request); // in rpcdump.cpp | ||
| extern UniValue importprivkey(const JSONRPCRequest& request); | ||
|
|
@@ -3835,16 +3988,9 @@ static const CRPCCommand commands[] = | |
| { "wallet", "dumpprivkey", &dumpprivkey, {"address"} }, | ||
| { "wallet", "dumpwallet", &dumpwallet, {"filename"} }, | ||
| { "wallet", "encryptwallet", &encryptwallet, {"passphrase"} }, | ||
| { "wallet", "getbalance", &getbalance, {"account","minconf","include_watchonly"} }, | ||
| { "wallet", "getnewaddress", &getnewaddress, {"label|account","address_type"} }, | ||
| { "wallet", "getrawchangeaddress", &getrawchangeaddress, {"address_type"} }, | ||
| { "wallet", "getreceivedbyaddress", &getreceivedbyaddress, {"address","minconf"} }, | ||
| { "wallet", "gettransaction", &gettransaction, {"txid","include_watchonly"} }, | ||
| { "wallet", "getunconfirmedbalance", &getunconfirmedbalance, {} }, | ||
|
|
@@ -3856,7 +4002,6 @@ static const CRPCCommand commands[] = | |
| { "wallet", "importprunedfunds", &importprunedfunds, {"rawtransaction","txoutproof"} }, | ||
| { "wallet", "importpubkey", &importpubkey, {"pubkey","label","rescan"} }, | ||
| { "wallet", "keypoolrefill", &keypoolrefill, {"newsize"} }, | ||
| { "wallet", "listaddressgroupings", &listaddressgroupings, {} }, | ||
| { "wallet", "listlockunspent", &listlockunspent, {} }, | ||
| { "wallet", "listreceivedbylabel", &listreceivedbylabel, {"minconf","include_empty","include_watchonly"} }, | ||
|
|
@@ -3867,12 +4012,9 @@ static const CRPCCommand commands[] = | |
| { "wallet", "listunspent", &listunspent, {"minconf","maxconf","addresses","include_unsafe","query_options"} }, | ||
| { "wallet", "listwallets", &listwallets, {} }, | ||
| { "wallet", "lockunspent", &lockunspent, {"unlock","transactions"} }, | ||
| { "wallet", "sendfrom", &sendfrom, {"fromaccount","toaddress","amount","minconf","comment","comment_to"} }, | ||
| { "wallet", "sendmany", &sendmany, {"fromaccount","amounts","minconf","comment","subtractfeefrom","replaceable","conf_target","estimate_mode"} }, | ||
| { "wallet", "sendtoaddress", &sendtoaddress, {"address","amount","comment","comment_to","subtractfeefromamount","replaceable","conf_target","estimate_mode"} }, | ||
| { "wallet", "settxfee", &settxfee, {"amount"} }, | ||
| { "wallet", "signmessage", &signmessage, {"address","message"} }, | ||
| { "wallet", "signrawtransactionwithwallet", &signrawtransactionwithwallet, {"hexstring","prevtxs","sighashtype"} }, | ||
|
|
@@ -3882,6 +4024,26 @@ static const CRPCCommand commands[] = | |
| { "wallet", "removeprunedfunds", &removeprunedfunds, {"txid"} }, | ||
| { "wallet", "rescanblockchain", &rescanblockchain, {"start_height", "stop_height"} }, | ||
|
|
||
| /** Account functions (deprecated) */ | ||
| { "wallet", "getaccountaddress", &getlabeladdress, {"account"} }, | ||
| { "wallet", "getaccount", &getaccount, {"address"} }, | ||
| { "wallet", "getaddressesbyaccount", &getaddressesbyaccount, {"account"} }, | ||
| { "wallet", "getaddressinfo", &getaddressinfo, {"address"} }, | ||
|
There was a problem hiding this comment. I don't think There was a problem hiding this comment.
Good catch. getaddressinfo should remain where it was above "getbalance". It's my fault for accidentally moving it into this section during a rebase. |
||
| { "wallet", "getreceivedbyaccount", &getreceivedbylabel, {"account","minconf"} }, | ||
| { "wallet", "listaccounts", &listaccounts, {"minconf","include_watchonly"} }, | ||
| { "wallet", "listreceivedbyaccount", &listreceivedbylabel, {"minconf","include_empty","include_watchonly"} }, | ||
| { "wallet", "setaccount", &setlabel, {"address","account"} }, | ||
| { "wallet", "move", &movecmd, {"fromaccount","toaccount","amount","minconf","comment"} }, | ||
|
|
||
| /** Label functions (to replace non-balance account functions) */ | ||
| { "wallet", "getlabeladdress", &getlabeladdress, {"label"} }, | ||
| { "wallet", "getlabel", &getlabel, {"bitcoinaddress"} }, | ||
| { "wallet", "getaddressesbylabel", &getaddressesbylabel, {"label"} }, | ||
| { "wallet", "getreceivedbylabel", &getreceivedbylabel, {"label","minconf"} }, | ||
| { "wallet", "listlabels", &listlabels, {"purpose"} }, | ||
| { "wallet", "listreceivedbylabel", &listreceivedbylabel, {"minconf","include_empty","include_watchonly"} }, | ||
| { "wallet", "setlabel", &setlabel, {"bitcoinaddress","label"} }, | ||
|
|
||
| { "generating", "generate", &generate, {"nblocks","maxtries"} }, | ||
| }; | ||
|
|
||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The behaviour for this RPC is weird. If called for a label that doesn't exist, it creates a new label, and then adds a new address as the 'label address' for that label. That's not very intuitive, and I think it's a bad experience (for example if a user typos an existing label name). Can we change this so that the rpc returns an error if called with a non-existent label name?