Merge bitcoin/bitcoin#32721: wallet, rpc: Remove deprecated balances from getwalletinfo and getunconfirmedbalance

c3fe85e wallet, rpc, test: Remove deprecated getunconfirmedbalance (Ava Chow)
0ec2551 wallet, rpc: Remove deprecated balances from getwalletinfo (Ava Chow)

Pull request description:

  `getwalletinfo` result fields `balance`, `immature_balance`, and `unconfirmed_balance`, and the `getunconfirmedbalance` RPC have all been deprecated since 0.19.0. It's been long enough that they should either be removed or undeprecated. The functionality provided by these RPCs is provided by `getbalances`.

ACKs for top commit:
  davidgumberg:
    ACK bitcoin/bitcoin@c3fe85e
  rkrux:
    ACK c3fe85e
  BrandonOdiwuor:
    ACK c3fe85e removing the deprecated `balance, unconfirmed_balance, immature_balance` fields from `getwalletinfo` and `getunconfirmedbalance` RPCs, as this infomation can be found on the `getbalances` RPC
  w0xlt:
    reACK bitcoin/bitcoin@c3fe85e

Tree-SHA512: c7c4acfd9cabc7517ba813b95281a6c6a717a417312afd9346298669b4f7bd37724ad977148ce42db7fd47fc3d1f5a8482d8ff2e7b9cb74756b171a5b8b91ef2

Author: glozow

src/wallet/rpc/coins.cpp

@@ -214,29 +214,6 @@ RPCHelpMan getbalance()
     };
 }
 
-RPCHelpMan getunconfirmedbalance()
-{
-    return RPCHelpMan{"getunconfirmedbalance",
-                "DEPRECATED\nIdentical to getbalances().mine.untrusted_pending\n",
-                {},
-                RPCResult{RPCResult::Type::NUM, "", "The balance"},
-                RPCExamples{""},
-        [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
-{
-    const std::shared_ptr<const CWallet> pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
-
-    // Make sure the results are valid at least up to the most recent block
-    // the user could have gotten from another RPC command prior to now
-    pwallet->BlockUntilSyncedToCurrentChain();
-
-    LOCK(pwallet->cs_wallet);
-
-    return ValueFromAmount(GetBalance(*pwallet).m_mine_untrusted_pending);
-},
-    };
-}
-
 RPCHelpMan lockunspent()
 {
     return RPCHelpMan{

src/wallet/rpc/wallet.cpp

@@ -42,9 +42,6 @@ static RPCHelpMan getwalletinfo()
                         {RPCResult::Type::STR, "walletname", "the wallet name"},
                         {RPCResult::Type::NUM, "walletversion", "the wallet version"},
                         {RPCResult::Type::STR, "format", "the database format (only sqlite)"},
-                        {RPCResult::Type::STR_AMOUNT, "balance", "DEPRECATED. Identical to getbalances().mine.trusted"},
-                        {RPCResult::Type::STR_AMOUNT, "unconfirmed_balance", "DEPRECATED. Identical to getbalances().mine.untrusted_pending"},
-                        {RPCResult::Type::STR_AMOUNT, "immature_balance", "DEPRECATED. Identical to getbalances().mine.immature"},
                         {RPCResult::Type::NUM, "txcount", "the total number of transactions in the wallet"},
                         {RPCResult::Type::NUM, "keypoolsize", "how many new keys are pre-generated (only counts external keys)"},
                         {RPCResult::Type::NUM, "keypoolsize_hd_internal", /*optional=*/true, "how many new keys are pre-generated for internal use (used for change outputs, only appears if the wallet is using this feature, otherwise external keys are used)"},
@@ -86,13 +83,9 @@ static RPCHelpMan getwalletinfo()
     UniValue obj(UniValue::VOBJ);
 
     size_t kpExternalSize = pwallet->KeypoolCountExternalKeys();
-    const auto bal = GetBalance(*pwallet);
     obj.pushKV("walletname", pwallet->GetName());
     obj.pushKV("walletversion", pwallet->GetVersion());
     obj.pushKV("format", pwallet->GetDatabase().Format());
-    obj.pushKV("balance", ValueFromAmount(bal.m_mine_trusted));
-    obj.pushKV("unconfirmed_balance", ValueFromAmount(bal.m_mine_untrusted_pending));
-    obj.pushKV("immature_balance", ValueFromAmount(bal.m_mine_immature));
     obj.pushKV("txcount",       (int)pwallet->mapWallet.size());
     obj.pushKV("keypoolsize", (int64_t)kpExternalSize);
 
@@ -973,7 +966,6 @@ RPCHelpMan restorewallet();
 RPCHelpMan getreceivedbyaddress();
 RPCHelpMan getreceivedbylabel();
 RPCHelpMan getbalance();
-RPCHelpMan getunconfirmedbalance();
 RPCHelpMan lockunspent();
 RPCHelpMan listlockunspent();
 RPCHelpMan getbalances();
@@ -1033,7 +1025,6 @@ std::span<const CRPCCommand> GetWalletRPCCommands()
         {"wallet", &getreceivedbyaddress},
         {"wallet", &getreceivedbylabel},
         {"wallet", &gettransaction},
-        {"wallet", &getunconfirmedbalance},
         {"wallet", &getbalances},
         {"wallet", &getwalletinfo},
         {"wallet", &importdescriptors},

test/functional/wallet_balance.py

@@ -167,12 +167,6 @@ def test_balances(*, fee_node_1=0):
             # TODO: fix getbalance tracking of coin spentness depth
             assert_equal(self.nodes[0].getbalance(minconf=1), Decimal('0'))
             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[1].getunconfirmedbalance(), Decimal('30') - fee_node_1)  # Doesn't include output of node 0's send since it was spent
-            # 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'))
 

test/functional/wallet_basic.py

@@ -69,9 +69,9 @@ def run_test(self):
 
         self.generate(self.nodes[0], 1, sync_fun=self.no_op)
 
-        walletinfo = self.nodes[0].getwalletinfo()
-        assert_equal(walletinfo['immature_balance'], 50)
-        assert_equal(walletinfo['balance'], 0)
+        balances = self.nodes[0].getbalances()
+        assert_equal(balances["mine"]["immature"], 50)
+        assert_equal(balances["mine"]["trusted"], 0)
 
         self.sync_all(self.nodes[0:3])
         self.generate(self.nodes[1], COINBASE_MATURITY + 1, sync_fun=lambda: self.sync_all(self.nodes[0:3]))
@@ -118,8 +118,7 @@ def run_test(self):
         # but 10 will go to node2 and the rest will go to node0
         balance = self.nodes[0].getbalance()
         assert_equal(set([txout1['value'], txout2['value']]), set([10, balance]))
-        walletinfo = self.nodes[0].getwalletinfo()
-        assert_equal(walletinfo['immature_balance'], 0)
+        assert_equal(self.nodes[0].getbalances()["mine"]["immature"], 0)
 
         # Have node0 mine a block, thus it will collect its own fee.
         self.generate(self.nodes[0], 1, sync_fun=lambda: self.sync_all(self.nodes[0:3]))

test/functional/wallet_multiwallet.py

@@ -203,8 +203,7 @@ def wallet_file(name):
         self.nodes[0].loadwallet("w5")
         assert_equal(set(node.listwallets()), {"w4", "w5"})
         w5 = wallet("w5")
-        w5_info = w5.getwalletinfo()
-        assert_equal(w5_info['immature_balance'], 50)
+        assert_equal(w5.getbalances()["mine"]["immature"], 50)
 
         competing_wallet_dir = os.path.join(self.options.tmpdir, 'competing_walletdir')
         os.mkdir(competing_wallet_dir)
@@ -226,7 +225,7 @@ def wallet_file(name):
         self.generatetoaddress(node, nblocks=1, address=wallets[0].getnewaddress(), sync_fun=self.no_op)
         for wallet_name, wallet in zip(wallet_names, wallets):
             info = wallet.getwalletinfo()
-            assert_equal(info['immature_balance'], 50 if wallet is wallets[0] else 0)
+            assert_equal(wallet.getbalances()["mine"]["immature"], 50 if wallet is wallets[0] else 0)
             assert_equal(info['walletname'], wallet_name)
 
         # accessing invalid wallet fails

test/functional/wallet_reorgsrestore.py

@@ -100,15 +100,15 @@ def test_reorg_handling_during_unclean_shutdown(self):
         # Restart to ensure node and wallet are flushed
         self.restart_node(0)
         wallet = node.get_wallet_rpc("reorg_crash")
-        assert_greater_than(wallet.getwalletinfo()['immature_balance'], 0)
+        assert_greater_than(wallet.getbalances()["mine"]["immature"], 0)
 
         # Disconnect tip and sync wallet state
         tip = wallet.getbestblockhash()
         wallet.invalidateblock(tip)
         wallet.syncwithvalidationinterfacequeue()
 
         # Tip was disconnected, ensure coinbase has been abandoned
-        assert_equal(wallet.getwalletinfo()['immature_balance'], 0)
+        assert_equal(wallet.getbalances()["mine"]["immature"], 0)
         coinbase_tx_id = wallet.getblock(tip, verbose=1)["tx"][0]
         assert_equal(wallet.gettransaction(coinbase_tx_id)['details'][0]['abandoned'], True)
 
@@ -131,12 +131,12 @@ def test_reorg_handling_during_unclean_shutdown(self):
         assert(node.getbestblockhash() != tip)
         # Ensure wallet state is consistent now
         assert_equal(wallet.gettransaction(coinbase_tx_id)['details'][0]['abandoned'], True)
-        assert_equal(wallet.getwalletinfo()['immature_balance'], 0)
+        assert_equal(wallet.getbalances()["mine"]["immature"], 0)
 
         # And finally, verify the state if the block ends up being into the best chain again
         node.reconsiderblock(tip)
         assert_equal(wallet.gettransaction(coinbase_tx_id)['details'][0]['abandoned'], False)
-        assert_greater_than(wallet.getwalletinfo()['immature_balance'], 0)
+        assert_greater_than(wallet.getbalances()["mine"]["immature"], 0)
 
     def run_test(self):
         # Send a tx from which to conflict outputs later