[packages] return DIFFERENT_WITNESS for same-txid-different-witness tx

The previous interface required callers to guess that the tx had been
swapped and look up the tx again by txid to find a `MEMPOOL_ENTRY`
result. This is a confusing interface.

Instead, explicitly tell the caller that this transaction was
`DIFFERENT_WITNESS` in the result linked to the mempool entry's wtxid.
This gives the caller all the information they need in 1 lookup, and
they can query the mempool for the other transaction if needed.

Author: glozow

src/validation.cpp

@@ -1274,9 +1274,10 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package,
             // transaction for the mempool one. Note that we are ignoring the validity of the
             // package transaction passed in.
             // TODO: allow witness replacement in packages.
-            auto iter = m_pool.GetIter(wtxid);
+            auto iter = m_pool.GetIter(txid);
             assert(iter != std::nullopt);
-            results.emplace(txid, MempoolAcceptResult::MempoolTx(iter.value()->GetTxSize(), iter.value()->GetFee()));
+            // Provide the wtxid of the mempool tx so that the caller can look it up in the mempool.
+            results.emplace(wtxid, MempoolAcceptResult::MempoolTxDifferentWitness(iter.value()->GetTx().GetWitnessHash()));
         } else {
             // Transaction does not already exist in the mempool.
             txns_new.push_back(tx);

src/validation.h

@@ -161,8 +161,12 @@ struct MempoolAcceptResult {
         VALID, //!> Fully validated, valid.
         INVALID, //!> Invalid.
         MEMPOOL_ENTRY, //!> Valid, transaction was already in the mempool.
+        DIFFERENT_WITNESS, //!> Not validated. A same-txid-different-witness tx (see m_other_wtxid) already exists in the mempool and was not replaced.
     };
+    /** Result type. Present in all MempoolAcceptResults. */
     const ResultType m_result_type;
+
+    /** Contains information about why the transaction failed. */
     const TxValidationState m_state;
 
     // The following fields are only present when m_result_type = ResultType::VALID or MEMPOOL_ENTRY
@@ -173,6 +177,10 @@ struct MempoolAcceptResult {
     /** Raw base fees in satoshis. */
     const std::optional<CAmount> m_base_fees;
 
+    // The following field is only present when m_result_type = ResultType::DIFFERENT_WITNESS
+    /** The wtxid of the transaction in the mempool which has the same txid but different witness. */
+    const std::optional<uint256> m_other_wtxid;
+
     static MempoolAcceptResult Failure(TxValidationState state) {
         return MempoolAcceptResult(state);
     }
@@ -185,6 +193,10 @@ struct MempoolAcceptResult {
         return MempoolAcceptResult(vsize, fees);
     }
 
+    static MempoolAcceptResult MempoolTxDifferentWitness(const uint256& other_wtxid) {
+        return MempoolAcceptResult(other_wtxid);
+    }
+
 // Private constructors. Use static methods MempoolAcceptResult::Success, etc. to construct.
 private:
     /** Constructor for failure case */
@@ -201,6 +213,10 @@ struct MempoolAcceptResult {
     /** Constructor for already-in-mempool case. It wouldn't replace any transactions. */
     explicit MempoolAcceptResult(int64_t vsize, CAmount fees)
         : m_result_type(ResultType::MEMPOOL_ENTRY), m_vsize{vsize}, m_base_fees(fees) {}
+
+    /** Constructor for witness-swapped case. */
+    explicit MempoolAcceptResult(const uint256& other_wtxid)
+        : m_result_type(ResultType::DIFFERENT_WITNESS), m_other_wtxid(other_wtxid) {}
 };
 
 /**
@@ -210,7 +226,7 @@ struct PackageMempoolAcceptResult
 {
     const PackageValidationState m_state;
     /**
-    * Map from (w)txid to finished MempoolAcceptResults. The client is responsible
+    * Map from wtxid to finished MempoolAcceptResults. The client is responsible
     * for keeping track of the transaction objects themselves. If a result is not
     * present, it means validation was unfinished for that transaction. If there
     * was a package-wide error (see result in m_state), m_tx_results will be empty.