Skip to content

Commit

Permalink
Merge bugfix_rpc_getbalance_hacky-0.21
Browse files Browse the repository at this point in the history
  • Loading branch information
@luke-jr
luke-jr committed Dec 13, 2021
3 parents 1e916ec + 927a487 + 2fc80ce commit 1413e85
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 26 deletions.
17 changes: 15 additions & 2 deletions src/wallet/rpcwallet.cpp
View file
Expand Up @@ -783,7 +783,7 @@ static RPCHelpMan getbalance()
"thus affected by options which limit spendability such as -spendzeroconfchange.\n",
{
{"dummy", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "Remains for backward compatibility. Must be excluded or set to \"*\"."},
{"minconf", RPCArg::Type::NUM, /* default */ "0", "Only include transactions confirmed at least this many times."},
{"minconf", RPCArg::Type::NUM, /* default */ "1", "Only include transactions confirmed at least this many times. (Requires dummy=\"*\")"},
{"include_watchonly", RPCArg::Type::BOOL, /* default */ "true for watch-only wallets, otherwise false", "Also include balance in watch-only addresses (see 'importaddress')"},
{"avoid_reuse", RPCArg::Type::BOOL, /* 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."},
},
Expand Down Expand Up @@ -816,6 +816,10 @@ static RPCHelpMan getbalance()
}

int min_depth = 0;
if (!dummy_value.isNull()) {
// Default min_depth to 1 when dummy="*"
min_depth = 1;
}
if (!request.params[1].isNull()) {
min_depth = request.params[1].get_int();
}
Expand All @@ -826,6 +830,15 @@ static RPCHelpMan getbalance()

const auto bal = pwallet->GetBalance(min_depth, avoid_reuse);

if (!dummy_value.isNull()) {
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));
},
};
Expand Down Expand Up @@ -4564,7 +4577,7 @@ static const CRPCCommand commands[] =
{ "wallet", "encryptwallet", &encryptwallet, {"passphrase"} },
{ "wallet", "getaddressesbylabel", &getaddressesbylabel, {"label"} },
{ "wallet", "getaddressinfo", &getaddressinfo, {"address"} },
{ "wallet", "getbalance", &getbalance, {"dummy","minconf","include_watchonly","avoid_reuse"} },
{ "wallet", "getbalance", &getbalance, {"dummy||account","minconf","include_watchonly","avoid_reuse"} },
{ "wallet", "getnewaddress", &getnewaddress, {"label","address_type"} },
{ "wallet", "getrawchangeaddress", &getrawchangeaddress, {"address_type"} },
{ "wallet", "getreceivedbyaddress", &getreceivedbyaddress, {"address","minconf"} },
Expand Down
53 changes: 53 additions & 0 deletions src/wallet/wallet.cpp
View file
Expand Up @@ -2153,6 +2153,59 @@ CWallet::Balance CWallet::GetBalance(const int min_depth, bool avoid_reuse) cons
return ret;
}

// Calculate total balance in a different way from GetBalance. The biggest
// difference is that GetBalance sums up all unspent TxOuts paying to the
// wallet, while this sums up both spent and unspent TxOuts paying to the
// wallet, and then subtracts the values of TxIns spending from the wallet. This
// also has fewer restrictions on which unconfirmed transactions are considered
// trusted.
CAmount CWallet::GetLegacyBalance(const isminefilter& filter, int minDepth) const
{
LOCK(cs_wallet);

CAmount balance = 0;
for (const auto& entry : mapWallet) {
const CWalletTx& wtx = entry.second;
const int depth = wtx.GetDepthInMainChain();
if (depth < 0 || !chain().checkFinalTx(*wtx.tx) || wtx.IsImmatureCoinBase()) {
continue;
}

if (depth == 0) {
bool have_conflicts = false;
for (const CTxIn& txin : wtx.tx->vin) {
if (mapTxSpends.count(txin.prevout) > 1) {
have_conflicts = true;
break;
}
}
if (have_conflicts && !wtx.InMempool()) {
// Rather than include two conflicting unconfirmed transactions in the same balance, only include ones in our mempool (which cannot contain conflicts)
continue;
}
}

// Loop through tx outputs and add incoming payments. For outgoing txs,
// treat change outputs specially, as part of the amount debited.
CAmount debit = wtx.GetDebit(filter);
const bool outgoing = debit > 0;
for (const CTxOut& out : wtx.tx->vout) {
if (outgoing && IsChange(out)) {
debit -= out.nValue;
} else if (IsMine(out) & filter && depth >= minDepth) {
balance += out.nValue;
}
}

// For outgoing txs, subtract amount debited.
if (outgoing) {
balance -= debit;
}
}

return balance;
}

CAmount CWallet::GetAvailableBalance(const CCoinControl* coinControl) const
{
LOCK(cs_wallet);
Expand Down
1 change: 1 addition & 0 deletions src/wallet/wallet.h
View file
Expand Up @@ -942,6 +942,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
CAmount m_watchonly_immature{0};
};
Balance GetBalance(int min_depth = 0, bool avoid_reuse = true) const;
CAmount GetLegacyBalance(const isminefilter& filter, int minDepth) const;
CAmount GetAvailableBalance(const CCoinControl* coinControl = nullptr) const;

OutputType TransactionChangeType(const Optional<OutputType>& change_type, const std::vector<CRecipient>& vecSend);
Expand Down
38 changes: 14 additions & 24 deletions test/functional/wallet_balance.py
View file
Expand Up @@ -90,14 +90,13 @@ def run_test(self):
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')])
Expand Down Expand Up @@ -171,13 +170,12 @@ def test_balances(*, fee_node_1=0):
# 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
Expand Down Expand Up @@ -212,14 +210,6 @@ def test_balances(*, fee_node_1=0):
self.nodes[1].generatetoaddress(2, ADDRESS_WATCHONLY)
self.sync_all()

# 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']
Expand All @@ -240,7 +230,7 @@ def test_balances(*, fee_node_1=0):
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']
Expand All @@ -258,16 +248,16 @@ def test_balances(*, fee_node_1=0):
# Now confirm tx_replace
block_reorg = self.nodes[1].generatetoaddress(1, ADDRESS_WATCHONLY)[0]
self.sync_all()
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)
self.sync_blocks()
self.nodes[0].syncwithvalidationinterfacequeue()
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.nodes[0].generatetoaddress(1, ADDRESS_WATCHONLY)
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

# Now confirm tx_orig
self.restart_node(1, ['-persistmempool=0'])
Expand All @@ -276,7 +266,7 @@ def test_balances(*, fee_node_1=0):
self.nodes[1].sendrawtransaction(tx_orig)
self.nodes[1].generatetoaddress(1, ADDRESS_WATCHONLY)
self.sync_all()
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 __name__ == '__main__':
Expand Down

0 comments on commit 1413e85

Please sign in to comment.