Merge bitcoin#30000: p2p: index TxOrphanage by wtxid, allow entries with same txid

0fb17bf [log] updates in TxOrphanage (glozow)
b16da7e [functional test] attackers sending mutated orphans (glozow)
6675f64 [unit test] TxOrphanage handling of same-txid-different-witness txns (glozow)
8923edf [p2p] allow entries with the same txid in TxOrphanage (glozow)
c31f148 [refactor] TxOrphanage::EraseTx by wtxid (glozow)
efcc593 [refactor] TxOrphanage::HaveTx only by wtxid (glozow)
7e475b9 [p2p] don't query orphanage by txid (glozow)

Pull request description:

  Part of bitcoin#27463 in the "make orphan handling more robust" section.

  Currently the main map in `TxOrphanage` is indexed by txid; we do not allow 2 transactions with the same txid into TxOrphanage. This means that if we receive a transaction and want to store it in orphanage, we'll fail to do so if a same-txid-different-witness version of the tx already exists in the orphanage. The existing orphanage entry can stay until it expires 20 minutes later, or until we find that it is invalid.

  This means an attacker can try to block/delay us accepting an orphan transaction by sending a mutated version of the child ahead of time. See included test.

  Prior to bitcoin#28970, we don't rely on the orphanage for anything and it would be relatively difficult to guess what transaction will go to a node's orphanage. After the parent(s) are accepted, if anybody sends us the correct transaction, we'll end up accepting it. However, this is a bit more painful for 1p1c: it's easier for an attacker to tell when a tx is going to hit a node's orphanage, and we need to store the correct orphan + receive the parent before we'll consider the package. If we start out with a bad orphan, we can't evict it until we receive the parent + try the 1p1c, and then we'll need to download the real child, put it in orphanage, download the parent again, and then retry 1p1c.

ACKs for top commit:
  AngusP:
    ACK 0fb17bf
  itornaza:
    trACK 0fb17bf
  instagibbs:
    ACK 0fb17bf
  theStack:
    ACK 0fb17bf
  sr-gi:
    crACK [0fb17bf](bitcoin@0fb17bf)
  stickies-v:
    ACK 0fb17bf

Tree-SHA512: edcbac7287c628bc27036920c2d4e4f63ec65087fbac1de9319c4f541515d669fc4e5fdc30c8b9a248b720da42b89153d388e91c7bf5caf4bc5b3b931ded1f59

Author: ryanofsky

src/net_processing.cpp

@@ -2293,7 +2293,23 @@ bool PeerManagerImpl::AlreadyHaveTx(const GenTxid& gtxid, bool include_reconside
 
     const uint256& hash = gtxid.GetHash();
 
-    if (m_orphanage.HaveTx(gtxid)) return true;
+    if (gtxid.IsWtxid()) {
+        // Normal query by wtxid.
+        if (m_orphanage.HaveTx(Wtxid::FromUint256(hash))) return true;
+    } else {
+        // Never query by txid: it is possible that the transaction in the orphanage has the same
+        // txid but a different witness, which would give us a false positive result. If we decided
+        // not to request the transaction based on this result, an attacker could prevent us from
+        // downloading a transaction by intentionally creating a malleated version of it.  While
+        // only one (or none!) of these transactions can ultimately be confirmed, we have no way of
+        // discerning which one that is, so the orphanage can store multiple transactions with the
+        // same txid.
+        //
+        // While we won't query by txid, we can try to "guess" what the wtxid is based on the txid.
+        // A non-segwit transaction's txid == wtxid. Query this txid "casted" to a wtxid. This will
+        // help us find non-segwit transactions, saving bandwidth, and should have no false positives.
+        if (m_orphanage.HaveTx(Wtxid::FromUint256(hash))) return true;
+    }
 
     if (include_reconsiderable && m_recent_rejects_reconsiderable.contains(hash)) return true;
 
@@ -3239,7 +3255,7 @@ void PeerManagerImpl::ProcessInvalidTx(NodeId nodeid, const CTransactionRef& ptx
 
     // If the tx failed in ProcessOrphanTx, it should be removed from the orphanage unless the
     // tx was still missing inputs. If the tx was not in the orphanage, EraseTx does nothing and returns 0.
-    if (Assume(state.GetResult() != TxValidationResult::TX_MISSING_INPUTS) && m_orphanage.EraseTx(ptx->GetHash()) > 0) {
+    if (Assume(state.GetResult() != TxValidationResult::TX_MISSING_INPUTS) && m_orphanage.EraseTx(ptx->GetWitnessHash()) > 0) {
         LogDebug(BCLog::TXPACKAGES, "   removed orphan tx %s (wtxid=%s)\n", ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString());
     }
 }
@@ -3257,7 +3273,7 @@ void PeerManagerImpl::ProcessValidTx(NodeId nodeid, const CTransactionRef& tx, c
 
     m_orphanage.AddChildrenToWorkSet(*tx);
     // If it came from the orphanage, remove it. No-op if the tx is not in txorphanage.
-    m_orphanage.EraseTx(tx->GetHash());
+    m_orphanage.EraseTx(tx->GetWitnessHash());
 
     LogDebug(BCLog::MEMPOOL, "AcceptToMemoryPool: peer=%d: accepted %s (wtxid=%s) (poolsz %u txn, %u kB)\n",
              nodeid,

src/test/fuzz/txorphan.cpp

@@ -104,21 +104,20 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage)
                     {
                         CTransactionRef ref = orphanage.GetTxToReconsider(peer_id);
                         if (ref) {
-                            bool have_tx = orphanage.HaveTx(GenTxid::Txid(ref->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(ref->GetWitnessHash()));
-                            Assert(have_tx);
+                            Assert(orphanage.HaveTx(ref->GetWitnessHash()));
                         }
                     }
                 },
                 [&] {
-                    bool have_tx = orphanage.HaveTx(GenTxid::Txid(tx->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(tx->GetWitnessHash()));
+                    bool have_tx = orphanage.HaveTx(tx->GetWitnessHash());
                     // AddTx should return false if tx is too big or already have it
                     // tx weight is unknown, we only check when tx is already in orphanage
                     {
                         bool add_tx = orphanage.AddTx(tx, peer_id);
                         // have_tx == true -> add_tx == false
                         Assert(!have_tx || !add_tx);
                     }
-                    have_tx = orphanage.HaveTx(GenTxid::Txid(tx->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(tx->GetWitnessHash()));
+                    have_tx = orphanage.HaveTx(tx->GetWitnessHash());
                     {
                         bool add_tx = orphanage.AddTx(tx, peer_id);
                         // if have_tx is still false, it must be too big
@@ -127,15 +126,15 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage)
                     }
                 },
                 [&] {
-                    bool have_tx = orphanage.HaveTx(GenTxid::Txid(tx->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(tx->GetWitnessHash()));
+                    bool have_tx = orphanage.HaveTx(tx->GetWitnessHash());
                     // EraseTx should return 0 if m_orphans doesn't have the tx
                     {
-                        Assert(have_tx == orphanage.EraseTx(tx->GetHash()));
+                        Assert(have_tx == orphanage.EraseTx(tx->GetWitnessHash()));
                     }
-                    have_tx = orphanage.HaveTx(GenTxid::Txid(tx->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(tx->GetWitnessHash()));
+                    have_tx = orphanage.HaveTx(tx->GetWitnessHash());
                     // have_tx should be false and EraseTx should fail
                     {
-                        Assert(!have_tx && !orphanage.EraseTx(tx->GetHash()));
+                        Assert(!have_tx && !orphanage.EraseTx(tx->GetWitnessHash()));
                     }
                 },
                 [&] {

src/test/orphanage_tests.cpp

@@ -30,8 +30,8 @@ class TxOrphanageTest : public TxOrphanage
     CTransactionRef RandomOrphan() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
     {
         LOCK(m_mutex);
-        std::map<Txid, OrphanTx>::iterator it;
-        it = m_orphans.lower_bound(Txid::FromUint256(InsecureRand256()));
+        std::map<Wtxid, OrphanTx>::iterator it;
+        it = m_orphans.lower_bound(Wtxid::FromUint256(InsecureRand256()));
         if (it == m_orphans.end())
             it = m_orphans.begin();
         return it->second.tx;
@@ -70,6 +70,16 @@ static CTransactionRef MakeTransactionSpending(const std::vector<COutPoint>& out
     return MakeTransactionRef(tx);
 }
 
+// Make another (not necessarily valid) tx with the same txid but different wtxid.
+static CTransactionRef MakeMutation(const CTransactionRef& ptx)
+{
+    CMutableTransaction tx(*ptx);
+    tx.vin[0].scriptWitness.stack.push_back({5});
+    auto mutated_tx = MakeTransactionRef(tx);
+    assert(ptx->GetHash() == mutated_tx->GetHash());
+    return mutated_tx;
+}
+
 static bool EqualTxns(const std::set<CTransactionRef>& set_txns, const std::vector<CTransactionRef>& vec_txns)
 {
     if (vec_txns.size() != set_txns.size()) return false;
@@ -180,6 +190,49 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
     BOOST_CHECK(orphanage.CountOrphans() == 0);
 }
 
+BOOST_AUTO_TEST_CASE(same_txid_diff_witness)
+{
+    FastRandomContext det_rand{true};
+    TxOrphanage orphanage;
+    NodeId peer{0};
+
+    std::vector<COutPoint> empty_outpoints;
+    auto parent = MakeTransactionSpending(empty_outpoints, det_rand);
+
+    // Create children to go into orphanage.
+    auto child_normal = MakeTransactionSpending({{parent->GetHash(), 0}}, det_rand);
+    auto child_mutated = MakeMutation(child_normal);
+
+    const auto& normal_wtxid = child_normal->GetWitnessHash();
+    const auto& mutated_wtxid = child_mutated->GetWitnessHash();
+    BOOST_CHECK(normal_wtxid != mutated_wtxid);
+
+    BOOST_CHECK(orphanage.AddTx(child_normal, peer));
+    // EraseTx fails as transaction by this wtxid doesn't exist.
+    BOOST_CHECK_EQUAL(orphanage.EraseTx(mutated_wtxid), 0);
+    BOOST_CHECK(orphanage.HaveTx(normal_wtxid));
+    BOOST_CHECK(!orphanage.HaveTx(mutated_wtxid));
+
+    // Must succeed. Both transactions should be present in orphanage.
+    BOOST_CHECK(orphanage.AddTx(child_mutated, peer));
+    BOOST_CHECK(orphanage.HaveTx(normal_wtxid));
+    BOOST_CHECK(orphanage.HaveTx(mutated_wtxid));
+
+    // Outpoints map should track all entries: check that both are returned as children of the parent.
+    std::set<CTransactionRef> expected_children{child_normal, child_mutated};
+    BOOST_CHECK(EqualTxns(expected_children, orphanage.GetChildrenFromSamePeer(parent, peer)));
+
+    // Erase by wtxid: mutated first
+    BOOST_CHECK_EQUAL(orphanage.EraseTx(mutated_wtxid), 1);
+    BOOST_CHECK(orphanage.HaveTx(normal_wtxid));
+    BOOST_CHECK(!orphanage.HaveTx(mutated_wtxid));
+
+    BOOST_CHECK_EQUAL(orphanage.EraseTx(normal_wtxid), 1);
+    BOOST_CHECK(!orphanage.HaveTx(normal_wtxid));
+    BOOST_CHECK(!orphanage.HaveTx(mutated_wtxid));
+}
+
+
 BOOST_AUTO_TEST_CASE(get_children)
 {
     FastRandomContext det_rand{true};

src/txorphanage.cpp

@@ -23,7 +23,7 @@ bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer)
 
     const Txid& hash = tx->GetHash();
     const Wtxid& wtxid = tx->GetWitnessHash();
-    if (m_orphans.count(hash))
+    if (m_orphans.count(wtxid))
         return false;
 
     // Ignore big transactions, to avoid a
@@ -40,30 +40,28 @@ bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer)
         return false;
     }
 
-    auto ret = m_orphans.emplace(hash, OrphanTx{tx, peer, GetTime() + ORPHAN_TX_EXPIRE_TIME, m_orphan_list.size()});
+    auto ret = m_orphans.emplace(wtxid, OrphanTx{tx, peer, GetTime() + ORPHAN_TX_EXPIRE_TIME, m_orphan_list.size()});
     assert(ret.second);
     m_orphan_list.push_back(ret.first);
-    // Allow for lookups in the orphan pool by wtxid, as well as txid
-    m_wtxid_to_orphan_it.emplace(tx->GetWitnessHash(), ret.first);
     for (const CTxIn& txin : tx->vin) {
         m_outpoint_to_orphan_it[txin.prevout].insert(ret.first);
     }
 
-    LogPrint(BCLog::TXPACKAGES, "stored orphan tx %s (wtxid=%s) (mapsz %u outsz %u)\n", hash.ToString(), wtxid.ToString(),
+    LogPrint(BCLog::TXPACKAGES, "stored orphan tx %s (wtxid=%s), weight: %u (mapsz %u outsz %u)\n", hash.ToString(), wtxid.ToString(), sz,
              m_orphans.size(), m_outpoint_to_orphan_it.size());
     return true;
 }
 
-int TxOrphanage::EraseTx(const Txid& txid)
+int TxOrphanage::EraseTx(const Wtxid& wtxid)
 {
     LOCK(m_mutex);
-    return EraseTxNoLock(txid);
+    return EraseTxNoLock(wtxid);
 }
 
-int TxOrphanage::EraseTxNoLock(const Txid& txid)
+int TxOrphanage::EraseTxNoLock(const Wtxid& wtxid)
 {
     AssertLockHeld(m_mutex);
-    std::map<Txid, OrphanTx>::iterator it = m_orphans.find(txid);
+    std::map<Wtxid, OrphanTx>::iterator it = m_orphans.find(wtxid);
     if (it == m_orphans.end())
         return 0;
     for (const CTxIn& txin : it->second.tx->vin)
@@ -85,10 +83,12 @@ int TxOrphanage::EraseTxNoLock(const Txid& txid)
         m_orphan_list[old_pos] = it_last;
         it_last->second.list_pos = old_pos;
     }
-    const auto& wtxid = it->second.tx->GetWitnessHash();
-    LogPrint(BCLog::TXPACKAGES, "   removed orphan tx %s (wtxid=%s)\n", txid.ToString(), wtxid.ToString());
+    const auto& txid = it->second.tx->GetHash();
+    // Time spent in orphanage = difference between current and entry time.
+    // Entry time is equal to ORPHAN_TX_EXPIRE_TIME earlier than entry's expiry.
+    LogPrint(BCLog::TXPACKAGES, "   removed orphan tx %s (wtxid=%s) after %ds\n", txid.ToString(), wtxid.ToString(),
+             GetTime() + ORPHAN_TX_EXPIRE_TIME - it->second.nTimeExpire);
     m_orphan_list.pop_back();
-    m_wtxid_to_orphan_it.erase(it->second.tx->GetWitnessHash());
 
     m_orphans.erase(it);
     return 1;
@@ -101,16 +101,16 @@ void TxOrphanage::EraseForPeer(NodeId peer)
     m_peer_work_set.erase(peer);
 
     int nErased = 0;
-    std::map<Txid, OrphanTx>::iterator iter = m_orphans.begin();
+    std::map<Wtxid, OrphanTx>::iterator iter = m_orphans.begin();
     while (iter != m_orphans.end())
     {
-        std::map<Txid, OrphanTx>::iterator maybeErase = iter++; // increment to avoid iterator becoming invalid
-        if (maybeErase->second.fromPeer == peer)
-        {
-            nErased += EraseTxNoLock(maybeErase->second.tx->GetHash());
+        // increment to avoid iterator becoming invalid after erasure
+        const auto& [wtxid, orphan] = *iter++;
+        if (orphan.fromPeer == peer) {
+            nErased += EraseTxNoLock(wtxid);
         }
     }
-    if (nErased > 0) LogPrint(BCLog::TXPACKAGES, "Erased %d orphan tx from peer=%d\n", nErased, peer);
+    if (nErased > 0) LogPrint(BCLog::TXPACKAGES, "Erased %d orphan transaction(s) from peer=%d\n", nErased, peer);
 }
 
 void TxOrphanage::LimitOrphans(unsigned int max_orphans, FastRandomContext& rng)
@@ -124,12 +124,12 @@ void TxOrphanage::LimitOrphans(unsigned int max_orphans, FastRandomContext& rng)
         // Sweep out expired orphan pool entries:
         int nErased = 0;
         int64_t nMinExpTime = nNow + ORPHAN_TX_EXPIRE_TIME - ORPHAN_TX_EXPIRE_INTERVAL;
-        std::map<Txid, OrphanTx>::iterator iter = m_orphans.begin();
+        std::map<Wtxid, OrphanTx>::iterator iter = m_orphans.begin();
         while (iter != m_orphans.end())
         {
-            std::map<Txid, OrphanTx>::iterator maybeErase = iter++;
+            std::map<Wtxid, OrphanTx>::iterator maybeErase = iter++;
             if (maybeErase->second.nTimeExpire <= nNow) {
-                nErased += EraseTxNoLock(maybeErase->second.tx->GetHash());
+                nErased += EraseTxNoLock(maybeErase->second.tx->GetWitnessHash());
             } else {
                 nMinExpTime = std::min(maybeErase->second.nTimeExpire, nMinExpTime);
             }
@@ -142,7 +142,7 @@ void TxOrphanage::LimitOrphans(unsigned int max_orphans, FastRandomContext& rng)
     {
         // Evict a random orphan:
         size_t randompos = rng.randrange(m_orphan_list.size());
-        EraseTxNoLock(m_orphan_list[randompos]->first);
+        EraseTxNoLock(m_orphan_list[randompos]->second.tx->GetWitnessHash());
         ++nEvicted;
     }
     if (nEvicted > 0) LogPrint(BCLog::TXPACKAGES, "orphanage overflow, removed %u tx\n", nEvicted);
@@ -159,7 +159,7 @@ void TxOrphanage::AddChildrenToWorkSet(const CTransaction& tx)
             for (const auto& elem : it_by_prev->second) {
                 // Get this source peer's work set, emplacing an empty set if it didn't exist
                 // (note: if this peer wasn't still connected, we would have removed the orphan tx already)
-                std::set<Txid>& orphan_work_set = m_peer_work_set.try_emplace(elem->second.fromPeer).first->second;
+                std::set<Wtxid>& orphan_work_set = m_peer_work_set.try_emplace(elem->second.fromPeer).first->second;
                 // Add this tx to the work set
                 orphan_work_set.insert(elem->first);
                 LogPrint(BCLog::TXPACKAGES, "added %s (wtxid=%s) to peer %d workset\n",
@@ -169,14 +169,10 @@ void TxOrphanage::AddChildrenToWorkSet(const CTransaction& tx)
     }
 }
 
-bool TxOrphanage::HaveTx(const GenTxid& gtxid) const
+bool TxOrphanage::HaveTx(const Wtxid& wtxid) const
 {
     LOCK(m_mutex);
-    if (gtxid.IsWtxid()) {
-        return m_wtxid_to_orphan_it.count(Wtxid::FromUint256(gtxid.GetHash()));
-    } else {
-        return m_orphans.count(Txid::FromUint256(gtxid.GetHash()));
-    }
+    return m_orphans.count(wtxid);
 }
 
 CTransactionRef TxOrphanage::GetTxToReconsider(NodeId peer)
@@ -187,10 +183,10 @@ CTransactionRef TxOrphanage::GetTxToReconsider(NodeId peer)
     if (work_set_it != m_peer_work_set.end()) {
         auto& work_set = work_set_it->second;
         while (!work_set.empty()) {
-            Txid txid = *work_set.begin();
+            Wtxid wtxid = *work_set.begin();
             work_set.erase(work_set.begin());
 
-            const auto orphan_it = m_orphans.find(txid);
+            const auto orphan_it = m_orphans.find(wtxid);
             if (orphan_it != m_orphans.end()) {
                 return orphan_it->second.tx;
             }
@@ -215,7 +211,7 @@ void TxOrphanage::EraseForBlock(const CBlock& block)
 {
     LOCK(m_mutex);
 
-    std::vector<Txid> vOrphanErase;
+    std::vector<Wtxid> vOrphanErase;
 
     for (const CTransactionRef& ptx : block.vtx) {
         const CTransaction& tx = *ptx;
@@ -226,8 +222,7 @@ void TxOrphanage::EraseForBlock(const CBlock& block)
             if (itByPrev == m_outpoint_to_orphan_it.end()) continue;
             for (auto mi = itByPrev->second.begin(); mi != itByPrev->second.end(); ++mi) {
                 const CTransaction& orphanTx = *(*mi)->second.tx;
-                const auto& orphanHash = orphanTx.GetHash();
-                vOrphanErase.push_back(orphanHash);
+                vOrphanErase.push_back(orphanTx.GetWitnessHash());
             }
         }
     }
@@ -238,7 +233,7 @@ void TxOrphanage::EraseForBlock(const CBlock& block)
         for (const auto& orphanHash : vOrphanErase) {
             nErased += EraseTxNoLock(orphanHash);
         }
-        LogPrint(BCLog::TXPACKAGES, "Erased %d orphan tx included or conflicted by block\n", nErased);
+        LogPrint(BCLog::TXPACKAGES, "Erased %d orphan transaction(s) included or conflicted by block\n", nErased);
     }
 }
 

src/txorphanage.h

@@ -23,8 +23,8 @@ class TxOrphanage {
     /** Add a new orphan transaction */
     bool AddTx(const CTransactionRef& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
 
-    /** Check if we already have an orphan transaction (by txid or wtxid) */
-    bool HaveTx(const GenTxid& gtxid) const EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
+    /** Check if we already have an orphan transaction (by wtxid only) */
+    bool HaveTx(const Wtxid& wtxid) const EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
 
     /** Extract a transaction from a peer's work set
      *  Returns nullptr if there are no transactions to work on.
@@ -33,8 +33,8 @@ class TxOrphanage {
      */
     CTransactionRef GetTxToReconsider(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
 
-    /** Erase an orphan by txid */
-    int EraseTx(const Txid& txid) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
+    /** Erase an orphan by wtxid */
+    int EraseTx(const Wtxid& wtxid) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
 
     /** Erase all orphans announced by a peer (eg, after that peer disconnects) */
     void EraseForPeer(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
@@ -77,12 +77,12 @@ class TxOrphanage {
         size_t list_pos;
     };
 
-    /** Map from txid to orphan transaction record. Limited by
+    /** Map from wtxid to orphan transaction record. Limited by
      *  -maxorphantx/DEFAULT_MAX_ORPHAN_TRANSACTIONS */
-    std::map<Txid, OrphanTx> m_orphans GUARDED_BY(m_mutex);
+    std::map<Wtxid, OrphanTx> m_orphans GUARDED_BY(m_mutex);
 
     /** Which peer provided the orphans that need to be reconsidered */
-    std::map<NodeId, std::set<Txid>> m_peer_work_set GUARDED_BY(m_mutex);
+    std::map<NodeId, std::set<Wtxid>> m_peer_work_set GUARDED_BY(m_mutex);
 
     using OrphanMap = decltype(m_orphans);
 
@@ -102,12 +102,8 @@ class TxOrphanage {
     /** Orphan transactions in vector for quick random eviction */
     std::vector<OrphanMap::iterator> m_orphan_list GUARDED_BY(m_mutex);
 
-    /** Index from wtxid into the m_orphans to lookup orphan
-     *  transactions using their witness ids. */
-    std::map<Wtxid, OrphanMap::iterator> m_wtxid_to_orphan_it GUARDED_BY(m_mutex);
-
-    /** Erase an orphan by txid */
-    int EraseTxNoLock(const Txid& txid) EXCLUSIVE_LOCKS_REQUIRED(m_mutex);
+    /** Erase an orphan by wtxid */
+    int EraseTxNoLock(const Wtxid& wtxid) EXCLUSIVE_LOCKS_REQUIRED(m_mutex);
 };
 
 #endif // BITCOIN_TXORPHANAGE_H