From 1a4931685939dc3a99f2fe16c64ffa02a8f95000 Mon Sep 17 00:00:00 2001 From: Samika Kashyap Date: Tue, 15 Oct 2024 12:23:20 -0700 Subject: [PATCH 1/4] feat: turn on event validation by default --- one/src/daemon.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/one/src/daemon.rs b/one/src/daemon.rs index 19888ab6..483cf0d3 100644 --- a/one/src/daemon.rs +++ b/one/src/daemon.rs @@ -165,12 +165,7 @@ pub struct DaemonOpts { authentication: bool, /// Enable event validation; Requires using the experimental-features flag - #[arg( - long, - default_value_t = false, - env = "CERAMIC_ONE_EVENT_VALIDATION", - requires = "experimental_features" - )] + #[arg(long, default_value_t = true, env = "CERAMIC_ONE_EVENT_VALIDATION")] event_validation: bool, /// Flight SQL bind address; Requires using the experimental-features flag From 96098ba2606bf1e71b96da5232a16d868f798198 Mon Sep 17 00:00:00 2001 From: Samika Kashyap Date: Mon, 21 Oct 2024 11:42:08 -0700 Subject: [PATCH 2/4] fix: clap is weirs with bools, we need to parse them and explicitly define an action on them to set them --- one/src/daemon.rs | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/one/src/daemon.rs b/one/src/daemon.rs index 483cf0d3..99928267 100644 --- a/one/src/daemon.rs +++ b/one/src/daemon.rs @@ -14,7 +14,7 @@ use ceramic_interest_svc::InterestService; use ceramic_kubo_rpc::Multiaddr; use ceramic_metrics::{config::Config as MetricsConfig, MetricsHandle}; use ceramic_p2p::{load_identity, DiskStorage, Keychain, Libp2pConfig}; -use clap::Args; +use clap::{ArgAction, Args}; use recon::{FullInterests, Recon, ReconInterestProvider}; use signal_hook::consts::signal::*; use signal_hook_tokio::Signals; @@ -164,8 +164,15 @@ pub struct DaemonOpts { )] authentication: bool, - /// Enable event validation; Requires using the experimental-features flag - #[arg(long, default_value_t = true, env = "CERAMIC_ONE_EVENT_VALIDATION")] + /// Enable event validation + #[arg( + long, + default_value = "true", + value_parser = clap::value_parser!(bool), + num_args = 0..=1, + action = ArgAction::Set, + env = "CERAMIC_ONE_EVENT_VALIDATION" + )] event_validation: bool, /// Flight SQL bind address; Requires using the experimental-features flag @@ -335,14 +342,9 @@ pub async fn run(opts: DaemonOpts) -> Result<()> { // Construct services from pool let interest_svc = Arc::new(InterestService::new(sqlite_pool.clone())); + let event_validation = opts.event_validation; let event_svc = Arc::new( - EventService::try_new( - sqlite_pool.clone(), - true, - opts.event_validation, - rpc_providers, - ) - .await?, + EventService::try_new(sqlite_pool.clone(), true, event_validation, rpc_providers).await?, ); let network = opts.network.to_network(&opts.local_network_id)?; From 948ab2b64538f9dbed85c25491a8757ac7e5472c Mon Sep 17 00:00:00 2001 From: Samika Kashyap Date: Tue, 22 Oct 2024 11:09:39 -0700 Subject: [PATCH 3/4] fix: use option in event_validation flag --- one/src/daemon.rs | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/one/src/daemon.rs b/one/src/daemon.rs index 99928267..ee0420ba 100644 --- a/one/src/daemon.rs +++ b/one/src/daemon.rs @@ -164,16 +164,9 @@ pub struct DaemonOpts { )] authentication: bool, - /// Enable event validation - #[arg( - long, - default_value = "true", - value_parser = clap::value_parser!(bool), - num_args = 0..=1, - action = ArgAction::Set, - env = "CERAMIC_ONE_EVENT_VALIDATION" - )] - event_validation: bool, + /// Enable event validation, default value if this is not set is true + #[arg(long, env = "CERAMIC_ONE_EVENT_VALIDATION")] + event_validation: Option, /// Flight SQL bind address; Requires using the experimental-features flag #[arg( @@ -342,11 +335,10 @@ pub async fn run(opts: DaemonOpts) -> Result<()> { // Construct services from pool let interest_svc = Arc::new(InterestService::new(sqlite_pool.clone())); - let event_validation = opts.event_validation; + let event_validation = opts.event_validation.unwrap_or(true); let event_svc = Arc::new( EventService::try_new(sqlite_pool.clone(), true, event_validation, rpc_providers).await?, ); - let network = opts.network.to_network(&opts.local_network_id)?; // Setup tokio-metrics From 25173228a6db0ad09eafb5e4ee642ef856b731bc Mon Sep 17 00:00:00 2001 From: Samika Kashyap Date: Tue, 22 Oct 2024 11:30:33 -0700 Subject: [PATCH 4/4] chore: add comment and set default_value to true, using default_value_t=Some(true) will break the code. The default_value_t cannot be used here : The default_value_t attribute in Clap requires the type to implement Display because it needs to be able to print the default value in the help text. However, Option does not implement Display, even if T does. To work around this, we use default_value instead This works because Clap will parse the string true into a bool and then wrap it in Some(...). We have additionally also handled this in the code parsing using an unwrap_or(). The default_value is added more for semantic purposes to aid readability of the code --- one/src/daemon.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/one/src/daemon.rs b/one/src/daemon.rs index ee0420ba..c730ea39 100644 --- a/one/src/daemon.rs +++ b/one/src/daemon.rs @@ -14,7 +14,7 @@ use ceramic_interest_svc::InterestService; use ceramic_kubo_rpc::Multiaddr; use ceramic_metrics::{config::Config as MetricsConfig, MetricsHandle}; use ceramic_p2p::{load_identity, DiskStorage, Keychain, Libp2pConfig}; -use clap::{ArgAction, Args}; +use clap::Args; use recon::{FullInterests, Recon, ReconInterestProvider}; use signal_hook::consts::signal::*; use signal_hook_tokio::Signals; @@ -164,8 +164,9 @@ pub struct DaemonOpts { )] authentication: bool, - /// Enable event validation, default value if this is not set is true - #[arg(long, env = "CERAMIC_ONE_EVENT_VALIDATION")] + /// Enable event validation, true by default + /// default value in args is added here for readability, removal of this param does not change the behavior + #[arg(long, default_value = "true", env = "CERAMIC_ONE_EVENT_VALIDATION")] event_validation: Option, /// Flight SQL bind address; Requires using the experimental-features flag