[txorphanage] support multiple announcers

glozow · glozow · commit e810842acda6 · 2025-01-06T09:02:05.000-05:00
Add ability to add and track multiple announcers per orphan transaction,
erasing announcers but not the entire orphan.

The tx creation code in orphanage_tests needs to be updated so that each
tx is unique, because the CountOrphans() check assumes that calling
EraseForPeer necessarily means its orphans are deleted.

Unused for now.
diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp
@@ -843,7 +843,9 @@ static UniValue OrphanToJSON(const TxOrphanage::OrphanTxBase& orphan)
     o.pushKV("entry", int64_t{TicksSinceEpoch<std::chrono::seconds>(orphan.nTimeExpire - ORPHAN_TX_EXPIRE_TIME)});
     o.pushKV("expiration", int64_t{TicksSinceEpoch<std::chrono::seconds>(orphan.nTimeExpire)});
     UniValue from(UniValue::VARR);
-    from.push_back(orphan.fromPeer); // only one fromPeer for now
+    for (const auto fromPeer: orphan.announcers) {
+        from.push_back(fromPeer);
+    }
     o.pushKV("from", from);
     return o;
 }
diff --git a/src/test/orphanage_tests.cpp b/src/test/orphanage_tests.cpp
@@ -132,7 +132,7 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
         tx.vin[0].prevout.hash = Txid::FromUint256(m_rng.rand256());
         tx.vin[0].scriptSig << OP_1;
         tx.vout.resize(1);
-        tx.vout[0].nValue = 1*CENT;
+        tx.vout[0].nValue = i*CENT;
         tx.vout[0].scriptPubKey = GetScriptForDestination(PKHash(key.GetPubKey()));
 
         orphanage.AddTx(MakeTransactionRef(tx), i);
@@ -148,7 +148,7 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
         tx.vin[0].prevout.n = 0;
         tx.vin[0].prevout.hash = txPrev->GetHash();
         tx.vout.resize(1);
-        tx.vout[0].nValue = 1*CENT;
+        tx.vout[0].nValue = i*CENT;
         tx.vout[0].scriptPubKey = GetScriptForDestination(PKHash(key.GetPubKey()));
         SignatureData empty;
         BOOST_CHECK(SignSignature(keystore, *txPrev, tx, 0, SIGHASH_ALL, empty));
diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp
@@ -16,8 +16,11 @@ bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer)
 {
     const Txid& hash = tx->GetHash();
     const Wtxid& wtxid = tx->GetWitnessHash();
-    if (m_orphans.count(wtxid))
+    if (auto it{m_orphans.find(wtxid)}; it != m_orphans.end()) {
+        AddAnnouncer(wtxid, peer);
+        // No new orphan entry was created. An announcer may have been added.
         return false;
+    }
 
     // Ignore big transactions, to avoid a
     // send-big-orphans memory exhaustion attack. If a peer has a legitimate
@@ -33,7 +36,7 @@ bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer)
         return false;
     }
 
-    auto ret = m_orphans.emplace(wtxid, OrphanTx{{tx, peer, Now<NodeSeconds>() + ORPHAN_TX_EXPIRE_TIME}, m_orphan_list.size()});
+    auto ret = m_orphans.emplace(wtxid, OrphanTx{{tx, {peer}, Now<NodeSeconds>() + ORPHAN_TX_EXPIRE_TIME}, m_orphan_list.size()});
     assert(ret.second);
     m_orphan_list.push_back(ret.first);
     for (const CTxIn& txin : tx->vin) {
@@ -45,6 +48,20 @@ bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer)
     return true;
 }
 
+bool TxOrphanage::AddAnnouncer(const Wtxid& wtxid, NodeId peer)
+{
+    const auto it = m_orphans.find(wtxid);
+    if (it != m_orphans.end()) {
+        Assume(!it->second.announcers.empty());
+        const auto ret = it->second.announcers.insert(peer);
+        if (ret.second) {
+            LogDebug(BCLog::TXPACKAGES, "added peer=%d as announcer of orphan tx %s\n", peer, wtxid.ToString());
+            return true;
+        }
+    }
+    return false;
+}
+
 int TxOrphanage::EraseTx(const Wtxid& wtxid)
 {
     std::map<Wtxid, OrphanTx>::iterator it = m_orphans.find(wtxid);
@@ -89,9 +106,15 @@ void TxOrphanage::EraseForPeer(NodeId peer)
     while (iter != m_orphans.end())
     {
         // increment to avoid iterator becoming invalid after erasure
-        const auto& [wtxid, orphan] = *iter++;
-        if (orphan.fromPeer == peer) {
-            nErased += EraseTx(wtxid);
+        auto& [wtxid, orphan] = *iter++;
+        auto orphan_it = orphan.announcers.find(peer);
+        if (orphan_it != orphan.announcers.end()) {
+            orphan.announcers.erase(peer);
+
+            // No remaining annnouncers: clean up entry
+            if (orphan.announcers.empty()) {
+                nErased += EraseTx(orphan.tx->GetWitnessHash());
+            }
         }
     }
     if (nErased > 0) LogDebug(BCLog::TXPACKAGES, "Erased %d orphan transaction(s) from peer=%d\n", nErased, peer);
@@ -110,7 +133,7 @@ void TxOrphanage::LimitOrphans(unsigned int max_orphans, FastRandomContext& rng)
         {
             std::map<Wtxid, OrphanTx>::iterator maybeErase = iter++;
             if (maybeErase->second.nTimeExpire <= nNow) {
-                nErased += EraseTx(maybeErase->second.tx->GetWitnessHash());
+                nErased += EraseTx(maybeErase->first);
             } else {
                 nMinExpTime = std::min(maybeErase->second.nTimeExpire, nMinExpTime);
             }
@@ -123,7 +146,7 @@ void TxOrphanage::LimitOrphans(unsigned int max_orphans, FastRandomContext& rng)
     {
         // Evict a random orphan:
         size_t randompos = rng.randrange(m_orphan_list.size());
-        EraseTx(m_orphan_list[randompos]->second.tx->GetWitnessHash());
+        EraseTx(m_orphan_list[randompos]->first);
         ++nEvicted;
     }
     if (nEvicted > 0) LogDebug(BCLog::TXPACKAGES, "orphanage overflow, removed %u tx\n", nEvicted);
@@ -135,13 +158,17 @@ void TxOrphanage::AddChildrenToWorkSet(const CTransaction& tx)
         const auto it_by_prev = m_outpoint_to_orphan_it.find(COutPoint(tx.GetHash(), i));
         if (it_by_prev != m_outpoint_to_orphan_it.end()) {
             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<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);
-                LogDebug(BCLog::TXPACKAGES, "added %s (wtxid=%s) to peer %d workset\n",
-                         tx.GetHash().ToString(), tx.GetWitnessHash().ToString(), elem->second.fromPeer);
+                // Belt and suspenders, each orphan should always have at least 1 announcer.
+                if (!Assume(!elem->second.announcers.empty())) continue;
+                for (const auto announcer: elem->second.announcers) {
+                    // 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<Wtxid>& orphan_work_set = m_peer_work_set.try_emplace(announcer).first->second;
+                    // Add this tx to the work set
+                    orphan_work_set.insert(elem->first);
+                    LogDebug(BCLog::TXPACKAGES, "added %s (wtxid=%s) to peer %d workset\n",
+                             tx.GetHash().ToString(), tx.GetWitnessHash().ToString(), announcer);
+                }
             }
         }
     }
@@ -152,6 +179,12 @@ bool TxOrphanage::HaveTx(const Wtxid& wtxid) const
     return m_orphans.count(wtxid);
 }
 
+bool TxOrphanage::HaveTxFromPeer(const Wtxid& wtxid, NodeId peer) const
+{
+    auto it = m_orphans.find(wtxid);
+    return (it != m_orphans.end() && it->second.announcers.contains(peer));
+}
+
 CTransactionRef TxOrphanage::GetTxToReconsider(NodeId peer)
 {
     auto work_set_it = m_peer_work_set.find(peer);
@@ -219,7 +252,7 @@ std::vector<CTransactionRef> TxOrphanage::GetChildrenFromSamePeer(const CTransac
         const auto it_by_prev = m_outpoint_to_orphan_it.find(COutPoint(parent->GetHash(), i));
         if (it_by_prev != m_outpoint_to_orphan_it.end()) {
             for (const auto& elem : it_by_prev->second) {
-                if (elem->second.fromPeer == nodeid) {
+                if (elem->second.announcers.contains(nodeid)) {
                     iters.emplace_back(elem);
                 }
             }
@@ -258,7 +291,7 @@ std::vector<std::pair<CTransactionRef, NodeId>> TxOrphanage::GetChildrenFromDiff
         const auto it_by_prev = m_outpoint_to_orphan_it.find(COutPoint(parent->GetHash(), i));
         if (it_by_prev != m_outpoint_to_orphan_it.end()) {
             for (const auto& elem : it_by_prev->second) {
-                if (elem->second.fromPeer != nodeid) {
+                if (!elem->second.announcers.contains(nodeid)) {
                     iters.emplace_back(elem);
                 }
             }
@@ -273,7 +306,9 @@ std::vector<std::pair<CTransactionRef, NodeId>> TxOrphanage::GetChildrenFromDiff
     std::vector<std::pair<CTransactionRef, NodeId>> children_found;
     children_found.reserve(iters.size());
     for (const auto& child_iter : iters) {
-        children_found.emplace_back(child_iter->second.tx, child_iter->second.fromPeer);
+        // Use first peer in announcers list
+        auto peer = *(child_iter->second.announcers.begin());
+        children_found.emplace_back(child_iter->second.tx, peer);
     }
     return children_found;
 }
@@ -283,7 +318,7 @@ std::vector<TxOrphanage::OrphanTxBase> TxOrphanage::GetOrphanTransactions() cons
     std::vector<OrphanTxBase> ret;
     ret.reserve(m_orphans.size());
     for (auto const& o : m_orphans) {
-        ret.push_back({o.second.tx, o.second.fromPeer, o.second.nTimeExpire});
+        ret.push_back({o.second.tx, o.second.announcers, o.second.nTimeExpire});
     }
     return ret;
 }
diff --git a/src/txorphanage.h b/src/txorphanage.h
@@ -30,9 +30,15 @@ class TxOrphanage {
     /** Add a new orphan transaction */
     bool AddTx(const CTransactionRef& tx, NodeId peer);
 
+    /** Add an additional announcer to an orphan if it exists. Otherwise, do nothing. */
+    bool AddAnnouncer(const Wtxid& wtxid, NodeId peer);
+
     /** Check if we already have an orphan transaction (by wtxid only) */
     bool HaveTx(const Wtxid& wtxid) const;
 
+    /** Check if a {tx, peer} exists in the orphanage.*/
+    bool HaveTxFromPeer(const Wtxid& wtxid, NodeId peer) const;
+
     /** Extract a transaction from a peer's work set
      *  Returns nullptr if there are no transactions to work on.
      *  Otherwise returns the transaction reference, and removes
@@ -43,7 +49,8 @@ class TxOrphanage {
     /** Erase an orphan by wtxid */
     int EraseTx(const Wtxid& wtxid);
 
-    /** Erase all orphans announced by a peer (eg, after that peer disconnects) */
+    /** Maybe erase all orphans announced by a peer (eg, after that peer disconnects). If an orphan
+     * has been announced by another peer, don't erase, just remove this peer from the list of announcers. */
     void EraseForPeer(NodeId peer);
 
     /** Erase all orphans included in or invalidated by a new block */
@@ -75,7 +82,8 @@ class TxOrphanage {
     /** Allows providing orphan information externally */
     struct OrphanTxBase {
         CTransactionRef tx;
-        NodeId fromPeer;
+        /** Peers added with AddTx or AddAnnouncer. */
+        std::set<NodeId> announcers;
         NodeSeconds nTimeExpire;
     };
 
