From bb3c1098a1a6102e1df9df9e12532dc5f902e53f Mon Sep 17 00:00:00 2001 From: Simon Wicky Date: Wed, 13 Nov 2024 12:01:22 +0100 Subject: [PATCH 1/4] fix no address deserialization bug --- Cargo.lock | 1 - common/client-core/config-types/Cargo.toml | 1 - common/client-core/config-types/src/lib.rs | 8 +++++--- common/config/src/serde_helpers.rs | 13 ++++++++++++- 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3d734c42943..2a09a35770a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4883,7 +4883,6 @@ dependencies = [ "nym-sphinx-addressing", "nym-sphinx-params", "serde", - "serde_with", "thiserror", "url", ] diff --git a/common/client-core/config-types/Cargo.toml b/common/client-core/config-types/Cargo.toml index 83b4d715a27..dca84036b90 100644 --- a/common/client-core/config-types/Cargo.toml +++ b/common/client-core/config-types/Cargo.toml @@ -9,7 +9,6 @@ license.workspace = true [dependencies] humantime-serde = { workspace = true } serde = { workspace = true, features = ["derive"] } -serde_with = { workspace = true, features = ["macros"] } thiserror.workspace = true url = { workspace = true, features = ["serde"] } diff --git a/common/client-core/config-types/src/lib.rs b/common/client-core/config-types/src/lib.rs index 30b34a79763..6159f8e0577 100644 --- a/common/client-core/config-types/src/lib.rs +++ b/common/client-core/config-types/src/lib.rs @@ -2,10 +2,10 @@ // SPDX-License-Identifier: Apache-2.0 use nym_config::defaults::NymNetworkDetails; +use nym_config::serde_helpers::{de_maybe_stringified, ser_maybe_stringified}; use nym_sphinx_addressing::Recipient; use nym_sphinx_params::{PacketSize, PacketType}; use serde::{Deserialize, Serialize}; -use serde_with::{serde_as, DisplayFromStr}; use std::time::Duration; use url::Url; @@ -643,7 +643,6 @@ impl Default for ReplySurbs { } } -#[serde_as] #[derive(Debug, Clone, Copy, Deserialize, PartialEq, Serialize)] #[serde(default, deny_unknown_fields)] pub struct StatsReporting { @@ -651,7 +650,10 @@ pub struct StatsReporting { pub enabled: bool, /// Address of the stats collector. If this is none, no reporting will happen, regardless of `enabled` - #[serde_as(as = "Option")] + #[serde( + serialize_with = "ser_maybe_stringified", + deserialize_with = "de_maybe_stringified" + )] pub provider_address: Option, /// With what frequence will statistics be sent diff --git a/common/config/src/serde_helpers.rs b/common/config/src/serde_helpers.rs index 27c11f21be0..1271bf47cbb 100644 --- a/common/config/src/serde_helpers.rs +++ b/common/config/src/serde_helpers.rs @@ -1,7 +1,7 @@ // Copyright 2023 - Nym Technologies SA // SPDX-License-Identifier: Apache-2.0 -use serde::{Deserialize, Deserializer}; +use serde::{Deserialize, Deserializer, Serializer}; use std::fmt::Display; use std::path::PathBuf; use std::str::FromStr; @@ -20,6 +20,17 @@ where } } +pub fn ser_maybe_stringified(field: &Option, serializer: S) -> Result +where + S: Serializer, + T: Display, +{ + match field { + Some(inner) => serializer.serialize_str(&inner.to_string()), + None => serializer.serialize_str(""), + } +} + pub fn de_maybe_string<'de, D>(deserializer: D) -> Result, D::Error> where D: Deserializer<'de>, From 0e20d6e5b8d88b6cdc024ebb72eafa27d6454e86 Mon Sep 17 00:00:00 2001 From: Simon Wicky Date: Wed, 13 Nov 2024 13:45:32 +0100 Subject: [PATCH 2/4] bug fix in stats_id generation --- common/client-core/src/client/base_client/mod.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/common/client-core/src/client/base_client/mod.rs b/common/client-core/src/client/base_client/mod.rs index 760b7376c37..4276303f2d1 100644 --- a/common/client-core/src/client/base_client/mod.rs +++ b/common/client-core/src/client/base_client/mod.rs @@ -736,8 +736,10 @@ where //make sure we don't accidentally get the same id as gateways are reporting let client_stats_id = format!( - "stats_id_{:x}", - sha2::Sha256::digest(self_address.identity().to_bytes()) + "{:x}", + sha2::Sha256::digest( + ["stats_id".as_bytes(), &self_address.identity().to_bytes()].concat() + ) ); let stats_reporter = Self::start_statistics_control( self.config, From 38a556f8ebbaa74d9398d94fed4507cc2c2e2d15 Mon Sep 17 00:00:00 2001 From: Simon Wicky Date: Thu, 14 Nov 2024 11:49:46 +0100 Subject: [PATCH 3/4] better stats id generation --- Cargo.lock | 1 + common/client-core/src/client/base_client/mod.rs | 11 ++--------- common/statistics/Cargo.toml | 1 + common/statistics/src/lib.rs | 14 ++++++++++++++ 4 files changed, 18 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2a09a35770a..1b2b50cfd67 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6605,6 +6605,7 @@ dependencies = [ "nym-task", "serde", "serde_json", + "sha2 0.10.8", "si-scale", "sysinfo", "thiserror", diff --git a/common/client-core/src/client/base_client/mod.rs b/common/client-core/src/client/base_client/mod.rs index 4276303f2d1..2c363ac517a 100644 --- a/common/client-core/src/client/base_client/mod.rs +++ b/common/client-core/src/client/base_client/mod.rs @@ -49,13 +49,13 @@ use nym_sphinx::addressing::nodes::NodeIdentity; use nym_sphinx::params::PacketType; use nym_sphinx::receiver::{ReconstructedMessage, SphinxMessageReceiver}; use nym_statistics_common::clients::ClientStatsSender; +use nym_statistics_common::generate_client_stats_id; use nym_task::connections::{ConnectionCommandReceiver, ConnectionCommandSender, LaneQueueLengths}; use nym_task::{TaskClient, TaskHandle}; use nym_topology::provider_trait::TopologyProvider; use nym_topology::HardcodedTopologyProvider; use nym_validator_client::{nyxd::contract_traits::DkgQueryClient, UserAgent}; use rand::rngs::OsRng; -use sha2::Digest; use std::fmt::Debug; use std::os::raw::c_int as RawFd; use std::path::Path; @@ -734,17 +734,10 @@ where self.user_agent.clone(), ); - //make sure we don't accidentally get the same id as gateways are reporting - let client_stats_id = format!( - "{:x}", - sha2::Sha256::digest( - ["stats_id".as_bytes(), &self_address.identity().to_bytes()].concat() - ) - ); let stats_reporter = Self::start_statistics_control( self.config, self.user_agent.clone(), - client_stats_id, + generate_client_stats_id(&self_address.identity().to_base58_string()), input_sender.clone(), shutdown.fork("statistics_control"), ); diff --git a/common/statistics/Cargo.toml b/common/statistics/Cargo.toml index 9a8242f34a4..0be446424f3 100644 --- a/common/statistics/Cargo.toml +++ b/common/statistics/Cargo.toml @@ -15,6 +15,7 @@ log = { workspace = true } sysinfo = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } +sha2 = { workspace = true } thiserror = { workspace = true } time = { workspace = true } tokio = { workspace = true } diff --git a/common/statistics/src/lib.rs b/common/statistics/src/lib.rs index dc55054ad09..2cbee637982 100644 --- a/common/statistics/src/lib.rs +++ b/common/statistics/src/lib.rs @@ -11,6 +11,8 @@ #![warn(clippy::todo)] #![warn(clippy::dbg_macro)] +use sha2::Digest; + /// Client specific statistics interfaces and events. pub mod clients; /// Statistics related errors. @@ -19,3 +21,15 @@ pub mod error; pub mod gateways; /// Statistics reporting abstractions and implementations. pub mod report; + +const CLIENT_ID_PREFIX: &str = "client_stats_id"; + +pub fn generate_client_stats_id(id_key: &str) -> String { + generate_stats_id(CLIENT_ID_PREFIX, id_key) +} + +fn generate_stats_id(prefix: &str, id_key: &str) -> String { + let mut hash_input = prefix.to_owned(); + hash_input.push_str(id_key); + format!("{:x}", sha2::Sha256::digest(hash_input)) +} From f61df41a1539191c6a1cfd6f7e04f1a78fe8df32 Mon Sep 17 00:00:00 2001 From: Simon Wicky Date: Thu, 14 Nov 2024 13:11:46 +0100 Subject: [PATCH 4/4] andrew's nitpicking --- Cargo.lock | 1 + common/client-core/src/client/base_client/mod.rs | 2 +- common/statistics/Cargo.toml | 1 + common/statistics/src/lib.rs | 15 +++++++++------ 4 files changed, 12 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1b2b50cfd67..a297c3e41ff 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6600,6 +6600,7 @@ dependencies = [ "futures", "log", "nym-credentials-interface", + "nym-crypto", "nym-metrics", "nym-sphinx", "nym-task", diff --git a/common/client-core/src/client/base_client/mod.rs b/common/client-core/src/client/base_client/mod.rs index 2c363ac517a..b9ea8ef8ae4 100644 --- a/common/client-core/src/client/base_client/mod.rs +++ b/common/client-core/src/client/base_client/mod.rs @@ -737,7 +737,7 @@ where let stats_reporter = Self::start_statistics_control( self.config, self.user_agent.clone(), - generate_client_stats_id(&self_address.identity().to_base58_string()), + generate_client_stats_id(*self_address.identity()), input_sender.clone(), shutdown.fork("statistics_control"), ); diff --git a/common/statistics/Cargo.toml b/common/statistics/Cargo.toml index 0be446424f3..9f8b8db1776 100644 --- a/common/statistics/Cargo.toml +++ b/common/statistics/Cargo.toml @@ -21,6 +21,7 @@ time = { workspace = true } tokio = { workspace = true } si-scale = { workspace = true } +nym-crypto = { path = "../crypto" } nym-sphinx = { path = "../nymsphinx" } nym-credentials-interface = { path = "../credentials-interface" } nym-metrics = { path = "../nym-metrics" } diff --git a/common/statistics/src/lib.rs b/common/statistics/src/lib.rs index 2cbee637982..eccf217d777 100644 --- a/common/statistics/src/lib.rs +++ b/common/statistics/src/lib.rs @@ -11,6 +11,7 @@ #![warn(clippy::todo)] #![warn(clippy::dbg_macro)] +use nym_crypto::asymmetric::ed25519; use sha2::Digest; /// Client specific statistics interfaces and events. @@ -24,12 +25,14 @@ pub mod report; const CLIENT_ID_PREFIX: &str = "client_stats_id"; -pub fn generate_client_stats_id(id_key: &str) -> String { - generate_stats_id(CLIENT_ID_PREFIX, id_key) +pub fn generate_client_stats_id(id_key: ed25519::PublicKey) -> String { + generate_stats_id(CLIENT_ID_PREFIX, id_key.to_base58_string()) } -fn generate_stats_id(prefix: &str, id_key: &str) -> String { - let mut hash_input = prefix.to_owned(); - hash_input.push_str(id_key); - format!("{:x}", sha2::Sha256::digest(hash_input)) +fn generate_stats_id>(prefix: &str, id_seed: M) -> String { + let mut hasher = sha2::Sha256::new(); + hasher.update(prefix); + hasher.update(&id_seed); + let output = hasher.finalize(); + format!("{:x}", output) }