Skip to content

Commit

Permalink
[Product Data] Config deserialization bug fix (#5126)
Browse files Browse the repository at this point in the history
* fix no address deserialization bug

* bug fix in stats_id generation

* better stats id generation

* andrew's nitpicking
  • Loading branch information
simonwicky authored Nov 14, 2024
1 parent 43e4224 commit b8c1014
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 13 deletions.
3 changes: 2 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion common/client-core/config-types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }

Expand Down
8 changes: 5 additions & 3 deletions common/client-core/config-types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -643,15 +643,17 @@ impl Default for ReplySurbs {
}
}

#[serde_as]
#[derive(Debug, Clone, Copy, Deserialize, PartialEq, Serialize)]
#[serde(default, deny_unknown_fields)]
pub struct StatsReporting {
/// Is stats reporting enabled
pub enabled: bool,

/// Address of the stats collector. If this is none, no reporting will happen, regardless of `enabled`
#[serde_as(as = "Option<DisplayFromStr>")]
#[serde(
serialize_with = "ser_maybe_stringified",
deserialize_with = "de_maybe_stringified"
)]
pub provider_address: Option<Recipient>,

/// With what frequence will statistics be sent
Expand Down
9 changes: 2 additions & 7 deletions common/client-core/src/client/base_client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -734,15 +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!(
"stats_id_{:x}",
sha2::Sha256::digest(self_address.identity().to_bytes())
);
let stats_reporter = Self::start_statistics_control(
self.config,
self.user_agent.clone(),
client_stats_id,
generate_client_stats_id(*self_address.identity()),
input_sender.clone(),
shutdown.fork("statistics_control"),
);
Expand Down
13 changes: 12 additions & 1 deletion common/config/src/serde_helpers.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright 2023 - Nym Technologies SA <[email protected]>
// 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;
Expand All @@ -20,6 +20,17 @@ where
}
}

pub fn ser_maybe_stringified<S, T>(field: &Option<T>, serializer: S) -> Result<S::Ok, S::Error>
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<Option<String>, D::Error>
where
D: Deserializer<'de>,
Expand Down
2 changes: 2 additions & 0 deletions common/statistics/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,13 @@ 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 }
si-scale = { workspace = true }

nym-crypto = { path = "../crypto" }
nym-sphinx = { path = "../nymsphinx" }
nym-credentials-interface = { path = "../credentials-interface" }
nym-metrics = { path = "../nym-metrics" }
Expand Down
17 changes: 17 additions & 0 deletions common/statistics/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@
#![warn(clippy::todo)]
#![warn(clippy::dbg_macro)]

use nym_crypto::asymmetric::ed25519;
use sha2::Digest;

/// Client specific statistics interfaces and events.
pub mod clients;
/// Statistics related errors.
Expand All @@ -19,3 +22,17 @@ 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: ed25519::PublicKey) -> String {
generate_stats_id(CLIENT_ID_PREFIX, id_key.to_base58_string())
}

fn generate_stats_id<M: AsRef<[u8]>>(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)
}

0 comments on commit b8c1014

Please sign in to comment.