diff --git a/crates/ethportal-api/src/types/ping_extensions/extensions/type_0.rs b/crates/ethportal-api/src/types/ping_extensions/extensions/type_0.rs index 277487acd..540abce9f 100644 --- a/crates/ethportal-api/src/types/ping_extensions/extensions/type_0.rs +++ b/crates/ethportal-api/src/types/ping_extensions/extensions/type_0.rs @@ -1,4 +1,4 @@ -use std::{fmt::Display, str::FromStr}; +use std::{fmt::Display, iter::repeat, str::FromStr}; use alloy::primitives::U256; use anyhow::{bail, ensure}; @@ -24,7 +24,7 @@ use crate::{ #[derive(PartialEq, Eq, Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct ClientInfoRadiusCapabilities { - pub client_info: Option, + pub client_info: String, pub data_radius: Distance, pub capabilities: VariableList, } @@ -32,14 +32,14 @@ pub struct ClientInfoRadiusCapabilities { impl ClientInfoRadiusCapabilities { pub fn new(radius: Distance, capabilities: Vec) -> Self { Self { - client_info: Some(ClientInfo::trin_client_info()), + client_info: ClientInfo::trin_client_info().to_string(), data_radius: radius, capabilities: VariableList::from(capabilities), } } pub fn new_with_client_info( - client_info: Option, + client_info: String, radius: Distance, capabilities: Vec, ) -> Self { @@ -50,6 +50,13 @@ impl ClientInfoRadiusCapabilities { } } + /// Returns [ClientInfo] type. + /// + /// See [ClientInfo::from_str_or_empty] for exact behaviour. + pub fn get_client_info(&self) -> ClientInfo { + ClientInfo::from_str_or_empty(&self.client_info) + } + /// ClientType is not robust and should not be used for any critical logic. /// It can't be used to reliably identify the client type from ClientInfoRadiusCapabilities, /// since clients can include amendments to their client name, an example of this is Trin @@ -59,11 +66,7 @@ impl ClientInfoRadiusCapabilities { /// For projects built on Portal like Glados, it is recommended the respective projects /// maintain their own client type parsing logic. pub fn get_client_type(&self) -> ClientType { - if let Some(client_info) = &self.client_info { - ClientType::from(client_info.client_name.as_str()) - } else { - ClientType::Unknown(None) - } + ClientType::from(self.get_client_info().client_name.as_str()) } } @@ -83,11 +86,7 @@ impl Encode for ClientInfoRadiusCapabilities { + ::ssz_fixed_len() + as Encode>::ssz_fixed_len(); let mut encoder = SszEncoder::container(buf, offset); - let client_info = match &self.client_info { - Some(client_info) => client_info.to_string(), - None => "".to_string(), - }; - let bytes: Vec = client_info.as_bytes().to_vec(); + let bytes: Vec = self.client_info.as_bytes().to_vec(); let client_info: VariableList = VariableList::from(bytes); encoder.append(&client_info); @@ -112,15 +111,9 @@ impl Decode for ClientInfoRadiusCapabilities { let data_radius: U256 = decoder.decode_next()?; let capabilities: VariableList = decoder.decode_next()?; - let string = String::from_utf8(client_info.to_vec()).map_err(|_| { + let client_info = String::from_utf8(client_info.to_vec()).map_err(|_| { ssz::DecodeError::BytesInvalid(format!("Invalid utf8 string: {client_info:?}")) })?; - let client_info = match string.as_str() { - "" => None, - _ => Some(ClientInfo::from_str(&string).map_err(|err| { - ssz::DecodeError::BytesInvalid(format!("Failed to parse client info: {err:?}")) - })?), - }; Ok(Self { client_info, @@ -157,6 +150,42 @@ impl ClientInfo { programming_language_version: format!("rustc{PROGRAMMING_LANGUAGE_VERSION}"), } } + + /// Parses a string `s` to return value of this type. + /// + /// Unlike [FromStr::from_str], this function doesn't fail. This means that if input doesn't + /// follow strict format, parsing might result in completely wrong interpretation (e.g. + /// `client_version` might be set to `operating_system`). + pub fn from_str_or_empty(s: &str) -> Self { + let mut parts = s.split('/'); + + let client_name = parts.next().unwrap_or_default(); + + let client_version_and_short_commit = parts.next().unwrap_or_default(); + let (client_version, short_commit) = client_version_and_short_commit + .splitn(2, '-') + .chain(repeat("")) + .next_tuple() + .expect("must have enough elemets"); + + let os_and_cpu = parts.next().unwrap_or_default(); + let (operating_system, cpu_architecture) = os_and_cpu + .splitn(2, '-') + .chain(repeat("")) + .next_tuple() + .expect("muct have enough elements"); + + let programming_language_version = parts.next().unwrap_or_default(); + + Self { + client_name: client_name.to_string(), + client_version: client_version.to_string(), + short_commit: short_commit.to_string(), + operating_system: operating_system.to_string(), + cpu_architecture: cpu_architecture.to_string(), + programming_language_version: programming_language_version.to_string(), + } + } } impl Display for ClientInfo { @@ -177,15 +206,15 @@ impl Display for ClientInfo { impl FromStr for ClientInfo { type Err = anyhow::Error; - fn from_str(string: &str) -> Result { - ensure!(string.len() <= 200, "Client info string is too long"); - let parts: Vec<&str> = string.split('/').collect(); + fn from_str(s: &str) -> Result { + ensure!(s.len() <= 200, "Client info string is too long"); + let parts: Vec<&str> = s.split('/').collect(); if parts.len() != 4 { bail!( "Invalid client info string: should have 4 /'s instead got {} | {}", parts.len(), - string + s ); } @@ -249,8 +278,130 @@ mod tests { utils::bytes::{hex_decode, hex_encode}, }; + mod client_info { + use super::*; + + #[test] + fn from_str() { + let client_info = ClientInfo::trin_client_info(); + let string = client_info.to_string(); + let decoded = ClientInfo::from_str(&string).unwrap(); + assert_eq!(client_info, decoded); + } + + #[rstest] + /// Fails because there are not enough parts + #[case("trin/0.1.1-2b00d730/linux-x86_64")] + /// Fails because there are too many parts + #[case("trin/0.1.1-2b00d730/linux-x86_64/rustc1.81.0/extra")] + /// Fails because the short commit is missing + #[case("trin/0.1.1/linux-x86_64/rustc1.81.0")] + /// Fails because the CPU architecture is missing + #[case("trin/0.1.1-2b00d730/linux/rustc1.81.0")] + /// Fails because client string is too long + #[case(&"t".repeat(201))] + #[should_panic] + fn from_str_invalid(#[case] string: &str) { + ClientInfo::from_str(string).unwrap(); + } + + #[rstest] + /// Regular client info format + #[case::regular( + "trin/0.1.1-2b00d730/linux-x86_64/rustc1.81.0", + "0.1.1", + "2b00d730", + "linux", + "x86_64", + "rustc1.81.0" + )] + /// Only Client name + #[case::only_client_name("trin", "", "", "", "", "")] + /// Only Client name and slashes + #[case::only_client_name_with_slashes("trin///", "", "", "", "", "")] + /// Only Client name and slashes and dashes + #[case::only_client_name_with_slashes_and_dashes("trin/-/-/", "", "", "", "", "")] + /// Short commit is missing + #[case::missing_commit( + "trin/0.1.1/linux-x86_64/rustc1.81.0", + "0.1.1", + "", + "linux", + "x86_64", + "rustc1.81.0" + )] + /// CPU architecture is missing + #[case::missing_cpu_architecture( + "trin/0.1.1-2b00d730/linux/rustc1.81.0", + "0.1.1", + "2b00d730", + "linux", + "", + "rustc1.81.0" + )] + /// Programming language is missing + #[case::missing_programming_language( + "trin/0.1.1-2b00d730/linux-x86_64", + "0.1.1", + "2b00d730", + "linux", + "x86_64", + "" + )] + /// Extra part + #[case::extra_part( + "trin/0.1.1-2b00d730/linux-x86_64/rustc1.81.0/extra", + "0.1.1", + "2b00d730", + "linux", + "x86_64", + "rustc1.81.0" + )] + fn from_str_or_empty( + #[case] string: &str, + #[case] client_version: &str, + #[case] short_commit: &str, + #[case] operating_system: &str, + #[case] cpu_architecture: &str, + #[case] programming_language_version: &str, + ) { + assert_eq!( + ClientInfo::from_str_or_empty(string), + ClientInfo { + client_name: "trin".to_string(), + client_version: client_version.to_string(), + short_commit: short_commit.to_string(), + operating_system: operating_system.to_string(), + cpu_architecture: cpu_architecture.to_string(), + programming_language_version: programming_language_version.to_string(), + }, + ); + } + + #[rstest] + #[case("")] + #[case("/")] + #[case("//")] + #[case("///")] + #[case("////")] + #[case("/-/-/")] + fn from_empty_string(#[case] string: &str) { + assert_eq!( + ClientInfo::from_str_or_empty(string), + ClientInfo { + client_name: "".to_string(), + client_version: "".to_string(), + short_commit: "".to_string(), + operating_system: "".to_string(), + cpu_architecture: "".to_string(), + programming_language_version: "".to_string(), + }, + ); + } + } + #[test] - fn test_client_info_radius_capabilities() { + fn client_info_radius_capabilities() { let radius = Distance::from(U256::from(42)); let capabilities = vec![ PingExtensionType::Capabilities, @@ -276,42 +427,17 @@ mod tests { } } - #[test] - fn test_client_info_from_str() { - let client_info = ClientInfo::trin_client_info(); - let string = client_info.to_string(); - let decoded = ClientInfo::from_str(&string).unwrap(); - assert_eq!(client_info, decoded); - } - - #[rstest] - /// Fails because there are not enough parts - #[case("trin/0.1.1-2b00d730/linux-x86_64")] - /// Fails because there are too many parts - #[case("trin/0.1.1-2b00d730/linux-x86_64/rustc1.81.0/extra")] - /// Fails because the short commit is missing - #[case("trin/0.1.1/linux-x86_64/rustc1.81.0")] - /// Fails because the CPU architecture is missing - #[case("trin/0.1.1-2b00d730/linux/rustc1.81.0")] - /// Fails because client string is too long - #[case(&"t".repeat(201))] - #[should_panic] - fn test_client_info_from_str_invalid(#[case] string: &str) { - ClientInfo::from_str(string).unwrap(); - } - #[test] fn message_encoding_ping_capabilities_with_client_info() { let data_radius = Distance::from(U256::MAX - U256::from(1)); - let client_info = - ClientInfo::from_str("trin/v0.1.1-b61fdc5c/linux-x86_64/rustc1.81.0").unwrap(); + let client_info = "trin/v0.1.1-b61fdc5c/linux-x86_64/rustc1.81.0".to_string(); let capabilities = vec![ PingExtensionType::Capabilities, PingExtensionType::BasicRadius, PingExtensionType::Error, ]; let capabilities_payload = ClientInfoRadiusCapabilities::new_with_client_info( - Some(client_info), + client_info, data_radius, capabilities, ); @@ -340,8 +466,11 @@ mod tests { PingExtensionType::BasicRadius, PingExtensionType::Error, ]; - let capabilities_payload = - ClientInfoRadiusCapabilities::new_with_client_info(None, data_radius, capabilities); + let capabilities_payload = ClientInfoRadiusCapabilities::new_with_client_info( + String::default(), + data_radius, + capabilities, + ); let payload = CustomPayload::from(capabilities_payload); let ping = Ping { enr_seq: 1, @@ -362,15 +491,14 @@ mod tests { #[test] fn message_encoding_pong_capabilities_with_client_info() { let data_radius = Distance::from(U256::MAX - U256::from(1)); - let client_info = - ClientInfo::from_str("trin/v0.1.1-b61fdc5c/linux-x86_64/rustc1.81.0").unwrap(); + let client_info = "trin/v0.1.1-b61fdc5c/linux-x86_64/rustc1.81.0".to_string(); let capabilities = vec![ PingExtensionType::Capabilities, PingExtensionType::BasicRadius, PingExtensionType::Error, ]; let capabilities_payload = ClientInfoRadiusCapabilities::new_with_client_info( - Some(client_info), + client_info, data_radius, capabilities, ); @@ -399,8 +527,11 @@ mod tests { PingExtensionType::BasicRadius, PingExtensionType::Error, ]; - let capabilities_payload = - ClientInfoRadiusCapabilities::new_with_client_info(None, data_radius, capabilities); + let capabilities_payload = ClientInfoRadiusCapabilities::new_with_client_info( + String::default(), + data_radius, + capabilities, + ); let payload = CustomPayload::from(capabilities_payload); let pong = Pong { enr_seq: 1, diff --git a/testing/ef-tests/tests/build_proofs.rs b/testing/ef-tests/tests/build_proofs.rs index e43ccd5f9..04140d6cb 100644 --- a/testing/ef-tests/tests/build_proofs.rs +++ b/testing/ef-tests/tests/build_proofs.rs @@ -92,10 +92,10 @@ fn block_body_execution_payload_proof() { .expect("cannot find test asset"); let content: BeaconBlockBodyBellatrix = serde_yaml::from_str(&value).unwrap(); let expected_execution_payload_proof = [ - "0x6f0e62bdce10586442ef0e4576f7f89d32d58259dd922f5a77ceff213600f5a3", + "0x2b76d38371c161b639e922b0c42031f99dba98e903dd63ccd21a633143aabbaa", "0xf5a5fd42d16a20302798ef6ed309979b43003d2320d9f0e8ea9831a92759fb4b", "0xdb56114e00fdd4c1f85c892bf35ac9a89289aaecb1ebd0a96cde606a748b5d71", - "0x2d296d5f5bc5ffd16277f9ed45c8336bdbec9f6ec9297415031ceaaa4da7680c", + "0xab95e3d062c58f9e1108fc4248ab84a4c87c142ba956ee7e77a6933cf40ea7aa", ] .map(|x| B256::from_str(x).unwrap()); let proof = content.build_execution_payload_proof(); @@ -104,10 +104,10 @@ fn block_body_execution_payload_proof() { assert_eq!(proof, expected_execution_payload_proof.to_vec()); let expected_block_hash_proof = [ - "0x2efbd2cd6514292c8a5888a40958fd3c31aac7dd7d192414fbd8f34731076b09", + "0x7ffe241ea60187fdb0187bfa22de35d1f9bed7ab061d9401fd47e34a54fbede1", "0xf5a5fd42d16a20302798ef6ed309979b43003d2320d9f0e8ea9831a92759fb4b", - "0x18a00335b0da8726239fccb1b19079b2e0f82584a8515d56a633fad33b446eb9", - "0x6542b54766ab0730a18ae5cf17493a2e9ab8c3cc9ae3e73336d2ea2d37895807", + "0x661a812e736b705fc21352b256146b2fc7622861a29b4fa774dcd5164543fea3", + "0xb7a4dae09ab0b87e06da935cc55d6f14a6c85dabc2b5472b44df4c6da99bb078", ] .map(|x| B256::from_str(x).unwrap()) .to_vec(); diff --git a/testing/ethportal-peertest/src/scenarios/ping.rs b/testing/ethportal-peertest/src/scenarios/ping.rs index 74f3081e4..5456f4107 100644 --- a/testing/ethportal-peertest/src/scenarios/ping.rs +++ b/testing/ethportal-peertest/src/scenarios/ping.rs @@ -208,8 +208,8 @@ pub async fn test_ping_capabilities_payload_type(target: &Client, peertest: &Pee assert_eq!(capabilities_payload.data_radius, Distance::MAX); assert_eq!( - capabilities_payload.client_info, - Some(ClientInfo::trin_client_info()) + capabilities_payload.get_client_info(), + ClientInfo::trin_client_info(), ); assert_eq!(capabilities_payload.capabilities.len(), 3); assert_eq!(result.enr_seq, bootnode_sequence);