Skip to content

Commit dc12d34

Browse files
lexnvbkchrgithub-actions[bot]
committed
net: Strip previous p2p prefix before concatenating addresses (#11078)
This PR ensures that a `MultiaddrWithPeerId` will always remain valid after a concatenating process. The litep2p code will concatanate the provided multiaddr-with-peerID regardless if the address already contains a `p2p/..` prefix: https://github.com/paritytech/polkadot-sdk/blob/e85be1c07adedcc86554affa4f6af536c4a2efc1/substrate/client/network/src/litep2p/service.rs#L427-L434 This then can lead to addresses for reserved peers to never be dialed by the network backend as they are considered invalid. This has been discovered by running the networking benchmarks: https://github.com/paritytech/polkadot-sdk/blob/e85be1c07adedcc86554affa4f6af536c4a2efc1/substrate/client/network/benches/notifications_protocol.rs#L143-L148 Part of the cleanups for: - #10425 --------- Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> Co-authored-by: Bastian Köcher <git@kchr.de> Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com> (cherry picked from commit e30a412)
1 parent 00fbc91 commit dc12d34

File tree

2 files changed

+27
-2
lines changed

2 files changed

+27
-2
lines changed

prdoc/pr_11078.prdoc

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
title: 'net: Strip previous p2p prefix before concatenating addresses'
2+
doc:
3+
- audience: Node Dev
4+
description: |-
5+
This PR ensures that a `MultiaddrWithPeerId` will always remain valid after a concatenating process.
6+
7+
The litep2p code will concatanate the provided multiaddr-with-peerID regardless if the address already contains a `p2p/..` prefix:
8+
9+
https://github.com/paritytech/polkadot-sdk/blob/e85be1c07adedcc86554affa4f6af536c4a2efc1/substrate/client/network/src/litep2p/service.rs#L427-L434
10+
11+
12+
This then can lead to addresses for reserved peers to never be dialed by the network backend as they are considered invalid.
13+
14+
This has been discovered by running the networking benchmarks:
15+
https://github.com/paritytech/polkadot-sdk/blob/e85be1c07adedcc86554affa4f6af536c4a2efc1/substrate/client/network/benches/notifications_protocol.rs#L143-L148
16+
17+
Part of the cleanups for:
18+
- https://github.com/paritytech/polkadot-sdk/issues/10425
19+
crates:
20+
- name: sc-network
21+
bump: patch

substrate/client/network/src/config.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,8 +170,12 @@ pub struct MultiaddrWithPeerId {
170170
impl MultiaddrWithPeerId {
171171
/// Concatenates the multiaddress and peer ID into one multiaddress containing both.
172172
pub fn concat(&self) -> Multiaddr {
173-
let proto = multiaddr::Protocol::P2p(From::from(self.peer_id));
174-
self.multiaddr.clone().with(proto)
173+
let mut addr = self.multiaddr.clone();
174+
// Ensure that the address not already contains the `p2p` protocol.
175+
if matches!(addr.iter().last(), Some(multiaddr::Protocol::P2p(_))) {
176+
addr.pop();
177+
}
178+
addr.with(multiaddr::Protocol::P2p(From::from(self.peer_id)))
175179
}
176180
}
177181

0 commit comments

Comments
 (0)