wallet: fix mixed-input transaction accounting in history RPCs #34872

pull w0xlt wants to merge 13 commits into bitcoin:master from w0xlt:wallet-mixed-input-history-only changing 11 files +993 −59
  1. w0xlt commented at 10:47 PM on March 19, 2026: contributor

    When a wallet owns only some inputs of a transaction, such as in CoinJoins, payjoins, or other collaborative transactions, wallet history RPCs currently apply normal send accounting.

    That works when all inputs are wallet-owned, but breaks for mixed-input transactions.

    Example:

    Inputs:

    1.00 BTC wallet-owned
    2.00 BTC foreign
    

    Outputs:

    0.80 BTC wallet-owned
    2.19 BTC non-wallet
    

    The wallet can safely know:

    wallet_debit  = 1.00
    wallet_credit = 0.80
    wallet_net    = -0.20
    

    But it 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. This is especially important after restore, rescan, or descriptor import, where any local transaction-intent metadata may be missing.

    This PR adds a conservative fallback for that no-metadata case:

    • classify wallet transactions as having no wallet inputs, partial wallet inputs, or all wallet inputs
    • keep existing send/fee accounting when all inputs are wallet-owned
    • also keep normal per-output send/fee accounting for mixed-input transactions when every non-wallet input has a known zero value
    • otherwise report a single unattributed aggregate send entry with no address, no vout, and no fee, carrying the negative total of wallet-owned inputs spent
    • report wallet-owned outputs as receive entries, so the transaction's entries sum to the wallet's net change
    • expose involves_mixed_inputs, wallet_debit, and wallet_credit
    • document that vout and fee are optional when attribution is unavailable

    This does not try to fully solve collaborative transaction attribution. A richer layer with foreign-output / fee-share / intent metadata would still be useful for wallet-created transactions. This PR only makes the fallback honest when that metadata is unavailable.

    Addresses #14136

  2. DrahtBot added the label Wallet on Mar 19, 2026
  3. DrahtBot commented at 10:47 PM on March 19, 2026: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

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

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

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

    Type Reviewers
    Concept NACK achow101
    Concept ACK rkrux

    If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #35587 (Remove boost as a unit test runner by rustaceanrob)
    • #35501 (wallet: store all witness variants of a transaction by achow101)
    • #27865 (wallet: Track no-longer-spendable TXOs separately by achow101)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

    LLM Linter (✨ experimental)

    Possible places where named args for integral literals may be used (e.g. func(x, /*named_arg=*/0) in C++, and func(x, named_arg=0) in Python):

    • AppendWalletTxEntries(*pwallet, wtx, 0, true, ret, filter_label) in src/wallet/rpc/transactions.cpp
    • AppendWalletTxEntries(wallet, tx, 0, true, transactions, filter_label, include_change) in src/wallet/rpc/transactions.cpp
    • AppendWalletTxEntries(wallet, it->second, -100000000, true, removed, filter_label, include_change) in src/wallet/rpc/transactions.cpp

    <sup>2026-07-08 08:32:03</sup>

  4. luke-jr commented at 2:56 PM on March 20, 2026: member

    This won't show even the true send anymore... not sure that's a good idea.

    See #25991 for what is needed to fix this properly.

  5. w0xlt commented at 5:40 AM on March 22, 2026: contributor

    The goal here, in this PR, is a smaller immediate fix: for mixed-input transactions, the history RPCs should stop reporting send entries and full fees when the wallet cannot attribute them objectively.

    #25991 is an interesting direction, but as written it depends on manually providing additional wallet-local per-output metadata. Without that metadata, we still need a safe fallback, and that is what this PR provides.

  6. rkrux commented at 12:24 PM on March 23, 2026: contributor

    As I read the related issue #14136, it's unfortunate that the RPC shows incorrect information in the result - glad that this PR tries to address it.

    However, I'm leaning towards a NACK because this PR removes the send category details of such transactions altogether, which is not correct. The wallet does participate (though partially) in sending funds within these transactions and not showing those can be problematic as well.

    From #34872#issue-4104631274

    Fixes this by introducing CachedTxIsFromMeForAccounting,

    I am also not in favour of having two different views of CachedTxIsFromMe - one without accounting and one with. Having two views is also a sign that there is mismatch between what the wallet shows to the user and what the wallet does for the user.

    From #34872 (comment):

    Without that metadata, we still need a safe fallback, and that is what this PR provides.

    Due to the above reasons, I don't believe this is a safe fallback, and it only replaces one issue by another.

    From #34872#issue-4104631274

    This is because the accounting code treats any nDebit > 0 (i.e. any wallet-owned input) as a full wallet spend.

    It appears that this assumption made by the wallet doesn't hold true anymore and maybe it needs to be reworked for this issue to be completely fixed.

  7. w0xlt commented at 8:15 PM on March 23, 2026: contributor

    @rkrux Thanks for the review. I agree this isn’t a complete solution for mixed-input accounting.

    The goal here is narrower: to prevent history RPCs from reporting send entries and full fees that the wallet cannot objectively attribute from the transaction alone.

    So I see this as an improvement over the current behavior.

    A complete solution likely requires additional attribution metadata. However, that’s tricky - even if we add something like #25991, or a more automated way to set it, the metadata would not survive an importdescriptors restore, since there’s currently no way to import it.

    In those cases, we would still need a conservative fallback for history RPCs when attribution is unavailable.

    Do you see another way to address this without relying on additional metadata?

  8. achow101 commented at 8:55 PM on May 19, 2026: member

    NACK

    I think it's even more misleading to not show any send entries at all for transactions where we participate in the inputs.

  9. murchandamus commented at 9:56 PM on May 19, 2026: member

    At first glance, I would expect our wallet to be able to recognize which inputs are ours, and I’m astonished that it would just assume that all of the inputs must be ours when it recognizes one. Would it perhaps be possible to recognize such transactions as “multi user transactions” and only apply the inputs that came from our wallet as debits? If our wallet’s balance is decreased by the transaction, we might recognize any outputs from the transaction to our wallet as change output, whereas if the wallet’s balance is increased by the transaction (e.g., a Payjoin that paid us), we would treat the output as an incoming payment. The transaction should then be labeled as having us paid the amount that our wallet balance increased.

  10. achow101 commented at 10:23 PM on May 19, 2026: member

    it would just assume that all of the inputs must be ours when it recognizes one

    That's not really what it is doing. It is correctly treating transactions that have any inputs belonging to the wallet as that transaction belonging to the wallet. The "assumption" comes from CachedTxGetDebit only retrieving the total amount of our inputs, which only works when all of the inputs of the transaction belong to the wallet.

    But without storing more information, there is nothing that we can do to fix that. It shouldn't be astonishing that we are unable to calculate fees when the wallet does not know how much value the inputs that it doesn't know about are providing to the transaction.

    The transaction should then be labeled as having us paid the amount that our wallet balance increased.

    This isn't about annotating Bitcoin transactions, it's about annotating logical financial transactions and trying to determine what they are from a single Bitcoin transaction. Because a Bitcoin transaction can have multiple outputs, it can have multiple logical transactions, and the wallet tries to display these as such. For example, a multi party transaction could conceivably contain an output created by someone else in that transaction which pays us, while the same transaction simultaneously has an output that we create paying someone else. These should show as two entries in listtransactions - one showing a receive and one showing the send. The issue is with annotating the rest of the outputs (we don't store the logical transactions, so how do we know that the remaining outputs are not sends initiated by the user), and showing the correct amount for the fee (we don't know about the inputs that aren't ours, thus we cannot accurately calculate the fees).

    The solution presented in this PR is to omit all of that logical transaction information entirely for multi party transactions, which I disagree with.

  11. w0xlt force-pushed on May 20, 2026
  12. w0xlt commented at 1:41 PM on May 20, 2026: contributor

    @achow101 @murchandamus Thanks for the feedback.

    I pushed a new proposal that addresses it.

    As I understand it, a complete solution has two layers:

    1. A conservative fallback for mixed-input transactions when the wallet has no extra attribution metadata.

    
2. A richer metadata layer, e.g. foreign-output / ownership / fee-share marks, for wallet-created collaborative transactions.

    The idea here to solve the layer 1 only. Layer 2 is still useful, but it cannot replace the fallback. If a wallet is restored from descriptors, rescanned, or imported without the original local metadata, the wallet still needs a reliable way to report history without inventing send outputs or fees.

  13. w0xlt commented at 1:47 PM on May 20, 2026: contributor

    The new proposal:



    In master, wallet history accounting effectively does this when any input is ours:

    wallet_debit = sum(wallet-owned inputs)
    fee = wallet_debit - total_outputs
    send entries = all non-change outputs
    

    That works only when all inputs are wallet-owned.

    Example:

    Inputs:

        1.00 BTC mine
        2.00 BTC foreign
    

    Outputs:

        0.80 BTC mine
        2.19 BTC foreign
    

    Master can calculate wallet_debit = 1.00, but it does not know/store the foreign input value reliably in wallet history. So subtracting all outputs from only wallet-owned inputs gives wrong accounting:

    fee = 1.00 - 2.99 = -1.99

    It can also report foreign outputs as wallet send entries, even though the wallet cannot know from the transaction alone whether those outputs are wallet's payment, another participant’s change, or something else.

    This new push changes the previous fallback model to:

    wallet_debit  = sum(wallet-owned inputs)
    wallet_credit = sum(wallet-owned outputs)
    amount        = wallet_credit - wallet_debit
    

    For the same example:

    wallet_debit  = 1.00
    wallet_credit = 0.80
    amount        = -0.20
    fee_known     = false
    category      = mixed
    

    So the wallet reports the part it can prove: “the wallet balance went down by 0.20 BTC.” It does not guess the fee share or recipient attribution.


  14. w0xlt commented at 1:49 PM on May 20, 2026: contributor

    Moving this to draft for further discussion.

  15. w0xlt marked this as a draft on May 20, 2026
  16. w0xlt commented at 2:02 PM on May 20, 2026: contributor

    PR description updated.

  17. rkrux commented at 2:50 PM on May 20, 2026: contributor

    Do you see another way to address this without relying on additional metadata?

    Sorry, I had forgotten about this PR. I would prefer a basic conservative solution first without relying on additional metadata by letting the wallet return correct information (as much as it can) while not showing incorrect responses based on any missing transaction data.

    Based on a cursory glance, the new proposal seems to be in this^ direction, will review.

  18. DrahtBot added the label CI failed on May 20, 2026
  19. w0xlt commented at 5:13 PM on May 20, 2026: contributor

    CI error unrelated

  20. DrahtBot removed the label CI failed on May 21, 2026
  21. rkrux commented at 2:16 PM on May 21, 2026: contributor

    Concept ACK 29fcb54dac4a8f4b45573ba20b97874eec81be9d

    The new approach lgtm, will review when taken out of draft.

  22. w0xlt marked this as ready for review on May 21, 2026
  23. w0xlt commented at 5:07 PM on May 21, 2026: contributor

    Done.

  24. w0xlt force-pushed on Jun 15, 2026
  25. w0xlt force-pushed on Jun 15, 2026
  26. DrahtBot added the label CI failed on Jun 15, 2026
  27. DrahtBot removed the label CI failed on Jun 15, 2026
  28. w0xlt force-pushed on Jun 18, 2026
  29. w0xlt commented at 12:27 AM on June 19, 2026: contributor

    PR description updated.

  30. DrahtBot added the label Needs rebase on Jun 19, 2026
  31. w0xlt force-pushed on Jun 22, 2026
  32. w0xlt force-pushed on Jun 22, 2026
  33. DrahtBot added the label CI failed on Jun 22, 2026
  34. DrahtBot removed the label Needs rebase on Jun 22, 2026
  35. DrahtBot removed the label CI failed on Jun 22, 2026
  36. w0xlt force-pushed on Jun 23, 2026
  37. wallet: cache transaction input ownership
    Add a cached helper that classifies wallet transaction inputs as all,
    partial, or none owned by the wallet. This lets history code distinguish
    mixed-input transactions without repeating input ownership scans.
    3d1134d11f
  38. wallet: add history accounting helper
    Add a cached history accounting helper that returns input ownership,
    wallet debit, wallet credit, and fee information in one place before
    the RPC code consumes it.
    2d5e4ee602
  39. wallet: invalidate child input caches on parent insertion
    Invalidate cached input ownership for wallet transactions that spend
    outputs from a newly inserted parent, so later imports can refresh child
    accounting.
    7eab5306be
  40. wallet,rpc: rename transaction entry helper 40d2e72f16
  41. wallet,rpc: report unattributable mixed-input history conservatively
    Use aggregate wallet debit for mixed-input transaction history when
    outputs and fees cannot be attributed to specific inputs. Expose wallet
    debit and credit fields so callers can identify the conservative
    accounting.
    0d3f1141ef
  42. w0xlt force-pushed on Jul 7, 2026
  43. w0xlt commented at 8:07 PM on July 7, 2026: contributor

    Split the PR into more commits based on offline feedback

  44. DrahtBot added the label CI failed on Jul 7, 2026
  45. w0xlt force-pushed on Jul 7, 2026
  46. DrahtBot removed the label CI failed on Jul 7, 2026
  47. test: cover conservative mixed-input history accounting
    Add functional coverage for conservative mixed-input history entries in
    gettransaction, listtransactions, and listsinceblock, including filtered
    history and removed entries.
    46f70e01ed
  48. wallet: cache zero-value foreign input state
    Track whether every non-wallet input of a mixed-input wallet
    transaction is known to spend a zero-value output.
    
    This is not used for attribution yet. It lets later history accounting
    differentiate unknown or nonzero foreign contributions from foreign
    inputs that provably cannot fund outputs or fees.
    e04e0906bd
  49. wallet,rpc: attribute zero-value foreign mixed inputs
    Use normal per-output send and fee accounting for mixed-input
    transactions when every non-wallet input is known to have zero value.
    
    Unknown or positive-value foreign inputs remain conservative because
    the wallet cannot attribute recipient outputs or its fee share from the
    transaction alone.
    056ccd7bab
  50. test: cover zero-value foreign input accounting
    Add functional coverage for a mixed-input transaction that spends a
    wallet-owned input together with a zero-value pay-to-anchor input.
    
    The test asserts that the wallet keeps normal send and fee entries,
    while still marking the transaction as involving mixed inputs.
    9b422a2237
  51. test: cover late mixed-input parent import
    Exercise the case where a wallet first sees a mixed-input child
    transaction before seeing the parent that creates a zero-value foreign
    input.
    
    After the parent is imported, the child history accounting must be
    recomputed and switch from conservative aggregate accounting to normal
    send and fee attribution.
    a4193188c5
  52. test: cover known nonzero foreign inputs
    Add coverage showing that a mixed-input transaction remains
    conservatively reported when the wallet knows a foreign input has
    nonzero value but still cannot attribute the fee share.
    242acb8af8
  53. test: cover zero-value wallet inputs
    Add coverage for mixed-input transactions where the wallet-owned input
    has zero value, ensuring history still reports the wallet credit with a
    zero aggregate debit.
    cf1b676c3d
  54. doc: add mixed-input history RPC release note
    Document how wallet history RPCs report transactions that spend both
    wallet-owned and non-wallet inputs.
    
    Mention the normal attribution allowed for known zero-value foreign
    inputs, the conservative aggregate fallback, and the new mixed-input
    metadata fields.
    eba63bfe77
  55. in test/functional/wallet_gettransaction_mixed_inputs.py:614 in 92a5b26e0c
     609 | +        since = [entry for entry in alice.listsinceblock(before_blockhash)["transactions"] if entry["txid"] == txid]
     610 | +        self.assert_mixed_history_entries(since, txid, expected_net, alice_change_address, change_vout, alice_debit, alice_credit)
     611 | +
     612 | +        # The same conservative shape holds after confirmation.
     613 | +        node.generatetoaddress(1, funder.getnewaddress(), called_by_framework=True)
     614 | +        node.syncwithvalidationinterfacequeue()
    


    maflcko commented at 5:38 AM on July 8, 2026:

    What is this sync needed for on block events? Also, the called_by_framework in the prior line doesn't make sense?


    w0xlt commented at 8:33 AM on July 8, 2026:

    Fixed. Thanks.

  56. w0xlt force-pushed on Jul 8, 2026

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-07-09 06:47 UTC