diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 8b0e6ff1a86..ea0b747d09b 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -5962,8 +5962,8 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // { LOCK(m_tx_download_mutex); - for (const GenTxid& gtxid : m_txdownloadman.GetRequestsToSend(pto->GetId(), current_time)) { - vGetData.emplace_back(gtxid.IsWtxid() ? MSG_WTX : (MSG_TX | GetFetchFlags(*peer)), gtxid.GetHash()); + for (const GenTxidVariant& gtxid : m_txdownloadman.GetRequestsToSend(pto->GetId(), current_time)) { + vGetData.emplace_back(gtxid.IsWtxid() ? MSG_WTX : (MSG_TX | GetFetchFlags(*peer)), gtxid.ToUint256()); if (vGetData.size() >= MAX_GETDATA_SZ) { MakeAndPushMessage(*pto, NetMsgType::GETDATA, vGetData); vGetData.clear(); diff --git a/src/node/txdownloadman.h b/src/node/txdownloadman.h index 328d24c686f..5c5e61cdca7 100644 --- a/src/node/txdownloadman.h +++ b/src/node/txdownloadman.h @@ -141,7 +141,7 @@ public: bool AddTxAnnouncement(NodeId peer, const GenTxidVariant& gtxid, std::chrono::microseconds now); /** Get getdata requests to send. */ - std::vector GetRequestsToSend(NodeId nodeid, std::chrono::microseconds current_time); + std::vector GetRequestsToSend(NodeId nodeid, std::chrono::microseconds current_time); /** Should be called when a notfound for a tx has been received. */ void ReceivedNotFound(NodeId nodeid, const std::vector& gtxids); diff --git a/src/node/txdownloadman_impl.cpp b/src/node/txdownloadman_impl.cpp index 5b72fd96cd2..a8fe8971bed 100644 --- a/src/node/txdownloadman_impl.cpp +++ b/src/node/txdownloadman_impl.cpp @@ -43,7 +43,7 @@ bool TxDownloadManager::AddTxAnnouncement(NodeId peer, const GenTxidVariant& gtx { return m_impl->AddTxAnnouncement(peer, gtxid, now); } -std::vector TxDownloadManager::GetRequestsToSend(NodeId nodeid, std::chrono::microseconds current_time) +std::vector TxDownloadManager::GetRequestsToSend(NodeId nodeid, std::chrono::microseconds current_time) { return m_impl->GetRequestsToSend(nodeid, current_time); } @@ -218,7 +218,7 @@ bool TxDownloadManagerImpl::AddTxAnnouncement(NodeId peer, const GenTxidVariant& const bool overloaded = !info.m_relay_permissions && m_txrequest.CountInFlight(peer) >= MAX_PEER_TX_REQUEST_IN_FLIGHT; if (overloaded) delay += OVERLOADED_PEER_TX_DELAY; - m_txrequest.ReceivedInv(peer, GenTxid::FromVariant(gtxid), info.m_preferred, now + delay); + m_txrequest.ReceivedInv(peer, gtxid, info.m_preferred, now + delay); return false; } @@ -255,31 +255,31 @@ bool TxDownloadManagerImpl::MaybeAddOrphanResolutionCandidate(const std::vector< // Treat finding orphan resolution candidate as equivalent to the peer announcing all missing parents. // In the future, orphan resolution may include more explicit steps for (const auto& parent_txid : unique_parents) { - m_txrequest.ReceivedInv(nodeid, GenTxid::Txid(parent_txid), info.m_preferred, now + delay); + m_txrequest.ReceivedInv(nodeid, parent_txid, info.m_preferred, now + delay); } LogDebug(BCLog::TXPACKAGES, "added peer=%d as a candidate for resolving orphan %s\n", nodeid, wtxid.ToString()); return true; } -std::vector TxDownloadManagerImpl::GetRequestsToSend(NodeId nodeid, std::chrono::microseconds current_time) +std::vector TxDownloadManagerImpl::GetRequestsToSend(NodeId nodeid, std::chrono::microseconds current_time) { - std::vector requests; - std::vector> expired; + std::vector requests; + std::vector> expired; auto requestable = m_txrequest.GetRequestable(nodeid, current_time, &expired); - for (const auto& entry : expired) { - LogDebug(BCLog::NET, "timeout of inflight %s %s from peer=%d\n", entry.second.IsWtxid() ? "wtx" : "tx", - entry.second.GetHash().ToString(), entry.first); + for (const auto& [expired_nodeid, gtxid] : expired) { + LogDebug(BCLog::NET, "timeout of inflight %s %s from peer=%d\n", gtxid.IsWtxid() ? "wtx" : "tx", + gtxid.ToUint256().ToString(), expired_nodeid); } - for (const GenTxid& gtxid : requestable) { - if (!AlreadyHaveTx(gtxid.ToVariant(), /*include_reconsiderable=*/false)) { + for (const GenTxidVariant& gtxid : requestable) { + if (!AlreadyHaveTx(gtxid, /*include_reconsiderable=*/false)) { LogDebug(BCLog::NET, "Requesting %s %s peer=%d\n", gtxid.IsWtxid() ? "wtx" : "tx", - gtxid.GetHash().ToString(), nodeid); + gtxid.ToUint256().ToString(), nodeid); requests.emplace_back(gtxid); - m_txrequest.RequestedTx(nodeid, gtxid.GetHash(), current_time + GETDATA_TX_INTERVAL); + m_txrequest.RequestedTx(nodeid, gtxid.ToUint256(), 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.ForgetTxHash(gtxid.GetHash()); + m_txrequest.ForgetTxHash(gtxid.ToUint256()); } } return requests; diff --git a/src/node/txdownloadman_impl.h b/src/node/txdownloadman_impl.h index 6387edb251f..e5bbc1e8030 100644 --- a/src/node/txdownloadman_impl.h +++ b/src/node/txdownloadman_impl.h @@ -166,7 +166,7 @@ public: bool AddTxAnnouncement(NodeId peer, const GenTxidVariant& gtxid, std::chrono::microseconds now); /** Get getdata requests to send. */ - std::vector GetRequestsToSend(NodeId nodeid, std::chrono::microseconds current_time); + std::vector GetRequestsToSend(NodeId nodeid, std::chrono::microseconds current_time); /** Marks a tx as ReceivedResponse in txrequest. */ void ReceivedNotFound(NodeId nodeid, const std::vector& gtxids); diff --git a/src/test/fuzz/txdownloadman.cpp b/src/test/fuzz/txdownloadman.cpp index 30c69f0a461..af6265678d6 100644 --- a/src/test/fuzz/txdownloadman.cpp +++ b/src/test/fuzz/txdownloadman.cpp @@ -382,7 +382,7 @@ FUZZ_TARGET(txdownloadman_impl, .init = initialize) // TxDownloadManager should not be telling us to request things we already have. // Exclude m_lazy_recent_rejects_reconsiderable because it may request low-feerate parent of orphan. for (const auto& gtxid : getdata_requests) { - Assert(!txdownload_impl.AlreadyHaveTx(gtxid.ToVariant(), /*include_reconsiderable=*/false)); + Assert(!txdownload_impl.AlreadyHaveTx(gtxid, /*include_reconsiderable=*/false)); } }, [&] { diff --git a/src/test/fuzz/txrequest.cpp b/src/test/fuzz/txrequest.cpp index 931ddf03288..1cab9bc471c 100644 --- a/src/test/fuzz/txrequest.cpp +++ b/src/test/fuzz/txrequest.cpp @@ -19,7 +19,7 @@ namespace { constexpr int MAX_TXHASHES = 16; constexpr int MAX_PEERS = 16; -//! Randomly generated GenTxids used in this test (length is MAX_TXHASHES). +//! Randomly generated txhashes used in this test (length is MAX_TXHASHES). uint256 TXHASHES[MAX_TXHASHES]; //! Precomputed random durations (positive and negative, each ~exponentially distributed). @@ -204,7 +204,8 @@ public: } // Call TxRequestTracker's implementation. - m_tracker.ReceivedInv(peer, is_wtxid ? GenTxid::Wtxid(TXHASHES[txhash]) : GenTxid::Txid(TXHASHES[txhash]), preferred, reqtime); + auto gtxid = is_wtxid ? GenTxidVariant{Wtxid::FromUint256(TXHASHES[txhash])} : GenTxidVariant{Txid::FromUint256(TXHASHES[txhash])}; + m_tracker.ReceivedInv(peer, gtxid, preferred, reqtime); } void RequestedTx(int peer, int txhash, std::chrono::microseconds exptime) @@ -246,13 +247,14 @@ public: //! list of (sequence number, txhash, is_wtxid) tuples. std::vector> result; - std::vector> expected_expired; + std::vector> expected_expired; for (int txhash = 0; txhash < MAX_TXHASHES; ++txhash) { // Mark any expired REQUESTED announcements as COMPLETED. for (int peer2 = 0; peer2 < MAX_PEERS; ++peer2) { Announcement& ann2 = m_announcements[txhash][peer2]; if (ann2.m_state == State::REQUESTED && ann2.m_time <= m_now) { - expected_expired.emplace_back(peer2, ann2.m_is_wtxid ? GenTxid::Wtxid(TXHASHES[txhash]) : GenTxid::Txid(TXHASHES[txhash])); + auto gtxid = ann2.m_is_wtxid ? GenTxidVariant{Wtxid::FromUint256(TXHASHES[txhash])} : GenTxidVariant{Txid::FromUint256(TXHASHES[txhash])}; + expected_expired.emplace_back(peer2, gtxid); ann2.m_state = State::COMPLETED; break; } @@ -270,7 +272,7 @@ public: std::sort(expected_expired.begin(), expected_expired.end()); // Compare with TxRequestTracker's implementation. - std::vector> expired; + std::vector> expired; const auto actual = m_tracker.GetRequestable(peer, m_now, &expired); std::sort(expired.begin(), expired.end()); assert(expired == expected_expired); @@ -278,7 +280,7 @@ public: m_tracker.PostGetRequestableSanityCheck(m_now); assert(result.size() == actual.size()); for (size_t pos = 0; pos < actual.size(); ++pos) { - assert(TXHASHES[std::get<1>(result[pos])] == actual[pos].GetHash()); + assert(TXHASHES[std::get<1>(result[pos])] == actual[pos].ToUint256()); assert(std::get<2>(result[pos]) == actual[pos].IsWtxid()); } } diff --git a/src/test/txrequest_tests.cpp b/src/test/txrequest_tests.cpp index d73692d8f98..0278f824f6c 100644 --- a/src/test/txrequest_tests.cpp +++ b/src/test/txrequest_tests.cpp @@ -61,7 +61,7 @@ struct Runner /** Which (peer, gtxid) combinations are known to be expired. These need to be accumulated here instead of * checked directly in the GetRequestable return value to avoid introducing a dependency between the various * parallel tests. */ - std::multiset> expired; + std::multiset> expired; }; std::chrono::microseconds TxRequestTest::RandomTime8s() { return std::chrono::microseconds{1 + m_rng.randbits(23)}; } @@ -103,17 +103,17 @@ public: void ForgetTxHash(const uint256& txhash) { auto& runner = m_runner; - runner.actions.emplace_back(m_now, [=,&runner]() { + runner.actions.emplace_back(m_now, [=, &runner]() { runner.txrequest.ForgetTxHash(txhash); runner.txrequest.SanityCheck(); }); } /** Schedule a ReceivedInv call at the Scheduler's current time. */ - void ReceivedInv(NodeId peer, const GenTxid& gtxid, bool pref, std::chrono::microseconds reqtime) + void ReceivedInv(NodeId peer, const GenTxidVariant& gtxid, bool pref, std::chrono::microseconds reqtime) { auto& runner = m_runner; - runner.actions.emplace_back(m_now, [=,&runner]() { + runner.actions.emplace_back(m_now, [=, &runner]() { runner.txrequest.ReceivedInv(peer, gtxid, pref, reqtime); runner.txrequest.SanityCheck(); }); @@ -123,7 +123,7 @@ public: void DisconnectedPeer(NodeId peer) { auto& runner = m_runner; - runner.actions.emplace_back(m_now, [=,&runner]() { + runner.actions.emplace_back(m_now, [=, &runner]() { runner.txrequest.DisconnectedPeer(peer); runner.txrequest.SanityCheck(); }); @@ -133,7 +133,7 @@ public: void RequestedTx(NodeId peer, const uint256& txhash, std::chrono::microseconds exptime) { auto& runner = m_runner; - runner.actions.emplace_back(m_now, [=,&runner]() { + runner.actions.emplace_back(m_now, [=, &runner]() { runner.txrequest.RequestedTx(peer, txhash, exptime); runner.txrequest.SanityCheck(); }); @@ -143,7 +143,7 @@ public: void ReceivedResponse(NodeId peer, const uint256& txhash) { auto& runner = m_runner; - runner.actions.emplace_back(m_now, [=,&runner]() { + runner.actions.emplace_back(m_now, [=, &runner]() { runner.txrequest.ReceivedResponse(peer, txhash); runner.txrequest.SanityCheck(); }); @@ -160,18 +160,20 @@ public: * @param offset Offset with the current time to use (must be <= 0). This allows simulations of time going * backwards (but note that the ordering of this event only follows the scenario's m_now. */ - void Check(NodeId peer, const std::vector& expected, size_t candidates, size_t inflight, - size_t completed, const std::string& checkname, - std::chrono::microseconds offset = std::chrono::microseconds{0}) + void Check(NodeId peer, const std::vector& expected, size_t candidates, size_t inflight, + size_t completed, const std::string& checkname, + std::chrono::microseconds offset = std::chrono::microseconds{0}) { const auto comment = m_testname + " " + checkname; auto& runner = m_runner; const auto now = m_now; assert(offset.count() <= 0); - runner.actions.emplace_back(m_now, [=,&runner]() { - std::vector> expired_now; + runner.actions.emplace_back(m_now, [=, &runner]() { + std::vector> expired_now; auto ret = runner.txrequest.GetRequestable(peer, now + offset, &expired_now); - for (const auto& entry : expired_now) runner.expired.insert(entry); + for (const auto& entry : expired_now) { + runner.expired.insert(entry); + } runner.txrequest.SanityCheck(); runner.txrequest.PostGetRequestableSanityCheck(now + offset); size_t total = candidates + inflight + completed; @@ -189,12 +191,12 @@ public: * * Every expected expiration should be accounted for through exactly one call to this function. */ - void CheckExpired(NodeId peer, GenTxid gtxid) + void CheckExpired(NodeId peer, GenTxidVariant gtxid) { const auto& testname = m_testname; auto& runner = m_runner; - runner.actions.emplace_back(m_now, [=,&runner]() { - auto it = runner.expired.find(std::pair{peer, gtxid}); + runner.actions.emplace_back(m_now, [=, &runner]() { + auto it = runner.expired.find(std::pair{peer, gtxid}); BOOST_CHECK_MESSAGE(it != runner.expired.end(), "[" + testname + "] missing expiration"); if (it != runner.expired.end()) runner.expired.erase(it); }); @@ -233,10 +235,11 @@ public: return ret; } - /** Generate a random GenTxid; the txhash follows NewTxHash; the is_wtxid flag is random. */ - GenTxid NewGTxid(const std::vector>& orders = {}) + /** Generate a random GenTxid; the txhash follows NewTxHash; the transaction identifier is random. */ + GenTxidVariant NewGTxid(const std::vector>& orders = {}) { - return m_rng.randbool() ? GenTxid::Wtxid(NewTxHash(orders)) : GenTxid::Txid(NewTxHash(orders)); + const uint256 txhash{NewTxHash(orders)}; + return m_rng.randbool() ? GenTxidVariant{Wtxid::FromUint256(txhash)} : GenTxidVariant{Txid::FromUint256(txhash)}; } /** Generate a new random NodeId to use as peer. The same NodeId is never returned twice @@ -285,7 +288,7 @@ void TxRequestTest::BuildSingleTest(Scenario& scenario, int config) scenario.AdvanceTime(RandomTime8s()); auto expiry = RandomTime8s(); scenario.Check(peer, {gtxid}, 1, 0, 0, "s5"); - scenario.RequestedTx(peer, gtxid.GetHash(), scenario.Now() + expiry); + scenario.RequestedTx(peer, gtxid.ToUint256(), scenario.Now() + expiry); scenario.Check(peer, {}, 0, 1, 0, "s6"); if ((config >> 3) == 1) { // The request will time out @@ -299,7 +302,7 @@ void TxRequestTest::BuildSingleTest(Scenario& scenario, int config) scenario.AdvanceTime(std::chrono::microseconds{m_rng.randrange(expiry.count())}); scenario.Check(peer, {}, 0, 1, 0, "s9"); if ((config >> 3) == 3) { // A response will arrive for the transaction - scenario.ReceivedResponse(peer, gtxid.GetHash()); + scenario.ReceivedResponse(peer, gtxid.ToUint256()); scenario.Check(peer, {}, 0, 0, 0, "s10"); return; } @@ -309,7 +312,7 @@ void TxRequestTest::BuildSingleTest(Scenario& scenario, int config) if (config & 4) { // The peer will go offline scenario.DisconnectedPeer(peer); } else { // The transaction is no longer needed - scenario.ForgetTxHash(gtxid.GetHash()); + scenario.ForgetTxHash(gtxid.ToUint256()); } scenario.Check(peer, {}, 0, 0, 0, "s11"); } @@ -355,7 +358,7 @@ void TxRequestTest::BuildPriorityTest(Scenario& scenario, int config) // We possibly request from the selected peer. if (config & 8) { - scenario.RequestedTx(priopeer, gtxid.GetHash(), MAX_TIME); + scenario.RequestedTx(priopeer, gtxid.ToUint256(), MAX_TIME); scenario.Check(priopeer, {}, 0, 1, 0, "p7"); scenario.Check(otherpeer, {}, 1, 0, 0, "p8"); if (m_rng.randbool()) scenario.AdvanceTime(RandomTime8s()); @@ -365,7 +368,7 @@ void TxRequestTest::BuildPriorityTest(Scenario& scenario, int config) if (config & 16) { scenario.DisconnectedPeer(priopeer); } else { - scenario.ReceivedResponse(priopeer, gtxid.GetHash()); + scenario.ReceivedResponse(priopeer, gtxid.ToUint256()); } if (m_rng.randbool()) scenario.AdvanceTime(RandomTime8s()); scenario.Check(priopeer, {}, 0, 0, !(config & 16), "p8"); @@ -449,7 +452,7 @@ void TxRequestTest::BuildBigPriorityTest(Scenario& scenario, int peers) scenario.DisconnectedPeer(peer); scenario.Check(peer, {}, 0, 0, 0, "b4"); } else { - scenario.ReceivedResponse(peer, gtxid.GetHash()); + scenario.ReceivedResponse(peer, gtxid.ToUint256()); scenario.Check(peer, {}, 0, 0, request_order.size() > 0, "b5"); } if (request_order.size()) { @@ -510,8 +513,8 @@ void TxRequestTest::BuildWtxidTest(Scenario& scenario, int config) auto peerT = scenario.NewPeer(); auto peerW = scenario.NewPeer(); auto txhash = scenario.NewTxHash(); - auto txid{GenTxid::Txid(txhash)}; - auto wtxid{GenTxid::Wtxid(txhash)}; + auto txid{Txid::FromUint256(txhash)}; + auto wtxid{Wtxid::FromUint256(txhash)}; auto reqtimeT = m_rng.randbool() ? MIN_TIME : scenario.Now() + RandomTime8s(); auto reqtimeW = m_rng.randbool() ? MIN_TIME : scenario.Now() + RandomTime8s(); @@ -542,11 +545,11 @@ void TxRequestTest::BuildWtxidTest(Scenario& scenario, int config) // Let the preferred announcement be requested. It's not going to be delivered. auto expiry = RandomTime8s(); if (config & 2) { - scenario.RequestedTx(peerT, txid.GetHash(), scenario.Now() + expiry); + scenario.RequestedTx(peerT, txid.ToUint256(), scenario.Now() + expiry); scenario.Check(peerT, {}, 0, 1, 0, "w5"); scenario.Check(peerW, {}, 1, 0, 0, "w6"); } else { - scenario.RequestedTx(peerW, wtxid.GetHash(), scenario.Now() + expiry); + scenario.RequestedTx(peerW, wtxid.ToUint256(), scenario.Now() + expiry); scenario.Check(peerT, {}, 1, 0, 0, "w7"); scenario.Check(peerW, {}, 0, 1, 0, "w8"); } @@ -599,7 +602,7 @@ void TxRequestTest::BuildTimeBackwardsTest(Scenario& scenario) // Request from peer1. if (m_rng.randbool()) scenario.AdvanceTime(RandomTime8s()); auto expiry = scenario.Now() + RandomTime8s(); - scenario.RequestedTx(peer1, gtxid.GetHash(), expiry); + scenario.RequestedTx(peer1, gtxid.ToUint256(), expiry); scenario.Check(peer1, {}, 0, 1, 0, "r7"); scenario.Check(peer2, {}, 1, 0, 0, "r8"); @@ -638,20 +641,20 @@ void TxRequestTest::BuildWeirdRequestsTest(Scenario& scenario) // We request gtxid2 from *peer1* - no effect. if (m_rng.randbool()) scenario.AdvanceTime(RandomTime8s()); - scenario.RequestedTx(peer1, gtxid2.GetHash(), MAX_TIME); + scenario.RequestedTx(peer1, gtxid2.ToUint256(), MAX_TIME); scenario.Check(peer1, {gtxid1}, 1, 0, 0, "q4"); scenario.Check(peer2, {gtxid2}, 1, 0, 0, "q5"); // Now request gtxid1 from peer1 - marks it as REQUESTED. if (m_rng.randbool()) scenario.AdvanceTime(RandomTime8s()); auto expiryA = scenario.Now() + RandomTime8s(); - scenario.RequestedTx(peer1, gtxid1.GetHash(), expiryA); + scenario.RequestedTx(peer1, gtxid1.ToUint256(), expiryA); scenario.Check(peer1, {}, 0, 1, 0, "q6"); scenario.Check(peer2, {gtxid2}, 1, 0, 0, "q7"); // Request it a second time - nothing happens, as it's already REQUESTED. auto expiryB = expiryA + RandomTime8s(); - scenario.RequestedTx(peer1, gtxid1.GetHash(), expiryB); + scenario.RequestedTx(peer1, gtxid1.ToUint256(), expiryB); scenario.Check(peer1, {}, 0, 1, 0, "q8"); scenario.Check(peer2, {gtxid2}, 1, 0, 0, "q9"); @@ -668,7 +671,7 @@ void TxRequestTest::BuildWeirdRequestsTest(Scenario& scenario) // Requesting it yet again from peer1 doesn't do anything, as it's already COMPLETED. if (m_rng.randbool()) scenario.AdvanceTime(RandomTime8s()); - scenario.RequestedTx(peer1, gtxid1.GetHash(), MAX_TIME); + scenario.RequestedTx(peer1, gtxid1.ToUint256(), MAX_TIME); scenario.Check(peer1, {}, 0, 0, 1, "q14"); scenario.Check(peer2, {gtxid2, gtxid1}, 2, 0, 0, "q15"); @@ -680,13 +683,13 @@ void TxRequestTest::BuildWeirdRequestsTest(Scenario& scenario) // And request it from peer1 (weird as peer2 has the preference). if (m_rng.randbool()) scenario.AdvanceTime(RandomTime8s()); - scenario.RequestedTx(peer1, gtxid2.GetHash(), MAX_TIME); + scenario.RequestedTx(peer1, gtxid2.ToUint256(), MAX_TIME); scenario.Check(peer1, {}, 0, 1, 1, "q18"); scenario.Check(peer2, {gtxid1}, 2, 0, 0, "q19"); // If peer2 now (normally) requests gtxid2, the existing request by peer1 becomes COMPLETED. if (m_rng.randbool()) scenario.AdvanceTime(RandomTime8s()); - scenario.RequestedTx(peer2, gtxid2.GetHash(), MAX_TIME); + scenario.RequestedTx(peer2, gtxid2.ToUint256(), MAX_TIME); scenario.Check(peer1, {}, 0, 0, 2, "q20"); scenario.Check(peer2, {gtxid1}, 1, 1, 0, "q21"); diff --git a/src/txrequest.cpp b/src/txrequest.cpp index 03b4cd6b161..02f03435154 100644 --- a/src/txrequest.cpp +++ b/src/txrequest.cpp @@ -60,7 +60,7 @@ using SequenceNumber = uint64_t; /** An announcement. This is the data we track for each txid or wtxid that is announced to us by each peer. */ struct Announcement { /** Txid or wtxid that was announced. */ - const uint256 m_txhash; + const GenTxidVariant m_gtxid; /** For CANDIDATE_{DELAYED,BEST,READY} the reqtime; for REQUESTED the expiry. */ std::chrono::microseconds m_time; /** What peer the request was from. */ @@ -69,9 +69,6 @@ struct Announcement { const SequenceNumber m_sequence : 59; /** Whether the request is preferred. */ const bool m_preferred : 1; - /** Whether this is a wtxid request. */ - const bool m_is_wtxid : 1; - /** What state this announcement is in. */ State m_state : 3 {State::CANDIDATE_DELAYED}; State GetState() const { return m_state; } @@ -96,10 +93,9 @@ struct Announcement { } /** Construct a new announcement from scratch, initially in CANDIDATE_DELAYED state. */ - Announcement(const GenTxid& gtxid, NodeId peer, bool preferred, std::chrono::microseconds reqtime, + Announcement(const GenTxidVariant& gtxid, NodeId peer, bool preferred, std::chrono::microseconds reqtime, SequenceNumber sequence) - : m_txhash(gtxid.GetHash()), m_time(reqtime), m_peer(peer), m_sequence(sequence), m_preferred(preferred), - m_is_wtxid{gtxid.IsWtxid()} {} + : m_gtxid(gtxid), m_time(reqtime), m_peer(peer), m_sequence(sequence), m_preferred(preferred) {} }; //! Type alias for priorities. @@ -124,7 +120,7 @@ public: Priority operator()(const Announcement& ann) const { - return operator()(ann.m_txhash, ann.m_peer, ann.m_preferred); + return operator()(ann.m_gtxid.ToUint256(), ann.m_peer, ann.m_preferred); } }; @@ -148,7 +144,7 @@ struct ByPeerViewExtractor using result_type = ByPeerView; result_type operator()(const Announcement& ann) const { - return ByPeerView{ann.m_peer, ann.GetState() == State::CANDIDATE_BEST, ann.m_txhash}; + return ByPeerView{ann.m_peer, ann.GetState() == State::CANDIDATE_BEST, ann.m_gtxid.ToUint256()}; } }; @@ -172,7 +168,7 @@ public: result_type operator()(const Announcement& ann) const { const Priority prio = (ann.GetState() == State::CANDIDATE_READY) ? m_computer(ann) : 0; - return ByTxHashView{ann.m_txhash, ann.GetState(), prio}; + return ByTxHashView{ann.m_gtxid.ToUint256(), ann.GetState(), prio}; } }; @@ -280,7 +276,7 @@ std::map ComputeTxHashInfo(const Index& index, const Priori { std::map ret; for (const Announcement& ann : index) { - TxHashInfo& info = ret[ann.m_txhash]; + TxHashInfo& info = ret[ann.m_gtxid.ToUint256()]; // Classify how many announcements of each state we have for this txhash. info.m_candidate_delayed += (ann.GetState() == State::CANDIDATE_DELAYED); info.m_candidate_ready += (ann.GetState() == State::CANDIDATE_READY); @@ -299,11 +295,6 @@ std::map ComputeTxHashInfo(const Index& index, const Priori return ret; } -GenTxid ToGenTxid(const Announcement& ann) -{ - return ann.m_is_wtxid ? GenTxid::Wtxid(ann.m_txhash) : GenTxid::Txid(ann.m_txhash); -} - } // namespace /** Actual implementation for TxRequestTracker's data structure. */ @@ -409,7 +400,7 @@ private: // priority) comes last. Thus, if an existing _BEST exists for the same txhash that this announcement may // be preferred over, it must immediately follow the newly created _READY. auto it_next = std::next(it); - if (it_next == m_index.get().end() || it_next->m_txhash != it->m_txhash || + if (it_next == m_index.get().end() || it_next->m_gtxid.ToUint256() != it->m_gtxid.ToUint256() || it_next->GetState() == State::COMPLETED) { // This is the new best CANDIDATE_READY, and there is no IsSelected() announcement for this txhash // already. @@ -435,7 +426,7 @@ private: auto it_prev = std::prev(it); // The next best CANDIDATE_READY, if any, immediately precedes the REQUESTED or CANDIDATE_BEST // announcement in the ByTxHash index. - if (it_prev->m_txhash == it->m_txhash && it_prev->GetState() == State::CANDIDATE_READY) { + if (it_prev->m_gtxid.ToUint256() == it->m_gtxid.ToUint256() && it_prev->GetState() == State::CANDIDATE_READY) { // If one such CANDIDATE_READY exists (for this txhash), convert it to CANDIDATE_BEST. Modify(it_prev, [](Announcement& ann){ ann.SetState(State::CANDIDATE_BEST); }); } @@ -451,10 +442,10 @@ private: // This announcement has a predecessor that belongs to the same txhash. Due to ordering, and the // fact that 'it' is not COMPLETED, its predecessor cannot be COMPLETED here. - if (it != m_index.get().begin() && std::prev(it)->m_txhash == it->m_txhash) return false; + if (it != m_index.get().begin() && std::prev(it)->m_gtxid.ToUint256() == it->m_gtxid.ToUint256()) return false; // This announcement has a successor that belongs to the same txhash, and is not COMPLETED. - if (std::next(it) != m_index.get().end() && std::next(it)->m_txhash == it->m_txhash && + if (std::next(it) != m_index.get().end() && std::next(it)->m_gtxid.ToUint256() == it->m_gtxid.ToUint256() && std::next(it)->GetState() != State::COMPLETED) return false; return true; @@ -472,10 +463,10 @@ private: if (IsOnlyNonCompleted(it)) { // This is the last non-COMPLETED announcement for this txhash. Delete all. - uint256 txhash = it->m_txhash; + uint256 txhash = it->m_gtxid.ToUint256(); do { it = Erase(it); - } while (it != m_index.get().end() && it->m_txhash == txhash); + } while (it != m_index.get().end() && it->m_gtxid.ToUint256() == txhash); return false; } @@ -490,7 +481,7 @@ private: //! - REQUESTED announcements with expiry <= now are turned into COMPLETED. //! - CANDIDATE_DELAYED announcements with reqtime <= now are turned into CANDIDATE_{READY,BEST}. //! - CANDIDATE_{READY,BEST} announcements with reqtime > now are turned into CANDIDATE_DELAYED. - void SetTimePoint(std::chrono::microseconds now, std::vector>* expired) + void SetTimePoint(std::chrono::microseconds now, std::vector>* expired) { if (expired) expired->clear(); @@ -501,7 +492,7 @@ private: if (it->GetState() == State::CANDIDATE_DELAYED && it->m_time <= now) { PromoteCandidateReady(m_index.project(it)); } else if (it->GetState() == State::REQUESTED && it->m_time <= now) { - if (expired) expired->emplace_back(it->m_peer, ToGenTxid(*it)); + if (expired) expired->emplace_back(it->m_peer, it->m_gtxid); MakeCompleted(m_index.project(it)); } else { break; @@ -569,7 +560,7 @@ public: void ForgetTxHash(const uint256& txhash) { auto it = m_index.get().lower_bound(ByTxHashView{txhash, State::CANDIDATE_DELAYED, 0}); - while (it != m_index.get().end() && it->m_txhash == txhash) { + while (it != m_index.get().end() && it->m_gtxid.ToUint256() == txhash) { it = Erase(it); } } @@ -577,19 +568,19 @@ public: void GetCandidatePeers(const uint256& txhash, std::vector& result_peers) const { auto it = m_index.get().lower_bound(ByTxHashView{txhash, State::CANDIDATE_DELAYED, 0}); - while (it != m_index.get().end() && it->m_txhash == txhash && it->GetState() != State::COMPLETED) { + while (it != m_index.get().end() && it->m_gtxid.ToUint256() == txhash && it->GetState() != State::COMPLETED) { result_peers.push_back(it->m_peer); ++it; } } - void ReceivedInv(NodeId peer, const GenTxid& gtxid, bool preferred, - std::chrono::microseconds reqtime) + void ReceivedInv(NodeId peer, const GenTxidVariant& gtxid, bool preferred, + std::chrono::microseconds reqtime) { // Bail out if we already have a CANDIDATE_BEST announcement for this (txhash, peer) combination. The case // where there is a non-CANDIDATE_BEST announcement already will be caught by the uniqueness property of the // ByPeer index when we try to emplace the new object below. - if (m_index.get().count(ByPeerView{peer, true, gtxid.GetHash()})) return; + if (m_index.get().count(ByPeerView{peer, true, gtxid.ToUint256()})) return; // Try creating the announcement with CANDIDATE_DELAYED state (which will fail due to the uniqueness // of the ByPeer index if a non-CANDIDATE_BEST announcement already exists with the same txhash and peer). @@ -603,8 +594,8 @@ public: } //! Find the GenTxids to request now from peer. - std::vector GetRequestable(NodeId peer, std::chrono::microseconds now, - std::vector>* expired) + std::vector GetRequestable(NodeId peer, std::chrono::microseconds now, + std::vector>* expired) { // Move time. SetTimePoint(now, expired); @@ -624,10 +615,10 @@ public: }); // Convert to GenTxid and return. - std::vector ret; + std::vector ret; ret.reserve(selected.size()); std::transform(selected.begin(), selected.end(), std::back_inserter(ret), [](const Announcement* ann) { - return ToGenTxid(*ann); + return ann->m_gtxid; }); return ret; } @@ -654,7 +645,7 @@ public: // found announcement had a different state than CANDIDATE_BEST. If it did, invariants guarantee that no // other CANDIDATE_BEST or REQUESTED can exist. auto it_old = m_index.get().lower_bound(ByTxHashView{txhash, State::CANDIDATE_BEST, 0}); - if (it_old != m_index.get().end() && it_old->m_txhash == txhash) { + if (it_old != m_index.get().end() && it_old->m_gtxid.ToUint256() == txhash) { if (it_old->GetState() == State::CANDIDATE_BEST) { // The data structure's invariants require that there can be at most one CANDIDATE_BEST or one // REQUESTED announcement per txhash (but not both simultaneously), so we have to convert any @@ -738,8 +729,8 @@ void TxRequestTracker::PostGetRequestableSanityCheck(std::chrono::microseconds n m_impl->PostGetRequestableSanityCheck(now); } -void TxRequestTracker::ReceivedInv(NodeId peer, const GenTxid& gtxid, bool preferred, - std::chrono::microseconds reqtime) +void TxRequestTracker::ReceivedInv(NodeId peer, const GenTxidVariant& gtxid, bool preferred, + std::chrono::microseconds reqtime) { m_impl->ReceivedInv(peer, gtxid, preferred, reqtime); } @@ -754,8 +745,8 @@ void TxRequestTracker::ReceivedResponse(NodeId peer, const uint256& txhash) m_impl->ReceivedResponse(peer, txhash); } -std::vector TxRequestTracker::GetRequestable(NodeId peer, std::chrono::microseconds now, - std::vector>* expired) +std::vector TxRequestTracker::GetRequestable(NodeId peer, std::chrono::microseconds now, + std::vector>* expired) { return m_impl->GetRequestable(peer, now, expired); } diff --git a/src/txrequest.h b/src/txrequest.h index 98ad43b79aa..f38f4e486ec 100644 --- a/src/txrequest.h +++ b/src/txrequest.h @@ -132,8 +132,8 @@ public: * fetched. The new announcement is given the specified preferred and reqtime values, and takes its is_wtxid * from the specified gtxid. */ - void ReceivedInv(NodeId peer, const GenTxid& gtxid, bool preferred, - std::chrono::microseconds reqtime); + void ReceivedInv(NodeId peer, const GenTxidVariant& gtxid, bool preferred, + std::chrono::microseconds reqtime); /** Deletes all announcements for a given peer. * @@ -158,14 +158,13 @@ public: * exists, and for which the specified peer is the best choice among all (reqtime <= now) CANDIDATE * announcements with the same txhash (subject to preferredness rules, and tiebreaking using a deterministic * salted hash of peer and txhash). - * - The selected announcements are converted to GenTxids using their is_wtxid flag, and returned in - * announcement order (even if multiple were added at the same time, or when the clock went backwards while - * they were being added). This is done to minimize disruption from dependent transactions being requested - * out of order: if multiple dependent transactions are announced simultaneously by one peer, and end up - * being requested from them, the requests will happen in announcement order. + * - The selected announcements are returned in announcement order (even if multiple were added at the same + * time, or when the clock went backwards while they were being added). This is done to minimize disruption + * from dependent transactions being requested out of order: if multiple dependent transactions are announced + * simultaneously by one peer, and end up being requested from them, the requests will happen in announcement order. */ - std::vector GetRequestable(NodeId peer, std::chrono::microseconds now, - std::vector>* expired = nullptr); + std::vector GetRequestable(NodeId peer, std::chrono::microseconds now, + std::vector>* expired = nullptr); /** Marks a transaction as requested, with a specified expiry. *