wallet: store all witness variants of a transaction #35501

achow101 commented at 12:51 AM on June 10, 2026: member

When the wallet is presented with a transaction that has the same txid as one already known to the wallet, but has a different witness, instead of ignoring the transaction, store it alongside the known tx. This enables the wallet to be aware of all wtxid variants of its transactions. This also allows for the wallet to be able to calculate fees for replacements better as txs with different witnesses may have different feerates.

Specifically, the wallet stores these alternates in CWalletTx and extends the existing tx record type to essentially have a vector of transactions appended to the record. In CWalletTx, the single transaction is replaced with a map of wtxid to transaction so that all witness variants can still be represented by a single CWalletTx. For all of the various things that need the tx from a CWalletTx, a single witness variant is chosen to be the canonical tx and returned by GetTx(). This canonical tx is written into the same place as the previous single tx was written to in the tx record so that wallets can be loaded into previous versions.

To choose the canonical transaction, if any of the variants is confirmed, then that is the canonical one. Otherwise, the witness variant with the least weight is chosen.

An additional change I've included is to make CWalletTx RAII. This simplifies some of the implementation and enforces the assumption that a CWalletTx always has a transaction.

Lastly, gettransaction and listtransaction have a new field alternate_wtxids to inform users of the wtxids of the witness variants for a transaction, and of course, a test.

Closes #11240

wallet: Deserialize directly in CWalletTx's ctor

When loading a transaction, instead of constructing a CWalletTx with no
transaction, pass the DataStream into the constructor so that the
CWalletTx is RAII. This allows us to ensure that the transaction is
never a nullptr so that dereferences, especially once multiple txs are
stored, will not cause a segfault.

cb71cb3c7c

wallet: Make CWalletTx::tx private and use CWalletTx::GetTx to access

When CWalletTx will have multiple transactions, tx will no longer exist
and accessing the single canonical tx should be done through an getter
function.

45be7674bc

wallet: Replace CWalletTx::tx with m_txs for multiple transactions

Allow CWalletTx to contain multiple transactions, and write them in a
single record. The transactions are stored in a map of txid to txref,
and m_canonical_wtxid holds the wtxid of the tx that is "canonical".
"Canonical" will be defined in a future commit.

962b88d096

wallet: Replace CWalletTx::SetTx with AddTx

Instead of replacing the tx when a witness alternative appears, add it
to the set of wtxid alternates.

In order to determine whether the added transaction is the canonical
transaction, AddTx also needs to know how the state is changing, so it
will also update the state if it is being changed.

4ad42b5f46

wallet: Show alternate wtxids in gettransaction

If a wallet tranasction has alterate witness versions, list those wtxids
in gettransaction's output.

e2b8ebdec3

test: Test for wallet txs with alternate wtxids 21631e94b9

DrahtBot added the label Wallet on Jun 10, 2026

DrahtBot commented at 12:51 AM on June 10, 2026: contributor

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

Code Coverage & Benchmarks

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	polespinasa

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#35511 (RFC: consensus: Make CAmount a class by hodlinator)
#35294 (wallet: Update tx chain state during loading during AttachChain instead of before by achow101)
#34909 (wallet, refactor: modularise wallet by extracting out legacy wallet migration by rkrux)
#34872 (wallet: fix mixed-input transaction accounting in history RPCs by w0xlt)
#33034 (wallet: Store transactions in a separate sqlite table by achow101)
#32763 (wallet: Replace CWalletTx::mapValue and vOrderForm with explicit class members by achow101)
#29278 (Wallet: Add maxfeerate wallet startup option by ismaelsadeeq)
#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.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

listtransctions -> listtransactions [misspelled in the Python comment]
alternate_txids -> alternate_wtxids [the comment uses the wrong field name, which could confuse readers]

polespinasa commented at 7:22 AM on June 11, 2026: member

Concept ACK will review :)

rkrux commented at 9:34 AM on June 11, 2026: contributor

I have read the PR description and only glanced at the diff yet, I have the following questions.

This enables the wallet to be aware of all wtxid variants of its transactions.

All - Doesn't this introduce an attack vector by making the wallet persist all (potentially several) variants of the transaction, thereby bloating the walletdb and backup?
What's the benefit of storing all the variants once one of the variants have been confirmed for a while? If not much, I feel the rest of the variants should be removed from the database.

achow101 commented at 5:09 PM on June 11, 2026: member

All - Doesn't this introduce an attack vector by making the wallet persist all (potentially several) variants of the transaction, thereby bloating the walletdb and backup?

It doesn't because there is no way to unconditionally add a transaction to the wallet. Unconfirmed transactions must first come through the mempool. This means that they need to be fully valid, and if replacing an existing transaction, have a higher fee rate. Confirmed transactions must be included in valid blocks that are connected to the chain tip. Both of these put natural limits on how many transactions can be added. Fundamentally, I don't see this as being any different from what the wallet already does where it stores every new transaction relevant to it as that is also theoretically unbounded growth.

What's the benefit of storing all the variants once one of the variants have been confirmed for a while? If not much, I feel the rest of the variants should be removed from the database.

It's a lot more complicated to periodically go through transaction history and delete things that are no longer relevant, and I don't think it really benefits us that much to do that. It may also be useful to a user to see their full history, including transactions that don't get confirmed. We likewise do not delete conflicted transactions that have been conflicted for a long time.

rkrux commented at 2:18 PM on June 12, 2026: contributor

Yeah, I am realising now that this doesn't introduce an attack vector.

Unconfirmed transactions must first come through the mempool. This means that they need to be fully valid, and if replacing an existing transaction, have a higher fee rate.

I was concerned about the wallet storing several witness-malleated unconfirmed transactions but I see that there is a check in the mempool prechecks section where the same txid and different wtxid transactions are not allowed: https://github.com/bitcoin/bitcoin/blob/4c99ed10766c7fac1353d1402d1b99de4c656c08/src/validation.cpp#L823-L827

One way I see the wallet storing multiple witness variants would be when a low fee transaction is seen in the mempool, which makes the wallet store the first version. Then after an unclean node shutdown in absence of -persistmempool=0 (which could make the original transaction not persist on disk), a witness variant of this unconfirmed transaction comes in the mempool (maybe via a different peer) and the wallet ends up storing it.

Both of these put natural limits on how many transactions can be added

I can see that, practically, this would not lead to unbounded growth.