Repository navigation
net: isolated broadcast waits for the peer's pong - #1342
reardencode merged 2 commits into
Conversation
![rearden-grok[bot]](https://avatars.githubusercontent.com/u/730881?s=60&v=4){kind=small}
There was a problem hiding this comment.
REQUEST_CHANGES
isolated_broadcast_needs_getdata_and_pong burns about four seconds of real timeout. The note is on Duration::from_secs(2).
Nit: the doc comment on isolated_broadcast_sends_core_private_broadcast_version still describes the post-verack exchange as only the tx.
| NetAddr::Ip(peer_addr), | ||
| Magic::REGTEST, | ||
| dummy_tx(), | ||
| Duration::from_secs(2), |
There was a problem hiding this comment.
isolated_broadcast_needs_getdata_and_pong waits out this 2s deadline twice, back to back, so the test is about 4s. The cases are sequential, and each one blocks in hand_over_tx until timeout_at fires (no getdata, then no pong). That is not HANDSHAKE_TIMEOUT, but it is still a new default-suite test over the 2s wall in TESTING.md, with no reason a shorter cap would miss the branch. The peer is on localhost and answers the handshake immediately; pass a few hundred milliseconds so both failures still trip and the test stays under that cap.
Review on reardencode#1342: isolated_broadcast_needs_getdata_and_pong waited out a 2 s limit twice, about 4 s. The peer is on localhost, so 300 ms still trips both failures, and the test runs in about 0.6 s. The comment on isolated_broadcast_sends_core_private_broadcast_version now names INV, GETDATA and PING.
There was a problem hiding this comment.
APPROVE
Both open items are fixed.
- Fixed.
Duration::from_millis(300)is the whole handshake-plus-exchange budget, and the two cases run one after the other, so the waits are about 0.6s.commandsstill requiresinvand notxwhen the peer never asks, andinv,tx,pingwhen it asks but never pongs, so a deadline that expires during the handshake fails the test instead of passing for the wrong reason. Thecargo teststep was 160s on this head, 140s on the previous head, and 149s on master. The log has no per-test times and no stall; the +20s on that step is the same runner noise as the job times. - Fixed. The comment on
isolated_broadcast_sends_core_private_broadcast_versionnow includes theinv, thegetdata, and theping.
Isolated broadcast sent tx and closed the socket straight away. Core can drop a message that arrives just before the close (bitcoin/bitcoin#4432), and nothing told us whether the peer took the tx. The one-shot now does what Core's private broadcast does: inv, wait for a getdata naming the tx, tx, ping, and close on the matching pong. A peer that never asks for the tx gets no tx, and a missing pong is a failed send. The handshake and the exchange share one deadline, so one send still takes at most HANDSHAKE_TIMEOUT. Item 4 of reardencode#1275.
Review on reardencode#1342: isolated_broadcast_needs_getdata_and_pong waited out a 2 s limit twice, about 4 s. The peer is on localhost, so 300 ms still trips both failures, and the test runs in about 0.6 s. The comment on isolated_broadcast_sends_core_private_broadcast_version now names INV, GETDATA and PING.
e6a272a to
9f3377c
Compare
Item 4 of #1275.
Isolated broadcast sent our tx and hung up straight away. The peer could drop it, and we never knew if it arrived.
Now it works like Bitcoin Core's private broadcast: we offer the tx, send it only when the peer asks for it, then ping and wait for the pong before hanging up. If the peer never asks, or never answers the ping, the send counts as failed.
Tested with new tests for the normal case and for a peer that never asks or never answers. I also sent a tx this way to a real Bitcoin Core 29 node, and it took it into its mempool.