From 8e68fc246d09f1e6c6dfa8c676969d97c2eb4334 Mon Sep 17 00:00:00 2001
From: Suhas Daftuar <sdaftuar@gmail.com>
Date: Wed, 29 Jan 2020 14:09:08 -0500
Subject: [PATCH] Add wtxids to recentRejects instead of txids

Previously, we only added txids to recentRejects if we were sure that the
transaction couldn't have had the wrong witness (either because the witness was
malleated or stripped).

In preparation for wtxid-based relay, we can observe that txid == wtxid for
transactions that have no witness, and add the wtxid of rejected transactions,
provided the transaction wasn't a witness-stripped one. This means that we now
add more data to the filter (as prior to this commit, any transaction with a
witness that failed to be accepted was being skipped for inclusion in the
filter) but witness malleation should still not interfere with relay of a valid
segwit transaction, because the txid of a segwit transaction would not be added
to the filter after failing validation.

In the future, having wtxids in the recent rejects filter will allow us to
skip downloading the same wtxid multiple times, once our peers use wtxids for
transaction relay.
---
 src/net_processing.cpp | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index efc90f85e46..511d5b66e1f 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -2005,12 +2005,12 @@ void static ProcessOrphanTx(CConnman& connman, CTxMemPool& mempool, std::set<uin
             // Has inputs but not accepted to mempool
             // Probably non-standard or insufficient fee
             LogPrint(BCLog::MEMPOOL, "   removed orphan tx %s\n", orphanHash.ToString());
-            if (!orphanTx.HasWitness() && orphan_state.GetResult() != TxValidationResult::TX_WITNESS_MUTATED) {
+            if (orphanTx.HasWitness() || orphan_state.GetResult() != TxValidationResult::TX_WITNESS_MUTATED) {
                 // Do not use rejection cache for witness transactions or
                 // witness-stripped transactions, as they can have been malleated.
                 // See https://github.com/bitcoin/bitcoin/issues/8279 for details.
                 assert(recentRejects);
-                recentRejects->insert(orphanHash);
+                recentRejects->insert(orphanTx.GetWitnessHash());
             }
             EraseOrphanTx(orphanHash);
             done = true;
@@ -2908,14 +2908,15 @@ void ProcessMessage(
                 // We will continue to reject this tx since it has rejected
                 // parents so avoid re-requesting it from other peers.
                 recentRejects->insert(tx.GetHash());
+                recentRejects->insert(tx.GetWitnessHash());
             }
         } else {
-            if (!tx.HasWitness() && state.GetResult() != TxValidationResult::TX_WITNESS_MUTATED) {
+            if (tx.HasWitness() || state.GetResult() != TxValidationResult::TX_WITNESS_MUTATED) {
                 // Do not use rejection cache for witness transactions or
                 // witness-stripped transactions, as they can have been malleated.
                 // See https://github.com/bitcoin/bitcoin/issues/8279 for details.
                 assert(recentRejects);
-                recentRejects->insert(tx.GetHash());
+                recentRejects->insert(tx.GetWitnessHash());
                 if (RecursiveDynamicUsage(*ptx) < 100000) {
                     AddToCompactExtraTransactions(ptx);
                 }