From e9331cd6ab2c756c56e8b27a2de2a6d4884c0c06 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Tue, 10 Jun 2025 19:58:48 +0000 Subject: [PATCH 1/3] wallet: IsEquivalentTo should strip witness data in addition to scriptsigs --- src/wallet/transaction.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/wallet/transaction.cpp b/src/wallet/transaction.cpp index 561880482f8..a611a034d9e 100644 --- a/src/wallet/transaction.cpp +++ b/src/wallet/transaction.cpp @@ -13,8 +13,14 @@ bool CWalletTx::IsEquivalentTo(const CWalletTx& _tx) const { CMutableTransaction tx1 {*this->tx}; CMutableTransaction tx2 {*_tx.tx}; - for (auto& txin : tx1.vin) txin.scriptSig = CScript(); - for (auto& txin : tx2.vin) txin.scriptSig = CScript(); + for (auto& txin : tx1.vin) { + txin.scriptSig = CScript(); + txin.scriptWitness.SetNull(); + } + for (auto& txin : tx2.vin) { + txin.scriptSig = CScript(); + txin.scriptWitness.SetNull(); + } return CTransaction(tx1) == CTransaction(tx2); } From cbf9b2dab1d8800d63d65904ccfd64e1e439e510 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Tue, 10 Jun 2025 20:52:46 +0000 Subject: [PATCH 2/3] mempool: codify existing assumption about duplicate txids during removal Also explicitly check for txid equality rather than transaction equality as the former is a tighter constraint if witness data is included when comparing the full transactions. Co-authored-by: glozow --- src/txmempool.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 3a5a3fb306d..10ff535828a 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -652,7 +652,7 @@ void CTxMemPool::removeConflicts(const CTransaction &tx) auto it = mapNextTx.find(txin.prevout); if (it != mapNextTx.end()) { const CTransaction &txConflict = *it->second; - if (txConflict != tx) + if (Assume(txConflict.GetHash() != tx.GetHash())) { ClearPrioritisation(txConflict.GetHash()); removeRecursive(txConflict, MemPoolRemovalReason::CONFLICT); From 6efbd1e1dcdfbe9eae2d5c22abab3ee616a75ff2 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Tue, 10 Jun 2025 21:13:59 +0000 Subject: [PATCH 3/3] refactor: CTransaction equality should consider witness data It is not at all obvious that two transactions with differing witness data should test equal to each other. There was only a single instance of a caller relying on this behavior, and that one appears accidental (left-over from before segwit). That caller (in the wallet) has been fixed. Change the definition of transaction equality (and inequality) to use the wtxid instead. --- src/primitives/transaction.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h index bf86562886a..ad817270949 100644 --- a/src/primitives/transaction.h +++ b/src/primitives/transaction.h @@ -360,12 +360,12 @@ public: friend bool operator==(const CTransaction& a, const CTransaction& b) { - return a.hash == b.hash; + return a.GetWitnessHash() == b.GetWitnessHash(); } friend bool operator!=(const CTransaction& a, const CTransaction& b) { - return a.hash != b.hash; + return !operator==(a, b); } std::string ToString() const;