From c4234a5e788e4c8de3388d49c62b3341a8f99d3c Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 13 Apr 2020 17:57:11 -0400 Subject: [PATCH] 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: 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. 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 --- test/functional/wallet_abandonconflict.py | 4 +-- test/functional/wallet_balance.py | 38 ++++++++++++++++------- 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/test/functional/wallet_abandonconflict.py b/test/functional/wallet_abandonconflict.py index bbddc68036..e41b0b2638 100755 --- a/test/functional/wallet_abandonconflict.py +++ b/test/functional/wallet_abandonconflict.py @@ -107,8 +107,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_balance.py b/test/functional/wallet_balance.py index 6982c0d20f..ad5f3d11e9 100755 --- a/test/functional/wallet_balance.py +++ b/test/functional/wallet_balance.py @@ -109,7 +109,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: @@ -150,6 +150,20 @@ class WalletTest(BitcoinTestFramework): def test_balances(*, fee_node_1=0): + # getbalances + expected_balances_0 = {'mine': {'coinjoin': Decimal('0E-8'), + 'immature': Decimal('0E-8'), + 'trusted': Decimal('9.99'), # change from node 0's send + 'untrusted_pending': Decimal('960')}, + 'watchonly': {'immature': Decimal('50000'), + 'trusted': Decimal('500'), + 'untrusted_pending': Decimal('0E-8')}} + expected_balances_1 = {'mine': {'coinjoin': Decimal('0E-8'), + '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 @@ -162,11 +176,9 @@ class WalletTest(BitcoinTestFramework): assert_equal(self.nodes[1].getbalance(minconf=1), Decimal('0')) # getunconfirmedbalance assert_equal(self.nodes[0].getunconfirmedbalance(), Decimal('960')) # output of node 1's spend - assert_equal(self.nodes[0].getbalances()['mine']['untrusted_pending'], Decimal('960')) - assert_equal(self.nodes[0].getwalletinfo()["unconfirmed_balance"], Decimal('960')) - 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('960')) assert_equal(self.nodes[1].getwalletinfo()["unconfirmed_balance"], Decimal('30') - fee_node_1) test_balances(fee_node_1=Decimal('0.01')) @@ -176,15 +188,19 @@ class WalletTest(BitcoinTestFramework): #self.nodes[0].sendrawtransaction(txs[1]['hex']) # sending on both nodes is faster than waiting for propagation # disabled, no RBF in Dash self.sync_all() - self.log.info("Test getbalance and getunconfirmedbalance with conflicted unconfirmed inputs") - # test_balances(fee_node_1=Decimal('0.02')) + self.log.info("Test getbalance and getbalances.mine.untrusted_pending with conflicted unconfirmed inputs") + # test_balances(fee_node_1=Decimal('0.02')) # disabled, no RBF in Dash 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('969.99')) # node 1's send plus change from node 0's send - assert_equal(self.nodes[1].getbalance(), Decimal('29.99')) # change from node 0's send + balance_node0 = Decimal('969.99') # node 1's send plus change from node 0's send + balance_node1 = Decimal('29.99') # 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.98'), [Decimal('0.01')]) @@ -202,13 +218,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.default_wallet_name) self.nodes[0].sendtoaddress(dst, 0.1) self.sync_all() self.nodes[1].loadwallet(self.default_wallet_name) - 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