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

pull w0xlt wants to merge 16 commits into bitcoin:master from w0xlt:wallet-mixed-input-history-only changing 11 files +1095 −60
  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
    ACK Bicaru20
    Concept NACK achow101
    Concept ACK rkrux, arejula27

    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:

    • #36339 (wallet: avoid int overflow in listtransactions by kriss39)
    • #36257 (qa: assert_equals -> assert_true/assert_false by hodlinator)
    • #35786 (wallet: drop spent parents redundant cache invalidation and notification by furszy)

    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, *pwtx, 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-09-23 18:04:25</sup>

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

    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. in src/wallet/receive.cpp:206 in cde6efd3d2
     211 | -        //   2) the output is to us (received)
     212 | -        if (nDebit > 0)
     213 | +        // Only need to handle txouts if either:
     214 | +        //   1) every input is ours, so the output can be reported as sent
     215 | +        //   2) the output is ours, so it can be reported as received
     216 | +        if (all_inputs_mine)
    


    murchandamus commented at 9:15 PM on June 23, 2026:

    How would this resolve if there were a P2A input?


    w0xlt commented at 6:59 AM on September 22, 2026:

    The P2A case is covered now. If the wallet can verify that all non-wallet inputs have zero value, we use master’s existing per-output format, including the fee. There’s a test for this before and after confirmation.

  38. in doc/release-notes-34872.md:5 in 42044ec295 outdated
       0 | @@ -0,0 +1,26 @@
       1 | +Wallet
       2 | +------
       3 | +
       4 | +- Transactions that spend a mix of wallet-owned and non-wallet inputs are now
       5 | +  reported safely by the `gettransaction`, `listtransactions`, and
    


    murchandamus commented at 8:13 PM on June 30, 2026:

    In "doc: add mixed-input history RPC release note" (eba63bfe77c148a447f44ba0703cb211c7a3213d): I’m not sure how safety is an issue here. How about "reported correctly", "with improved attribution", "in more detail", or similar?

      broken down in more detail by the `gettransaction`, `listtransactions`, and
    

    w0xlt commented at 6:01 AM on September 22, 2026:

    Changed to “reported more accurately”. Thanks!

  39. w0xlt force-pushed on Jul 7, 2026
  40. w0xlt commented at 8:07 PM on July 7, 2026: contributor

    Split the PR into more commits based on offline feedback

  41. DrahtBot added the label CI failed on Jul 7, 2026
  42. w0xlt force-pushed on Jul 7, 2026
  43. DrahtBot removed the label CI failed on Jul 7, 2026
  44. 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.

  45. w0xlt force-pushed on Jul 8, 2026
  46. in src/wallet/test/wallet_tests.cpp:487 in 3d1134d11f outdated
     478 | @@ -479,6 +479,40 @@ BOOST_FIXTURE_TEST_CASE(ListCoinsTest, ListCoinsTestingSetup)
     479 |      BOOST_CHECK_EQUAL(list.begin()->second.size(), 2U);
     480 |  }
     481 |  
     482 | +BOOST_FIXTURE_TEST_CASE(CachedInputOwnershipTest, ListCoinsTestingSetup)
     483 | +{
     484 | +    LOCK(wallet->cs_wallet);
     485 | +
     486 | +    const COutPoint mine_outpoint{m_coinbase_txns[0]->GetHash(), 0};
     487 | +    const COutPoint foreign_outpoint{Txid::FromUint256(uint256::ONE), 0};
    


    murchandamus commented at 9:13 PM on July 15, 2026:

    In "wallet: cache transaction input ownership" (3d1134d11fbcbbe4e62a00be06f04648e1bf5e90):

    I’d be curious how an anyone-can-spend output such as a P2A output is categorized by InputIsMine(…), and it might also be a good addition to this test.


    w0xlt commented at 11:22 PM on September 21, 2026:

    This behavior predates the PR. InputIsMine() checks whether the spent output belongs to the wallet. Being anyone-can-spend does not automatically imply ownership:

      const CWalletTx* prev = wallet.GetWalletTx(txin.prevout.hash);
      if (prev && txin.prevout.n < prev->GetTx()->vout.size()) {
          return wallet.IsMine(prev->GetTx()->vout[txin.prevout.n]);
      }
      return false;
    

    Added a P2A case with its parent stored in the wallet, checking InputIsMine() == false, NONE for P2A alone, and PARTIAL alongside a wallet-owned input.

  47. in src/wallet/rpc/transactions.cpp:298 in 40d2e72f16 outdated
     294 | @@ -295,7 +295,7 @@ static void MaybePushAddress(UniValue & entry, const CTxDestination &dest)
     295 |  }
     296 |  
     297 |  /**
     298 | - * List transactions based on the given criteria.
     299 | + * Append RPC entries for a wallet transaction based on the given criteria.
    


    murchandamus commented at 9:18 PM on July 15, 2026:

    In "wallet,rpc: rename transaction entry helper" (40d2e72f16c099a15688fa0cc26b7f0bf258636c): I’m not sure I understand the motivation for this rename. This RPC would print transactions to the terminal based on the given criteria, right? What is it appending to?

    Could you please add a short explanation to the commit message why this RPC was renamed?


    w0xlt commented at 1:12 AM on September 22, 2026:

    Added the rationale to the commit message. This internal helper appends entries for one wallet transaction to the caller's result container.

    Could you please add a short explanation to the commit message why this RPC was renamed?

    The public RPC names remain unchanged.

  48. in src/wallet/rpc/transactions.cpp:309 in 0d3f1141ef outdated
     304 | +    entry.pushKV("involves_mixed_inputs", true);
     305 | +    entry.pushKV("wallet_debit", ValueFromAmount(accounting.debit));
     306 | +    entry.pushKV("wallet_credit", ValueFromAmount(accounting.credit));
     307 | +}
     308 | +
     309 | +// When a mixed-input transaction has unattributable outputs or fee share, the
    


    murchandamus commented at 9:21 PM on July 15, 2026:

    In "wallet,rpc: report unattributable mixed-input history conservatively" (0d3f1141ef76ef3cbd527f5f1a96518a252f2aed): What do you mean with "unattributable outputs"? Do you perhaps mean that no outputs go to our wallet, i.e., that credit is zero?


    w0xlt commented at 1:19 AM on September 22, 2026:

    I meant that we cannot determine the wallet’s contribution to individual outputs or the fee. This does not imply zero credit: wallet-owned outputs are still reported as receives. Clarified the comment.

  49. in src/wallet/rpc/transactions.cpp:307 in 0d3f1141ef outdated
     302 | +static void PushMixedInputFields(UniValue& entry, const WalletTxHistoryAccounting& accounting)
     303 | +{
     304 | +    entry.pushKV("involves_mixed_inputs", true);
     305 | +    entry.pushKV("wallet_debit", ValueFromAmount(accounting.debit));
     306 | +    entry.pushKV("wallet_credit", ValueFromAmount(accounting.credit));
     307 | +}
    


    murchandamus commented at 9:25 PM on July 15, 2026:

    In "wallet,rpc: report unattributable mixed-input history conservatively" (0d3f1141ef76ef3cbd527f5f1a96518a252f2aed): Even if there are mixed inputs, either the debit or credit could have a greater magnitude. Could you perhaps add an explanation in the commit message why a transaction that overall reduces the balance in the wallet would not be categorized as a send, and a transaction that overall increases the balance in the wallet as a receive? I assume it is because it would be hard to distinguish e.g., a Payjoin in which we get paid and the sender gets a change output from a Payjoin transaction in which we get paid by a sender and also pay another receiver, but it would be good to document the rationale somewhere.


    murchandamus commented at 9:27 PM on July 15, 2026:

    I still think that there might be a special case here, in which there are mixed inputs, but all outputs go to our wallet, in which we might want to categorize it as a receive?


    w0xlt commented at 2:56 AM on September 22, 2026:

    Added the rationale to the commit message: receives preserve each output’s details; the send records wallet inputs used. Added tests where all outputs belong to the wallet, before and after confirmation.

  50. in src/wallet/rpc/transactions.cpp:470 in 0d3f1141ef
     464 | @@ -424,7 +465,11 @@ RPCMethod listtransactions()
     465 |                  "transactions specified in the 'skip' argument. A transaction can have multiple entries in this RPC response. \n"
     466 |                  "For instance, a wallet transaction that pays three addresses — one wallet-owned and two external — will produce \n"
     467 |                  "four entries. The payment to the wallet-owned address appears both as a send entry and as a receive entry. \n"
     468 | -                "As a result, the RPC response will contain one entry in the receive category and three entries in the send category.\n",
     469 | +                "As a result, the RPC response will contain one entry in the receive category and three entries in the send category.\n"
     470 | +                "A transaction with both wallet-owned and non-wallet inputs cannot attribute the foreign outputs or its \n"
     471 | +                "fee share, so it reports a single unattributed aggregate send entry (marked with involves_mixed_inputs and \n"
    


    murchandamus commented at 9:33 PM on July 15, 2026:

    In "wallet,rpc: report unattributable mixed-input history conservatively" (0d3f1141ef76ef3cbd527f5f1a96518a252f2aed): I find this description confusing. What is "unattributed" about the "aggregate send entry"?

    Perhaps: "

    -                so it reports a single unattributed aggregate send entry (marked with involves_mixed_inputs and \n"
    -                "without address, vout, or fee) carrying the negative total of wallet-owned inputs spent. Wallet-owned outputs \n"
    +                so it reports the sum of the wallet-owned inputs as a single aggregate send entry (marked with involves_mixed_inputs and \n"
    +                "without address, vout, or fee). Wallet-owned outputs \n"
    

    w0xlt commented at 4:06 AM on September 22, 2026:

    Taken, thanks.

  51. in src/wallet/rpc/transactions.cpp:487 in 0d3f1141ef outdated
     483 | @@ -439,17 +484,17 @@ RPCMethod listtransactions()
     484 |                          {
     485 |                              {RPCResult::Type::STR, "address",  /*optional=*/true, "The bitcoin address of the transaction (not returned if the output does not have an address, e.g. OP_RETURN null data)."},
     486 |                              {RPCResult::Type::STR, "category", "The transaction category.\n"
     487 | -                                "\"send\"                  Transactions sent.\n"
     488 | +                                "\"send\"                  Transactions sent. For mixed-input transactions whose outputs or fee share cannot be attributed, a single unattributed aggregate send entry carries the negative total of wallet-owned inputs spent.\n"
    


    murchandamus commented at 9:35 PM on July 15, 2026:

    In "wallet,rpc: report unattributable mixed-input history conservatively" (0d3f1141ef76ef3cbd527f5f1a96518a252f2aed):

    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 foreign-owned inputs and foreign-owned outputs, we call it "not attributable". If that’s the correct understanding, perhaps we can find a more speaking description of that?


    w0xlt commented at 4:41 AM on September 22, 2026:

    Updated the help text. With mixed inputs, we report one send entry for all wallet inputs, regardless of who owns the outputs.

    If every non-wallet input is known to have zero value, we use master’s existing per-output format: a separate send entry for each non-change output.

  52. in src/wallet/rpc/transactions.cpp:605 in 0d3f1141ef
     600 | @@ -556,7 +601,11 @@ RPCMethod listsinceblock()
     601 |          "listsinceblock",
     602 |          "Get all transactions in blocks since block [blockhash], or all transactions if omitted.\n"
     603 |                  "If \"blockhash\" is no longer a part of the main chain, transactions from the fork point onward are included.\n"
     604 | -                "Additionally, if include_removed is set, transactions affecting the wallet which were removed are returned in the \"removed\" array.\n",
     605 | +                "Additionally, if include_removed is set, transactions affecting the wallet which were removed are returned in the \"removed\" array.\n"
     606 | +                "A transaction with both wallet-owned and non-wallet inputs cannot attribute the foreign outputs or its \n"
    


    murchandamus commented at 9:38 PM on July 15, 2026:

    In "wallet,rpc: report unattributable mixed-input history conservatively" (0d3f1141ef76ef3cbd527f5f1a96518a252f2aed): Also here: "cannot attribute" needs more context.


    w0xlt commented at 5:02 AM on September 22, 2026:

    Clarified in both RPC descriptions that the wallet cannot determine how much of each output or the fee was paid from its inputs.

  53. in src/wallet/rpc/transactions.cpp:807 in 0d3f1141ef outdated
     809 | +    const WalletTxHistoryAccounting accounting{CachedTxGetHistoryAccounting(*pwallet, wtx)};
     810 |  
     811 | -    entry.pushKV("amount", ValueFromAmount(nNet - nFee));
     812 | -    if (CachedTxIsFromMe(*pwallet, wtx))
     813 | -        entry.pushKV("fee", ValueFromAmount(nFee));
     814 | +    entry.pushKV("amount", ValueFromAmount(accounting.credit - accounting.debit + accounting.fee.value_or(0)));
    


    murchandamus commented at 9:40 PM on July 15, 2026:

    In "wallet,rpc: report unattributable mixed-input history conservatively" (0d3f1141ef76ef3cbd527f5f1a96518a252f2aed): I’m confused by this line. If all of credit, debit, and fee are stored as positive numbers, then surely it should be credit - debit - fee. If fee and debit are stored as negative numbers, it should be credit + debit + fee. If fee is stored as a negative number and debit is stored as a positive number, what are we even doing here? 😅


    w0xlt commented at 3:35 AM on September 22, 2026:

    On master, the code uses opposite fee formulas:

    • CachedTxGetAmounts(): wallet input value − total output value.
    • gettransaction: total output value − wallet input value.

    For transactions funded entirely by the wallet, these represent the same fee with opposite signs.

    The new helper uses the first convention and tells all three history RPCs whether the wallet’s fee share is known.

    Changing the sign is an implementation choice arising from that consolidation, rather than a requirement of mixed-input accounting itself.

    Regarding the question above:

    • accounting.debit sums the full value of the wallet’s spent inputs.
    • accounting.credit sums the wallet-owned outputs, including change.

    accounting.credit - accounting.debit already includes the fee. gettransaction adds the known fee back so amount excludes it, and reports fee separately as negative.


    w0xlt commented at 3:51 AM on September 22, 2026:

    Added a code comment explaining why the fee is added back.

  54. in src/wallet/rpc/transactions.cpp:479 in 056ccd7bab outdated
     474 | @@ -466,7 +475,8 @@ RPCMethod listtransactions()
     475 |                  "For instance, a wallet transaction that pays three addresses — one wallet-owned and two external — will produce \n"
     476 |                  "four entries. The payment to the wallet-owned address appears both as a send entry and as a receive entry. \n"
     477 |                  "As a result, the RPC response will contain one entry in the receive category and three entries in the send category.\n"
     478 | -                "A transaction with both wallet-owned and non-wallet inputs cannot attribute the foreign outputs or its \n"
     479 | +                "A transaction with both wallet-owned and non-wallet inputs is reported with normal per-output send/receive entries if \n"
     480 | +                "all non-wallet inputs have known zero value. Otherwise the wallet cannot attribute the foreign outputs or its \n"
    


    murchandamus commented at 9:49 PM on July 15, 2026:

    In "wallet,rpc: attribute zero-value foreign mixed inputs" (056ccd7bab93110d65b5c028f1502238c3a587b9):

    The "known" seems obsolete, as we always know the value of spent TXOs.

    -"all non-wallet inputs have known zero value. Otherwise the wallet cannot attribute the foreign outputs or its \n"
    +"all non-wallet inputs have zero value. Otherwise the wallet cannot attribute the foreign outputs or its \n"
    

    w0xlt commented at 5:12 AM on September 22, 2026:

    The wallet may not have the parent transaction needed to check a non-wallet input’s value. Clarified the wording.

  55. in src/wallet/rpc/transactions.cpp:616 in 056ccd7bab outdated
     611 | @@ -602,7 +612,8 @@ RPCMethod listsinceblock()
     612 |          "Get all transactions in blocks since block [blockhash], or all transactions if omitted.\n"
     613 |                  "If \"blockhash\" is no longer a part of the main chain, transactions from the fork point onward are included.\n"
     614 |                  "Additionally, if include_removed is set, transactions affecting the wallet which were removed are returned in the \"removed\" array.\n"
     615 | -                "A transaction with both wallet-owned and non-wallet inputs cannot attribute the foreign outputs or its \n"
     616 | +                "A transaction with both wallet-owned and non-wallet inputs is reported with normal per-output send/receive entries if \n"
     617 | +                "all non-wallet inputs have known zero value. Otherwise the wallet cannot attribute the foreign outputs or its \n"
    


    murchandamus commented at 9:49 PM on July 15, 2026:

    In "wallet,rpc: attribute zero-value foreign mixed inputs" (056ccd7bab93110d65b5c028f1502238c3a587b9):

    "known" seems to be extraneous as above.


    w0xlt commented at 5:13 AM on September 22, 2026:

    Applied the same clarification as above here.

  56. in src/wallet/rpc/transactions.cpp:750 in 056ccd7bab outdated
     746 | @@ -736,8 +747,7 @@ RPCMethod gettransaction()
     747 |                      RPCResult::Type::OBJ, "", "", Cat(Cat<std::vector<RPCResult>>(
     748 |                      {
     749 |                          {RPCResult::Type::STR_AMOUNT, "amount", "The amount in " + CURRENCY_UNIT},
     750 | -                        {RPCResult::Type::STR_AMOUNT, "fee", /*optional=*/true, "The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the\n"
     751 | -                                     "'send' category of transactions."},
     752 | +                        {RPCResult::Type::STR_AMOUNT, "fee", /*optional=*/true, "The amount of the wallet-attributable fee in " + CURRENCY_UNIT + ". This is negative and only available when known."},
    


    murchandamus commented at 9:54 PM on July 15, 2026:

    In "wallet,rpc: attribute zero-value foreign mixed inputs" (056ccd7bab93110d65b5c028f1502238c3a587b9): If someone only saw this RPC description, "only available when known" would be confusing. How about something along the lines of:

    -{RPCResult::Type::STR_AMOUNT, "fee", /*optional=*/true, "The amount of the wallet-attributable fee in " + CURRENCY_UNIT + ". This is negative and only available when known."},
    +{RPCResult::Type::STR_AMOUNT, "fee", /*optional=*/true, "The amount of the wallet-attributable fee in " + CURRENCY_UNIT + ". The fee is negative and will only be reported for the 'send' category. The fee cannot be calculated for transactions with multiple senders."},
    

    w0xlt commented at 5:26 AM on September 22, 2026:

    Clarified that the fee is reported when all inputs belong to the wallet, or when the transaction has mixed inputs and the wallet can verify that every non-wallet input has zero value. Otherwise, the fee field is omitted.

  57. in src/wallet/receive.cpp:209 in 056ccd7bab outdated
     205 | +        // Even when the values of all foreign inputs are known (e.g. their
     206 | +        // funding transactions are in the wallet), a nonzero foreign
     207 | +        // contribution keeps the fee unattributed: the total fee may be
     208 | +        // computable, but the wallet's share of it is not. Only when every
     209 | +        // foreign input provably contributes zero value is the entire fee
     210 | +        // attributable to the wallet.
    


    murchandamus commented at 9:56 PM on July 15, 2026:

    In "wallet,rpc: attribute zero-value foreign mixed inputs" (056ccd7bab93110d65b5c028f1502238c3a587b9): This explanation is excellent. "not computable" might be a better expression for the above cases where I found "attributable" to be insufficiently clear.


    w0xlt commented at 5:43 AM on September 22, 2026:

    Clarified the RPC descriptions and removed the remaining “unattributed” wording from the help text. Thanks!

  58. in src/wallet/receive.cpp:240 in 056ccd7bab outdated
     236 | @@ -229,9 +237,9 @@ void CachedTxGetAmounts(const CWallet& wallet, const CWalletTx& wtx,
     237 |          const CTxOut& txout = wtx.tx->vout[i];
     238 |          bool ismine = wallet.IsMine(txout);
     239 |          // Only need to handle txouts if either:
     240 | -        //   1) every input is ours, so the output can be reported as sent
     241 | +        //   1) every spent input value is ours, so the output can be reported as sent
    


    murchandamus commented at 9:57 PM on July 15, 2026:

    In "wallet,rpc: attribute zero-value foreign mixed inputs" (056ccd7bab93110d65b5c028f1502238c3a587b9): Do you mean "every spent input with nonzero value"? Or perhaps "the total input value was contributed by our wallet"?


    w0xlt commented at 5:51 AM on September 22, 2026:

    Updated the comment to say that the wallet contributes the total input value. Thanks!

  59. in doc/release-notes-34872.md:7 in eba63bfe77 outdated
       0 | @@ -0,0 +1,26 @@
       1 | +Wallet
       2 | +------
       3 | +
       4 | +- Transactions that spend a mix of wallet-owned and non-wallet inputs are now
       5 | +  reported safely by the `gettransaction`, `listtransactions`, and
       6 | +  `listsinceblock` RPCs. Previously such transactions reported a misleading fee
       7 | +  and attributed non-wallet outputs as wallet sends (see issue #14136).
    


    murchandamus commented at 10:01 PM on July 15, 2026:

    In "doc: add mixed-input history RPC release note" (eba63bfe77c148a447f44ba0703cb211c7a3213d):

      and considered all non-wallet outputs to be wallet sends (see issue [#14136](/bitcoin-bitcoin/14136/)).
    

    w0xlt commented at 6:07 AM on September 22, 2026:

    Applied your suggestion. Thanks!

  60. in doc/release-notes-34872.md:9 in eba63bfe77 outdated
       0 | @@ -0,0 +1,26 @@
       1 | +Wallet
       2 | +------
       3 | +
       4 | +- Transactions that spend a mix of wallet-owned and non-wallet inputs are now
       5 | +  reported safely by the `gettransaction`, `listtransactions`, and
       6 | +  `listsinceblock` RPCs. Previously such transactions reported a misleading fee
       7 | +  and attributed non-wallet outputs as wallet sends (see issue #14136).
       8 | +
       9 | +  When every non-wallet input has a known zero value (for example a
    


    murchandamus commented at 10:06 PM on July 15, 2026:

    In "doc: add mixed-input history RPC release note" (eba63bfe77c148a447f44ba0703cb211c7a3213d):

    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 first validate a transaction. Shouldn’t it therefore also be available when we store a transaction in the wallet? If not, the wallet should learn it at the time a block is processed, and the value should be in the revert data?


    w0xlt commented at 6:24 AM on September 22, 2026:

    Yes, it makes sense. The node knows these values during validation, and block undo data contains them. To verify that every non-wallet input has zero value, the wallet currently uses only parent transactions stored in the wallet. If a parent is missing, it cannot verify that input’s value. Learning and retaining those values could be a follow-up improvement.

    Updated the release note to say “the wallet can verify”.

  61. in doc/release-notes-34872.md:11 in eba63bfe77 outdated
       6 | +  `listsinceblock` RPCs. Previously such transactions reported a misleading fee
       7 | +  and attributed non-wallet outputs as wallet sends (see issue #14136).
       8 | +
       9 | +  When every non-wallet input has a known zero value (for example a
      10 | +  pay-to-anchor output), the wallet can still attribute the full fee and all
      11 | +  sent outputs, so these transactions continue to be reported with the usual
    


    murchandamus commented at 10:08 PM on July 15, 2026:

    In "doc: add mixed-input history RPC release note" (eba63bfe77c148a447f44ba0703cb211c7a3213d):

    I think my issue with "attribute" might be that the verb always needs a target for the attribution that I was missing. Maybe it makes sense when we describe it as the wallet being able to attribute it to itself?

      pay-to-anchor output), the wallet can still attribute the full fee and all
      sent outputs to itself, so these transactions continue to be reported with the usual
    

    w0xlt commented at 6:31 AM on September 22, 2026:

    Added “to itself”, as suggested. Thanks!

  62. in doc/release-notes-34872.md:13 in eba63bfe77 outdated
       8 | +
       9 | +  When every non-wallet input has a known zero value (for example a
      10 | +  pay-to-anchor output), the wallet can still attribute the full fee and all
      11 | +  sent outputs, so these transactions continue to be reported with the usual
      12 | +  per-output `send`/`receive` entries. Otherwise the wallet cannot attribute the
      13 | +  foreign outputs or its fee share, so it reports a single unattributed
    


    murchandamus commented at 10:09 PM on July 15, 2026:

    In "doc: add mixed-input history RPC release note" (eba63bfe77c148a447f44ba0703cb211c7a3213d):

      per-output `send`/`receive` entries. Otherwise the wallet cannot compute its share of the
      foreign outputs and the fee, so it reports a single unattributed
    

    w0xlt commented at 6:39 AM on September 22, 2026:

    Applied your suggestion. Thanks!

  63. in doc/release-notes-34872.md:14 in eba63bfe77 outdated
       9 | +  When every non-wallet input has a known zero value (for example a
      10 | +  pay-to-anchor output), the wallet can still attribute the full fee and all
      11 | +  sent outputs, so these transactions continue to be reported with the usual
      12 | +  per-output `send`/`receive` entries. Otherwise the wallet cannot attribute the
      13 | +  foreign outputs or its fee share, so it reports a single unattributed
      14 | +  aggregate `send` summary carrying the negative total of wallet-owned inputs
    


    murchandamus commented at 10:10 PM on July 15, 2026:

    In "doc: add mixed-input history RPC release note" (eba63bfe77c148a447f44ba0703cb211c7a3213d):

    Maybe:

      foreign outputs or its fee share, so it reports the negative total of the wallet-owned inputs
      as the `send` amount
    

    w0xlt commented at 6:44 AM on September 22, 2026:

    Done. Thanks.

  64. in doc/release-notes-34872.md:26 in eba63bfe77 outdated
      21 | +  `wallet_debit`, and `wallet_credit` fields, which let consumers detect the
      22 | +  conservative case without depending on the `category` value. The `vout` field
      23 | +  is now optional in the result schema: it is omitted from unattributed
      24 | +  aggregate mixed-input send summaries, which do not correspond to a single
      25 | +  output. The `fee` field is likewise omitted when the wallet cannot determine
      26 | +  the fee. (#34872)
    


    murchandamus commented at 10:16 PM on July 15, 2026:

    In "doc: add mixed-input history RPC release note" (eba63bfe77c148a447f44ba0703cb211c7a3213d):

    The fee can always be determined per Σ(inputs)−Σ(outputs), rather we cannot determine our share of the fee.

      output. The `fee` field is likewise omitted when the wallet cannot determine its share of
      the fee. (#34872)
    

    w0xlt commented at 6:50 AM on September 22, 2026:

    Done. Thanks.

  65. murchandamus commented at 10:19 PM on July 15, 2026: member

    Thanks for working on this. This sounds like a great improvement regarding multi-user transaction accounting. I have a few questions and I especially found the use of "attributable" somewhat confusing at times.

  66. DrahtBot added the label Needs rebase on Aug 4, 2026
  67. sedited commented at 9:49 AM on September 9, 2026: contributor

    @w0xlt can you rebase and respond to murch's review comments?

  68. w0xlt force-pushed on Sep 21, 2026
  69. DrahtBot removed the label Needs rebase on Sep 22, 2026
  70. w0xlt force-pushed on Sep 22, 2026
  71. w0xlt force-pushed on Sep 22, 2026
  72. DrahtBot added the label CI failed on Sep 22, 2026
  73. DrahtBot removed the label CI failed on Sep 22, 2026
  74. w0xlt force-pushed on Sep 22, 2026
  75. w0xlt force-pushed on Sep 22, 2026
  76. DrahtBot added the label CI failed on Sep 22, 2026
  77. w0xlt force-pushed on Sep 22, 2026
  78. w0xlt force-pushed on Sep 22, 2026
  79. w0xlt force-pushed on Sep 22, 2026
  80. w0xlt force-pushed on Sep 22, 2026
  81. w0xlt force-pushed on Sep 22, 2026
  82. w0xlt force-pushed on Sep 22, 2026
  83. w0xlt force-pushed on Sep 22, 2026
  84. w0xlt force-pushed on Sep 22, 2026
  85. w0xlt force-pushed on Sep 22, 2026
  86. w0xlt force-pushed on Sep 22, 2026
  87. w0xlt force-pushed on Sep 22, 2026
  88. w0xlt force-pushed on Sep 22, 2026
  89. w0xlt force-pushed on Sep 22, 2026
  90. w0xlt commented at 7:08 AM on September 22, 2026: contributor

    @sedited Rebased and all comments addressed.

  91. DrahtBot removed the label CI failed on Sep 22, 2026
  92. DrahtBot added the label Needs rebase on Sep 22, 2026
  93. 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.
    394b1c7415
  94. 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.
    a6f29fbb20
  95. 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.
    baf994d73e
  96. wallet,rpc: rename transaction entry helper
    Rename ListTransactions to AppendWalletTxEntries to clarify that the
    helper appends entries for a single wallet transaction to the
    caller-provided result container (ret).
    
    The helper is shared by listtransactions, listsinceblock, and
    gettransaction. The public RPC names and behavior are unchanged.
    a4d011550e
  97. wallet: require the wallet lock for CachedTxGetAmounts
    Make the existing caller-held wallet lock requirement explicit and remove
    the redundant internal lock. AppendWalletTxEntries is the only caller and
    already requires cs_wallet. Leave accounting behavior unchanged.
    7bdcbb8d39
  98. 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.
    
    Keep one receive entry for each wallet-owned output, showing its amount,
    address, label, and vout. Keep the send entry to record the wallet inputs
    used in the transaction. Use this format even when every output belongs
    to the wallet.
    a625026843
  99. test: cover basic mixed-input history accounting
    Add the functional test fixture and shared assertions for conservative
    mixed-input history in gettransaction, listtransactions, and listsinceblock.
    Check both participants, label filtering, confirmation, and reorg removal
    and reinclusion.
    75b5c6782c
  100. test: preserve change outputs in mixed-input history
    Check that a wallet-owned change output remains a receive entry beside the
    aggregate send. Their amounts must sum to the wallet's net change, both
    before and after confirmation.
    53a5d6635f
  101. test: cover mixed inputs with all outputs wallet-owned
    Check positive, zero, and negative net changes when every output belongs
    to the wallet. Preserve the aggregate send and each output's receive
    metadata, including label filtering, before and after confirmation.
    23c13144e7
  102. 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.
    0a2939417b
  103. 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.
    c167e7b01d
  104. 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.
    eaecc34e13
  105. 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.
    9ceb54bb0f
  106. 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.
    8c62d0a82f
  107. 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.
    f187f742d0
  108. 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.
    66763e7068
  109. w0xlt force-pushed on Sep 23, 2026
  110. DrahtBot removed the label Needs rebase on Sep 23, 2026
  111. arejula27 commented at 10:17 PM on September 26, 2026: contributor

    Concept ACK.

    I agree it is better to omit information than to show information that is wrong.

    I think the issue is that the history RPCs show two different things in the same list: the UTXO history and the balance history (how much the wallet balance changed, and whether we paid someone). This is hard because they are different paradigms: for simple transactions it works well, but for complex ones (like the ones this PR aims to fix) it breaks.

    Using the net change as the amount was already discussed earlier in this thread. 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 the wallet UTXOs the transaction spent and created, so no per output information is lost. Something like this (only to illustrate the idea, not a concrete proposal for the field names or layout):

    {
      "category": "receive",
      "amount": 1.99,
      // "fee" omitted: we cannot know it
      "involves_mixed_inputs": true,
      "utxos": {
        "spent":   [{"txid": "...", "vout": 0, "amount": 1.00}],
        "created": [{"vout": 0, "address": "...", "amount": 1.50},
                    {"vout": 1, "address": "...", "amount": 1.49}]
      }
    }
    

    For reference, I asked an LLM (Claude Opus) to look at how other wallets handle transactions with foreign inputs: BDK, Electrum, Electron Cash, BlueWallet, Sparrow, Wasabi, Dojo, CLN, LND and the Core GUI. All of them use the net change as the amount of the transaction, and those that cannot compute the fee (BDK, Electrum, Sparrow, Wasabi) leave it out, as this PR does.

    I reviewed this PR as part of the Spanish PR Review Club and wanted to share my view. I know the output format has already been discussed at length, so if that is decided and the PR is now at the code review stage, feel free to ignore this suggestion.

  112. in src/wallet/rpc/transactions.cpp:362 in 66763e7068
     360 |      std::list<COutputEntry> listSent;
     361 |  
     362 | +    const WalletTxHistoryAccounting accounting{CachedTxGetHistoryAccounting(wallet, wtx)};
     363 | +    const bool is_mixed_input{IsMixedInput(accounting)};
     364 | +    const bool needs_unattributed_aggregate_send{NeedsUnattributedAggregateSend(accounting)};
     365 |      CachedTxGetAmounts(wallet, wtx, listReceived, listSent, nFee, include_change);
    


    Bicaru20 commented at 1:34 PM on September 29, 2026:

    In a6250268436f7af45fd5bdab7384eb3ad75a3786 I think here we could avoid calling CachedTxGetHistoryAccounting since we already do that in CachedTxGetAmounts. For instance, we could modify CachedTxGetAmounts to return the accounting struct. Sth like this:

    --- a/src/wallet/receive.cpp
    +++ b/src/wallet/receive.cpp
    @@ -214,7 +214,7 @@ WalletTxHistoryAccounting CachedTxGetHistoryAccounting(const CWallet& wallet, co
         return {input_ownership, debit, credit, fee};
     }
     
    -void CachedTxGetAmounts(const CWallet& wallet, const CWalletTx& wtx,
    +WalletTxHistoryAccounting CachedTxGetAmounts(const CWallet& wallet, const CWalletTx& wtx,
                       std::list<COutputEntry>& listReceived,
                       std::list<COutputEntry>& listSent, CAmount& nFee,
                       bool include_change)
    @@ -267,7 +267,7 @@ void CachedTxGetAmounts(const CWallet& wallet, const CWalletTx& wtx,
             if (ismine)
                 listReceived.push_back(output);
         }
    -
    +    return accounting;
     }
     
     bool CachedTxIsFromMe(const CWallet& wallet, const CWalletTx& wtx)
    diff --git a/src/wallet/receive.h b/src/wallet/receive.h
    index 2328235e4e..e2139327d5 100644
    --- a/src/wallet/receive.h
    +++ b/src/wallet/receive.h
    @@ -46,7 +46,7 @@ struct WalletTxHistoryAccounting
     };
     WalletTxHistoryAccounting CachedTxGetHistoryAccounting(const CWallet& wallet, const CWalletTx& wtx)
         EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
    -void CachedTxGetAmounts(const CWallet& wallet, const CWalletTx& wtx,
    +WalletTxHistoryAccounting CachedTxGetAmounts(const CWallet& wallet, const CWalletTx& wtx,
                             std::list<COutputEntry>& listReceived,
                             std::list<COutputEntry>& listSent,
                             CAmount& nFee,
    diff --git a/src/wallet/rpc/transactions.cpp b/src/wallet/rpc/transactions.cpp
    index 6f7e9270a2..08c98a3db6 100644
    --- a/src/wallet/rpc/transactions.cpp
    +++ b/src/wallet/rpc/transactions.cpp
    @@ -356,10 +356,9 @@ static void AppendWalletTxEntries(const CWallet& wallet, const CWalletTx& wtx, i
         std::list<COutputEntry> listReceived;
         std::list<COutputEntry> listSent;
     
    -    const WalletTxHistoryAccounting accounting{CachedTxGetHistoryAccounting(wallet, wtx)};
    +    const WalletTxHistoryAccounting accounting{CachedTxGetAmounts(wallet, wtx, listReceived, listSent, nFee, include_change)};
         const bool is_mixed_input{IsMixedInput(accounting)};
         const bool needs_unattributed_aggregate_send{NeedsUnattributedAggregateSend(accounting)};
    -    CachedTxGetAmounts(wallet, wtx, listReceived, listSent, nFee, include_change);
    
    
  113. Bicaru20 commented at 2:35 PM on September 29, 2026: contributor

    ACK 66763e7068e0264fef85c259b677e319c90f2201

    I think this is the rigth approach so 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 when the wallet do not have enough information instead of seeing wrong data.

    I tested the code in regtest and everything seems to work the way it is supposed to. I just left a comment about a possible small optimitation.

  114. DrahtBot requested review from arejula27 on Sep 29, 2026
  115. DrahtBot requested review from rkrux on Sep 29, 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-10-04 20:51 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me