backport: bitcoin#12892 - [wallet] [rpc] introduce 'label' API for wa…

…llet

Add label API to wallet RPC.

This is one step towards dashpay#3816 ("Remove bolt-on account system") although it doesn't
actually remove anything yet.

These initially mirror the account functions, with the following differences:

- These functions aren't DEPRECATED in the help
- Help mentions 'label' instead of accounts. In the language used, labels are
  associated with addresses, instead of addresses associated with labels. (unlike
  with accounts.)
- Labels have no balance
  - No balances in `listlabels`
  - `listlabels` has no minconf or watchonly argument
- Like in the GUI, labels can be set on any address, not just receiving addreses
- Unlike accounts, labels can be deleted.
  Being unable to delete them is a common annoyance (see dashpay#1231).
  Currently only by reassigning all addresses using `setlabel`, but an explicit
  call `deletelabel` which assigns all address to the default label may make
  sense.

Thanks to Pierre Rochard for test fixes.

src/rpc/client.cpp

@@ -60,6 +60,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
     { "listreceivedbylabel", 1, "addlocked" },
     { "listreceivedbylabel", 2, "include_empty" },
     { "listreceivedbylabel", 3, "include_watchonly" },
+    { "getlabeladdress", 1, "force" },
     { "getbalance", 1, "minconf" },
     { "getbalance", 2, "addlocked" },
     { "getbalance", 3, "include_watchonly" },

src/wallet/wallet.cpp

@@ -4753,6 +4753,12 @@ std::set<CTxDestination> CWallet::GetLabelAddresses(const std::string& label) co
     return result;
 }
 
+void CWallet::DeleteLabel(const std::string& label)
+{
+    WalletBatch batch(*database);
+    batch.EraseAccount(label);
+}
+
 bool CReserveKey::GetReservedKey(CPubKey& pubkey, bool fInternalIn)
 {
     if (nIndex == -1)

src/wallet/wallet.h

@@ -618,7 +618,7 @@ class CWalletKey
 };
 
 /**
- * Internal transfers.
+ * DEPRECATED Internal transfers.
  * Database key is acentry<account><counter>.
  */
 class CAccountingEntry
@@ -1104,6 +1104,7 @@ class CWallet final : public CCryptoKeyStore, public CValidationInterface
     std::map<CTxDestination, CAmount> GetAddressBalances();
 
     std::set<CTxDestination> GetLabelAddresses(const std::string& label) const;
+    void DeleteLabel(const std::string& label);
 
     isminetype IsMine(const CTxIn& txin) const;
     /**
@@ -1288,7 +1289,7 @@ class CReserveKey final : public CReserveScript
 
 
 /**
- * Account information.
+ * DEPRECATED Account information.
  * Stored in wallet with key "acc"+string account name.
  */
 class CAccount

src/wallet/walletdb.cpp

@@ -169,6 +169,11 @@ bool WalletBatch::WriteAccount(const std::string& strAccount, const CAccount& ac
     return WriteIC(std::make_pair(std::string("acc"), strAccount), account);
 }
 
+bool WalletBatch::EraseAccount(const std::string& strAccount)
+{
+    return EraseIC(std::make_pair(std::string("acc"), strAccount));
+}
+
 bool WalletBatch::WriteAccountingEntry(const uint64_t nAccEntryNum, const CAccountingEntry& acentry)
 {
     return WriteIC(std::make_pair(std::string("acentry"), std::make_pair(acentry.strAccount, nAccEntryNum)), acentry);

src/wallet/walletdb.h

@@ -162,6 +162,7 @@ class WalletBatch
     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);
 
     bool ReadPrivateSendSalt(uint256& salt);
     bool WritePrivateSendSalt(const uint256& salt);

test/functional/wallet_labels.py

@@ -12,6 +12,7 @@
     - sendfrom (with account arguments)
     - move (with account arguments)
 """
+from collections import defaultdict
 
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import assert_equal
@@ -80,9 +81,12 @@ def run_test(self):
         # recognize the label/address associations.
         labels = [Label(name) for name in ("a", "b", "c", "d", "e")]
         for label in labels:
-            label.add_receive_address(node.getlabeladdress(label.name))
+            label.add_receive_address(node.getlabeladdress(label=label.name, force=True))
             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.
         for label in labels:
@@ -117,7 +121,7 @@ def run_test(self):
 
         # Check that setlabel can assign a label to a new unused address.
         for label in labels:
-            address = node.getlabeladdress("")
+            address = node.getlabeladdress(label="", force=True)
             node.setlabel(address, label.name)
             label.add_address(address)
             label.verify(node)
@@ -130,6 +134,7 @@ def run_test(self):
                 addresses.append(node.getnewaddress())
             multisig_address = node.addmultisigaddress(5, addresses, label.name)['address']
             label.add_address(multisig_address)
+            label.purpose[multisig_address] = "send"
             label.verify(node)
             node.sendfrom("", multisig_address, 50)
         node.generate(101)
@@ -149,9 +154,7 @@ def run_test(self):
         change_label(node, labels[2].addresses[0], labels[2], labels[2])
 
         # Check that setlabel can set the label of an address which is
-        # already the receiving address of the label. It would probably make
-        # sense for this to be a no-op, but right now it resets the receiving
-        # address, causing getlabeladdress to return a brand new address.
+        # already the receiving address of the label. This is a no-op.
         change_label(node, labels[2].receive_address, labels[2], labels[2])
 
 class Label:
@@ -162,6 +165,8 @@ def __init__(self, name):
         self.receive_address = None
         # List of all addresses assigned with this label
         self.addresses = []
+        # Map of address to address purpose
+        self.purpose = defaultdict(lambda: "receive")
 
     def add_address(self, address):
         assert_equal(address not in self.addresses, True)
@@ -177,8 +182,15 @@ def verify(self, node):
             assert_equal(node.getlabeladdress(self.name), self.receive_address)
 
         for address in self.addresses:
+            assert_equal(
+                node.getaddressinfo(address)['labels'][0],
+                {"name": self.name,
+                 "purpose": self.purpose[address]})
             assert_equal(node.getaccount(address), self.name)
 
+        assert_equal(
+            node.getaddressesbylabel(self.name),
+            {address: {"purpose": self.purpose[address]} for address in self.addresses})
         assert_equal(
             set(node.getaddressesbyaccount(self.name)), set(self.addresses))
 
@@ -194,7 +206,7 @@ def change_label(node, address, old_label, new_label):
     # address of a different label should reset the receiving address of
     # the old label, causing getlabeladdress to return a brand new
     # address.
-    if address == old_label.receive_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)

test/functional/wallet_listreceivedby.py

@@ -144,7 +144,7 @@ def run_test(self):
         assert_equal(balance, balance_by_label + Decimal("0.1"))
 
         # Create a new label named "mynewlabel" that has a 0 balance
-        self.nodes[1].getlabeladdress("mynewlabel")
+        self.nodes[1].getlabeladdress("mynewlabel", force=True)
         received_by_label_json = [r for r in self.nodes[1].listreceivedbylabel(0, False, True) if r["label"] == "mynewlabel"][0]
 
         # Test includeempty of listreceivedbylabel