From 85017896843a2300e07f603b012d90b22ad676b4 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Fri, 9 Jun 2023 09:42:58 +0200 Subject: [PATCH 1/3] Ignore multiaddresses that can't be parsed in discovery --- full-node/src/run/network_service.rs | 16 +++++++++++++++- lib/src/libp2p/multiaddr.rs | 7 +++++-- lib/src/network/protocol/kademlia.rs | 6 ++---- lib/src/network/service.rs | 2 +- lib/src/network/service/requests_responses.rs | 2 +- light-base/src/network_service.rs | 17 ++++++++++++++++- wasm-node/CHANGELOG.md | 4 ++++ 7 files changed, 44 insertions(+), 10 deletions(-) diff --git a/full-node/src/run/network_service.rs b/full-node/src/run/network_service.rs index 77c112e7cc..e53c31ea64 100644 --- a/full-node/src/run/network_service.rs +++ b/full-node/src/run/network_service.rs @@ -916,11 +916,25 @@ async fn background_task(mut inner: Inner) { Ok(nodes) => { log::debug!("discovered; nodes={:?}", nodes); for (peer_id, addrs) in nodes { + let mut valid_addrs = Vec::with_capacity(addrs.len()); + for addr in addrs { + match Multiaddr::try_from(addr) { + Ok(a) => valid_addrs.push(a), + Err(err) => { + log::debug!( + "discovery-invalid-address; addr={}", + hex::encode(&err.addr) + ); + continue; + } + } + } + inner.network.discover( &Instant::now(), chain_index, peer_id, - addrs, + valid_addrs, ); } } diff --git a/lib/src/libp2p/multiaddr.rs b/lib/src/libp2p/multiaddr.rs index 7ba138baa3..f4aef822ed 100644 --- a/lib/src/libp2p/multiaddr.rs +++ b/lib/src/libp2p/multiaddr.rs @@ -148,7 +148,7 @@ impl TryFrom> for Multiaddr { ))(&bytes) .is_err() { - return Err(FromVecError {}); + return Err(FromVecError { addr: bytes }); } Ok(Multiaddr { bytes }) @@ -173,7 +173,10 @@ impl fmt::Display for Multiaddr { // TODO: more doc and properly derive Display #[derive(Debug, derive_more::Display, Clone, PartialEq, Eq)] -pub struct FromVecError {} +#[display(fmt = "Unable to parse multiaddress")] +pub struct FromVecError { + pub addr: Vec, +} // TODO: more doc and properly derive Display #[derive(Debug, derive_more::Display, Clone)] diff --git a/lib/src/network/protocol/kademlia.rs b/lib/src/network/protocol/kademlia.rs index 88915a7e03..a1a9a26e58 100644 --- a/lib/src/network/protocol/kademlia.rs +++ b/lib/src/network/protocol/kademlia.rs @@ -43,7 +43,7 @@ pub fn build_find_node_request(peer_id: &[u8]) -> Vec { // TODO: return a borrow of the response bytes ; we're limited by protobuf library pub fn decode_find_node_response( response_bytes: &[u8], -) -> Result)>, DecodeFindNodeResponseError> { +) -> Result>)>, DecodeFindNodeResponseError> { let mut parser = nom::combinator::all_consuming::<_, _, nom::error::Error<&[u8]>, _>( nom::combinator::complete(protobuf::message_decode! { #[optional] response_ty = 1 => protobuf::enum_tag_decode, @@ -71,9 +71,7 @@ pub fn decode_find_node_response( let mut multiaddrs = Vec::with_capacity(peer.addrs.len()); for addr in peer.addrs { - let addr = multiaddr::Multiaddr::try_from(addr.to_vec()) - .map_err(DecodeFindNodeResponseError::BadMultiaddr)?; - multiaddrs.push(addr); + multiaddrs.push(addr.to_vec()); } result.push((peer_id, multiaddrs)); diff --git a/lib/src/network/service.rs b/lib/src/network/service.rs index 3dca5496e2..9c2cbc3b27 100644 --- a/lib/src/network/service.rs +++ b/lib/src/network/service.rs @@ -1274,7 +1274,7 @@ pub enum Event { KademliaDiscoveryResult { operation_id: KademliaOperationId, - result: Result)>, DiscoveryError>, + result: Result>)>, DiscoveryError>, }, /*Transactions { peer_id: PeerId, diff --git a/lib/src/network/service/requests_responses.rs b/lib/src/network/service/requests_responses.rs index 8e9a7791af..fefbaa8acd 100644 --- a/lib/src/network/service/requests_responses.rs +++ b/lib/src/network/service/requests_responses.rs @@ -935,7 +935,7 @@ pub enum RequestResult { StorageProof(Result), CallProof(Result), KademliaFindNode( - Result)>, KademliaFindNodeError>, + Result>)>, KademliaFindNodeError>, ), } diff --git a/light-base/src/network_service.rs b/light-base/src/network_service.rs index 9fa144e525..74036126ac 100644 --- a/light-base/src/network_service.rs +++ b/light-base/src/network_service.rs @@ -1177,11 +1177,26 @@ async fn update_round( ); for (peer_id, addrs) in nodes { + let mut valid_addrs = Vec::with_capacity(addrs.len()); + for addr in addrs { + match Multiaddr::try_from(addr) { + Ok(a) => valid_addrs.push(a), + Err(err) => { + log::debug!( + target: "connections", + "Discovery => InvalidAddress({})", + hex::encode(&err.addr) + ); + continue; + } + } + } + guarded.network.discover( &shared.platform.now(), chain_index, peer_id, - addrs, + valid_addrs, ); } } diff --git a/wasm-node/CHANGELOG.md b/wasm-node/CHANGELOG.md index 4499d5c600..76470c945e 100644 --- a/wasm-node/CHANGELOG.md +++ b/wasm-node/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Changed + +- Multiaddresses that can't be parsed during the discovery process are now silently ignored instead of causing the entire list of discovered nodes to be discarded. + ## 1.0.9 - 2023-06-08 ### Added From 0d65c2abce2d77e1aa2447fc524e8c62e412e089 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Fri, 9 Jun 2023 09:43:22 +0200 Subject: [PATCH 2/3] PR link --- wasm-node/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wasm-node/CHANGELOG.md b/wasm-node/CHANGELOG.md index 76470c945e..88534d8d27 100644 --- a/wasm-node/CHANGELOG.md +++ b/wasm-node/CHANGELOG.md @@ -4,7 +4,7 @@ ### Changed -- Multiaddresses that can't be parsed during the discovery process are now silently ignored instead of causing the entire list of discovered nodes to be discarded. +- Multiaddresses that can't be parsed during the discovery process are now silently ignored instead of causing the entire list of discovered nodes to be discarded. ([#705](https://github.com/smol-dot/smoldot/pull/705)) ## 1.0.9 - 2023-06-08 From 2da970d37b268e6f0fdb691d62404d8bbf903622 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Fri, 9 Jun 2023 09:48:13 +0200 Subject: [PATCH 3/3] Rustfmt --- lib/src/network/service/requests_responses.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/src/network/service/requests_responses.rs b/lib/src/network/service/requests_responses.rs index fefbaa8acd..20c2b873e2 100644 --- a/lib/src/network/service/requests_responses.rs +++ b/lib/src/network/service/requests_responses.rs @@ -934,9 +934,7 @@ pub enum RequestResult { State(Result), StorageProof(Result), CallProof(Result), - KademliaFindNode( - Result>)>, KademliaFindNodeError>, - ), + KademliaFindNode(Result>)>, KademliaFindNodeError>), } /// Undecoded but valid block announce.