← Watch feed
wallet: Store all witness variants of a transaction
Public commit record
85/100 · Strong
What the developer wrote
Authored by furszy
wallet: Store all witness variants of a transaction
A transaction can have several valid witnesses that share its txid but
differ in wtxid, e.g. when a taproot output is spent via the key path
in one variant and the script path in another.
CWalletTx now keeps all of them in a map indexed by wtxid (m_txs) and
marks one as canonical (m_canonical_wtxid): a confirmed variant if there
is one, otherwise the one with a witness and the lowest weight. GetTx()
and serialization return the canonical variant, so existing callers
don't need to change.
The other variants are stored in their own wtxvariant records keyed by
(txid, wtxid) and merged back into the CWalletTx at load. The tx record
keeps its old format, holding the canonical transaction, so older soft
versions can still read and rewrite it without dropping those records.
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Explains rationale or failure mode
What changed, and why it matters
This Bitcoin Core wallet change lets the wallet remember multiple valid versions of the same transaction that look identical on-chain by transaction ID (txid) but differ by witness transaction ID (wtxid). For example, a Taproot spend can be signed in two different valid ways. Previously the wallet only kept one witness version, which could cause it to lose track of the actual confirmed version or show wrong balances/labels. The fix stores all variants and picks a canonical one, with older wallet software still able to read the main record safely.
Recommended action
Treat as a wallet correctness/data-integrity improvement rather than an active exploit. Reviewers should verify that canonical selection cannot be manipulated by an attacker-supplied variant, that ErasePrefix correctly removes all wtxvariant records, and that downgrade/upgrade scenarios do not leave orphaned or inconsistent records. No urgent patch deployment is indicated absent a disclosed vulnerability.
Security signals we found
01
Data integrity / consistency fix for wallet transaction state
02
Taproot witness malleability handling
03
Database schema addition (wtxvariant records) with backward-compatible canonical tx record
04
Potential balance/reporting correctness issue if wrong witness variant is treated as canonical
05
No explicit bugfix CVE or security disclosure in commit message
Evidence from the diff
CWalletTx is refactored from holding a single CTransactionRef tx to a map m_txs keyed by wtxid plus a m_canonical_wtxid. Serialization still writes the canonical transaction in the existing tx record, preserving backward read/write compatibility, while new wtxvariant records keyed by (txid, wtxid) persist alternate witness variants. On load, variants are merged back into m_txs. Canonical selection prefers a confirmed variant, otherwise the witnessed variant with lowest weight. GetTx(), GetHash(), GetWitnessHash(), IsCoinBase(), and SetTx() are updated to operate on the canonical variant. Export and migration paths now pass the variant map when deserializing copies.
Changed components
src/wallet/transaction.hsrc/wallet/walletdb.cppsrc/wallet/walletdb.hsrc/wallet/wallet.cppsrc/wallet/export.cppInspect captured patch +77 / −16
diff --git a/src/wallet/export.cpp b/src/wallet/export.cpp
index d9ca2b34..67bd7eb4 100644
--- a/src/wallet/export.cpp
+++ b/src/wallet/export.cpp
@@ -161,7 +161,7 @@ util::Result<std::string> ExportWatchOnlyWallet(const CWallet& wallet, const fs:
for (const auto& [txid, wtx] : wallet.mapWallet) {
DataStream wtx_ser;
wtx_ser << wtx;
- CWalletTx copy_wtx(deserialize, wtx_ser);
+ CWalletTx copy_wtx(deserialize, wtx_ser, wtx.GetTxs());
if (!watchonly_wallet->LoadToWallet(std::move(copy_wtx))) {
return util::Error{strprintf(_("Error: Could not add tx %s to watchonly wallet"), txid.GetHex())};
}
diff --git a/src/wallet/transaction.h b/src/wallet/transaction.h
index 8f1806bf..96ebeb37 100644
--- a/src/wallet/transaction.h
+++ b/src/wallet/transaction.h
@@ -234,15 +234,21 @@ public:
mutable bool fChangeCached;
mutable CAmount nChangeCached;
- CWalletTx(CTransactionRef tx, const TxState& state) : m_state(state), tx(std::move(Assert(tx)))
+ CWalletTx(CTransactionRef tx, const TxState& state) : m_state(state)
{
+ Assert(tx);
+ m_canonical_wtxid = tx->GetWitnessHash();
+ m_txs.emplace(tx->GetWitnessHash(), std::move(tx));
Init();
}
template <typename Stream>
- CWalletTx(deserialize_type, Stream& s) : m_state(TxStateInactive{})
+ CWalletTx(deserialize_type, Stream& s, const std::map<Wtxid, CTransactionRef>& variants) : m_state(TxStateInactive{})
{
Unserialize(s);
+ // Merge witness variants
+ m_txs.insert(variants.begin(), variants.end());
+ Assert(m_txs.contains(GetWitnessHash()));
}
void Init()
@@ -297,7 +303,7 @@ public:
uint32_t dummy_int = 0; // Used to be fTimeReceivedIsTxTime
uint256 serializedHash = TxStateSerializedBlockHash(m_state);
int serializedIndex = TxStateSerializedIndex(m_state);
- s << TX_WITH_WITNESS(tx) << serializedHash << dummy_vector1 << serializedIndex << dummy_vector2 << string_values << msgs_reqs << dummy_int << nTimeReceived << dummy_bool << dummy_bool;
+ s << TX_WITH_WITNESS(GetTx()) << serializedHash << dummy_vector1 << serializedIndex << dummy_vector2 << string_values << msgs_reqs << dummy_int << nTimeReceived << dummy_bool << dummy_bool;
}
template<typename Stream>
@@ -313,7 +319,10 @@ public:
int serializedIndex;
std::map<std::string, std::string> string_values;
std::vector<std::pair<std::string, std::string>> msgs_reqs;
- s >> TX_WITH_WITNESS(tx) >> serialized_block_hash >> dummy_vector1 >> serializedIndex >> dummy_vector2 >> string_values >> msgs_reqs >> dummy_int >> nTimeReceived >> dummy_bool >> dummy_bool;
+ CTransactionRef canonical_tx;
+ s >> TX_WITH_WITNESS(canonical_tx) >> serialized_block_hash >> dummy_vector1 >> serializedIndex >> dummy_vector2 >> string_values >> msgs_reqs >> dummy_int >> nTimeReceived >> dummy_bool >> dummy_bool;
+ m_canonical_wtxid = canonical_tx->GetWitnessHash();
+ m_txs.emplace(m_canonical_wtxid, std::move(canonical_tx));
m_state = TxStateInterpretSerialized({serialized_block_hash, serializedIndex});
@@ -342,11 +351,12 @@ public:
}
}
- CTransactionRef GetTx() const { return tx; }
+ CTransactionRef GetTx() const { return m_txs.at(m_canonical_wtxid); }
void SetTx(CTransactionRef arg)
{
- tx = std::move(arg);
+ Assert(arg);
+ m_txs.emplace(arg->GetWitnessHash(), std::move(arg));
}
//! make sure balances are recalculated
@@ -379,9 +389,11 @@ public:
bool isInactive() const { return state<TxStateInactive>(); }
bool isUnconfirmed() const { return !isAbandoned() && !isBlockConflicted() && !isMempoolConflicted() && !isConfirmed(); }
bool isConfirmed() const { return state<TxStateConfirmed>(); }
- const Txid& GetHash() const LIFETIMEBOUND { return tx->GetHash(); }
- const Wtxid& GetWitnessHash() const LIFETIMEBOUND { return tx->GetWitnessHash(); }
- bool IsCoinBase() const { return tx->IsCoinBase(); }
+ const Txid& GetHash() const LIFETIMEBOUND { return GetTx()->GetHash(); }
+ const Wtxid& GetWitnessHash() const LIFETIMEBOUND { return GetTx()->GetWitnessHash(); }
+ bool IsCoinBase() const { return GetTx()->IsCoinBase(); }
+
+ const std::map<Wtxid, CTransactionRef>& GetTxs() const { return m_txs; }
// Disable copying of CWalletTx objects to prevent bugs where instances get
// copied in and out of the mapWallet map, and fields are updated in the
@@ -393,7 +405,9 @@ public:
CWalletTx(CWalletTx&&) = default;
private:
- CTransactionRef tx;
+ Wtxid m_canonical_wtxid;
+ std::map<Wtxid, CTransactionRef> m_txs;
+
};
struct WalletTxOrderComparator {
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 54233ea5..36a50660 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -4049,7 +4049,7 @@ util::Result<void> CWallet::ApplyMigrationData(WalletBatch& local_wallet_batch,
const Txid& hash = wtx->GetHash();
DataStream wtx_ser;
wtx_ser << *wtx;
- CWalletTx copy_wtx(deserialize, wtx_ser);
+ CWalletTx copy_wtx(deserialize, wtx_ser, wtx->GetTxs());
if (!data.watchonly_wallet->LoadToWallet(std::move(copy_wtx))) {
return util::Error{strprintf(_("Error: Could not add watchonly tx %s to watchonly wallet"), wtx->GetHash().GetHex())};
}
diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp
index 8f208223..ae6ee848 100644
--- a/src/wallet/walletdb.cpp
+++ b/src/wallet/walletdb.cpp
@@ -52,6 +52,7 @@ const std::string POOL{"pool"};
const std::string PURPOSE{"purpose"};
const std::string SETTINGS{"settings"};
const std::string TX{"tx"};
+const std::string WTX_VARIANT{"wtxvariant"};
const std::string VERSION{"version"};
const std::string WALLETDESCRIPTOR{"walletdescriptor"};
const std::string WALLETDESCRIPTORCACHE{"walletdescriptorcache"};
@@ -97,12 +98,24 @@ bool WalletBatch::ErasePurpose(const std::string& strAddress)
bool WalletBatch::WriteTx(const CWalletTx& wtx)
{
- return WriteIC(std::make_pair(DBKeys::TX, wtx.GetHash()), wtx);
+ const Txid txid = wtx.GetHash();
+ // Persist all witness variants. Including the canonical one
+ for (const auto& [wtxid, tx] : wtx.GetTxs()) {
+ if (!WriteWtxVariant(txid, tx)) return false;
+ }
+ return WriteIC(std::make_pair(DBKeys::TX, txid), wtx);
}
bool WalletBatch::EraseTx(Txid hash)
{
- return EraseIC(std::make_pair(DBKeys::TX, hash.ToUint256()));
+ if (!EraseIC(std::make_pair(DBKeys::TX, hash.ToUint256()))) return false;
+ // Drop all witness variant records too, so none are left dangling
+ return m_batch->ErasePrefix(DataStream() << DBKeys::WTX_VARIANT << hash);
+}
+
+bool WalletBatch::WriteWtxVariant(const Txid& txid, const CTransactionRef& tx)
+{
+ return WriteIC(std::make_pair(DBKeys::WTX_VARIANT, std::make_pair(txid, tx->GetWitnessHash())), TX_WITH_WITNESS(tx));
}
bool WalletBatch::WriteKeyMetadata(const CKeyMetadata& meta, const CPubKey& pubkey, const bool overwrite)
@@ -980,6 +993,37 @@ static DBErrors LoadAddressBookRecords(CWallet* pwallet, DatabaseBatch& batch) E
return result;
}
+static std::map<Wtxid, CTransactionRef> ReadWtxVariants(DatabaseBatch& batch, const Txid& txid)
+{
+ std::map<Wtxid, CTransactionRef> variants;
+
+ DataStream prefix;
+ prefix << DBKeys::WTX_VARIANT << txid;
+ std::unique_ptr<DatabaseCursor> cursor = batch.GetNewPrefixCursor(prefix);
+ if (!cursor) {
+ throw std::runtime_error(strprintf("Error getting database cursor for '%s' records", DBKeys::WTX_VARIANT));
+ }
+
+ DataStream key;
+ DataStream value;
+ while (true) {
+ DatabaseCursor::Status status = cursor->Next(key, value);
+ if (status == DatabaseCursor::Status::DONE) break;
+ if (status == DatabaseCursor::Status::FAIL) {
+ throw std::runtime_error(strprintf("Error reading '%s' record", DBKeys::WTX_VARIANT));
+ }
+ CTransactionRef tx;
+ value >> TX_WITH_WITNESS(tx);
+ if (tx->GetHash() != txid) {
+ throw std::runtime_error(strprintf("Corrupted witness variant, tx hash differs"));
+ }
+ if (!variants.emplace(tx->GetWitnessHash(), std::move(tx)).second) {
+ throw std::runtime_error(strprintf("Duplicate witness variant"));
+ }
+ }
+ return variants;
+}
+
static DBErrors LoadTxRecords(CWallet* pwallet, DatabaseBatch& batch, bool& any_unordered) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)
{
AssertLockHeld(pwallet->cs_wallet);
@@ -988,12 +1032,12 @@ static DBErrors LoadTxRecords(CWallet* pwallet, DatabaseBatch& batch, bool& any_
// Load tx record
any_unordered = false;
LoadResult tx_res = LoadRecords(pwallet, batch, DBKeys::TX,
- [&any_unordered] (CWallet* pwallet, DataStream& key, DataStream& value, std::string& err) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) {
+ [&any_unordered, &batch] (CWallet* pwallet, DataStream& key, DataStream& value, std::string& err) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) {
DBErrors result = DBErrors::LOAD_OK;
Txid hash;
key >> hash;
try {
- CWalletTx wtx{deserialize, value};
+ CWalletTx wtx{deserialize, value, ReadWtxVariants(batch, hash)};
if (wtx.GetHash() != hash) {
result = std::max(result, DBErrors::NEED_RESCAN);
}
diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h
index 8397fff9..cf624657 100644
--- a/src/wallet/walletdb.h
+++ b/src/wallet/walletdb.h
@@ -7,6 +7,7 @@
#define BITCOIN_WALLET_WALLETDB_H
#include <key.h>
+#include <primitives/transaction.h>
#include <primitives/transaction_identifier.h>
#include <script/sign.h>
#include <wallet/db.h>
@@ -79,6 +80,7 @@ extern const std::string POOL;
extern const std::string PURPOSE;
extern const std::string SETTINGS;
extern const std::string TX;
+extern const std::string WTX_VARIANT;
extern const std::string VERSION;
extern const std::string WALLETDESCRIPTOR;
extern const std::string WALLETDESCRIPTORCKEY;
@@ -230,6 +232,7 @@ public:
bool WriteTx(const CWalletTx& wtx);
bool EraseTx(Txid hash);
+ bool WriteWtxVariant(const Txid& txid, const CTransactionRef& tx);
bool WriteKeyMetadata(const CKeyMetadata& meta, const CPubKey& pubkey, bool overwrite);
bool WriteKey(const CPubKey& vchPubKey, const CPrivKey& vchPrivKey, const CKeyMetadata &keyMeta);
Risk score
Our methodology →Why this scored 35/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.