Merge #18451: test: shift coverage from getunconfirmedbalance to getbalances

0306d78cb49d1684cc96ba3512b582a1fdaf78cc Use getbalances in wallet_address_types tests (Jon Atack)
7eacdc5167c8db94df84e206db85817bc64e4921 Shift coverage from getunconfirmedbalance to getbalances in wallet_abandonconflict tests (Jon Atack)
3e6f7377f600e47e5e3d439fc5d6ccf3db210038 Improve getbalances coverage in wallet_balance tests (Jon Atack)

Pull request description:

  <strike>This PR updates several tests and then removes the `getunconfirmedbalance` RPC which was deprecated in facfb4111d14a3b06c46690a2cca7ca91cea8a96 a year ago.

  Next steps: remove the deprecated `getwalletinfo` fields and the `getbalance` RPC in follow-ups, if there seems to be consensus on those removals.</strike>

  Update:

  `getunconfirmedbalance` RPC was deprecated in facfb4111d14a3b06c46690a2cca7ca91cea8a96 a year ago, but following the review comments below, this PR now only updates the test coverage to use `getbalances` while still leaving basic coverage for `getunconfirmedbalance` in wallet_balance.py.

  That said, I've seen 3 regular contributors confused in the past 10 days by "DEPRECATED" warnings in the code that are not following the deprecation policy in [JSON-RPC-interface.md#versioning](https://github.com/bitcoin/bitcoin/blob/master/doc/JSON-RPC-interface.md#versioning).

  ISTM these warnings should either be removed, or the calls deprecated (`-deprecatedrpc`), or the policy updated to describe these warnings as a pre-deprecation practice.

ACKs for top commit:
  jnewbery:
    utACK 0306d78cb

Tree-SHA512: 692e43e9bed5afa97d905740666e365f0b64e559e1c75a6a398236d9e943894e3477947fc11324f420a6feaffa0c0c1532aa983c50090ca39d06551399e6ddd1

3 files changed, 30 insertions, 19 deletions

diff --git a/test/functional/wallet_abandonconflict.py b/test/functional/wallet_abandonconflict.py
index 1122daaf83..d63ab28d61 100755
--- a/test/functional/wallet_abandonconflict.py
+++ b/test/functional/wallet_abandonconflict.py
@@ -110,8 +110,8 @@ class AbandonConflictTest(BitcoinTestFramework):
         assert_equal(newbalance, balance - signed3_change)
         # Unconfirmed received funds that are not in mempool, also shouldn't show
         # up in unconfirmed balance
-        unconfbalance = self.nodes[0].getunconfirmedbalance() + self.nodes[0].getbalance()
-        assert_equal(unconfbalance, newbalance)
+        balances = self.nodes[0].getbalances()['mine']
+        assert_equal(balances['untrusted_pending'] + balances['trusted'], newbalance)
         # Also shouldn't show up in listunspent
         assert not txABC2 in [utxo["txid"] for utxo in self.nodes[0].listunspent(0)]
         balance = newbalance
diff --git a/test/functional/wallet_address_types.py b/test/functional/wallet_address_types.py
index 79b6db986b..065764dcac 100755
--- a/test/functional/wallet_address_types.py
+++ b/test/functional/wallet_address_types.py
@@ -97,12 +97,9 @@ class AddressTypeTest(BitcoinTestFramework):
                 connect_nodes(self.nodes[i], j)
         self.sync_all()
 
-    def get_balances(self, confirmed=True):
-        """Return a list of confirmed or unconfirmed balances."""
-        if confirmed:
-            return [self.nodes[i].getbalance() for i in range(4)]
-        else:
-            return [self.nodes[i].getunconfirmedbalance() for i in range(4)]
+    def get_balances(self, key='trusted'):
+        """Return a list of balances."""
+        return [self.nodes[i].getbalances()['mine'][key] for i in range(4)]
 
     # Quick test of python bech32 implementation
     def test_python_bech32(self, addr):
@@ -307,7 +304,7 @@ class AddressTypeTest(BitcoinTestFramework):
             self.nodes[from_node].sendmany("", sends)
             self.sync_mempools()
 
-            unconf_balances = self.get_balances(False)
+            unconf_balances = self.get_balances('untrusted_pending')
             self.log.debug("Check unconfirmed balances: {}".format(unconf_balances))
             assert_equal(unconf_balances[from_node], 0)
             for n, to_node in enumerate(range(from_node + 1, from_node + 4)):
diff --git a/test/functional/wallet_balance.py b/test/functional/wallet_balance.py
index a5f9a047ed..58eb34e66c 100755
--- a/test/functional/wallet_balance.py
+++ b/test/functional/wallet_balance.py
@@ -107,7 +107,7 @@ class WalletTest(BitcoinTestFramework):
         # First argument of getbalance must be set to "*"
         assert_raises_rpc_error(-32, "dummy first argument must be excluded or set to \"*\"", self.nodes[1].getbalance, "")
 
-        self.log.info("Test getbalance and getunconfirmedbalance with unconfirmed inputs")
+        self.log.info("Test balances with unconfirmed inputs")
 
         # Before `test_balance()`, we have had two nodes with a balance of 50
         # each and then we:
@@ -148,6 +148,18 @@ class WalletTest(BitcoinTestFramework):
 
 
         def test_balances(*, fee_node_1=0):
+            # getbalances
+            expected_balances_0 = {'mine':      {'immature':          Decimal('0E-8'),
+                                                 'trusted':           Decimal('9.99'),  # change from node 0's send
+                                                 'untrusted_pending': Decimal('60.0')},
+                                   'watchonly': {'immature':          Decimal('5000'),
+                                                 'trusted':           Decimal('50.0'),
+                                                 'untrusted_pending': Decimal('0E-8')}}
+            expected_balances_1 = {'mine':      {'immature':          Decimal('0E-8'),
+                                                 'trusted':           Decimal('0E-8'),  # node 1's send had an unsafe input
+                                                 'untrusted_pending': Decimal('30.0') - fee_node_1}}  # Doesn't include output of node 0's send since it was spent
+            assert_equal(self.nodes[0].getbalances(), expected_balances_0)
+            assert_equal(self.nodes[1].getbalances(), expected_balances_1)
             # getbalance without any arguments includes unconfirmed transactions, but not untrusted transactions
             assert_equal(self.nodes[0].getbalance(), Decimal('9.99'))  # change from node 0's send
             assert_equal(self.nodes[1].getbalance(), Decimal('0'))  # node 1's send had an unsafe input
@@ -160,11 +172,9 @@ class WalletTest(BitcoinTestFramework):
             assert_equal(self.nodes[1].getbalance(minconf=1), Decimal('0'))
             # getunconfirmedbalance
             assert_equal(self.nodes[0].getunconfirmedbalance(), Decimal('60'))  # output of node 1's spend
-            assert_equal(self.nodes[0].getbalances()['mine']['untrusted_pending'], Decimal('60'))
-            assert_equal(self.nodes[0].getwalletinfo()["unconfirmed_balance"], Decimal('60'))
-
             assert_equal(self.nodes[1].getunconfirmedbalance(), Decimal('30') - fee_node_1)  # Doesn't include output of node 0's send since it was spent
-            assert_equal(self.nodes[1].getbalances()['mine']['untrusted_pending'], Decimal('30') - fee_node_1)
+            # getwalletinfo.unconfirmed_balance
+            assert_equal(self.nodes[0].getwalletinfo()["unconfirmed_balance"], Decimal('60'))
             assert_equal(self.nodes[1].getwalletinfo()["unconfirmed_balance"], Decimal('30') - fee_node_1)
 
         test_balances(fee_node_1=Decimal('0.01'))
@@ -174,15 +184,19 @@ class WalletTest(BitcoinTestFramework):
         self.nodes[0].sendrawtransaction(txs[1]['hex'])  # sending on both nodes is faster than waiting for propagation
         self.sync_all()
 
-        self.log.info("Test getbalance and getunconfirmedbalance with conflicted unconfirmed inputs")
+        self.log.info("Test getbalance and getbalances.mine.untrusted_pending with conflicted unconfirmed inputs")
         test_balances(fee_node_1=Decimal('0.02'))
 
         self.nodes[1].generatetoaddress(1, ADDRESS_WATCHONLY)
         self.sync_all()
 
         # balances are correct after the transactions are confirmed
-        assert_equal(self.nodes[0].getbalance(), Decimal('69.99'))  # node 1's send plus change from node 0's send
-        assert_equal(self.nodes[1].getbalance(), Decimal('29.98'))  # change from node 0's send
+        balance_node0 = Decimal('69.99')  # node 1's send plus change from node 0's send
+        balance_node1 = Decimal('29.98')  # change from node 0's send
+        assert_equal(self.nodes[0].getbalances()['mine']['trusted'], balance_node0)
+        assert_equal(self.nodes[1].getbalances()['mine']['trusted'], balance_node1)
+        assert_equal(self.nodes[0].getbalance(), balance_node0)
+        assert_equal(self.nodes[1].getbalance(), balance_node1)
 
         # Send total balance away from node 1
         txs = create_transactions(self.nodes[1], self.nodes[0].getnewaddress(), Decimal('29.97'), [Decimal('0.01')])
@@ -200,13 +214,13 @@ class WalletTest(BitcoinTestFramework):
 
         # check mempool transactions count for wallet unconfirmed balance after
         # dynamically loading the wallet.
-        before = self.nodes[1].getunconfirmedbalance()
+        before = self.nodes[1].getbalances()['mine']['untrusted_pending']
         dst = self.nodes[1].getnewaddress()
         self.nodes[1].unloadwallet('')
         self.nodes[0].sendtoaddress(dst, 0.1)
         self.sync_all()
         self.nodes[1].loadwallet('')
-        after = self.nodes[1].getunconfirmedbalance()
+        after = self.nodes[1].getbalances()['mine']['untrusted_pending']
         assert_equal(before + Decimal('0.1'), after)
 
         # Create 3 more wallet txs, where the last is not accepted to the