From 879a3cf2c2367d51310204d21030f3b218582c30 Mon Sep 17 00:00:00 2001
From: Suhas Daftuar <sdaftuar@gmail.com>
Date: Fri, 7 Feb 2020 04:30:41 -0500
Subject: Make TX_WITNESS_STRIPPED its own rejection reason

Previously, TX_WITNESS_MUTATED could be returned during transaction validation
for either transactions that had a witness that was non-standard, or for
transactions that had no witness but were invalid due to segwit validation
rules.

However, for txid/wtxid-relay considerations, net_processing distinguishes the
witness stripped case separately, because it affects whether a wtxid should be
able to be added to the reject filter. It is safe to add the wtxid of a
witness-mutated transaction to the filter (as that wtxid shouldn't collide with
the txid, and hence it wouldn't interfere with transaction relay from
txid-relay peers), but it is not safe to add the wtxid (== txid) of a
witness-stripped transaction to the filter, because that would interfere with
relay of another transaction with the same txid (but different wtxid) when
relaying from txid-relay peers.

Also updates the comment explaining this logic, and explaining that we can get
rid of this complexity once there's a sufficient deployment of wtxid-relaying
peers on the network.
---
 src/consensus/validation.h | 6 +++++-
 src/net_processing.cpp     | 5 +++--
 src/validation.cpp         | 2 +-
 3 files changed, 9 insertions(+), 4 deletions(-)

(limited to 'src')

diff --git a/src/consensus/validation.h b/src/consensus/validation.h
index 3a90cd69b3..f7d476951b 100644
--- a/src/consensus/validation.h
+++ b/src/consensus/validation.h
@@ -31,11 +31,15 @@ enum class TxValidationResult {
     TX_MISSING_INPUTS,        //!< transaction was missing some of its inputs
     TX_PREMATURE_SPEND,       //!< transaction spends a coinbase too early, or violates locktime/sequence locks
     /**
-     * Transaction might be missing a witness, have a witness prior to SegWit
+     * Transaction might have a witness prior to SegWit
      * activation, or witness may have been malleated (which includes
      * non-standard witnesses).
      */
     TX_WITNESS_MUTATED,
+    /**
+     * Transaction is missing a witness.
+     */
+    TX_WITNESS_STRIPPED,
     /**
      * Tx already in mempool or conflicts with a tx in the chain
      * (if it conflicts with another tx in mempool, we use MEMPOOL_POLICY as it failed to reach the RBF threshold)
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 0527bd2022..8893397e83 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -1092,6 +1092,7 @@ static bool MaybePunishNodeForTx(NodeId nodeid, const TxValidationState& state,
     case TxValidationResult::TX_MISSING_INPUTS:
     case TxValidationResult::TX_PREMATURE_SPEND:
     case TxValidationResult::TX_WITNESS_MUTATED:
+    case TxValidationResult::TX_WITNESS_STRIPPED:
     case TxValidationResult::TX_CONFLICT:
     case TxValidationResult::TX_MEMPOOL_POLICY:
         break;
@@ -1934,7 +1935,7 @@ 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 (orphan_state.GetResult() != TxValidationResult::TX_WITNESS_STRIPPED) {
                 // We can add the wtxid of this transaction to our reject filter.
                 // Do not add txids of witness transactions or witness-stripped
                 // transactions to the filter, as they can have been malleated;
@@ -2708,7 +2709,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec
                 recentRejects->insert(tx.GetWitnessHash());
             }
         } else {
-            if (tx.HasWitness() || state.GetResult() != TxValidationResult::TX_WITNESS_MUTATED) {
+            if (state.GetResult() != TxValidationResult::TX_WITNESS_STRIPPED) {
                 // We can add the wtxid of this transaction to our reject filter.
                 // Do not add txids of witness transactions or witness-stripped
                 // transactions to the filter, as they can have been malleated;
diff --git a/src/validation.cpp b/src/validation.cpp
index 5a98b2cb92..a4adc81b5a 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -916,7 +916,7 @@ bool MemPoolAccept::PolicyScriptChecks(ATMPArgs& args, Workspace& ws, Precompute
         if (!tx.HasWitness() && CheckInputScripts(tx, state_dummy, m_view, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true, false, txdata) &&
                 !CheckInputScripts(tx, state_dummy, m_view, scriptVerifyFlags & ~SCRIPT_VERIFY_CLEANSTACK, true, false, txdata)) {
             // Only the witness is missing, so the transaction itself may be fine.
-            state.Invalid(TxValidationResult::TX_WITNESS_MUTATED,
+            state.Invalid(TxValidationResult::TX_WITNESS_STRIPPED,
                     state.GetRejectReason(), state.GetDebugMessage());
         }
         return false; // state filled in by CheckInputScripts
-- 
cgit v1.2.3