Skip to content

Commit

Permalink
litep2p/req-resp: Always provide main protocol name in responses (#6603)
Browse files Browse the repository at this point in the history
Request responses are initialized with a main protocol name, and
optional protocol names as a fallback.

Running litep2p in kusama as a validator has surfaced a `debug_asserts`
coming from the sync component:

https://github.com/paritytech/polkadot-sdk/blob/3906c578c96d97a8a099a4bdac4685acbe375a7c/substrate/client/network/sync/src/strategy/chain_sync.rs#L640-L646

The issue is that we initiate a request-response over the main protocol
name `/genesis/sync/2` but receive a response over the legacy procotol
`ksm/sync/2`. This behavior is correct because litep2p propagates to the
higher levels the protocol that responded.

In contrast, libp2p provides the main protocol name regardless of
negotiating a legacy protocol.

Because of this, higher level components assume that only the main
protocol name will respond.
To not break this assumption, this PR alings litep2p shim layer with the
libp2p behavior.

Closes: #6581

---------

Signed-off-by: Alexandru Vasile <[email protected]>
Co-authored-by: Dmitry Markin <[email protected]>
(cherry picked from commit 2a0b268)
  • Loading branch information
lexnv authored and github-actions[bot] committed Nov 27, 2024
1 parent 9fe2d73 commit 0046227
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 5 deletions.
16 changes: 16 additions & 0 deletions prdoc/pr_6603.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json

title: Always provide main protocol name in litep2p responses

doc:
- audience: [ Node Dev, Node Operator ]
description: |
This PR aligns litep2p behavior with libp2p. Previously, litep2p network backend
would provide the actual negotiated request-response protocol that produced a
response message. After this PR, only the main protocol name is reported to other
subsystems.

crates:
- name: sc-network
bump: patch
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ impl RequestResponseProtocol {
&mut self,
peer: litep2p::PeerId,
request_id: RequestId,
fallback: Option<litep2p::ProtocolName>,
_fallback: Option<litep2p::ProtocolName>,
response: Vec<u8>,
) {
match self.pending_inbound_responses.remove(&request_id) {
Expand All @@ -335,10 +335,7 @@ impl RequestResponseProtocol {
response.len(),
);

let _ = tx.send(Ok((
response,
fallback.map_or_else(|| self.protocol.clone(), Into::into),
)));
let _ = tx.send(Ok((response, self.protocol.clone())));
self.metrics.register_outbound_request_success(started.elapsed());
},
}
Expand Down

0 comments on commit 0046227

Please sign in to comment.