From a2c4e43e0797a1911f20cae8d98428fc2ea2003a Mon Sep 17 00:00:00 2001 From: Daniel Knopik Date: Mon, 17 Feb 2025 10:22:33 +0100 Subject: [PATCH] address review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: João Oliveira --- .../mainnet/ssv_domain_type.txt | 2 +- anchor/common/ssv_types/src/domain_type.rs | 3 +- anchor/network/src/handshake/codec.rs | 1 + anchor/network/src/handshake/mod.rs | 19 +++++++--- anchor/network/src/network.rs | 36 ++++++++++--------- 5 files changed, 36 insertions(+), 25 deletions(-) diff --git a/anchor/common/ssv_network_config/built_in_network_configs/mainnet/ssv_domain_type.txt b/anchor/common/ssv_network_config/built_in_network_configs/mainnet/ssv_domain_type.txt index 3ee928e4..b86234fa 100644 --- a/anchor/common/ssv_network_config/built_in_network_configs/mainnet/ssv_domain_type.txt +++ b/anchor/common/ssv_network_config/built_in_network_configs/mainnet/ssv_domain_type.txt @@ -1 +1 @@ -0x00000001 \ No newline at end of file +00000001 \ No newline at end of file diff --git a/anchor/common/ssv_types/src/domain_type.rs b/anchor/common/ssv_types/src/domain_type.rs index 762ed78d..b4a27678 100644 --- a/anchor/common/ssv_types/src/domain_type.rs +++ b/anchor/common/ssv_types/src/domain_type.rs @@ -7,8 +7,7 @@ impl FromStr for DomainType { type Err = String; fn from_str(hex_str: &str) -> Result { - let bytes = hex::decode(hex_str.strip_prefix("0x").unwrap_or(hex_str)) - .map_err(|e| format!("Invalid domain type hex: {}", e))?; + let bytes = hex::decode(hex_str).map_err(|e| format!("Invalid domain type hex: {}", e))?; if bytes.len() != 4 { return Err("Domain type must be 4 bytes".into()); } diff --git a/anchor/network/src/handshake/codec.rs b/anchor/network/src/handshake/codec.rs index 76fd2a30..88f47984 100644 --- a/anchor/network/src/handshake/codec.rs +++ b/anchor/network/src/handshake/codec.rs @@ -53,6 +53,7 @@ impl Codec { let envelope = node_info.seal(&self.keypair)?; let raw = envelope.encode_to_vec()?; io.write_all(&raw).await?; + io.flush().await?; io.close().await?; Ok(()) } diff --git a/anchor/network/src/handshake/mod.rs b/anchor/network/src/handshake/mod.rs index ca9a0227..3f1a6a0a 100644 --- a/anchor/network/src/handshake/mod.rs +++ b/anchor/network/src/handshake/mod.rs @@ -18,30 +18,35 @@ pub type Event = ::ToSwarm; #[derive(Debug)] pub enum Error { + /// We are not on the same network as the remote NetworkMismatch { ours: String, theirs: String }, + /// Serialization/Deserialization of the Node Info. NodeInfo(node_info::Error), + /// Error occurred while handling an incoming handshake. Inbound(InboundFailure), + /// Error occurred while handling an outgoing handshake. Outbound(OutboundFailure), } -/// Event emitted on handshake completion or failure. +/// We successfully completed a handshake. #[derive(Debug)] pub struct Completed { pub peer_id: PeerId, pub their_info: NodeInfo, } +/// The handshake either failed because of shaking with an incompatible peer or because of some +/// network failure. #[derive(Debug)] pub struct Failed { pub peer_id: PeerId, pub error: Box, } +/// Create a libp2p Behaviour to handle handshake requests. Events emitted from this event must be +/// fed into [`handle_event`]. pub fn create_behaviour(keypair: Keypair) -> Behaviour { - // NodeInfoProtocol is the protocol.ID used for handshake - const NODE_INFO_PROTOCOL: &str = "/ssv/info/0.0.1"; - - let protocol = StreamProtocol::new(NODE_INFO_PROTOCOL); + let protocol = StreamProtocol::new("/ssv/info/0.0.1"); Behaviour::with_codec( Codec::new(keypair), [(protocol, ProtocolSupport::Full)], @@ -59,6 +64,8 @@ fn verify_node_info(ours: &NodeInfo, theirs: &NodeInfo) -> Result<(), Error> { Ok(()) } +/// Handle an [`Event`] emitted by the passed [`Behaviour`]. The passed [`NodeInfo`] is used for +/// validating the remote peer's data and for responding to incoming requests. pub fn handle_event( our_node_info: &NodeInfo, behaviour: &mut Behaviour, @@ -148,6 +155,8 @@ fn handle_response( }) } +/// Send a handshake request to a specified peer. Should be called after establishing an outgoing +/// connection. pub fn initiate(our_node_info: &NodeInfo, behaviour: &mut Behaviour, peer_id: PeerId) { trace!(?peer_id, "initiating handshake"); behaviour.send_request(&peer_id, our_node_info.clone()); diff --git a/anchor/network/src/network.rs b/anchor/network/src/network.rs index 576d4932..16aff913 100644 --- a/anchor/network/src/network.rs +++ b/anchor/network/src/network.rs @@ -167,11 +167,13 @@ impl Network { } } AnchorBehaviourEvent::Handshake(event) => { - handshake::handle_event( + if let Some(result) = handshake::handle_event( &self.node_info, &mut self.swarm.behaviour_mut().handshake, event, - ); + ) { + self.handle_handshake_result(result); + } } // TODO handle other behaviour events _ => { @@ -237,27 +239,27 @@ impl Network { } } } + + fn handle_handshake_result(&mut self, result: Result) { + match result { + Ok(handshake::Completed { + peer_id, + their_info, + }) => { + debug!(%peer_id, ?their_info, "Handshake completed"); + // Update peer store with their_info + } + Err(handshake::Failed { peer_id, error }) => { + debug!(%peer_id, ?error, "Handshake failed"); + } + } + } } fn subnet_to_topic(subnet: SubnetId) -> IdentTopic { IdentTopic::new(format!("ssv.{}", *subnet)) } -fn handle_handshake_result(result: Result) { - match result { - Ok(handshake::Completed { - peer_id, - their_info, - }) => { - debug!(%peer_id, ?their_info, "Handshake completed"); - // Update peer store with their_info - } - Err(handshake::Failed { peer_id, error }) => { - debug!(%peer_id, ?error, "Handshake failed"); - } - } -} - async fn build_anchor_behaviour( local_keypair: Keypair, network_config: &Config,