highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-09-11 21:20:39 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#35925: wallet, rpc: Exclude non-owned addresses from listreceivedby*

Browse Source 
089c883c55 test: Add coverage for listreceivedby* excluding "send" addresses (pablomartin4btc)
873c054805 wallet: Exclude non-owned addresses from listreceivedby* (pablomartin4btc)

Pull request description:

  Fixes #16159.

  `listreceivedbyaddress`/`listreceivedbylabel` with `include_empty=true` walk the entire address book and return every entry that has no matching `mapTally` record — including addresses with a "send" purpose (foreign addresses that got a label via `setlabel`, the GUI, or `addmultisigaddress`) that this wallet never received funds to and doesn't own.

  This excludes those via `IsMine()` rather than the address book's `purpose` field, since `purpose` is set inconsistently across several code paths and `IsMine()` is the same check `mapTally` itself is already built from.

  Picks up prior work by kouloumos in #25973 and BrandonOdiwuor in #30972, both closed for
  inactivity:
  - [#25973](https://github.com/bitcoin/bitcoin/pull/25973) filtered on `purpose == "send"` directly. ryanofsky pointed out purpose "is set pretty haphazardly in code" and [suggested](https://github.com/bitcoin/bitcoin/pull/25973#discussion_r1269477246) `IsMine()` instead.
  - [#30972](https://github.com/bitcoin/bitcoin/pull/30972) implemented that, then furszy pointed out `IsMine()` only needs to run for addresses missing from `mapTally`, not every one. rkrux further suggested dropping the redundant re-lock in favor of `EXCLUSIVE_LOCKS_REQUIRED` directly on the lambda — matching the existing pattern in `wallet/interfaces.cpp` — and simplifying the branching.

  This PR carries that final approach forward on current master. The regression test is a small, standalone addition rather than reviving the test-file "split into subtests" refactor from the earlier PRs, which achow101 [flagged](https://github.com/bitcoin/bitcoin/pull/30972#issuecomment-3688186614) on #30972 as unrelated stylistic churn.

ACKs for top commit:
  polespinasa:
    lgtm re-ACK 089c883c55
  jeanpablojp:
    ACK 089c883c55
  achow101:
    ACK 089c883c55

Tree-SHA512: d45488c93b9294258faaab5d1891ca5e8c4b8d0d4feb298403c7c3f20d6aa08989d548cddd25ccd47a1ed969e4a309ee68ad1541c6121fed39ed534c78c256e7
This commit is contained in:
Ava Chow
2026-08-10 12:37:54 -07:00
parent 1d386c250f 089c883c55
commit e8cc21c57f
2 changed files with 19 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
src/wallet/rpc/transactions.cpp
View File

@@ -139,12 +139,17 @@ static UniValue ListReceived(const CWallet& wallet, const UniValue& params, cons
UniValue ret(UniValue::VARR);
std::map<std::string, tallyitem> label_tally;
const auto& func = [&](const CTxDestination& address, const std::string& label, bool is_change, const std::optional<AddressPurpose>& purpose) {
const auto& func = [&](const CTxDestination& address, const std::string& label, bool is_change,
const std::optional<AddressPurpose>& purpose) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) {
if (is_change) return; // no change addresses
// Entries in mapTally are only ever added for wallet.IsMine() addresses (see the tally
// loop above), so it's only addresses missing from mapTally that need the IsMine() check.
auto it = mapTally.find(address);
if (it == mapTally.end() && !fIncludeEmpty)
return;
if (it == mapTally.end()) {
if (!fIncludeEmpty) return;
if (!wallet.IsMine(address)) return; // exclude addresses not owned by the wallet (e.g. "send" purpose)
}
CAmount nAmount = 0;
int nConf = std::numeric_limits<int>::max();

11
test/functional/wallet_listreceivedby.py
View File

@@ -101,6 +101,17 @@ class ReceivedByTest(BitcoinTestFramework):
res = self.nodes[1].listreceivedbyaddress(0, True, True, other_addr)
assert_equal(len(res), 0)
self.log.info("listreceivedbyaddress and listreceivedbylabel exclude not owned addresses")
# setlabel assigns a "send" purpose when the wallet doesn't own the address.
send_label = "external-address"
external_addr = self.nodes[0].getnewaddress(send_label)
self.nodes[1].setlabel(external_addr, send_label)
assert_equal(self.nodes[1].getaddressinfo(external_addr)["ismine"], False)
assert_array_result(self.nodes[1].listreceivedbyaddress(minconf=0, include_empty=True),
{"address": external_addr}, {}, True)
assert_array_result(self.nodes[1].listreceivedbylabel(minconf=0, include_empty=True),
{"label": send_label}, {}, True)
self.log.info("getreceivedbyaddress Test")
# Send from node 0 to 1