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

pull pablomartin4btc wants to merge 2 commits into bitcoin:master from pablomartin4btc:16159-listreceivedby-exclude-send changing 2 files +19 −3

pablomartin4btc commented at 3:07 AM on August 7, 2026: member
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 filtered on purpose == "send" directly. ryanofsky pointed out purpose "is set pretty haphazardly in code" and suggested IsMine() instead.
- #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 on #30972 as unrelated stylistic churn.
DrahtBot added the label Wallet on Aug 7, 2026
DrahtBot commented at 3:08 AM on August 7, 2026: contributor



The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35925.



Reviews

See the guideline and AI policy for information on the review process.

Type Reviewers

ACK polespinasa, jeanpablojp, achow101

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.
pablomartin4btc renamed this:
wallet: Exclude non-owned addresses from listreceivedby*
wallet, rpc: Exclude non-owned addresses from listreceivedby*
on Aug 7, 2026
polespinasa commented at 7:58 AM on August 7, 2026: member

Concept ACK

Just as a proof-of-bug, the console screen shows listreceivedbyaddress from wallet1 where the address from wallet2 is displayed.

<img width="1589" height="553" alt="imagen" src="https://github.com/user-attachments/assets/478f3753-e5d0-4a05-acaf-3674be8d1650" />

Type	Reviewers
ACK	polespinasa, jeanpablojp, achow101

in src/wallet/rpc/transactions.cpp:142 in f3dccd20bc

 138 | @@ -139,12 +139,14 @@ static UniValue ListReceived(const CWallet& wallet, const UniValue& params, cons
 139 |      UniValue ret(UniValue::VARR);
 140 |      std::map<std::string, tallyitem> label_tally;
 141 |  
 142 | -    const auto& func = [&](const CTxDestination& address, const std::string& label, bool is_change, const std::optional<AddressPurpose>& purpose) {
 143 | +    const auto& func = [&](const CTxDestination& address, const std::string& label, bool is_change, const std::optional<AddressPurpose>& purpose) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) {

polespinasa commented at 8:15 AM on August 7, 2026:

in f3dccd20bcd2848095d883d8cdc8bf24605c046f wallet: Exclude non-owned addresses from listreceivedby*

nit: consider spliting the function declaration line into multiple lines, it is really long.

pablomartin4btc commented at 1:19 PM on August 7, 2026:

Done, thanks!

in src/wallet/rpc/transactions.cpp:148 in f3dccd20bc outdated
```
 146 |          auto it = mapTally.find(address);
 147 | -        if (it == mapTally.end() && !fIncludeEmpty)
 148 | -            return;
 149 | +        if (it == mapTally.end()) {
 150 | +            if (!fIncludeEmpty) return;
 151 | +            if (!wallet.IsMine(address)) return; // exclude addresses not owned by the wallet (e.g. "send" purpose)
```
polespinasa commented at 8:22 AM on August 7, 2026:

in f3dccd2 wallet: Exclude non-owned addresses from listreceivedby*

nit: Actually even if the address is found in mapTally we should discard it if !IsMine(). This is not a problem right now because inside the mapTally loop we already check IsMine(), but maybe it is safer to check IsMine() regardless. Idk how expensive that is tho as we have to go back to m_cached_spks to check.
pablomartin4btc commented at 1:22 PM on August 7, 2026:

Good catch to double check, but I don't think we need the extra call: mapTally is only ever populated inside the tally loop above, gated by if (!wallet.IsMine(address)) continue; — so every entry that makes it into mapTally is already guaranteed IsMine(). And since ListReceived() holds wallet.cs_wallet for its entire duration, that can't change mid-function either. Re-checking would be pure redundant work (and non-free, as you noted, since it goes through m_cached_spks) for zero behavioural difference.

Added a comment above the lookup instead, documenting that invariant so it's clear why the check is scoped to the it == mapTally.end() branch:

// 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.

in test/functional/wallet_listreceivedby.py:104 in 4f94c2e82a

 100 | @@ -101,6 +101,17 @@ def run_test(self):
 101 |          res = self.nodes[1].listreceivedbyaddress(0, True, True, other_addr)
 102 |          assert_equal(len(res), 0)
 103 |  
 104 | +        self.log.info("listreceivedbyaddress/listreceivedbylabel Test - excludes addresses with 'send' purpose")

polespinasa commented at 8:26 AM on August 7, 2026:

in 4f94c2e82aa06c226d5d9cd0ad4766658818252b test: Add coverage for listreceivedby* excluding "send" addresses

nit: Consider just simply:

self.log.info("listreceivedbyaddress and listreceivedbylabel exclude not owned addresses")

pablomartin4btc commented at 1:18 PM on August 7, 2026:

Done, thanks!

polespinasa commented at 8:30 AM on August 7, 2026: member

ACK 4f94c2e82aa06c226d5d9cd0ad4766658818252b

Left some minor nits, but looks good to me as is

wallet: Exclude non-owned addresses from listreceivedby*

listreceivedbyaddress/listreceivedbylabel with include_empty=true
walked the full address book and returned every entry with no
matching mapTally record, including addresses with a "send" purpose
(foreign addresses labeled via setlabel, the GUI, or
addmultisigaddress) that the wallet never received funds to and does
not own.

Filter these out 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.

Fixes #16159.

Co-authored-by: Brandon Odiwuor <brandon.odiwuor@gmail.com>

873c054805

test: Add coverage for listreceivedby* excluding "send" addresses

Regression test for #16159: an address labeled via setlabel by a
wallet that doesn't own it is assigned a "send" purpose and must not
appear in listreceivedbyaddress/listreceivedbylabel results, even
with include_empty=true.

Co-authored-by: Andreas Kouloumos <kouloumosa@gmail.com>

089c883c55

pablomartin4btc force-pushed on Aug 7, 2026
pablomartin4btc commented at 1:26 PM on August 7, 2026: member
-<ins>Updates</ins>:
- Addressed @polespinasa's feedback.
polespinasa commented at 1:26 PM on August 7, 2026: member

lgtm re-ACK 089c883c558e01c2a18a92f861fa7de9a7cc607a
jeanpablojp commented at 5:31 PM on August 8, 2026: none

ACK 089c883c558e01c2a18a92f861fa7de9a7cc607a

I have tested the code. The comment on the tally loop holds: mapTally has a single insertion in the file and it sits behind IsMine(), so only the missing-entry branch needs the check. IsMine(const CTxDestination&) requires cs_wallet, so the annotation on the lambda is needed, and both RPC entry points take the lock.

Reverting the logic while keeping the new test makes it fail: the not owned address is still listed. I also checked a watch-only descriptor address (ismine=true, solvable=false), the case this filter could plausibly exclude, and both RPCs still return it. Built the merge with master (128456b): no thread-safety warnings, unit suite and wallet_listreceivedby.py pass.
achow101 commented at 7:20 PM on August 10, 2026: member

ACK 089c883c558e01c2a18a92f861fa7de9a7cc607a
achow101 merged this on Aug 10, 2026
achow101 closed this on Aug 10, 2026

Contributors

Labels

Linked (view graph)

#16159 listrecievedbyaddress with include_empty not filtering out "send" side of address book #25973 wallet: Filter-out "send" addresses from `listreceivedby*`#30972 Wallet: "listreceivedby*" fix

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

Code Coverage & Benchmarks

Reviews