Merge bitcoin/bitcoin#35925: wallet, rpc: Exclude non-owned addresses from listreceivedby*
What the developer wrote
Authored by Ava Chow
Merge bitcoin/bitcoin#35925: wallet, rpc: Exclude non-owned addresses from listreceivedby*
089c883c558e01c2a18a92f861fa7de9a7cc607a test: Add coverage for listreceivedby* excluding "send" addresses (pablomartin4btc)
873c0548059b2860b3718551078763d227186bd6 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 089c883c558e01c2a18a92f861fa7de9a7cc607a
jeanpablojp:
ACK 089c883c558e01c2a18a92f861fa7de9a7cc607a
achow101:
ACK 089c883c558e01c2a18a92f861fa7de9a7cc607a
Tree-SHA512: d45488c93b9294258faaab5d1891ca5e8c4b8d0d4feb298403c7c3f20d6aa08989d548cddd25ccd47a1ed969e4a309ee68ad1541c6121fed39ed534c78c256e7
What changed, and why it matters
This update fixes a wallet reporting bug. When a user asked their Bitcoin wallet to list every address that had received money, including empty ones, the result incorrectly included foreign addresses that the wallet merely knew about (for example, an address labeled as a 'send' recipient). The fix ensures only addresses actually owned by the wallet are shown. It is a privacy/information-disclosure issue, not a theft-of-funds bug.
No urgent action required; this is a low-severity privacy fix. Users and integrators relying on listreceivedbyaddress/listreceivedbylabel with include_empty=true should upgrade to avoid seeing unrelated external addresses in results. Review whether any downstream tools parsed these foreign entries.
Security signals we found
Information disclosure: wallet RPCs leaked existence/labels of foreign addresses
Incorrect access-control boundary: non-owned addresses exposed in 'received' report
Fix uses IsMine() rather than address purpose metadata, avoiding inconsistent purpose handling
Evidence from the diff
listreceivedbyaddress and listreceivedbylabel with include_empty=true iterate the address book and return entries lacking a mapTally record. Previously this included non-owned addresses that had a label via setlabel, the GUI, or addmultisigaddress. The patch adds an IsMine() check for addresses missing from mapTally, since mapTally is only populated for IsMine addresses. A functional test verifies that a setlabel’d external address is excluded.
Changed components
src/wallet/rpc/transactions.cppwallet listreceivedbyaddress RPCwallet listreceivedbylabel RPCInspect captured patch +19 / −3
### src/wallet/rpc/transactions.cpp
@@ -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();
### test/functional/wallet_listreceivedby.py
@@ -101,6 +101,17 @@ def run_test(self):
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 1Why this scored 28/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.