Skip to content

Commit

Permalink
Merge bitcoin/bitcoin#31190: TxDownloadManager followups
Browse files Browse the repository at this point in the history
5dc94d1 fuzz fix: assert MAX_PEER_TX_ANNOUNCEMENTS is not exceeded (glozow)
8351562 [fuzz] allow negative time jumps in txdownloadman_impl (glozow)
917ab81 [doc] comment fixups from n30110 (glozow)

Pull request description:

  Addresses some remaining followups from #30110:
  - bitcoin/bitcoin#30110 (comment)
  - bitcoin/bitcoin#30110 (comment)
  - bitcoin/bitcoin#30110 (comment)

ACKs for top commit:
  naumenkogs:
    ACK bitcoin/bitcoin@5dc94d1
  instagibbs:
    ACK 5dc94d1
  theStack:
    ACK 5dc94d1

Tree-SHA512: 568de8822b2ba73407d2231d9c8c83e6c53fb929b598102b6135c16805752954b3b9b53f4e698856d4422fd8ac2f58ce7d033e9d8e101ed09986578b7605df66
  • Loading branch information
fanquake committed Nov 7, 2024
2 parents 3a5f602 + 5dc94d1 commit 018e5fc
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 6 deletions.
3 changes: 2 additions & 1 deletion src/node/txdownloadman.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 3 additions & 1 deletion src/node/txdownloadman_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down
7 changes: 4 additions & 3 deletions src/test/fuzz/txdownloadman.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion src/test/txdownload_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down

0 comments on commit 018e5fc

Please sign in to comment.