#34872 wallet: fix mixed-input transaction accounting in history RPCs
| Reviewability | Reviews | Agreement | Size | ||||
|---|---|---|---|---|---|---|---|
| #34872 wallet: fix mixed-input transaction accounting in history RPCs | Needs rebase | 0 (+1) -1 | Mild | 270 + 828 tests | |||
|
|
|
| ||||
Reviewability: ReadyStalePaused Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs. Size: lines added or modified outside tests, then in tests; greener = smaller. | |||||||
Goal
Classifies transaction input ownership in wallet history accounting helpers and updates gettransaction, listtransactions, and listsinceblock for mixed-input transactions. When foreign inputs have unknown or non-zero value, the RPCs report a single unattributed aggregate send debit with no fee or recipient address, alongside normal receive entries for wallet outputs. If foreign inputs are known to have zero value (such as PayToAnchor outputs), standard per-output send and fee attribution is retained, and new RPC fields (involves_mixed_inputs, wallet_debit, and wallet_credit) are added.
Problem: When a wallet transaction spends a mix of wallet and external inputs (such as CoinJoins or Payjoins), wallet history RPCs assume all inputs belong to the wallet. This fabricates false recipient send entries and produces incorrect or negative fee calculations, which cannot be reconstructed from chain data alone after a rescan or descriptor import.
Evidence of importance:
- Addresses issue #14136 ('rpc: Wrong `gettransaction` info for a coinjoin'), where gettransaction reports inaccurate send entries and fees for collaborative transactions
- Description notes that without intent metadata, the wallet 'cannot reliably know, from wallet history alone, the foreign input value, the total fee, the wallet's fee share, or which non-wallet output should be attributed as the user's payment'
Scope: Changes RPC output schema by making fee and vout optional on unattributed send entries, adds wallet input ownership caching and invalidation across parent-child spends, and adds a dedicated 700-line functional test file.
Risk of a mistake: medium (wallet, rpc_api). Mistakes in wallet history accounting or cache invalidation could return incorrect balances and transaction details to external applications or break downstream parsers expecting mandatory fee and vout fields.
Changed since the previous assessment: Refines locking semantics by requiring cs_wallet for CachedTxGetAmounts, renames ListTransactions to AppendWalletTxEntries, and adds cache invalidation for spending transactions via MarkOutputsDirty on parent insertion.
Reviewability
Ready · Needs rebase
Conflicts with master; review the logic now, the rebase is mechanical.
Waiting on (from the thread read):
- reviewer: further review of the updated PR 2026-10-07
Agreement
Mild
NACK from achow101 not withdrawn (omitting send entries entirely is misleading when the wallet participates in transaction inputs)
Thread notes: Added author's comment c:6028458814 discussing follow-up GUI drafts with arejula27; PR is awaiting further reviewer feedback.
Objections, suggestions, and questions:
| Reviewer | Type | Harm, request, or question | Status | Must be | Author replied | Quote |
|---|---|---|---|---|---|---|
| luke-jr (contributor) | objection, approach | mixed-input transactions will not show even true send entries | fixed · fix pushed · author reworked the proposal to retain aggregate send and receive entries | changed | yes | 2026-03-20: 'This won't show even the true send anymore... not sure that's a good idea.' Status from: 2026-05-20: w0xlt: 'I pushed a new proposal that addresses it.' |
| rkrux (member) | objection, approach | omitting send category details altogether hides the wallet's partial participation in mixed-input transactions | fixed · fix pushed · author pushed a reworked proposal and rkrux confirmed the new approach lgtm | changed | yes | 2026-03-23: 'this PR removes the `send` category details of such transactions altogether, which is not correct.' Status from: 2026-05-21: rkrux: 'The new approach lgtm, will review when taken out of draft.' |
| achow101 (member) | objection, approach | omitting send entries entirely is misleading when the wallet participates in transaction inputs | fixed · fix pushed · author pushed a reworked proposal retaining send accounting entries | changed | yes | 2026-05-19: 'I think it's even more misleading to not show any send entries at all for transactions where we participate in the inputs.' Status from: 2026-05-20: w0xlt: 'I pushed a new proposal that addresses it.' |
| maflcko (member) | question, correctness | what is the sync needed for on block events and why called_by_framework is used | answered · author fixed the test script | answered | yes | 2026-07-08: 'What is this sync needed for on block events? Also, the `called_by_framework` in the prior line doesn't make sense?' Status from: 2026-07-08: w0xlt: 'Fixed. Thanks.' |
| murchandamus (member) | suggestion, interface | find a clearer description than 'cannot be attributed' or 'unattributed' for mixed-input transactions | answered · author clarified RPC descriptions and removed unattributed wording | answered | yes | 2026-07-15: 'The "cannot be attributed" here also confuses me. FWIU at this point in the review, when no outputs go to ourselves, or all outputs go ourselves, we consider it "attributable", but if there are both f' Status from: 2026-09-22: w0xlt: 'Clarified the RPC descriptions and removed the remaining “unattributed” wording from the help text. Thanks!' |
| murchandamus (member) | objection, correctness | why non-wallet spent TXO values are not always known when stored in the wallet or processed from blocks | answered · author explained wallet parent tx limitations, updated release notes, and added tests | answered | yes | 2026-07-15: 'The "known zero value" keeps throwing me for a loop. Transactions can only spend TXOs that are in our UTXO set when we first see them. Therefore, we should always know every spent TXO’s value when we ' Status from: 2026-09-22: w0xlt: 'To verify that every non-wallet input has zero value, the wallet currently uses only parent transactions stored in the w' |
| arejula27 (contributor) | suggestion, interface | represent mixed transactions as a single entry with net change amount and a subfield with spent and created wallet UTXOs | accepted · reviewer accepted deferring the format change to a follow-up PR | answered | yes | 2026-09-26: 'What I would suggest discussing is keeping it but splitting both views inside the entry: a single entry per mixed transaction whose `amount` is the net change (`credit - debit`), and a subfield with t' Status from: 2026-10-06: arejula27: 'Yes, I'd be happy to open a draft PR on top of this one' |
| Bicaru20 (contributor) | suggestion, maintenance | avoid duplicate call to CachedTxGetHistoryAccounting by returning the struct from CachedTxGetAmounts | answered · fix pushed · author adopted the suggestion in code | answered | yes | 2026-09-29: 'I think here we could avoid calling `CachedTxGetHistoryAccounting` since we already do that in `CachedTxGetAmounts`.' Status from: 2026-10-06: w0xlt: 'Thanks. Done. Added you as co-author.' |
Support:
- rkrux (member) (Concept ACK, 2026-05-21): The new approach lgtm, will review when taken out of draft. [not substantive]
- murchandamus (member) (2026-07-15) [read]: This sounds like a great improvement regarding multi-user transaction accounting.
- arejula27 (contributor) (Concept ACK, 2026-09-26) [read]: I agree it is better to omit information than to show information that is wrong.
- Bicaru20 (contributor) (ACK, 2026-09-29) [both]: history RPC never show information the wallet cannot actually know. This way the wallet will be more honest and less confusing when showing the histroy through RPCs and the user will be able to see wh
Participants:
- luke-jr (contributor, objection, 1 statements 2026-03-20 to 2026-03-20): objected that mixed-input transactions would not show even true send entries
- rkrux (member, support, 3 statements 2026-03-23 to 2026-05-21): initially leaned NACK, then gave Concept ACK to the reworked proposal
- achow101 (member, objection, 2 statements 2026-05-19 to 2026-05-19): NACKed omitting send entries entirely when the wallet participates in inputs
- murchandamus (member, objection, 24 statements 2026-05-19 to 2026-07-15): supported concept as an improvement but questioned terminology and whether spent TXO values could be known
- maflcko (member, question, 1 statements 2026-07-08 to 2026-07-08): asked about test synchronization and called_by_framework in test script
- sedited (member, neutral, 1 statements 2026-09-09 to 2026-09-09): asked author to rebase and respond to review comments
- arejula27 (contributor, support, 2 statements 2026-09-26 to 2026-10-06): Concept ACK; suggested format change but agreed to defer it to a follow-up PR
- Bicaru20 (contributor, support, 2 statements 2026-09-29 to 2026-09-29): ACK; tested on regtest and suggested small optimization avoiding duplicate accounting call
Files
839 lines under test/bench/ci.
- test/functional/wallet_gettransaction_mixed_inputs.py +700/-0
- src/wallet/rpc/transactions.cpp +103/-37
- src/wallet/receive.cpp +91/-14
- test/functional/wallet_listtransactions.py +69/-11
- src/wallet/test/wallet_tests.cpp +58/-0
- doc/release-notes-34872.md +26/-0
- src/wallet/wallet.cpp +19/-0
- src/wallet/receive.h +15/-2
- src/wallet/transaction.h +12/-0
- src/wallet/wallet.h +4/-0
- test/functional/test_runner.py +1/-0
Card
The short text the ranking pass compares across PRs.
Fixes wallet history accounting in gettransaction, listtransactions, and listsinceblock for mixed-input transactions such as CoinJoins and Payjoins. Currently, the wallet assumes all inputs are wallet-owned, creating fabricated recipient entries and reporting incorrect or negative fees (issue #14136). The PR replaces speculative attribution with a conservative fallback that reports an unattributed aggregate send debit and omits the fee unless foreign inputs are verified zero-value (such as anchor outputs). It also adds involves_mixed_inputs, wallet_debit, and wallet_credit metadata fields to the RPC output.
Data
thread read through 2026-10-07 (73 statements), head 9d82e07236
labels: Wallet, Needs rebase
Categories
Wallet
P2 · bug fix
Fixes a long-standing accounting flaw where history RPCs assume all inputs belong to the wallet for mixed-input transactions like CoinJoins. As noted in issue #14136, this 'fabricates false recipient send entries and produces incorrect or negative fee calculations' in gettransaction and listtransactions.
Membership: Carries the Wallet label and modifies wallet transaction accounting and history RPCs in src/wallet/.
Factors: security/stability 0, bug 2, performance 0, user value 2, leverage 0