Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions src/wallet/rpc/coins.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,51 @@ RPCHelpMan getreceivedbylabel()
};
}

RPCHelpMan listaddressbalances()
{
return RPCHelpMan{"listaddressbalances",
"\nLists addresses of this wallet and their balances\n",
{
{"minamount", RPCArg::Type::NUM, RPCArg::Default{0}, "Minimum balance in " + CURRENCY_UNIT + " an address should have to be shown in the list"},
},
RPCResult{
RPCResult::Type::OBJ_DYN, "", "Balances of addresses",
{
{RPCResult::Type::STR_AMOUNT, "address", "the amount in " + CURRENCY_UNIT},
}
},
RPCExamples{
HelpExampleCli("listaddressbalances", "")
+ HelpExampleCli("listaddressbalances", "10")
+ HelpExampleRpc("listaddressbalances", "")
+ HelpExampleRpc("listaddressbalances", "10")
},
[&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
{
const std::shared_ptr<const CWallet> pwallet = GetWalletForJSONRPCRequest(request);
if (!pwallet) return UniValue::VNULL;

LOCK(pwallet->cs_wallet);

CAmount nMinAmount = 0;
if (!request.params[0].isNull())
nMinAmount = AmountFromValue(request.params[0]);

if (nMinAmount < 0)
throw JSONRPCError(RPC_TYPE_ERROR, "Invalid amount");

UniValue jsonBalances(UniValue::VOBJ);
std::map<CTxDestination, CAmount> balances = GetAddressBalances(*pwallet);
for (auto& balance : balances)
if (balance.second >= nMinAmount)
jsonBalances.pushKV(EncodeDestination(balance.first), ValueFromAmount(balance.second));

return jsonBalances;
},
};
}


RPCHelpMan getbalance()
{
return RPCHelpMan{"getbalance",
Expand Down
46 changes: 1 addition & 45 deletions src/wallet/rpc/wallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,51 +42,6 @@ bool HaveKey(const SigningProvider& wallet, const CKey& key)
return wallet.HaveKey(key.GetPubKey().GetID()) || wallet.HaveKey(key2.GetPubKey().GetID());
}

static RPCHelpMan listaddressbalances()
{
return RPCHelpMan{"listaddressbalances",
"\nLists addresses of this wallet and their balances\n",
{
{"minamount", RPCArg::Type::NUM, RPCArg::Default{0}, "Minimum balance in " + CURRENCY_UNIT + " an address should have to be shown in the list"},
},
RPCResult{
RPCResult::Type::ARR, "", "",
{
{RPCResult::Type::STR_AMOUNT, "amount", "The Dash address and the amount in " + CURRENCY_UNIT},
}
},
RPCExamples{
HelpExampleCli("listaddressbalances", "")
+ HelpExampleCli("listaddressbalances", "10")
+ HelpExampleRpc("listaddressbalances", "")
+ HelpExampleRpc("listaddressbalances", "10")
},
[&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
{
const std::shared_ptr<const CWallet> pwallet = GetWalletForJSONRPCRequest(request);
if (!pwallet) return UniValue::VNULL;

LOCK(pwallet->cs_wallet);

CAmount nMinAmount = 0;
if (!request.params[0].isNull())
nMinAmount = AmountFromValue(request.params[0]);

if (nMinAmount < 0)
throw JSONRPCError(RPC_TYPE_ERROR, "Invalid amount");

UniValue jsonBalances(UniValue::VOBJ);
std::map<CTxDestination, CAmount> balances = GetAddressBalances(*pwallet);
for (auto& balance : balances)
if (balance.second >= nMinAmount)
jsonBalances.pushKV(EncodeDestination(balance.first), ValueFromAmount(balance.second));

return jsonBalances;
},
};
}


static RPCHelpMan setcoinjoinrounds()
{
return RPCHelpMan{"setcoinjoinrounds",
Expand Down Expand Up @@ -1138,6 +1093,7 @@ RPCHelpMan importelectrumwallet();
// coins
RPCHelpMan getreceivedbyaddress();
RPCHelpMan getreceivedbylabel();
RPCHelpMan listaddressbalances();
RPCHelpMan getbalance();
RPCHelpMan getunconfirmedbalance();
RPCHelpMan lockunspent();
Expand Down
1 change: 1 addition & 0 deletions test/functional/create_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class CreateCache(BitcoinTestFramework):

def set_test_params(self):
self.num_nodes = 0
self.uses_wallet = True

def setup_network(self):
pass
Expand Down
5 changes: 4 additions & 1 deletion test/functional/test_framework/test_framework.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ def __init__(self):
self._requires_wallet = False
# Disable ThreadOpenConnections by default, so that adding entries to
# addrman will not result in automatic connections to them.
self.uses_wallet = False
Comment on lines 144 to +147
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: Two overlapping wallet flags: _requires_wallet and uses_wallet

BitcoinTestFramework.__init__ now owns two separately-purposed wallet flags: _requires_wallet (set by skip_if_no_wallet() at line 1071, gates import_deterministic_coinbase_privkeys() at lines 466 and 1820) and uses_wallet (controls whether -disablewallet is appended by TestNode at test_node.py:117). The distinction is undocumented and easy to get wrong — e.g. the missed propagation in add_dynamically_node() at line 619 is partly enabled by the fact that tests must now remember both flags. Consider either deriving one from the other (self.uses_wallet = self.uses_wallet or self._requires_wallet) or adding an explanatory comment at the definition site so future test authors don't set only one and wonder why the wallet is still disabled.

source: unknown

🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `test/functional/test_framework/test_framework.py`:
- [SUGGESTION] lines 144-147: Two overlapping wallet flags: `_requires_wallet` and `uses_wallet`
  `BitcoinTestFramework.__init__` now owns two separately-purposed wallet flags: `_requires_wallet` (set by `skip_if_no_wallet()` at line 1071, gates `import_deterministic_coinbase_privkeys()` at lines 466 and 1820) and `uses_wallet` (controls whether `-disablewallet` is appended by `TestNode` at test_node.py:117). The distinction is undocumented and easy to get wrong — e.g. the missed propagation in `add_dynamically_node()` at line 619 is partly enabled by the fact that tests must now remember both flags. Consider either deriving one from the other (`self.uses_wallet = self.uses_wallet or self._requires_wallet`) or adding an explanatory comment at the definition site so future test authors don't set only one and wonder why the wallet is still disabled.

self.disable_autoconnect = True
self.set_test_params()
assert self.wallet_names is None or len(self.wallet_names) <= self.num_nodes
Expand Down Expand Up @@ -577,6 +578,7 @@ def get_bin_from_version(version, bin_name, bin_default):
use_valgrind=self.options.valgrind,
descriptors=self.options.descriptors,
v2transport=self.options.v2transport,
uses_wallet=self.uses_wallet,
)
self.nodes.append(test_node_i)
if not test_node_i.version_is_at_least(160000):
Expand Down Expand Up @@ -955,7 +957,7 @@ def _initialize_chain(self):
cache_node_dir,
chain=self.chain,
extra_conf=["bind=127.0.0.1"],
extra_args=['-disablewallet', f"-mocktime={TIME_GENESIS_BLOCK}"],
extra_args=[f"-mocktime={TIME_GENESIS_BLOCK}"],
extra_args_from_options=self.extra_args_from_options,
rpchost=None,
timewait=self.rpc_timeout,
Expand All @@ -966,6 +968,7 @@ def _initialize_chain(self):
coverage_dir=None,
cwd=self.options.tmpdir,
descriptors=self.options.descriptors,
uses_wallet=self.uses_wallet,
))
self.start_node(CACHE_NODE_ID)
cache_node = self.nodes[CACHE_NODE_ID]
Expand Down
4 changes: 2 additions & 2 deletions test/functional/test_framework/test_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class TestNode():
To make things easier for the test writer, any unrecognised messages will
be dispatched to the RPC connection."""

def __init__(self, i, datadir, extra_args_from_options, *, chain, rpchost, timewait, timeout_factor, bitcoind, bitcoin_cli, mocktime, coverage_dir, cwd, extra_conf=None, extra_args=None, use_cli=False, start_perf=False, use_valgrind=False, version=None, descriptors=False, v2transport=False):
def __init__(self, i, datadir, extra_args_from_options, *, chain, rpchost, timewait, timeout_factor, bitcoind, bitcoin_cli, mocktime, coverage_dir, cwd, extra_conf=None, extra_args=None, use_cli=False, start_perf=False, use_valgrind=False, version=None, descriptors=False, v2transport=False, uses_wallet=False):
"""
Kwargs:
start_perf (bool): If True, begin profiling the node with `perf` as soon as
Expand Down Expand Up @@ -114,7 +114,7 @@ def __init__(self, i, datadir, extra_args_from_options, *, chain, rpchost, timew
if self.mocktime != 0:
self.args.append(f"-mocktime={mocktime}")

if self.descriptors is None:
if uses_wallet is not None and not uses_wallet and self.descriptors is None:
self.args.append("-disablewallet")
Comment on lines 72 to 118
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 Nitpick: Partial backport of bitcoin#28710 uses_wallet infrastructure

bitcoin#33064 relies on the uses_wallet plumbing introduced upstream as part of bitcoin#28710 (legacy wallet removal, commit c847dee). That broader series is not backported to Dash — only the minimum-viable subset needed for bitcoin#33064 was added in follow-up commit 53cd792 (default uses_wallet=False, uses_wallet= kwarg on TestNode.__init__, and the gated -disablewallet branch at line 117). This is sufficient for the current fix, but worth recording: if future backports pull in more of bitcoin#28710's descriptor-only semantics, a more complete port of that series may become warranted.

source: ['claude']


# Use valgrind, expect for previous release binaries
Expand Down
2 changes: 2 additions & 0 deletions test/functional/test_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -926,6 +926,8 @@ def _get_uncovered_rpc_commands(self):
covered_cmds = set({'generate'})
# TODO: implement functional tests for voteraw
covered_cmds.add('voteraw')
# TODO: implement functional tests for importelectrumwallet
covered_cmds.add('importelectrumwallet')
# TODO: implement functional tests for getmerkleblocks
covered_cmds.add('getmerkleblocks')

Expand Down
9 changes: 9 additions & 0 deletions test/functional/wallet_balance.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,15 @@ def run_test(self):
assert_equal(self.nodes[0].getbalance("*", 1, True), 500)
assert_equal(self.nodes[1].getbalance(minconf=0, include_watchonly=True), 500)

self.log.info("Test listaddressbalances")
address_balances = {}
address_balances['yYdShjQSptFKitYLksFEUSwHe4hnbar5rf'] = Decimal('500.00000000')
if not self.options.descriptors:
address_balances['yVg3NBUHNEhgDceqwVUjsZHreC5PBHnUo9'] = Decimal('500.00000000')
assert_equal(address_balances, self.nodes[0].listaddressbalances())
assert_equal(address_balances, self.nodes[0].listaddressbalances(500))
assert_equal({}, self.nodes[0].listaddressbalances(501))

# Send 490 BTC from 0 to 1 and 960 BTC from 1 to 0.
txs = create_transactions(self.nodes[0], self.nodes[1].getnewaddress(), 490 , [Decimal('0.01')])
self.nodes[0].sendrawtransaction(txs[0]['hex'])
Expand Down
4 changes: 4 additions & 0 deletions test/functional/wallet_transactiontime_rescan.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,10 @@ def run_test(self):
restorewo_wallet.importaddress(wo2, rescan=False)
restorewo_wallet.importaddress(wo3, rescan=False)

self.log.info('Testing abortrescan when no rescan is in progress')
assert_equal(restorewo_wallet.getwalletinfo()['scanning'], False)
assert_equal(restorewo_wallet.abortrescan(), False)

# check user has 0 balance and no transactions
assert_equal(restorewo_wallet.getbalance(), 0)
assert_equal(len(restorewo_wallet.listtransactions()), 0)
Expand Down
Loading