[prep] Return the made-reconsiderable announcements in AddChildrenToWorkSet

This is preparation for the simulation fuzz test added in a later commit. Since
AddChildrenToWorkSet consumes randomness, there is no way for the simulator to
exactly predict its behavior. By returning the set of made-reconsiderable announcements
instead, the simulator can instead test that it is *a* valid choice, and then
apply it to its own data structures.

Author: sipa

src/node/txorphanage.cpp

@@ -217,7 +217,7 @@ class TxOrphanageImpl final : public TxOrphanage {
     void EraseForPeer(NodeId peer) override;
     void EraseForBlock(const CBlock& block) override;
     void LimitOrphans() override;
-    void AddChildrenToWorkSet(const CTransaction& tx, FastRandomContext& rng) override;
+    std::vector<std::pair<Wtxid, NodeId>> AddChildrenToWorkSet(const CTransaction& tx, FastRandomContext& rng) override;
     bool HaveTxToReconsider(NodeId peer) override;
     std::vector<CTransactionRef> GetChildrenFromSamePeer(const CTransactionRef& parent, NodeId nodeid) const override;
     size_t Size() const override { return m_unique_orphans; }
@@ -489,8 +489,9 @@ void TxOrphanageImpl::LimitOrphans()
     LogDebug(BCLog::TXPACKAGES, "orphanage overflow, removed %u tx (%u announcements)\n", original_unique_txns - remaining_unique_orphans, num_erased);
 }
 
-void TxOrphanageImpl::AddChildrenToWorkSet(const CTransaction& tx, FastRandomContext& rng)
+std::vector<std::pair<Wtxid, NodeId>> TxOrphanageImpl::AddChildrenToWorkSet(const CTransaction& tx, FastRandomContext& rng)
 {
+    std::vector<std::pair<Wtxid, NodeId>> ret;
     auto& index_by_wtxid = m_orphans.get<ByWtxid>();
     for (unsigned int i = 0; i < tx.vout.size(); i++) {
         const auto it_by_prev = m_outpoint_to_orphan_it.find(COutPoint(tx.GetHash(), i));
@@ -512,13 +513,17 @@ void TxOrphanageImpl::AddChildrenToWorkSet(const CTransaction& tx, FastRandomCon
 
                 // Mark this orphan as ready to be reconsidered.
                 static constexpr auto mark_reconsidered_modifier = [](auto& ann) { ann.m_reconsider = true; };
-                index_by_wtxid.modify(it, mark_reconsidered_modifier);
+                if (!it->m_reconsider) {
+                    index_by_wtxid.modify(it, mark_reconsidered_modifier);
+                    ret.emplace_back(wtxid, it->m_announcer);
+                }
 
                 LogDebug(BCLog::TXPACKAGES, "added %s (wtxid=%s) to peer %d workset\n",
                             it->m_tx->GetHash().ToString(), it->m_tx->GetWitnessHash().ToString(), it->m_announcer);
             }
         }
     }
+    return ret;
 }
 
 bool TxOrphanageImpl::HaveTx(const Wtxid& wtxid) const

src/node/txorphanage.h

@@ -92,7 +92,7 @@ class TxOrphanage {
     virtual void LimitOrphans() = 0;
 
     /** Add any orphans that list a particular tx as a parent into the from peer's work set */
-    virtual void AddChildrenToWorkSet(const CTransaction& tx, FastRandomContext& rng) = 0;
+    virtual std::vector<std::pair<Wtxid, NodeId>> AddChildrenToWorkSet(const CTransaction& tx, FastRandomContext& rng) = 0;
 
     /** Does this peer have any work to do? */
     virtual bool HaveTxToReconsider(NodeId peer) = 0;

src/test/orphanage_tests.cpp

@@ -185,7 +185,8 @@ BOOST_AUTO_TEST_CASE(peer_dos_limits)
         orphanage->AddTx(children.at(2), peer);
 
         // Make child0 ready to reconsider
-        orphanage->AddChildrenToWorkSet(*parents.at(0), det_rand);
+        const std::vector<std::pair<Wtxid, NodeId>> expected_set_c0{std::make_pair(children.at(0)->GetWitnessHash(), peer)};
+        BOOST_CHECK(orphanage->AddChildrenToWorkSet(*parents.at(0), det_rand) == expected_set_c0);
         BOOST_CHECK(orphanage->HaveTxToReconsider(peer));
 
         // Add 1 more orphan, causing the orphanage to be oversize. child1 is evicted.
@@ -204,9 +205,11 @@ BOOST_AUTO_TEST_CASE(peer_dos_limits)
         BOOST_CHECK(orphanage->HaveTx(children.at(3)->GetWitnessHash()));
         BOOST_CHECK(orphanage->HaveTx(children.at(4)->GetWitnessHash()));
 
-        // Eviction order is FIFO within the orphans that are ready to be reconsidered.
-        orphanage->AddChildrenToWorkSet(*parents.at(4), det_rand);
-        orphanage->AddChildrenToWorkSet(*parents.at(3), det_rand);
+        // Eviction order is FIFO within the orphans that are read
+        const std::vector<std::pair<Wtxid, NodeId>> expected_set_c4{std::make_pair(children.at(4)->GetWitnessHash(), peer)};
+        BOOST_CHECK(orphanage->AddChildrenToWorkSet(*parents.at(4), det_rand) == expected_set_c4);
+        const std::vector<std::pair<Wtxid, NodeId>> expected_set_c3{std::make_pair(children.at(3)->GetWitnessHash(), peer)};
+        BOOST_CHECK(orphanage->AddChildrenToWorkSet(*parents.at(3), det_rand) == expected_set_c3);
 
         // child5 is evicted immediately because it is the only non-reconsiderable orphan.
         orphanage->AddTx(children.at(5), peer);
@@ -857,7 +860,8 @@ BOOST_AUTO_TEST_CASE(peer_worksets)
         }
 
         // Parent accepted: child is added to 1 of 3 worksets.
-        orphanage->AddChildrenToWorkSet(*tx_missing_parent, det_rand);
+        auto newly_reconsiderable = orphanage->AddChildrenToWorkSet(*tx_missing_parent, det_rand);
+        BOOST_CHECK_EQUAL(newly_reconsiderable.size(), 1);
         int node0_reconsider = orphanage->HaveTxToReconsider(node0);
         int node1_reconsider = orphanage->HaveTxToReconsider(node1);
         int node2_reconsider = orphanage->HaveTxToReconsider(node2);