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); + TxRequestTracker m_txrequest GUARDED_BY(::cs_main); //! Next time to check for stale tip int64_t m_stale_tip_check_time; 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 @@ -70,7 +70,7 @@ */ class Tester { //! TxRequestTracker object being tested. - TxRequestTracker m_tracker; + TxRequestTracker m_tracker; //! States for txid/peer combinations in the naive data structure. enum class State { 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 @@ -32,7 +32,7 @@ */ struct Runner { /** The TxRequestTracker being tested. */ - TxRequestTracker txrequest; + TxRequestTracker txrequest; /** List of actions to be executed (in order of increasing timestamp). */ std::vector actions; diff --git a/src/txrequest.h b/src/txrequest.h --- a/src/txrequest.h +++ b/src/txrequest.h @@ -6,12 +6,11 @@ #define BITCOIN_TXREQUEST_H #include // For NodeId -#include #include -#include - #include +#include +#include /** * Data structure to keep track of, and schedule, transaction downloads from @@ -120,7 +119,7 @@ // Avoid littering this header file with implementation details. class InvRequestTrackerImplInterface { - friend class TxRequestTracker; + template friend class TxRequestTracker; // The base class is responsible for building the child implementation. // This is a hack that allows for hiding the concrete implementation details @@ -157,13 +156,22 @@ PostGetRequestableSanityCheck(std::chrono::microseconds now) const = 0; }; -class TxRequestTracker { +template class TxRequestTracker { + /* + * Only uint256-based InvId types are supported for now. + * FIXME: use a template constraint when C++20 is available. + */ + static_assert(std::is_base_of::value, + "InvRequestTracker inv id type should be uint256 or derived"); + const std::unique_ptr m_impl; public: //! Construct a TxRequestTracker. - explicit TxRequestTracker(bool deterministic = false); - ~TxRequestTracker(); + + explicit TxRequestTracker(bool deterministic = false) + : m_impl{InvRequestTrackerImplInterface::BuildImpl(deterministic)} {} + ~TxRequestTracker() = default; // Conceptually, the data structure consists of a collection of // "announcements", one for each peer/txid combination: @@ -191,15 +199,17 @@ * Does nothing if one already exists for that (txid, peer) combination * (whether it's CANDIDATE, REQUESTED, or COMPLETED). */ - void ReceivedInv(NodeId peer, const TxId &txid, bool preferred, - std::chrono::microseconds reqtime); + void ReceivedInv(NodeId peer, const InvId &txid, bool preferred, + std::chrono::microseconds reqtime) { + m_impl->ReceivedInv(peer, txid, preferred, reqtime); + } /** * Deletes all announcements for a given peer. * * It should be called when a peer goes offline. */ - void DisconnectedPeer(NodeId peer); + void DisconnectedPeer(NodeId peer) { m_impl->DisconnectedPeer(peer); } /** * Deletes all announcements for a given txid. @@ -208,7 +218,7 @@ * should ensure that new announcements for the same txid will not trigger * new ReceivedInv calls, at least in the short term after this call. */ - void ForgetTxId(const TxId &txid); + void ForgetTxId(const InvId &txid) { m_impl->ForgetTxId(txid); } /** * Find the txids to request now from peer. @@ -232,9 +242,25 @@ * peer, and end up being requested from them, the requests will happen * in announcement order. */ - std::vector + std::vector GetRequestable(NodeId peer, std::chrono::microseconds now, - std::vector> *expired = nullptr); + std::vector> *expired) { + InvRequestTrackerImplInterface::ClearExpiredFun clearExpired = + [expired]() { + if (expired) { + expired->clear(); + } + }; + InvRequestTrackerImplInterface::EmplaceExpiredFun emplaceExpired = + [expired](const NodeId &nodeid, const uint256 &txid) { + if (expired) { + expired->emplace_back(nodeid, InvId(txid)); + } + }; + std::vector hashes = + m_impl->GetRequestable(peer, now, clearExpired, emplaceExpired); + return std::vector(hashes.begin(), hashes.end()); + } /** * Marks a transaction as requested, with a specified expiry. @@ -247,8 +273,10 @@ * 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 TxId &txid, - std::chrono::microseconds expiry); + void RequestedTx(NodeId peer, const InvId &txid, + std::chrono::microseconds expiry) { + m_impl->RequestedTx(peer, txid, expiry); + } /** * Converts a CANDIDATE or REQUESTED announcement to a COMPLETED one. If no @@ -258,34 +286,42 @@ * a peer. When the transaction is not needed entirely anymore, ForgetTxId * should be called instead of, or in addition to, this call. */ - void ReceivedResponse(NodeId peer, const TxId &txid); + void ReceivedResponse(NodeId peer, const InvId &txid) { + m_impl->ReceivedResponse(peer, txid); + } // The operations below inspect the data structure. /** Count how many REQUESTED announcements a peer has. */ - size_t CountInFlight(NodeId peer) const; + size_t CountInFlight(NodeId peer) const { + return m_impl->CountInFlight(peer); + } /** Count how many CANDIDATE announcements a peer has. */ - size_t CountCandidates(NodeId peer) const; + size_t CountCandidates(NodeId peer) const { + return m_impl->CountCandidates(peer); + } /** * Count how many announcements a peer has (REQUESTED, CANDIDATE, and * COMPLETED combined). */ - size_t Count(NodeId peer) const; + size_t Count(NodeId peer) const { return m_impl->Count(peer); } /** * Count how many announcements are being tracked in total across all peers * and transaction hashes. */ - size_t Size() const; + size_t Size() const { return m_impl->Size(); } /** Access to the internal priority computation (testing only) */ - uint64_t ComputePriority(const TxId &txid, NodeId peer, - bool preferred) const; + uint64_t ComputePriority(const InvId &txid, NodeId peer, + bool preferred) const { + return m_impl->ComputePriority(txid, peer, preferred); + } /** Run internal consistency check (testing only). */ - void SanityCheck() const; + void SanityCheck() const { m_impl->SanityCheck(); } /** * Run a time-dependent internal consistency check (testing only). @@ -293,7 +329,9 @@ * This can only be called immediately after GetRequestable, with the same * 'now' parameter. */ - void PostGetRequestableSanityCheck(std::chrono::microseconds now) const; + void PostGetRequestableSanityCheck(std::chrono::microseconds now) const { + m_impl->PostGetRequestableSanityCheck(now); + } }; #endif // BITCOIN_TXREQUEST_H diff --git a/src/txrequest.cpp b/src/txrequest.cpp --- a/src/txrequest.cpp +++ b/src/txrequest.cpp @@ -6,7 +6,6 @@ #include #include -#include #include #include @@ -863,74 +862,3 @@ InvRequestTrackerImplInterface::BuildImpl(bool deterministic) { return std::make_unique(deterministic); } - -TxRequestTracker::TxRequestTracker(bool deterministic) - : m_impl{InvRequestTrackerImplInterface::BuildImpl(deterministic)} {} - -TxRequestTracker::~TxRequestTracker() = default; - -void TxRequestTracker::ForgetTxId(const TxId &txid) { - m_impl->ForgetTxId(txid); -} -void TxRequestTracker::DisconnectedPeer(NodeId peer) { - m_impl->DisconnectedPeer(peer); -} -size_t TxRequestTracker::CountInFlight(NodeId peer) const { - return m_impl->CountInFlight(peer); -} -size_t TxRequestTracker::CountCandidates(NodeId peer) const { - return m_impl->CountCandidates(peer); -} -size_t TxRequestTracker::Count(NodeId peer) const { - return m_impl->Count(peer); -} -size_t TxRequestTracker::Size() const { - return m_impl->Size(); -} -void TxRequestTracker::SanityCheck() const { - m_impl->SanityCheck(); -} - -void TxRequestTracker::PostGetRequestableSanityCheck( - std::chrono::microseconds now) const { - m_impl->PostGetRequestableSanityCheck(now); -} - -void TxRequestTracker::ReceivedInv(NodeId peer, const TxId &txid, - bool preferred, - std::chrono::microseconds reqtime) { - m_impl->ReceivedInv(peer, txid, preferred, reqtime); -} - -void TxRequestTracker::RequestedTx(NodeId peer, const TxId &txid, - std::chrono::microseconds expiry) { - m_impl->RequestedTx(peer, txid, expiry); -} - -void TxRequestTracker::ReceivedResponse(NodeId peer, const TxId &txid) { - m_impl->ReceivedResponse(peer, txid); -} - -std::vector TxRequestTracker::GetRequestable( - NodeId peer, std::chrono::microseconds now, - std::vector> *expired) { - InvRequestTrackerImplInterface::ClearExpiredFun clearExpired = [expired]() { - if (expired) { - expired->clear(); - } - }; - InvRequestTrackerImplInterface::EmplaceExpiredFun emplaceExpired = - [expired](const NodeId &nodeid, const uint256 &txid) { - if (expired) { - expired->emplace_back(nodeid, TxId(txid)); - } - }; - std::vector hashes = - m_impl->GetRequestable(peer, now, clearExpired, emplaceExpired); - return std::vector(hashes.begin(), hashes.end()); -} - -uint64_t TxRequestTracker::ComputePriority(const TxId &txid, NodeId peer, - bool preferred) const { - return m_impl->ComputePriority(txid, peer, preferred); -}