diff --git a/doc/release-notes.md b/doc/release-notes.md --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -11,3 +11,7 @@ as such, is subject to changes or removal in future releases. - The new RPC `getzmqnotifications` returns information about active ZMQ notifications. + - `getlabeladdress` has been removed and replaced with `getaccountaddress` + until v0.21 at which time `getaccountaddress` will also be removed. To + use `getaccountaddress` start `bitcoind` with the + `-deprecatedrpc=accounts` option. diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -50,7 +50,6 @@ {"listreceivedbylabel", 0, "minconf"}, {"listreceivedbylabel", 1, "include_empty"}, {"listreceivedbylabel", 2, "include_watchonly"}, - {"getlabeladdress", 1, "force"}, {"getbalance", 1, "minconf"}, {"getbalance", 2, "include_watchonly"}, {"getblockhash", 0, "height"}, diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -234,8 +234,8 @@ return dest; } -static UniValue getlabeladdress(const Config &config, - const JSONRPCRequest &request) { +static UniValue getaccountaddress(const Config &config, + const JSONRPCRequest &request) { std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); CWallet *const pwallet = wallet.get(); @@ -243,8 +243,7 @@ return NullUniValue; } - if (!IsDeprecatedRPCEnabled(gArgs, "accounts") && - request.strMethod == "getaccountaddress") { + if (!IsDeprecatedRPCEnabled(gArgs, "accounts")) { if (request.fHelp) { throw std::runtime_error( "getaccountaddress (Deprecated, will be removed in v0.21. To " @@ -257,57 +256,33 @@ "use this command, start bitcoind with -deprecatedrpc=accounts."); } - if (request.fHelp || request.params.size() < 1 || - request.params.size() > 2) { + if (request.fHelp || request.params.size() != 1) { throw std::runtime_error( - "getlabeladdress \"label\" ( force ) \n" - "\nReturns the default receiving address for this label. This will " - "reset to a fresh address once there's a transaction that spends " - "to it.\n" + "getaccountaddress \"account\"\n" + "\n\nDEPRECATED. Returns the current Bitcoin address for receiving" + "payments to this account.\n" "\nArguments:\n" - "1. \"label\" (string, required) The label for the " + "1. \"account\" (string, required) The account for the " "address. It can also be set to the empty string \"\" to represent " - "the default label.\n" - "2. \"force\" (bool, optional) Whether the label should be " - "created if it does not yet exist. If False, the RPC will return " - "an error if called with a label that doesn't exist.\n" - " Defaults to false (unless the " - "getaccountaddress method alias is being called, in which case " - "defaults to true for backwards compatibility).\n" + "the default account. The account does not need to exist, it will " + "be created and a new address created if there is no account by " + "the given name.\n" "\nResult:\n" - "\"address\" (string) The current receiving address for " - "the label.\n" + "\"address\" (string) The account bitcoin address\n" "\nExamples:\n" + - HelpExampleCli("getlabeladdress", "") + - HelpExampleCli("getlabeladdress", "\"\"") + - HelpExampleCli("getlabeladdress", "\"mylabel\"") + - HelpExampleRpc("getlabeladdress", "\"mylabel\"")); + HelpExampleCli("getaccountaddress", "") + + HelpExampleCli("getaccountaddress", "\"\"") + + HelpExampleCli("getaccountaddress", "\"myaccount\"") + + HelpExampleRpc("getaccountaddress", "\"myaccount\"")); } LOCK2(cs_main, pwallet->cs_wallet); - // Parse the label first so we don't generate a key if there's an error - std::string label = LabelFromValue(request.params[0]); - bool force = request.strMethod == "getaccountaddress"; - if (!request.params[1].isNull()) { - force = request.params[1].get_bool(); - } - - bool label_found = false; - for (const std::pair &item : - pwallet->mapAddressBook) { - if (item.second.name == label) { - label_found = true; - break; - } - } - if (!force && !label_found) { - throw JSONRPCError(RPC_WALLET_INVALID_LABEL_NAME, - std::string("No addresses with label " + label)); - } + // Parse the account first so we don't generate a key if there's an error + std::string account = LabelFromValue(request.params[0]); UniValue ret(UniValue::VSTR); - ret = EncodeDestination(GetLabelDestination(pwallet, label), config); + ret = EncodeDestination(GetLabelDestination(pwallet, account), config); return ret; } @@ -417,26 +392,37 @@ "Invalid Bitcoin address"); } + std::string old_label = pwallet->mapAddressBook[dest].name; std::string label = LabelFromValue(request.params[1]); if (IsMine(*pwallet, dest)) { - // Detect when changing the label of an address that is the receiving - // 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)) { - pwallet->DeleteLabel(old_label); - } - } - pwallet->SetAddressBook(dest, label, "receive"); + if (request.strMethod == "setaccount" && old_label != label && + dest == GetLabelDestination(pwallet, old_label)) { + // for setaccount, call GetLabelDestination so a new receive address + // is created for the old account + GetLabelDestination(pwallet, old_label, true); + } } else { pwallet->SetAddressBook(dest, label, "send"); } + // Detect when there are no addresses using this 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. + bool found_address = false; + for (const std::pair &item : + pwallet->mapAddressBook) { + if (item.second.name == label) { + found_address = true; + break; + } + } + if (!found_address) { + pwallet->DeleteLabel(old_label); + } + return NullUniValue; } @@ -5424,7 +5410,7 @@ { "wallet", "walletpassphrase", walletpassphrase, {"passphrase","timeout"} }, /** Account functions (deprecated) */ - { "wallet", "getaccountaddress", getlabeladdress, {"account"} }, + { "wallet", "getaccountaddress", getaccountaddress, {"account"} }, { "wallet", "getaccount", getaccount, {"address"} }, { "wallet", "getaddressesbyaccount", getaddressesbyaccount, {"account"} }, { "wallet", "getreceivedbyaccount", getreceivedbylabel, {"account","minconf"} }, @@ -5434,7 +5420,6 @@ { "wallet", "move", movecmd, {"fromaccount","toaccount","amount","minconf","comment"} }, /** Label functions (to replace non-balance account functions) */ - { "wallet", "getlabeladdress", getlabeladdress, {"label","force"} }, { "wallet", "getaddressesbylabel", getaddressesbylabel, {"label"} }, { "wallet", "getreceivedbylabel", getreceivedbylabel, {"label","minconf"} }, { "wallet", "listlabels", listlabels, {"purpose"} }, diff --git a/test/functional/rpc_deprecated.py b/test/functional/rpc_deprecated.py --- a/test/functional/rpc_deprecated.py +++ b/test/functional/rpc_deprecated.py @@ -35,7 +35,6 @@ # # The following 'label' RPC methods are usable both with and without the # -deprecatedrpc=accounts switch enabled. - # - getlabeladdress # - getaddressesbylabel # - getreceivedbylabel # - listlabels @@ -66,10 +65,6 @@ self.nodes[0].getaccountaddress, "label0") self.nodes[1].getaccountaddress("label1") - self.log.info("- getlabeladdress") - self.nodes[0].getlabeladdress("label0") - self.nodes[1].getlabeladdress("label1") - self.log.info("- getaddressesbyaccount") assert_raises_rpc_error(-32, "getaddressesbyaccount is deprecated", self.nodes[0].getaddressesbyaccount, "label0") diff --git a/test/functional/wallet_labels.py b/test/functional/wallet_labels.py --- a/test/functional/wallet_labels.py +++ b/test/functional/wallet_labels.py @@ -5,7 +5,7 @@ """Test label RPCs. RPCs tested are: - - getlabeladdress + - getaccountaddress - getaddressesbyaccount/getaddressesbylabel - listaddressgroupings - setlabel @@ -90,18 +90,24 @@ labels = [Label(name, accounts_api) for name in ("a", "b", "c", "d", "e")] for label in labels: - label.add_receive_address( - node.getlabeladdress(label=label.name, force=True)) + if accounts_api: + address = node.getaccountaddress(label.name) + else: + address = node.getnewaddress(label.name) + label.add_receive_address(address) label.verify(node) # Check all labels are returned by listlabels. assert_equal(node.listlabels(), [label.name for label in labels]) # Send a transaction to each label, and make sure this forces - # getlabeladdress to generate a new receiving address. + # getaccountaddress to generate a new receiving address. for label in labels: - node.sendtoaddress(label.receive_address, amount_to_send) - label.add_receive_address(node.getlabeladdress(label.name)) + if accounts_api: + node.sendtoaddress(label.receive_address, amount_to_send) + label.add_receive_address(node.getaccountaddress(label.name)) + else: + node.sendtoaddress(label.addresses[0], amount_to_send) label.verify(node) # Check the amounts received. @@ -114,10 +120,19 @@ # Check that sendfrom label reduces listaccounts balances. for i, label in enumerate(labels): to_label = labels[(i + 1) % len(labels)] - node.sendfrom(label.name, to_label.receive_address, amount_to_send) + if accounts_api: + node.sendfrom( + label.name, to_label.receive_address, amount_to_send) + else: + node.sendfrom( + label.name, to_label.addresses[0], amount_to_send) node.generate(1) for label in labels: - label.add_receive_address(node.getlabeladdress(label.name)) + if accounts_api: + address = node.getaccountaddress(label.name) + else: + address = node.getnewaddress(label.name) + label.add_receive_address(address) label.verify(node) assert_equal(node.getreceivedbylabel(label.name), 2) if accounts_api: @@ -133,12 +148,12 @@ # Check that setlabel can assign a label to a new unused address. for label in labels: - address = node.getlabeladdress(label="", force=True) + address = node.getnewaddress() node.setlabel(address, label.name) label.add_address(address) label.verify(node) if accounts_api: - assert(address not in node.getaddressesbyaccount("")) + assert address not in node.getaddressesbyaccount("") else: assert_raises_rpc_error(-11, "No addresses with label", node.getaddressesbylabel, "") @@ -161,19 +176,24 @@ # Check that setlabel can change the label of an address from a # different label. - change_label(node, labels[0].addresses[0], labels[0], labels[1]) - - # Check that setlabel can change the label of an address which - # is the receiving address of a different label. - change_label(node, labels[0].receive_address, labels[0], labels[1]) + change_label(node, labels[0].addresses[0], + labels[0], labels[1], accounts_api) # Check that setlabel can set the label of an address already # in the label. This is a no-op. - change_label(node, labels[2].addresses[0], labels[2], labels[2]) + change_label(node, labels[2].addresses[0], + labels[2], labels[2], accounts_api) + + if accounts_api: + # Check that setaccount can change the label of an address which + # is the receiving address of a different label. + change_label(node, labels[0].receive_address, + labels[0], labels[1], accounts_api) - # Check that setlabel can set the label of an address which is - # already the receiving address of the label. This is a no-op. - change_label(node, labels[2].receive_address, labels[2], labels[2]) + # Check that setaccount can set the label of an address which is + # already the receiving address of the label. This is a no-op. + change_label(node, labels[2].receive_address, + labels[2], labels[2], accounts_api) class Label: @@ -194,12 +214,15 @@ def add_receive_address(self, address): self.add_address(address) - self.receive_address = address + if self.accounts_api: + self.receive_address = address def verify(self, node): if self.receive_address is not None: assert self.receive_address in self.addresses - assert_equal(node.getlabeladdress(self.name), self.receive_address) + if self.accounts_api: + assert_equal(node.getaccountaddress( + self.name), self.receive_address) for address in self.addresses: assert_equal( @@ -219,22 +242,26 @@ self.name)), set(self.addresses)) -def change_label(node, address, old_label, new_label): +def change_label(node, address, old_label, new_label, accounts_api): assert_equal(address in old_label.addresses, True) - node.setlabel(address, new_label.name) + if accounts_api: + node.setaccount(address, new_label.name) + else: + node.setlabel(address, new_label.name) old_label.addresses.remove(address) new_label.add_address(address) - # Calling setlabel on an address which was previously the receiving - # address of a different label should reset the receiving address of - # the old label, causing getlabeladdress to return a brand new + # Calling setaccount on an address which was previously the receiving + # address of a different account should reset the receiving address of + # the old account, causing getaccountaddress to return a brand new # address. - if old_label.name != new_label.name and address == old_label.receive_address: - new_address = node.getlabeladdress(old_label.name) - assert_equal(new_address not in old_label.addresses, True) - assert_equal(new_address not in new_label.addresses, True) - old_label.add_receive_address(new_address) + if accounts_api: + if old_label.name != new_label.name and address == old_label.receive_address: + new_address = node.getaccountaddress(old_label.name) + assert_equal(new_address not in old_label.addresses, True) + assert_equal(new_address not in new_label.addresses, True) + old_label.add_receive_address(new_address) old_label.verify(node) new_label.verify(node)