rpc: introduce 'label' API for wallet #7729

Open
wants to merge 3 commits into
from
Jump to file or symbol
Failed to load files and symbols.
+393 −10
Split
View
@@ -152,6 +152,11 @@ UniValue getnewaddress(const JSONRPCRequest& request)
return CBitcoinAddress(keyID).ToString();
}
+void DeleteAccount(CWallet * const pwallet, std::string strAccount)
@promag

promag Jul 7, 2017

Contributor

Nit, shouldn't we follow the convention in new code in favor of consistency? In that case rename strAccount to account_name for instance?

+{
+ CWalletDB walletdb(pwallet->GetDBHandle());
+ walletdb.EraseAccount(strAccount);
+}
CBitcoinAddress GetAccountAddress(CWallet* const pwallet, std::string strAccount, bool bForceNew=false)
{
@@ -2922,6 +2927,236 @@ UniValue bumpfee(const JSONRPCRequest& request)
return result;
}
+UniValue getlabeladdress(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(
+ "getlabeladdress \"label\"\n"
+ "\nReturns the current 'label address' for this label.\n"
+ "\nArguments:\n"
+ "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"
@instagibbs

instagibbs Nov 18, 2016

Member

perhaps mention latest unused or something similar.

@laanwj

laanwj Nov 21, 2016

Owner

I still think we should get rid of this.

+ "\nExamples:\n"
+ + HelpExampleCli("getlabeladdress", "")
+ + HelpExampleCli("getlabeladdress", "\"\"")
+ + HelpExampleCli("getlabeladdress", "\"mylabel\"")
+ + HelpExampleRpc("getlabeladdress", "\"mylabel\"")
+ );
+
+ LOCK2(cs_main, pwallet->cs_wallet);
+
+ // Parse the label first so we don't generate a key if there's an error
+ std::string strLabel = AccountFromValue(request.params[0]);
@jnewbery

jnewbery Jun 14, 2017

Member

It'd be nice to eventually rename this function LabelFromValue(). That can be done in a follow-up PR.

+
+ UniValue ret(UniValue::VSTR);
+
+ ret = GetAccountAddress(pwallet, strLabel).ToString();
+ 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);
@promag

promag Jul 7, 2017

Contributor

Nit, rename ddata to dest_data.

+ 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"
+ " \"name\": \"labelname\" (string) The label\n"
+ " \"purpose\": \"string\" (string) Purpose of address (\"send\" for sending address, \"receive\" for receiving address)\n"
@jnewbery

jnewbery Jun 14, 2017

Member

If I'm understanding AddressBookDataToJSON() correctly, then the result will also include an array of destdata.

+ " },...\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);
+
+ CBitcoinAddress address(request.params[0].get_str());
+ if (!address.IsValid()) {
+ throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid Bitcoin address");
+ }
+
+ std::map<CTxDestination, CAddressBookData>::iterator mi = pwallet->mapAddressBook.find(address.Get());
+ 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 = AccountFromValue(request.params[0]);
+
+ // Find all addresses that have the given label
+ UniValue ret(UniValue::VOBJ);
+ for (const std::pair<CBitcoinAddress, CAddressBookData>& item : pwallet->mapAddressBook) {
+ if (item.second.name == strLabel) {
+ ret.push_back(Pair(item.first.ToString(), AddressBookDataToJSON(item.second, false)));
+ }
+ }
+ 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"
@jnewbery

jnewbery Jun 14, 2017

Member

nit: I don't think An empty string is the same as not providing this argument. is required

@promag

promag Jul 7, 2017

Contributor

Agree with @jnewbery, empty value should be an error then.

@promag

promag Jul 7, 2017

Contributor

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) {
+ ret.push_back(name);
+ }
+
+ return ret;
+}
+
+UniValue setlabel(const JSONRPCRequest& request)
+{
+ CWallet * const pwallet = GetWalletForJSONRPCRequest(request);
+ if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) {
+ return NullUniValue;
+ }
+
+ if (request.fHelp || request.params.size() < 1 || request.params.size() > 2)
+ throw std::runtime_error(
+ "setlabel \"bitcoinaddress\" \"label\"\n"
+ "\nSets the label associated with the given address.\n"
+ "\nArguments:\n"
+ "1. \"bitcoinaddress\" (string, required) The bitcoin address to be associated with an label.\n"
+ "2. \"label\" (string, required) The label to assign to the address.\n"
@JeremyRubin

JeremyRubin Mar 28, 2017

Contributor

Is the label optional or required?

If it's required update L2675 to if (fHelp || params.size() != 2) and L2693-2695 to std::string strLabel = AccountFromValue(params[1]);, if optional update the docstring.

+ "\nExamples:\n"
+ + HelpExampleCli("setlabel", "\"1D1ZrZNe3JUo7ZycKEYQQiQAWd9y54F4XZ\" \"tabby\"")
+ + HelpExampleRpc("setlabel", "\"1D1ZrZNe3JUo7ZycKEYQQiQAWd9y54F4XZ\", \"tabby\"")
+ );
+
+ LOCK2(cs_main, pwallet->cs_wallet);
+
+ CBitcoinAddress address(request.params[0].get_str());
+ if (!address.IsValid()) {
+ throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid Bitcoin address");
+ }
+
+ std::string strLabel;
+ if (request.params.size() > 1){
@jnewbery

jnewbery Jun 14, 2017

Member

What happens if request.params.size() == 1? Are you supposed to be able to remove a label using this rpc without a second argument? If so, I think you want to call pwallet->DelAddressBook.

+ strLabel = AccountFromValue(request.params[1]);
+ }
+
+ if (IsMine(*pwallet, address.Get()))
+ {
+ // Detect when changing the label of an address that is the 'label address' of another label:
@jnewbery

jnewbery Jun 14, 2017

Member

I think we should remove the concept of 'label address' and change this section to delete the label when removing the final address from that label.

+ // If so, delete the account record for it. Labels, unlike addresses can be deleted,
+ // and we wouldn't do this, the record would stick around forever.
+ if (pwallet->mapAddressBook.count(address.Get()))
+ {
+ std::string strOldLabel = pwallet->mapAddressBook[address.Get()].name;
+ if (strOldLabel != strLabel && address == GetAccountAddress(pwallet, strOldLabel)) {
+ DeleteAccount(pwallet, strOldLabel);
+ }
+ }
+ pwallet->SetAddressBook(address.Get(), strLabel, "receive");
+ } else {
+ pwallet->SetAddressBook(address.Get(), strLabel, "send");
+ }
+
+ return NullUniValue;
+}
+
extern UniValue abortrescan(const JSONRPCRequest& request); // in rpcdump.cpp
extern UniValue dumpprivkey(const JSONRPCRequest& request); // in rpcdump.cpp
extern UniValue importprivkey(const JSONRPCRequest& request);
@@ -2947,13 +3182,9 @@ static const CRPCCommand commands[] =
{ "wallet", "dumpprivkey", &dumpprivkey, true, {"address"} },
{ "wallet", "dumpwallet", &dumpwallet, true, {"filename"} },
{ "wallet", "encryptwallet", &encryptwallet, true, {"passphrase"} },
- { "wallet", "getaccountaddress", &getaccountaddress, true, {"account"} },
- { "wallet", "getaccount", &getaccount, true, {"address"} },
- { "wallet", "getaddressesbyaccount", &getaddressesbyaccount, true, {"account"} },
{ "wallet", "getbalance", &getbalance, false, {"account","minconf","include_watchonly"} },
{ "wallet", "getnewaddress", &getnewaddress, true, {"account"} },
{ "wallet", "getrawchangeaddress", &getrawchangeaddress, true, {} },
- { "wallet", "getreceivedbyaccount", &getreceivedbyaccount, false, {"account","minconf"} },
{ "wallet", "getreceivedbyaddress", &getreceivedbyaddress, false, {"address","minconf"} },
{ "wallet", "gettransaction", &gettransaction, false, {"txid","include_watchonly"} },
{ "wallet", "getunconfirmedbalance", &getunconfirmedbalance, false, {} },
@@ -2965,26 +3196,39 @@ static const CRPCCommand commands[] =
{ "wallet", "importprunedfunds", &importprunedfunds, true, {"rawtransaction","txoutproof"} },
{ "wallet", "importpubkey", &importpubkey, true, {"pubkey","label","rescan"} },
{ "wallet", "keypoolrefill", &keypoolrefill, true, {"newsize"} },
- { "wallet", "listaccounts", &listaccounts, false, {"minconf","include_watchonly"} },
{ "wallet", "listaddressgroupings", &listaddressgroupings, false, {} },
{ "wallet", "listlockunspent", &listlockunspent, false, {} },
- { "wallet", "listreceivedbyaccount", &listreceivedbyaccount, false, {"minconf","include_empty","include_watchonly"} },
{ "wallet", "listreceivedbyaddress", &listreceivedbyaddress, false, {"minconf","include_empty","include_watchonly"} },
{ "wallet", "listsinceblock", &listsinceblock, false, {"blockhash","target_confirmations","include_watchonly"} },
{ "wallet", "listtransactions", &listtransactions, false, {"account","count","skip","include_watchonly"} },
{ "wallet", "listunspent", &listunspent, false, {"minconf","maxconf","addresses","include_unsafe","query_options"} },
{ "wallet", "lockunspent", &lockunspent, true, {"unlock","transactions"} },
- { "wallet", "move", &movecmd, false, {"fromaccount","toaccount","amount","minconf","comment"} },
{ "wallet", "sendfrom", &sendfrom, false, {"fromaccount","toaddress","amount","minconf","comment","comment_to"} },
{ "wallet", "sendmany", &sendmany, false, {"fromaccount","amounts","minconf","comment","subtractfeefrom"} },
{ "wallet", "sendtoaddress", &sendtoaddress, false, {"address","amount","comment","comment_to","subtractfeefromamount"} },
- { "wallet", "setaccount", &setaccount, true, {"address","account"} },
{ "wallet", "settxfee", &settxfee, true, {"amount"} },
{ "wallet", "signmessage", &signmessage, true, {"address","message"} },
{ "wallet", "walletlock", &walletlock, true, {} },
{ "wallet", "walletpassphrasechange", &walletpassphrasechange, true, {"oldpassphrase","newpassphrase"} },
{ "wallet", "walletpassphrase", &walletpassphrase, true, {"passphrase","timeout"} },
{ "wallet", "removeprunedfunds", &removeprunedfunds, true, {"txid"} },
+
+ /** Account functions (deprecated) */
+ { "wallet", "getaccountaddress", &getaccountaddress, true, {"account"} },
+ { "wallet", "getaccount", &getaccount, true, {"address"} },
+ { "wallet", "getaddressesbyaccount", &getaddressesbyaccount, true, {"account"} },
+ { "wallet", "getreceivedbyaccount", &getreceivedbyaccount, false, {"account","minconf"} },
+ { "wallet", "listaccounts", &listaccounts, false, {"minconf","include_watchonly"} },
+ { "wallet", "listreceivedbyaccount", &listreceivedbyaccount, false, {"minconf","include_empty","include_watchonly"} },
+ { "wallet", "setaccount", &setaccount, true, {"address","account"} },
+ { "wallet", "move", &movecmd, false, {"fromaccount","toaccount","amount","minconf","comment"} },
+
+ /** Label functions (to replace non-balance account functions) */
+ { "wallet", "getlabeladdress", &getlabeladdress, true, {"label"} },
+ { "wallet", "getlabel", &getlabel, true, {"bitcoinaddress"} },
+ { "wallet", "getaddressesbylabel", &getaddressesbylabel, true, {"label"} },
+ { "wallet", "listlabels", &listlabels, false, {"purpose"} },
+ { "wallet", "setlabel", &setlabel, true, {"bitcoinaddress","label"} },
};
void RegisterWalletRPCCommands(CRPCTable &t)
View
@@ -561,7 +561,7 @@ class CWalletKey
};
/**
- * Internal transfers.
+ * DEPRECATED Internal transfers.
* Database key is acentry<account><counter>.
*/
class CAccountingEntry
@@ -1158,7 +1158,7 @@ class CReserveKey : public CReserveScript
/**
- * Account information.
+ * DEPRECATED Account information.
* Stored in wallet with key "acc"+string account name.
*/
class CAccount
View
@@ -167,6 +167,11 @@ bool CWalletDB::WriteAccount(const std::string& strAccount, const CAccount& acco
return WriteIC(std::make_pair(std::string("acc"), strAccount), account);
}
+bool CWalletDB::EraseAccount(const std::string& strAccount)
+{
+ return EraseIC(std::make_pair(std::string("acc"), strAccount));
+}
+
bool CWalletDB::WriteAccountingEntry(const uint64_t nAccEntryNum, const CAccountingEntry& acentry)
{
return WriteIC(std::make_pair(std::string("acentry"), std::make_pair(acentry.strAccount, nAccEntryNum)), acentry);
View
@@ -204,6 +204,7 @@ class CWalletDB
bool WriteAccountingEntry(const uint64_t nAccEntryNum, const CAccountingEntry& acentry);
bool ReadAccount(const std::string& strAccount, CAccount& account);
bool WriteAccount(const std::string& strAccount, const CAccount& account);
+ bool EraseAccount(const std::string& strAccount);
@jnewbery

jnewbery Jun 14, 2017

Member

I think the comment above may need updating to:

  • remove comments about accounting entries (or say they're deprecated)
  • comment that 'acc' database entries are for labels.
/// Write destination data key,value tuple to database
bool WriteDestData(const std::string &address, const std::string &key, const std::string &value);
@@ -114,6 +114,7 @@
'p2p-leaktests.py',
'wallet-encryption.py',
'uptime.py',
+ 'wallet-labels.py',
]
EXTENDED_SCRIPTS = [
Oops, something went wrong.