Merge bitcoin#11552: Improve wallet-accounts test

bc9c0a7 Improve wallet-accounts test (Russell Yanofsky)

Pull request description:

  Add comments and

  - Verify sending to a account causes getaccountaddress to generate new addresses.
  - Verify sending to a account causes getreceivedbyaccount to return amount received.
  - Verify ways setaccount updates the accounts of existing addresses.

Tree-SHA512: 4facc8d10fb23847081e8081839d4050faa855cca40f34012b82297bf1a8cfdb975b26314bc518dc6b3f5997452a7acf309f9627e0dff359347c628b37014bfa

test/functional/wallet-accounts.py

@@ -74,62 +74,135 @@ def run_test(self):
         # otherwise we're off by exactly the fee amount as that's mined
         # and matures in the next 100 blocks
         node.sendfrom("", common_address, fee)
-        accounts = ["a", "b", "c", "d", "e"]
         amount_to_send = 1.0
-        account_addresses = dict()
+
+        # Create accounts and make sure subsequent account API calls
+        # recognize the account/address associations.
+        accounts = [Account(name) for name in ("a", "b", "c", "d", "e")]
         for account in accounts:
-            address = node.getaccountaddress(account)
-            account_addresses[account] = address
-
-            node.getnewaddress(account)
-            assert_equal(node.getaccount(address), account)
-            assert(address in node.getaddressesbyaccount(account))
-
-            node.sendfrom("", address, amount_to_send)
-
+            account.add_receive_address(node.getaccountaddress(account.name))
+            account.verify(node)
+
+        # Send a transaction to each account, and make sure this forces
+        # getaccountaddress to generate a new receiving address.
+        for account in accounts:
+            node.sendtoaddress(account.receive_address, amount_to_send)
+            account.add_receive_address(node.getaccountaddress(account.name))
+            account.verify(node)
+
+        # Check the amounts received.
         node.generate(1)
-
-        for i in range(len(accounts)):
-            from_account = accounts[i]
+        for account in accounts:
+            assert_equal(
+                node.getreceivedbyaddress(account.addresses[0]), amount_to_send)
+            assert_equal(node.getreceivedbyaccount(account.name), amount_to_send)
+
+        # Check that sendfrom account reduces listaccounts balances.
+        for i, account in enumerate(accounts):
             to_account = accounts[(i+1) % len(accounts)]
-            to_address = account_addresses[to_account]
-            node.sendfrom(from_account, to_address, amount_to_send)
-
+            node.sendfrom(account.name, to_account.receive_address, amount_to_send)
         node.generate(1)
-
         for account in accounts:
-            address = node.getaccountaddress(account)
-            assert(address != account_addresses[account])
-            assert_equal(node.getreceivedbyaccount(account), 2)
-            node.move(account, "", node.getbalance(account))
-
+            account.add_receive_address(node.getaccountaddress(account.name))
+            account.verify(node)
+            assert_equal(node.getreceivedbyaccount(account.name), 2)
+            node.move(account.name, "", node.getbalance(account.name))
+            account.verify(node)
         node.generate(101)
-
         expected_account_balances = {"": 52000}
         for account in accounts:
-            expected_account_balances[account] = 0
-
+            expected_account_balances[account.name] = 0
         assert_equal(node.listaccounts(), expected_account_balances)
-
         assert_equal(node.getbalance(""), 52000)
 
+        # Check that setaccount can assign an account to a new unused address.
         for account in accounts:
             address = node.getaccountaddress("")
-            node.setaccount(address, account)
-            assert(address in node.getaddressesbyaccount(account))
+            node.setaccount(address, account.name)
+            account.add_address(address)
+            account.verify(node)
             assert(address not in node.getaddressesbyaccount(""))
 
+        # Check that addmultisigaddress can assign accounts.
         for account in accounts:
             addresses = []
             for x in range(10):
                 addresses.append(node.getnewaddress())
-            multisig_address = node.addmultisigaddress(5, addresses, account)
+            multisig_address = node.addmultisigaddress(5, addresses, account.name)
+            account.add_address(multisig_address)
+            account.verify(node)
             node.sendfrom("", multisig_address, 50)
-
         node.generate(101)
-
         for account in accounts:
-            assert_equal(node.getbalance(account), 50)
+            assert_equal(node.getbalance(account.name), 50)
+
+        # Check that setaccount can change the account of an address from a
+        # different account.
+        change_account(node, accounts[0].addresses[0], accounts[0], accounts[1])
+
+        # Check that setaccount can change the account of an address which
+        # is the receiving address of a different account.
+        change_account(node, accounts[0].receive_address, accounts[0], accounts[1])
+
+        # Check that setaccount can set the account of an address already
+        # in the account. This is a no-op.
+        change_account(node, accounts[2].addresses[0], accounts[2], accounts[2])
+
+        # Check that setaccount can set the account of an address which is
+        # already the receiving address of the account. It would probably make
+        # sense for this to be a no-op, but right now it resets the receiving
+        # address, causing getaccountaddress to return a brand new address.
+        change_account(node, accounts[2].receive_address, accounts[2], accounts[2])
+
+class Account:
+    def __init__(self, name):
+        # Account name
+        self.name = name
+        # Current receiving address associated with this account.
+        self.receive_address = None
+        # List of all addresses assigned with this account
+        self.addresses = []
+
+    def add_address(self, address):
+        assert_equal(address not in self.addresses, True)
+        self.addresses.append(address)
+
+    def add_receive_address(self, address):
+        self.add_address(address)
+        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.getaccountaddress(self.name), self.receive_address)
+
+        for address in self.addresses:
+            assert_equal(node.getaccount(address), self.name)
+
+        assert_equal(
+            set(node.getaddressesbyaccount(self.name)), set(self.addresses))
+
+
+def change_account(node, address, old_account, new_account):
+    assert_equal(address in old_account.addresses, True)
+    node.setaccount(address, new_account.name)
+
+    old_account.addresses.remove(address)
+    new_account.add_address(address)
+
+    # 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 address == old_account.receive_address:
+        new_address = node.getaccountaddress(old_account.name)
+        assert_equal(new_address not in old_account.addresses, True)
+        assert_equal(new_address not in new_account.addresses, True)
+        old_account.add_receive_address(new_address)
+
+    old_account.verify(node)
+    new_account.verify(node)
+
 
 if __name__ == '__main__':
     WalletAccountsTest().main()