Repository navigation
docs: add brief explanation of the backoff mechanism from 3SF-mini - #18
Merged
Merged
Conversation
tcoratger
pushed a commit
to tcoratger/ethlambda
that referenced
this pull request
Oct 30, 2026
…lass#18) * fix(p2p): stop writing a varint+snappy header for an empty MetaData request The spec's MetaData request has no body at all, but write_request encoded it to an empty Vec and still ran it through write_payload, which always writes a varint length prefix and a bare snappy stream header even for a zero-length slice: eleven bytes where the spec puts none. read_request had the matching problem in reverse, unconditionally calling decode_payload before looking at the protocol, which would block forever on a peer that actually sent the spec-correct zero bytes. Both directions now check for a MetaData protocol first and return before touching write_payload/decode_payload. The existing round-trip test could never have caught this, since our own encoder and decoder agreed with each other on the same wrong framing either way; two new tests pin the actual wire bytes and prove read_request accepts a truly empty stream. * refactor(p2p): one request_response::Behaviour per protocol Two outbound requests on different protocols, fired back to back on one connection (exactly what ConnectionEstablished does: send_status then request_metadata), could get matched to each other's substreams. The shared req_resp: request_response::Behaviour<Codec> field handed every outbound request to one FIFO pair (pending_outbound/requested_outbound in the pinned fork's handler.rs), drained in substream-negotiation completion order, not send order; the fork's own send_request_with_protocol only narrows which protocol string is offered per call; it does not change that shared queue. When two negotiations complete out of send order, write_request is handed the wrong (protocol, request) pair. Splitting the one behaviour into fourteen, one per protocol id (three lean, eleven beacon), makes the crossing structurally unreachable rather than merely less likely: NetworkBehaviour's derive nests each field's ConnectionHandler behind ConnectionHandlerSelect, which routes a negotiated substream back to exactly the field that offered its protocol, so each field's own queue never sees a request sent on another protocol. Every field is built through build_swarm's existing per-wire match, with either its one real protocol registration or none, so a lean node still offers nothing beacon-only and vice versa. send_request now goes through upstream request_response::Behaviour::send_request on the field named by the new ReqRespProtocol enum; the fork's send_request_with_protocol has no remaining call site. Splitting the behaviour also splits its OutboundRequestId sequence: each field now mints its own ids starting at 1, so two different protocols can legitimately produce the same numeric id for two unrelated requests. outbound_requests is keyed on the new composite ReqRespRequestId (protocol + id) instead, threaded through swarm_adapter's SendRequest command and reconstructed in handle_req_resp_message from the BehaviourEvent variant that produced each event. Left at the request-response layer's own default, fourteen fields would each carry a per-connection stream budget sized for one shared behaviour speaking every protocol, multiplying the aggregate budget by fourteen for no added traffic. Retuned per protocol shape instead: HANDSHAKE_MAX_CONCURRENT_STREAMS for the once-per-connection protocols (status, ping, metadata, goodbye) and the larger FETCH_MAX_CONCURRENT_STREAMS for the four protocols that carry real block and data-column fetch traffic. handle_behaviour_event's dispatch is deliberately exhaustive, with no wildcard arm, so a fifteenth protocol added later forces a decision here rather than falling through handle_swarm_event's own catch-all unnoticed. Adds a regression test that fires a real Status and a real MetaData request back to back over a live loopback connection, repeated across many fresh connections since the race is timing-dependent (negotiation completion order, not send order), and asserts every repetition's answer matches the protocol it was sent on. * refactor(p2p): move the req/resp behaviours into their own struct `lib.rs` carried the fourteen `request_response::Behaviour` fields, their fourteen `with_codec` calls, the two stream-budget constants and the three construction helpers, which was most of what `build_swarm` had grown into. All of it now lives in `req_resp::behaviour`, as a `ReqResp` struct that `Behaviour` nests as a single field: the outer `NetworkBehaviour` derive selects into this one and this one selects into its fourteen, so a negotiated substream still routes back to exactly the field that offered its protocol, which is the guarantee the split exists for. Construction is `ReqResp::new(codec, &wire)`, so the `is_lean`/`is_beacon` pair deciding each field's protocol list is no longer computed in `build_swarm` and threaded through fourteen call sites. `handle_behaviour_event` matched fourteen `BehaviourEvent` variants and repeated the same `handle_req_resp_message` call in each with a different `ReqRespProtocol` constant, which the formatter spread over four to six lines an arm. The tag is the whole of what a req/resp arm decides, so the match yields it as a value and the single call that consumes it sits below: each arm is one line, and the function goes from 119 lines to 40. Still exhaustive, with no wildcard arm, for the reason its doc comment gives. The regression test's own tagging match narrows the same way, so it no longer mirrors fourteen `BehaviourEvent` variants. `swarm_adapter`'s exhaustive send dispatch is unchanged beyond the extra `.req_resp` hop; the fields stay `pub(crate)` because picking one by `ReqRespProtocol` is what sending means. Net: `lib.rs` loses 322 lines, the new module is 271.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.