diff --git a/src/invrequest.h b/src/invrequest.h --- a/src/invrequest.h +++ b/src/invrequest.h @@ -51,7 +51,7 @@ * are forgotten only: * - If a peer goes offline, all its announcements are forgotten. * - If an inventory has been successfully received, or is otherwise no - * longer needed, the caller can call ForgetTxId, which removes all + * longer needed, the caller can call ForgetInvId, which removes all * announcements across all peers with the specified invid. * - If for a given invid only already-failed announcements remain, they are * all forgotten. @@ -119,7 +119,7 @@ // Avoid littering this header file with implementation details. class InvRequestTrackerImplInterface { - template friend class TxRequestTracker; + template friend class InvRequestTracker; // The base class is responsible for building the child implementation. // This is a hack that allows for hiding the concrete implementation details @@ -137,13 +137,13 @@ virtual void ReceivedInv(NodeId peer, const uint256 &invid, bool preferred, std::chrono::microseconds reqtime) = 0; virtual void DisconnectedPeer(NodeId peer) = 0; - virtual void ForgetTxId(const uint256 &invid) = 0; + virtual void ForgetInvId(const uint256 &invid) = 0; virtual std::vector GetRequestable(NodeId peer, std::chrono::microseconds now, ClearExpiredFun clearExpired, EmplaceExpiredFun emplaceExpired) = 0; - virtual void RequestedTx(NodeId peer, const uint256 &invid, - std::chrono::microseconds expiry) = 0; + virtual void RequestedData(NodeId peer, const uint256 &invid, + std::chrono::microseconds expiry) = 0; virtual void ReceivedResponse(NodeId peer, const uint256 &invid) = 0; virtual size_t CountInFlight(NodeId peer) const = 0; virtual size_t CountCandidates(NodeId peer) const = 0; @@ -156,7 +156,7 @@ PostGetRequestableSanityCheck(std::chrono::microseconds now) const = 0; }; -template class TxRequestTracker { +template class InvRequestTracker { /* * Only uint256-based InvId types are supported for now. * FIXME: use a template constraint when C++20 is available. @@ -167,11 +167,10 @@ const std::unique_ptr m_impl; public: - //! Construct a TxRequestTracker. - - explicit TxRequestTracker(bool deterministic = false) + //! Construct a InvRequestTracker. + explicit InvRequestTracker(bool deterministic = false) : m_impl{InvRequestTrackerImplInterface::BuildImpl(deterministic)} {} - ~TxRequestTracker() = default; + ~InvRequestTracker() = default; // Conceptually, the data structure consists of a collection of // "announcements", one for each peer/invid combination: @@ -218,7 +217,7 @@ * should ensure that new announcements for the same invid will not trigger * new ReceivedInv calls, at least in the short term after this call. */ - void ForgetTxId(const InvId &invid) { m_impl->ForgetTxId(invid); } + void ForgetInvId(const InvId &invid) { m_impl->ForgetInvId(invid); } /** * Find the invids to request now from peer. @@ -273,9 +272,9 @@ * never advise doing so). In this case it is converted to COMPLETED, as * we're no longer waiting for a response to it. */ - void RequestedTx(NodeId peer, const InvId &invid, - std::chrono::microseconds expiry) { - m_impl->RequestedTx(peer, invid, expiry); + void RequestedData(NodeId peer, const InvId &invid, + std::chrono::microseconds expiry) { + m_impl->RequestedData(peer, invid, expiry); } /** @@ -284,7 +283,7 @@ * happens. * * It should be called whenever an inventory or NOTFOUND was received from - * a peer. When the inventory is not needed entirely anymore, ForgetTxId + * a peer. When the inventory is not needed entirely anymore, ForgetInvId * should be called instead of, or in addition to, this call. */ void ReceivedResponse(NodeId peer, const InvId &invid) { diff --git a/src/invrequest.cpp b/src/invrequest.cpp --- a/src/invrequest.cpp +++ b/src/invrequest.cpp @@ -191,7 +191,7 @@ // Note: priority == 0 whenever state != CANDIDATE_READY. // // Uses: -// * Deleting all announcements with a given invid in ForgetTxId. +// * Deleting all announcements with a given invid in ForgetInvId. // * Finding the best CANDIDATE_READY to convert to CANDIDATE_BEST, when no // other CANDIDATE_READY or REQUESTED announcement exists for that invid. // * Determining when no more non-COMPLETED announcements for a given invid @@ -352,7 +352,7 @@ } // namespace -/** Actual implementation for TxRequestTracker's data structure. */ +/** Actual implementation for InvRequestTracker's data structure. */ class InvRequestTrackerImpl : public InvRequestTrackerImplInterface { //! The current sequence number. Increases for every announcement. This is //! used to sort invid returned by GetRequestable in announcement order. @@ -608,7 +608,7 @@ // CANDIDATE_READY announcements back to CANDIDATE_DELAYED. This is // an unusual edge case, and unlikely to matter in production. // However, it makes it much easier to specify and test - // TxRequestTracker::Impl's behaviour. + // InvRequestTracker::Impl's behaviour. auto it = std::prev(m_index.get().end()); if (it->IsSelectable() && it->m_time > now) { ChangeAndReselect(m_index.project(it), @@ -680,7 +680,7 @@ } } - void ForgetTxId(const uint256 &invid) { + void ForgetInvId(const uint256 &invid) { auto it = m_index.get().lower_bound( ByInvIdView{invid, State::CANDIDATE_DELAYED, 0}); while (it != m_index.get().end() && it->m_invid == invid) { @@ -748,14 +748,14 @@ return ret; } - void RequestedTx(NodeId peer, const uint256 &invid, - std::chrono::microseconds expiry) { + void RequestedData(NodeId peer, const uint256 &invid, + std::chrono::microseconds expiry) { auto it = m_index.get().find(ByPeerView{peer, true, invid}); if (it == m_index.get().end()) { // There is no CANDIDATE_BEST announcement, look for a _READY or - // _DELAYED instead. If the caller only ever invokes RequestedTx + // _DELAYED instead. If the caller only ever invokes RequestedData // with the values returned by GetRequestable, and no other - // non-const functions other than ForgetTxId and GetRequestable in + // non-const functions other than ForgetInvId and GetRequestable in // between, this branch will never execute (as invids returned by // GetRequestable always correspond to CANDIDATE_BEST // announcements). @@ -768,7 +768,7 @@ // we have nothing to do. Either this invid wasn't tracked at // all (and the caller should have called ReceivedInv), or it // was already requested and/or completed for other reasons and - // this is just a superfluous RequestedTx call. + // this is just a superfluous RequestedData call. return; } diff --git a/src/net_processing.h b/src/net_processing.h --- a/src/net_processing.h +++ b/src/net_processing.h @@ -207,7 +207,7 @@ BanMan *const m_banman; ChainstateManager &m_chainman; CTxMemPool &m_mempool; - TxRequestTracker m_txrequest GUARDED_BY(::cs_main); + InvRequestTracker m_txrequest GUARDED_BY(::cs_main); //! Next time to check for stale tip int64_t m_stale_tip_check_time; diff --git a/src/net_processing.cpp b/src/net_processing.cpp --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -935,7 +935,7 @@ } const CNodeState *state = State(nodeid); - // Decide the TxRequestTracker parameters for this announcement: + // Decide the InvRequestTracker parameters for this announcement: // - "preferred": if fPreferredDownload is set (= outbound, or PF_NOBAN // permission) // - "reqtime": current time plus delays for: @@ -1478,7 +1478,7 @@ { LOCK(cs_main); for (const auto &ptx : pblock->vtx) { - m_txrequest.ForgetTxId(ptx->GetId()); + m_txrequest.ForgetInvId(ptx->GetId()); } } } @@ -3350,7 +3350,7 @@ m_mempool.check(&::ChainstateActive().CoinsTip()); // As this version of the transaction was acceptable, we can forget // about any requests for it. - m_txrequest.ForgetTxId(tx.GetId()); + m_txrequest.ForgetInvId(tx.GetId()); RelayTransaction(tx.GetId(), m_connman); for (size_t i = 0; i < tx.vout.size(); i++) { auto it_by_prev = @@ -3398,7 +3398,7 @@ // Once added to the orphan pool, a tx is considered // AlreadyHave, and we shouldn't request it anymore. - m_txrequest.ForgetTxId(tx.GetId()); + m_txrequest.ForgetInvId(tx.GetId()); // DoS prevention: do not allow mapOrphanTransactions to grow // unbounded (see CVE-2012-3789) @@ -3417,12 +3417,12 @@ // We will continue to reject this tx since it has rejected // parents so avoid re-requesting it from other peers. recentRejects->insert(tx.GetId()); - m_txrequest.ForgetTxId(tx.GetId()); + m_txrequest.ForgetInvId(tx.GetId()); } } else { assert(recentRejects); recentRejects->insert(tx.GetId()); - m_txrequest.ForgetTxId(tx.GetId()); + m_txrequest.ForgetInvId(tx.GetId()); if (RecursiveDynamicUsage(*ptx) < 100000) { AddToCompactExtraTransactions(ptx); @@ -4366,7 +4366,7 @@ if (inv.IsMsgTx()) { // If we receive a NOTFOUND message for a tx we requested, // mark the announcement for it as completed in - // TxRequestTracker. + // InvRequestTracker. m_txrequest.ReceivedResponse(pfrom.GetId(), TxId(inv.hash)); } } @@ -5422,13 +5422,13 @@ for (const TxId &txid : requestable) { if (!AlreadyHaveTx(txid, m_mempool)) { addGetDataAndMaybeFlush(MSG_TX, txid); - m_txrequest.RequestedTx(pto->GetId(), txid, - current_time + GETDATA_TX_INTERVAL); + m_txrequest.RequestedData(pto->GetId(), txid, + current_time + GETDATA_TX_INTERVAL); } else { // We have already seen this transaction, no need to download. // This is just a belt-and-suspenders, as this should already be // called whenever a transaction becomes AlreadyHaveTx(). - m_txrequest.ForgetTxId(txid); + m_txrequest.ForgetInvId(txid); } } diff --git a/src/test/fuzz/txrequest.cpp b/src/test/fuzz/txrequest.cpp --- a/src/test/fuzz/txrequest.cpp +++ b/src/test/fuzz/txrequest.cpp @@ -54,23 +54,23 @@ } g_initializer; /** - * Tester class for TxRequestTracker + * Tester class for InvRequestTracker * * It includes a naive reimplementation of its behavior, for a limited set * of MAX_TXIDS distinct txids, and MAX_PEERS peer identifiers. * * All of the public member functions perform the same operation on - * an actual TxRequestTracker and on the state of the reimplementation. + * an actual InvRequestTracker and on the state of the reimplementation. * The output of GetRequestable is compared with the expected value * as well. * - * Check() calls the TxRequestTracker's sanity check, plus compares the + * Check() calls the InvRequestTracker's sanity check, plus compares the * output of the constant accessors (Size(), CountLoad(), CountTracked()) * with expected values. */ class Tester { - //! TxRequestTracker object being tested. - TxRequestTracker m_tracker; + //! InvRequestTracker object being tested. + InvRequestTracker m_tracker; //! States for txid/peer combinations in the naive data structure. enum class State { @@ -183,7 +183,7 @@ } } - // Call TxRequestTracker's implementation. + // Call InvRequestTracker's implementation. m_tracker.DisconnectedPeer(peer); } @@ -194,8 +194,8 @@ } Cleanup(txid); - // Call TxRequestTracker's implementation. - m_tracker.ForgetTxId(TXIDS[txid]); + // Call InvRequestTracker's implementation. + m_tracker.ForgetInvId(TXIDS[txid]); } void ReceivedInv(int peer, int txid, bool is_wtxid, bool preferred, @@ -218,7 +218,7 @@ } } - // Call TxRequestTracker's implementation. + // Call InvRequestTracker's implementation. m_tracker.ReceivedInv(peer, TXIDS[txid], preferred, reqtime); } @@ -242,8 +242,8 @@ m_events.push(exptime); } - // Call TxRequestTracker's implementation. - m_tracker.RequestedTx(peer, TXIDS[txid], exptime); + // Call InvRequestTracker's implementation. + m_tracker.RequestedData(peer, TXIDS[txid], exptime); } void ReceivedResponse(int peer, int txid) { @@ -253,7 +253,7 @@ Cleanup(txid); } - // Call TxRequestTracker's implementation. + // Call InvRequestTracker's implementation. m_tracker.ReceivedResponse(peer, TXIDS[txid]); } @@ -286,7 +286,7 @@ std::sort(result.begin(), result.end()); std::sort(expected_expired.begin(), expected_expired.end()); - // Compare with TxRequestTracker's implementation. + // Compare with InvRequestTracker's implementation. std::vector> expired; const auto actual = m_tracker.GetRequestable(peer, m_now, &expired); std::sort(expired.begin(), expired.end()); @@ -322,14 +322,14 @@ // Compare Size. assert(m_tracker.Size() == total); - // Invoke internal consistency check of TxRequestTracker object. + // Invoke internal consistency check of InvRequestTracker object. m_tracker.SanityCheck(); } }; } // namespace void test_one_input(const std::vector &buffer) { - // Tester object (which encapsulates a TxRequestTracker). + // Tester object (which encapsulates a InvRequestTracker). Tester tester; // Decode the input as a sequence of instructions with parameters diff --git a/src/test/txrequest_tests.cpp b/src/test/txrequest_tests.cpp --- a/src/test/txrequest_tests.cpp +++ b/src/test/txrequest_tests.cpp @@ -31,8 +31,8 @@ * The Scenario below is used to fill this. */ struct Runner { - /** The TxRequestTracker being tested. */ - TxRequestTracker txrequest; + /** The InvRequestTracker being tested. */ + InvRequestTracker txrequest; /** List of actions to be executed (in order of increasing timestamp). */ std::vector actions; @@ -61,12 +61,12 @@ /** * A proxy for a Runner that helps build a sequence of consecutive test actions - * on a TxRequestTracker. + * on a InvRequestTracker. * * Each Scenario is a proxy through which actions for the (sequential) execution * of various tests are added to a Runner. The actions from multiple scenarios * are then run concurrently, resulting in these tests being performed against a - * TxRequestTracker in parallel. Every test has its own unique txids and + * InvRequestTracker in parallel. Every test has its own unique txids and * NodeIds which are not reused in other tests, and thus they should be * independent from each other. Running them in parallel however means that we * verify the behavior (w.r.t. one test's txids and NodeIds) even when the @@ -98,7 +98,7 @@ void ForgetTxId(const TxId &txid) { auto &runner = m_runner; runner.actions.emplace_back(m_now, [=, &runner]() { - runner.txrequest.ForgetTxId(txid); + runner.txrequest.ForgetInvId(txid); runner.txrequest.SanityCheck(); }); } @@ -127,7 +127,7 @@ std::chrono::microseconds exptime) { auto &runner = m_runner; runner.actions.emplace_back(m_now, [=, &runner]() { - runner.txrequest.RequestedTx(peer, txid, exptime); + runner.txrequest.RequestedData(peer, txid, exptime); runner.txrequest.SanityCheck(); }); } @@ -142,7 +142,7 @@ } /** - * Schedule calls to verify the TxRequestTracker's state at the Scheduler's + * Schedule calls to verify the InvRequestTracker's state at the Scheduler's * current time. * * @param peer The peer whose state will be inspected. @@ -223,7 +223,7 @@ * such that both: * - priority(p1,T) > priority(p2,T) > priority(p3,T) * - priority(p2,T) > priority(p4,T) > priority(p5,T) - * where priority is the predicted internal TxRequestTracker's priority, + * where priority is the predicted internal InvRequestTracker's priority, * assuming all announcements are within the same preferredness class. */ TxId NewTxId(const std::vector> &orders = {}) {