RPC/Wallet: Hacky fix for getbalance bugs

This commit is contained in:
Luke Dashjr 2022-09-01 20:36:09 +00:00
parent 14bf4726c4
commit d09a74c7e4
2 changed files with 25 additions and 26 deletions

View File

@ -167,8 +167,8 @@ RPCHelpMan getbalance()
"The available balance is what the wallet considers currently spendable, and is\n"
"thus affected by options which limit spendability such as -spendzeroconfchange.\n",
{
{"dummy", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Remains for backward compatibility. Must be excluded or set to \"*\"."},
{"minconf", RPCArg::Type::NUM, RPCArg::Default{0}, "Only include transactions confirmed at least this many times."},
{"dummy|account", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Remains for backward compatibility. Must be excluded or set to \"*\"."},
{"minconf", RPCArg::Type::NUM, RPCArg::Default{1}, "Only include transactions confirmed at least this many times. (Requires dummy=\"*\")"},
{"include_watchonly", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true for watch-only wallets, otherwise false"}, "Also include balance in watch-only addresses (see 'importaddress')"},
{"avoid_reuse", RPCArg::Type::BOOL, RPCArg::Default{true}, "(only available if avoid_reuse wallet flag is set) Do not include balance in dirty outputs; addresses are considered dirty if they have previously been used in a transaction."},
},
@ -199,7 +199,7 @@ RPCHelpMan getbalance()
throw JSONRPCError(RPC_METHOD_DEPRECATED, "dummy first argument must be excluded or set to \"*\".");
}
const auto min_depth{self.Arg<int>("minconf")};
const auto min_depth{dummy_value ? self.Arg<int>("minconf") : 0};
bool include_watchonly = ParseIncludeWatchonly(request.params[2], *pwallet);
@ -207,6 +207,15 @@ RPCHelpMan getbalance()
const auto bal = GetBalance(*pwallet, min_depth, avoid_reuse);
if (dummy_value) {
isminefilter filter = ISMINE_SPENDABLE;
if (include_watchonly) filter = filter | ISMINE_WATCH_ONLY;
return ValueFromAmount(pwallet->GetLegacyBalance(filter, min_depth));
}
if (!request.params[1].isNull()) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "getbalance minconf option is only currently supported if dummy is set to \"*\"");
}
return ValueFromAmount(bal.m_mine_trusted + (include_watchonly ? bal.m_watchonly_trusted : 0));
},
};

View File

@ -106,14 +106,13 @@ class WalletTest(BitcoinTestFramework):
self.log.info("Test getbalance with different arguments")
assert_equal(self.nodes[0].getbalance("*"), 50)
assert_equal(self.nodes[0].getbalance("*", 1), 50)
assert_equal(self.nodes[0].getbalance(minconf=1), 50)
assert_raises_rpc_error(-8, "getbalance minconf option is only currently supported if dummy is set to \"*\"", self.nodes[0].getbalance, minconf=1)
assert_raises_rpc_error(-8, "getbalance minconf option is only currently supported if dummy is set to \"*\"", self.nodes[0].getbalance, minconf=0, include_watchonly=True)
if not self.options.descriptors:
assert_equal(self.nodes[0].getbalance(minconf=0, include_watchonly=True), 100)
assert_equal(self.nodes[0].getbalance("*", 1, True), 100)
else:
assert_equal(self.nodes[0].getbalance(minconf=0, include_watchonly=True), 50)
assert_equal(self.nodes[0].getbalance("*", 1, True), 50)
assert_equal(self.nodes[1].getbalance(minconf=0, include_watchonly=True), 50)
assert_raises_rpc_error(-8, "getbalance minconf option is only currently supported if dummy is set to \"*\"", self.nodes[1].getbalance, minconf=0, include_watchonly=True)
# Send 40 BTC from 0 to 1 and 60 BTC from 1 to 0.
txs = create_transactions(self.nodes[0], self.nodes[1].getnewaddress(), 40, [Decimal('0.01')])
@ -192,13 +191,12 @@ class WalletTest(BitcoinTestFramework):
# 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
# Same with minconf=0
assert_equal(self.nodes[0].getbalance(minconf=0), Decimal('9.99'))
assert_equal(self.nodes[1].getbalance(minconf=0), Decimal('0'))
# getbalance with a minconf incorrectly excludes coins that have been spent more recently than the minconf blocks ago
# 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'))
# getbalance with '*' and minconf=0 includes unconfirmed transactions, AND untrusted transactions
assert_equal(self.nodes[0].getbalance('*', 0), Decimal('69.99'))
assert_equal(self.nodes[1].getbalance('*', 0), Decimal('30') - fee_node_1)
# getbalance with '*' and minconf=1 includes only confirmed and sent transactions
assert_equal(self.nodes[0].getbalance('*', 1), Decimal('9.99'))
assert_equal(self.nodes[1].getbalance('*', 1), Decimal('-10') - fee_node_1)
# 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
@ -231,14 +229,6 @@ class WalletTest(BitcoinTestFramework):
self.nodes[1].sendrawtransaction(txs[0]['hex'])
self.generatetoaddress(self.nodes[1], 2, ADDRESS_WATCHONLY)
# getbalance with a minconf incorrectly excludes coins that have been spent more recently than the minconf blocks ago
# TODO: fix getbalance tracking of coin spentness depth
# getbalance with minconf=3 should still show the old balance
assert_equal(self.nodes[1].getbalance(minconf=3), Decimal('0'))
# getbalance with minconf=2 will show the new balance.
assert_equal(self.nodes[1].getbalance(minconf=2), Decimal('0'))
# check mempool transactions count for wallet unconfirmed balance after
# dynamically loading the wallet.
before = self.nodes[1].getbalances()['mine']['untrusted_pending']
@ -259,7 +249,7 @@ class WalletTest(BitcoinTestFramework):
self.log.info('Check that wallet txs not in the mempool are untrusted')
assert txid not in self.nodes[0].getrawmempool()
assert_equal(self.nodes[0].gettransaction(txid)['trusted'], False)
assert_equal(self.nodes[0].getbalance(minconf=0), 0)
assert_equal(self.nodes[0].getbalances()['mine']['trusted'], 0)
self.log.info("Test replacement and reorg of non-mempool tx")
tx_orig = self.nodes[0].gettransaction(txid)['hex']
@ -276,12 +266,12 @@ class WalletTest(BitcoinTestFramework):
# Now confirm tx_replace
block_reorg = self.generatetoaddress(self.nodes[1], 1, ADDRESS_WATCHONLY)[0]
assert_equal(self.nodes[0].getbalance(minconf=0), total_amount)
assert_equal(self.nodes[0].getbalances()['mine']['trusted'], total_amount)
self.log.info('Put txs back into mempool of node 1 (not node 0)')
self.nodes[0].invalidateblock(block_reorg)
self.nodes[1].invalidateblock(block_reorg)
assert_equal(self.nodes[0].getbalance(minconf=0), 0) # wallet txs not in the mempool are untrusted
assert_equal(self.nodes[0].getbalances()['mine']['trusted'], 0) # wallet txs not in the mempool are untrusted
self.generatetoaddress(self.nodes[0], 1, ADDRESS_WATCHONLY, sync_fun=self.no_op)
# Now confirm tx_orig
@ -290,7 +280,7 @@ class WalletTest(BitcoinTestFramework):
self.sync_blocks()
self.nodes[1].sendrawtransaction(tx_orig)
self.generatetoaddress(self.nodes[1], 1, ADDRESS_WATCHONLY)
assert_equal(self.nodes[0].getbalance(minconf=0), total_amount + 1) # The reorg recovered our fee of 1 coin
assert_equal(self.nodes[0].getbalances()['mine']['trusted'], total_amount + 1) # The reorg recovered our fee of 1 coin
if not self.options.descriptors:
self.log.info('Check if mempool is taken into account after import*')