From 917ab810d9380aa0c649847a647078b2f02d2643 Mon Sep 17 00:00:00 2001 From: glozow Date: Wed, 30 Oct 2024 21:13:01 -0400 Subject: [PATCH 1/3] [doc] comment fixups from n30110 --- src/node/txdownloadman.h | 3 ++- src/node/txdownloadman_impl.h | 4 +++- src/test/txdownload_tests.cpp | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/node/txdownloadman.h b/src/node/txdownloadman.h index 28ca90c5542..81b0c76e0a2 100644 --- a/src/node/txdownloadman.h +++ b/src/node/txdownloadman.h @@ -134,7 +134,8 @@ class TxDownloadManager { /** Deletes all txrequest announcements and orphans for a given peer. */ void DisconnectedPeer(NodeId nodeid); - /** New inv has been received. May be added as a candidate to txrequest. + /** Consider adding this tx hash to txrequest. Should be called whenever a new inv has been received. + * Also called internally when a transaction is missing parents so that we can request them. * @param[in] p2p_inv When true, only add this announcement if we don't already have the tx. * Returns true if this was a dropped inv (p2p_inv=true and we already have the tx), false otherwise. */ bool AddTxAnnouncement(NodeId peer, const GenTxid& gtxid, std::chrono::microseconds now, bool p2p_inv); diff --git a/src/node/txdownloadman_impl.h b/src/node/txdownloadman_impl.h index 48f02e607af..8039ddb3cbb 100644 --- a/src/node/txdownloadman_impl.h +++ b/src/node/txdownloadman_impl.h @@ -160,7 +160,9 @@ class TxDownloadManagerImpl { void ConnectedPeer(NodeId nodeid, const TxDownloadConnectionInfo& info); void DisconnectedPeer(NodeId nodeid); - /** New inv has been received. May be added as a candidate to txrequest. */ + /** Consider adding this tx hash to txrequest. Should be called whenever a new inv has been received. + * Also called internally when a transaction is missing parents so that we can request them. + */ bool AddTxAnnouncement(NodeId peer, const GenTxid& gtxid, std::chrono::microseconds now, bool p2p_inv); /** Get getdata requests to send. */ diff --git a/src/test/txdownload_tests.cpp b/src/test/txdownload_tests.cpp index 3eb57a63537..e401fbf59c4 100644 --- a/src/test/txdownload_tests.cpp +++ b/src/test/txdownload_tests.cpp @@ -27,7 +27,7 @@ struct Behaviors { bool m_ignore_inv_txid; bool m_ignore_inv_wtxid; - // Constructor. We are passing and casting ints because they are more readable in a table (see all_expected_results). + // Constructor. We are passing and casting ints because they are more readable in a table (see expected_behaviors). Behaviors(bool txid_rejects, bool wtxid_rejects, bool txid_recon, bool wtxid_recon, bool keep, bool txid_inv, bool wtxid_inv) : m_txid_in_rejects(txid_rejects), m_wtxid_in_rejects(wtxid_rejects), From 8351562bec6081eda2a600bfe4edeb264a9dee0b Mon Sep 17 00:00:00 2001 From: glozow Date: Wed, 30 Oct 2024 21:16:23 -0400 Subject: [PATCH 2/3] [fuzz] allow negative time jumps in txdownloadman_impl --- src/test/fuzz/txdownloadman.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/test/fuzz/txdownloadman.cpp b/src/test/fuzz/txdownloadman.cpp index eb903bb4703..90e04d8376f 100644 --- a/src/test/fuzz/txdownloadman.cpp +++ b/src/test/fuzz/txdownloadman.cpp @@ -430,8 +430,9 @@ FUZZ_TARGET(txdownloadman_impl, .init = initialize) } ); - // Jump ahead in time - time += fuzzed_data_provider.PickValueInArray(TIME_SKIPS); + auto time_skip = fuzzed_data_provider.PickValueInArray(TIME_SKIPS); + if (fuzzed_data_provider.ConsumeBool()) time_skip *= -1; + time += time_skip; CheckInvariants(txdownload_impl, max_orphan_count); } // Disconnect everybody, check that all data structures are empty. From 5dc94d13d419b8d5e543cb50edeb872335c090e7 Mon Sep 17 00:00:00 2001 From: glozow Date: Wed, 6 Nov 2024 09:29:07 -0300 Subject: [PATCH 3/3] fuzz fix: assert MAX_PEER_TX_ANNOUNCEMENTS is not exceeded Previously this assertion checked MAX_PEER_TX_REQUEST_IN_FLIGHT was not exceeded. However, this property is not actually enforced; it is just used to determine when a peer is overloaded. --- src/test/fuzz/txdownloadman.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/fuzz/txdownloadman.cpp b/src/test/fuzz/txdownloadman.cpp index 90e04d8376f..a313caa1117 100644 --- a/src/test/fuzz/txdownloadman.cpp +++ b/src/test/fuzz/txdownloadman.cpp @@ -287,7 +287,7 @@ static void CheckInvariants(const node::TxDownloadManagerImpl& txdownload_impl, // We should never have more than the maximum in-flight requests out for a peer. for (NodeId peer = 0; peer < NUM_PEERS; ++peer) { if (!HasRelayPermissions(peer)) { - Assert(txdownload_impl.m_txrequest.CountInFlight(peer) <= node::MAX_PEER_TX_REQUEST_IN_FLIGHT); + Assert(txdownload_impl.m_txrequest.Count(peer) <= node::MAX_PEER_TX_ANNOUNCEMENTS); } } txdownload_impl.m_txrequest.SanityCheck();