[wallet] [rpc] Remove getlabeladdress RPC

labels are associated with addresses (rather than addresses being
associated with labels, as was the case with accounts). The
getlabeladdress does not make sense in this model, so remove it.

getaccountaddress is still supported for one release as the accounts
API is deprecated.

Author: jnewbery

src/rpc/client.cpp

@@ -52,7 +52,6 @@ static const CRPCConvertParam vRPCConvertParams[] =
     { "listreceivedbylabel", 0, "minconf" },
     { "listreceivedbylabel", 1, "include_empty" },
     { "listreceivedbylabel", 2, "include_watchonly" },
-    { "getlabeladdress", 1, "force" },
     { "getbalance", 1, "minconf" },
     { "getbalance", 2, "include_watchonly" },
     { "getblockhash", 0, "height" },

src/wallet/rpcwallet.cpp

@@ -196,60 +196,43 @@ CTxDestination GetLabelDestination(CWallet* const pwallet, const std::string& la
     return dest;
 }
 
-static UniValue getlabeladdress(const JSONRPCRequest& request)
+static UniValue getaccountaddress(const JSONRPCRequest& request)
 {
     CWallet * const pwallet = GetWalletForJSONRPCRequest(request);
     if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) {
         return NullUniValue;
     }
 
-    if (!IsDeprecatedRPCEnabled("accounts") && request.strMethod == "getaccountaddress") {
+    if (!IsDeprecatedRPCEnabled("accounts")) {
         if (request.fHelp) {
             throw std::runtime_error("getaccountaddress (Deprecated, will be removed in V0.18. To use this command, start bitcoind with -deprecatedrpc=accounts)");
         }
         throw JSONRPCError(RPC_METHOD_DEPRECATED, "getaccountaddress is deprecated and will be removed in V0.18. To 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 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"
+            "1. \"account\"       (string, required) The account for the address. It can also be set to the empty string \"\" to represent 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<CTxDestination, CAddressBookData>& 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));
+    ret = EncodeDestination(GetLabelDestination(pwallet, account));
     return ret;
 }
 
@@ -335,23 +318,33 @@ static UniValue setlabel(const JSONRPCRequest& request)
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "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<CTxDestination, CAddressBookData>& item : pwallet->mapAddressBook) {
+        if (item.second.name == label) {
+            found_address = true;
+            break;
+        }
+    }
+    if (!found_address) {
+        pwallet->DeleteLabel(old_label);
+    }
+
     return NullUniValue;
 }
 
@@ -4260,7 +4253,7 @@ static const CRPCCommand commands[] =
     { "wallet",             "sethdseed",                        &sethdseed,                     {"newkeypool","seed"} },
 
     /** Account functions (deprecated) */
-    { "wallet",             "getaccountaddress",                &getlabeladdress,               {"account"} },
+    { "wallet",             "getaccountaddress",                &getaccountaddress,             {"account"} },
     { "wallet",             "getaccount",                       &getaccount,                    {"address"} },
     { "wallet",             "getaddressesbyaccount",            &getaddressesbyaccount,         {"account"} },
     { "wallet",             "getreceivedbyaccount",             &getreceivedbylabel,            {"account","minconf"} },
@@ -4270,7 +4263,6 @@ static const CRPCCommand commands[] =
     { "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"} },

test/functional/rpc_deprecated.py

@@ -40,7 +40,6 @@ def run_test(self):
         #
         # The following 'label' RPC methods are usable both with and without the
         # -deprecatedrpc=accounts switch enabled.
-        # - getlabeladdress
         # - getaddressesbylabel
         # - getreceivedbylabel
         # - listlabels
@@ -69,10 +68,6 @@ def run_test(self):
         assert_raises_rpc_error(-32, "getaccountaddress is deprecated", 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")
         self.nodes[1].getaddressesbyaccount("label1")

test/functional/wallet_labels.py

@@ -5,7 +5,7 @@
 """Test label RPCs.
 
 RPCs tested are:
-    - getlabeladdress
+    - getaccountaddress
     - getaddressesbyaccount/getaddressesbylabel
     - listaddressgroupings
     - setlabel
@@ -92,17 +92,24 @@ def _run_subtest(self, accounts_api, node):
         # recognize the label/address associations.
         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.
@@ -115,10 +122,17 @@ def _run_subtest(self, accounts_api, node):
         # 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:
@@ -134,12 +148,12 @@ def _run_subtest(self, accounts_api, node):
 
         # 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, "")
 
@@ -160,19 +174,20 @@ def _run_subtest(self, accounts_api, node):
 
         # 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:
     def __init__(self, name, accounts_api):
@@ -192,12 +207,14 @@ def add_address(self, address):
 
     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(
@@ -216,22 +233,26 @@ def verify(self, node):
             assert_equal(set(node.getaddressesbyaccount(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)