Skip to content

Commit

Permalink
Use crate::type::Multiaddr shared functionality
Browse files Browse the repository at this point in the history 
Signed-off-by: Alexandru Vasile <[email protected]>
  • Loading branch information
@lexnv
lexnv committed Nov 21, 2024
1 parent f09e25c commit 3906c57
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 120 deletions.
97 changes: 31 additions & 66 deletions substrate/client/network/src/litep2p/discovery.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -489,57 +489,46 @@ impl Discovery {
(false, None)
}

/// Verify the external address discovered by the identify protocol.
///
/// This ensures that:
/// - the address is not empty
/// - the address contains a valid IP address
/// - the address is for the local peer ID
/// - the address always contains `/p2p/<local_peer_id>` protocol.
///
/// The provided peer ID is only used for logging purposes.
fn verify_external_address(&mut self, address: Multiaddr, peer: PeerId) -> Option<Multiaddr> {
if address.is_empty() {
log::warn!(
/// Handle the observed address from the network.
fn handle_observed_addresses(&mut self, observed_address: Multiaddr, peer: PeerId) {
// Converting to and from `sc_network_types::multiaddr::Multiaddr` is cheap considering
// it is a small wrapper over litep2p `Multiaddr`.
let observed_address: sc_network_types::multiaddr::Multiaddr = observed_address.into();
if !observed_address.is_valid_external_address(self.local_peer_id.into()) {
log::debug!(
target: LOG_TARGET,
"🔍 Discovered empty address from {peer:?}",
"Ignoring invalid external address {observed_address} from {peer:?}",
);
return None;
};
return;
}

// Expect the address to contain a valid IP address.
if std::matches!(
address.iter().next(),
Some(
Protocol::Dns(_) |
Protocol::Dns4(_) |
Protocol::Dns6(_) |
Protocol::Ip6(_) |
Protocol::Ip4(_),
)
) {
log::warn!(
let Some(observed_address) = observed_address.ensure_peer_id(self.local_peer_id.into())
else {
log::debug!(
target: LOG_TARGET,
"🔍 Discovered external address from {peer:?} does not contain a valid IP address: {address}",
"Ignoring external address with different peer ID from {peer:?}",
);
return None;
return
};
let observed_address = observed_address.into();

if let Some(Protocol::P2p(peer_id)) = address.iter().last() {
// Invalid address if the reported peer ID is not the local peer ID.
if peer_id != *self.local_peer_id.as_ref() {
log::warn!(
target: LOG_TARGET,
"🔍 Discovered external address from {peer:?} that is not us: {address}",
);
return None
}
let (is_new, expired_address) = self.is_new_external_address(&observed_address, peer);

return Some(address)
if let Some(expired_address) = expired_address {
log::trace!(
target: LOG_TARGET,
"Removing expired external address expired={expired_address} is_new={is_new} observed={observed_address}",
);

self.pending_events
.push_back(DiscoveryEvent::ExternalAddressExpired { address: expired_address });
}

// Ensure the address contains the local peer ID.
Some(address.with(Protocol::P2p(self.local_peer_id.into())))
if is_new {
self.pending_events.push_back(DiscoveryEvent::ExternalAddressDiscovered {
address: observed_address.clone(),
});
}
}
}

Expand Down Expand Up @@ -649,31 +638,7 @@ impl Stream for Discovery {
observed_address,
..
})) => {
let observed_address = this.verify_external_address(observed_address, peer);

// Ensure that an external address with a different peer ID does not have
// side effects of evicting other external addresses via `ExternalAddressExpired`.
if let Some(observed_address) = observed_address {
let (is_new, expired_address) =
this.is_new_external_address(&observed_address, peer);

if let Some(expired_address) = expired_address {
log::trace!(
target: LOG_TARGET,
"Removing expired external address expired={expired_address} is_new={is_new} observed={observed_address}",
);

this.pending_events.push_back(DiscoveryEvent::ExternalAddressExpired {
address: expired_address,
});
}

if is_new {
this.pending_events.push_back(DiscoveryEvent::ExternalAddressDiscovered {
address: observed_address.clone(),
});
}
}
this.handle_observed_addresses(observed_address, peer);

return Poll::Ready(Some(DiscoveryEvent::Identified {
peer,
Expand Down
51 changes: 2 additions & 49 deletions substrate/client/network/src/peer_info.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ use libp2p::{
Info as IdentifyInfo,
},
identity::PublicKey,
multiaddr::Protocol,
ping::{Behaviour as Ping, Config as PingConfig, Event as PingEvent},
swarm::{
behaviour::{
Expand Down Expand Up @@ -184,53 +183,6 @@ impl PeerInfoBehaviour {
"Received pong from node we're not connected to {:?}", peer_id);
}
}

/// Verify the external address discovered by the identify protocol.
///
/// This ensures that:
/// - the address is not empty
/// - the address contains a valid IP address
/// - the address is for the local peer ID
fn verify_external_address(&mut self, address: &Multiaddr) -> bool {
if address.is_empty() {
log::debug!(
target: "sub-libp2p",
"🔍 Discovered empty address",
);
return false;
};

// Expect the address to contain a valid IP address.
if std::matches!(
address.iter().next(),
Some(
Protocol::Dns(_) |
Protocol::Dns4(_) |
Protocol::Dns6(_) |
Protocol::Ip6(_) |
Protocol::Ip4(_),
)
) {
log::debug!(
target: "sub-libp2p",
"🔍 Discovered external address does not contain a valid IP address: {address}",
);
return false;
};

if let Some(Protocol::P2p(peer_id)) = address.iter().last() {
// Invalid address if the reported peer ID is not the local peer ID.
if peer_id != self.local_peer_id {
log::debug!(
target: "sub-libp2p",
"🔍 Discovered external address that is not us: {address}",
);
return false
}
}

true
}
}

/// Gives access to the information about a node.
Expand Down Expand Up @@ -455,7 +407,8 @@ impl NetworkBehaviour for PeerInfoBehaviour {
},
FromSwarm::NewExternalAddrCandidate(e @ NewExternalAddrCandidate { addr }) => {
// Verify the external address is valid.
if self.verify_external_address(addr) {
let multiaddr: sc_network_types::multiaddr::Multiaddr = addr.clone().into();
if multiaddr.is_valid_external_address(self.local_peer_id.into()) {
self.ping.on_swarm_event(FromSwarm::NewExternalAddrCandidate(e));
self.identify.on_swarm_event(FromSwarm::NewExternalAddrCandidate(e));

Expand Down
10 changes: 5 additions & 5 deletions substrate/client/network/types/src/multiaddr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@ impl Multiaddr {
/// - the address is not empty
/// - the address contains a valid IP address
/// - the address is for the local peer ID
pub fn verify_external_address(&self, local_peer_id: PeerId) -> Result<(), ParseError> {
pub fn is_valid_external_address(&self, local_peer_id: PeerId) -> bool {
if self.is_empty() {
return Err(ParseError::InvalidMultiaddr);
return false;
}

// Expect the address to contain a valid IP address.
Expand All @@ -100,17 +100,17 @@ impl Multiaddr {
Protocol::Ip4(_),
)
) {
return Err(ParseError::InvalidMultiaddr);
return false;
}

if let Some(Protocol::P2p(peer_id)) = self.iter().last() {
// Invalid address if the reported peer ID is not the local peer ID.
if peer_id != *local_peer_id.as_ref() {
return Err(ParseError::InvalidMultiaddr);
return false;
}
}

Ok(())
true
}

/// Ensure the peer ID is present in the multiaddress.
Expand Down

0 comments on commit 3906c57

Please sign in to comment.