Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cli): Fix and clean client modes and fetch presets #279

Merged
merged 31 commits into from
Sep 30, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
dc30d18
changed authority to sequencer mode
antiyro Sep 23, 2024
26d2b50
adding cli rules for full
antiyro Sep 23, 2024
31696ba
improved error management
antiyro Sep 23, 2024
7c6c7d1
testing missed cases
antiyro Sep 23, 2024
31ec92c
added support for remote fetch of configfile
antiyro Sep 23, 2024
a1bbc71
fixing ci
antiyro Sep 23, 2024
bf28872
cleaned for ci
antiyro Sep 23, 2024
de4572a
fix(cli): ChainConfig fixes - new override behavior & simpler
akhercha Sep 26, 2024
507ca3d
fix(cli): Sync with main
akhercha Sep 26, 2024
a3ae3fa
fix(cli): Fixed path in tests after removal of set_workdir
akhercha Sep 27, 2024
cf757b1
fix(cli): Docstring
akhercha Sep 27, 2024
2ecb5f2
fix(cli): Re-added test config
akhercha Sep 27, 2024
b129599
fix(cli): Renamed stuff
akhercha Sep 27, 2024
d4cac2a
fix(cli): Moved ChainPreset to CLI
akhercha Sep 27, 2024
2948e87
fix(cli): Context instead of expect
akhercha Sep 27, 2024
d25ee54
fix(cli): Human readable durations
akhercha Sep 27, 2024
45189f9
Merge branch 'main' into fix/cli
akhercha Sep 27, 2024
b722f96
fix(cli): Parse duration tests
akhercha Sep 27, 2024
6c1556d
fix(cli): CLI durations
akhercha Sep 27, 2024
40df5c4
fix(cli): Non mandatory config for --devnet
akhercha Sep 27, 2024
c343e17
fix(cli): Checks during override
akhercha Sep 27, 2024
01c1d81
fix(cli): Override example
akhercha Sep 27, 2024
74dd1ed
fix(cli): better error
akhercha Sep 27, 2024
09802e0
fix(cli): Comments stuff
akhercha Sep 27, 2024
11034e6
reovered state root on full sync
antiyro Sep 27, 2024
9d5ac82
Merge branch 'fix/cli' of https://github.com/madara-alliance/madara i…
antiyro Sep 27, 2024
75b911e
fix(cli): Fixed override
akhercha Sep 27, 2024
05293fd
fix(cli): removed test from CLI
akhercha Sep 27, 2024
7ac59b4
fix(cli): Sequencer default address for devnet
akhercha Sep 27, 2024
4ad2c3b
fix(cli): Fixed path stuff
akhercha Sep 29, 2024
cfe3b3f
fix(cli): Fixed test
akhercha Sep 29, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix(cli): CLI durations
  • Loading branch information
akhercha committed Sep 27, 2024
commit 6c1556d4d892a147d255a57e193e66b77f4d43cd
2 changes: 2 additions & 0 deletions Cargo.lock

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

16 changes: 6 additions & 10 deletions crates/node/src/cli/chain_config_overrides.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
use std::time::Duration;

use anyhow::Context;
use mp_block::H160;
use mp_chain_config::{ChainConfig, StarknetVersion};
use serde::{Deserialize, Serialize};
use serde_yaml::Value;
use starknet_api::core::{ChainId, ContractAddress};
use std::time::Duration;

use mp_block::H160;
use mp_chain_config::{ChainConfig, StarknetVersion};
use mp_utils::parsers::parse_key_value;

/// Override chain config parameters.
/// Format: "--chain-config-override key1=value1 --chain-config-override key2=value2"
Expand Down Expand Up @@ -40,13 +43,6 @@ impl ChainConfigOverrideParams {
}
}

fn parse_key_value(s: &str) -> anyhow::Result<(String, String)> {
let mut parts = s.splitn(2, '=');
let key = parts.next().ok_or_else(|| anyhow::anyhow!("Invalid key-value pair"))?;
let value = parts.next().ok_or_else(|| anyhow::anyhow!("Invalid key-value pair"))?;
Ok((key.to_string(), value.to_string()))
}

/// Part of the Chain Config that we can override.
// We need this proxy structure to implement Serialize -
// which is not possible on the original ChainConfig because the bouncer config and
Expand Down
17 changes: 10 additions & 7 deletions crates/node/src/cli/l1.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
use std::time::Duration;

use url::Url;
const DEFAULT_GAS_PRICE_POLL_MS: u64 = 10_000;

fn parse_url(s: &str) -> Result<Url, url::ParseError> {
s.parse()
}
use mp_utils::parsers::{parse_duration, parse_url};

#[derive(Clone, Debug, clap::Args)]
pub struct L1SyncParams {
Expand All @@ -19,7 +18,11 @@ pub struct L1SyncParams {
#[clap(long, alias = "no-gas-price-sync")]
pub gas_price_sync_disabled: bool,

/// Time in milliseconds in which the gas price worker will fetch the gas price.
#[clap(long, default_value_t = DEFAULT_GAS_PRICE_POLL_MS, alias = "gas-price-poll")]
pub gas_price_poll_ms: u64,
/// Time in which the gas price worker will fetch the gas price.
#[clap(
long,
default_value = "10s",
value_parser = parse_duration,
)]
pub gas_price_poll: Duration,
}
32 changes: 24 additions & 8 deletions crates/node/src/cli/sync.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
use crate::cli::NetworkType;
use mc_sync::fetch::fetchers::FetchConfig;
use starknet_api::core::ChainId;
use std::time::Duration;

use starknet_api::core::ChainId;

use mc_sync::fetch::fetchers::FetchConfig;
use mp_utils::parsers::parse_duration;

use crate::cli::NetworkType;

#[derive(Clone, Debug, clap::Args)]
pub struct SyncParams {
/// Disable the sync service. The sync service is responsible for listening for new blocks on starknet and ethereum.
Expand All @@ -29,12 +33,24 @@ pub struct SyncParams {
pub gateway_key: Option<String>,

/// Polling interval, in seconds. This only affects the sync service once it has caught up with the blockchain tip.
#[clap(long, default_value = "4", value_name = "SECONDS")]
pub sync_polling_interval: u64,
#[clap(
long,
value_parser = parse_duration,
default_value = "4s",
value_name = "SYNC POLLING INTERVAL",
help = "Set the sync polling interval (e.g., '4s', '100ms', '1min')"
)]
pub sync_polling_interval: Duration,

/// Pending block polling interval, in seconds. This only affects the sync service once it has caught up with the blockchain tip.
#[clap(long, default_value = "2", value_name = "SECONDS")]
pub pending_block_poll_interval: u64,
#[clap(
long,
value_parser = parse_duration,
default_value = "2s",
value_name = "PENDING BLOCK POLL INTERVAL",
help = "Set the pending block poll interval (e.g., '2s', '500ms', '30s')"
)]
pub pending_block_poll_interval: Duration,

/// Disable sync polling. This currently means that the sync process will not import any more block once it has caught up with the
/// blockchain tip.
Expand All @@ -55,7 +71,7 @@ impl SyncParams {
let gateway = network.gateway();
let feeder_gateway = network.feeder_gateway();

let polling = if self.no_sync_polling { None } else { Some(Duration::from_secs(self.sync_polling_interval)) };
let polling = if self.no_sync_polling { None } else { Some(self.sync_polling_interval) };

#[cfg(feature = "sound")]
let sound = self.sound;
Expand Down
12 changes: 6 additions & 6 deletions crates/node/src/service/l1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub struct L1SyncService {
l1_gas_provider: GasPriceProvider,
chain_id: ChainId,
gas_price_sync_disabled: bool,
gas_price_poll_ms: Duration,
gas_price_poll: Duration,
}

impl L1SyncService {
Expand Down Expand Up @@ -53,15 +53,15 @@ impl L1SyncService {
};

let gas_price_sync_enabled = authority && !config.gas_price_sync_disabled;
let gas_price_poll_ms = Duration::from_secs(config.gas_price_poll_ms);
let gas_price_poll = config.gas_price_poll;

if gas_price_sync_enabled {
let eth_client = eth_client
.clone()
.context("L1 gas prices require the ethereum service to be enabled. Either disable gas prices syncing using `--no-gas-price-sync`, or remove the `--no-l1-sync` argument.")?;
// running at-least once before the block production service
log::info!("⏳ Getting initial L1 gas prices");
mc_eth::l1_gas_price::gas_price_worker_once(&eth_client, l1_gas_provider.clone(), gas_price_poll_ms)
mc_eth::l1_gas_price::gas_price_worker_once(&eth_client, l1_gas_provider.clone(), gas_price_poll)
.await
.context("Getting initial ethereum gas prices")?;
}
Expand All @@ -72,15 +72,15 @@ impl L1SyncService {
l1_gas_provider,
chain_id,
gas_price_sync_disabled: !gas_price_sync_enabled,
gas_price_poll_ms,
gas_price_poll,
})
}
}

#[async_trait::async_trait]
impl Service for L1SyncService {
async fn start(&mut self, join_set: &mut JoinSet<anyhow::Result<()>>) -> anyhow::Result<()> {
let L1SyncService { l1_gas_provider, chain_id, gas_price_sync_disabled, gas_price_poll_ms, .. } = self.clone();
let L1SyncService { l1_gas_provider, chain_id, gas_price_sync_disabled, gas_price_poll, .. } = self.clone();

if let Some(eth_client) = self.eth_client.take() {
// enabled
Expand All @@ -93,7 +93,7 @@ impl Service for L1SyncService {
chain_id.to_felt(),
l1_gas_provider,
gas_price_sync_disabled,
gas_price_poll_ms,
gas_price_poll,
)
.await
});
Expand Down
2 changes: 1 addition & 1 deletion crates/node/src/service/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ impl SyncService {
block_importer,
start_params: Some(telemetry),
disabled: config.sync_disabled,
pending_block_poll_interval: Duration::from_secs(config.pending_block_poll_interval),
pending_block_poll_interval: config.pending_block_poll_interval,
})
}
}
Expand Down
41 changes: 2 additions & 39 deletions crates/primitives/chain_config/src/chain_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ use serde::{Deserialize, Deserializer};
use starknet_api::core::{ChainId, ContractAddress, PatriciaKey};
use starknet_types_core::felt::Felt;

use mp_utils::serde::deserialize_duration;

use crate::StarknetVersion;

pub mod eth_core_contract_address {
Expand Down Expand Up @@ -294,30 +296,6 @@ impl<'de> Deserialize<'de> for ChainVersionedConstants {
}
}

fn parse_duration(s: &str) -> Result<Duration> {
let s = s.trim();
let split_index =
s.find(|c: char| !c.is_ascii_digit()).ok_or_else(|| anyhow::anyhow!("Invalid duration format: {}", s))?;

let (value_str, suffix) = s.split_at(split_index);
let value: u64 = value_str.parse().map_err(|_| anyhow::anyhow!("Invalid duration value: {}", value_str))?;

match suffix.trim() {
"ms" => Ok(Duration::from_millis(value)),
"s" => Ok(Duration::from_secs(value)),
"min" => Ok(Duration::from_secs(value * 60)),
_ => bail!("Invalid duration suffix: {}. Expected 'ms', 's', or 'min'.", suffix),
}
}

pub fn deserialize_duration<'de, D>(deserializer: D) -> Result<Duration, D::Error>
where
D: Deserializer<'de>,
{
let s = String::deserialize(deserializer)?;
parse_duration(&s).map_err(serde::de::Error::custom)
}

pub fn deserialize_starknet_version<'de, D>(deserializer: D) -> Result<StarknetVersion, D::Error>
where
D: Deserializer<'de>,
Expand Down Expand Up @@ -503,19 +481,4 @@ mod tests {
);
assert!(chain_config.exec_constants_by_protocol_version(StarknetVersion::new(0, 0, 0, 0)).is_err(),);
}

#[rstest]
fn test_parse_duration() {
assert_eq!(parse_duration("2s").unwrap(), Duration::from_secs(2));
assert_eq!(parse_duration("200ms").unwrap(), Duration::from_millis(200));
assert_eq!(parse_duration("5min").unwrap(), Duration::from_secs(300));
assert_eq!(parse_duration("1 min").unwrap(), Duration::from_secs(60));
assert_eq!(parse_duration("10 s").unwrap(), Duration::from_secs(10));
assert!(parse_duration("2x").is_err());
assert!(parse_duration("200").is_err());
assert!(parse_duration("5h").is_err());
assert!(parse_duration("ms200").is_err());
assert!(parse_duration("-5s").is_err());
assert!(parse_duration("5.5s").is_err());
}
}
2 changes: 2 additions & 0 deletions crates/primitives/utils/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,6 @@ async-trait.workspace = true
futures.workspace = true
rayon.workspace = true
rstest = { workspace = true }
serde.workspace = true
tokio = { workspace = true, features = ["signal"] }
url.workspace = true
2 changes: 2 additions & 0 deletions crates/primitives/utils/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#![allow(clippy::new_without_default)]

pub mod parsers;
pub mod serde;
pub mod service;

use std::sync::atomic::{AtomicBool, Ordering};
Expand Down
55 changes: 55 additions & 0 deletions crates/primitives/utils/src/parsers.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
use anyhow::{anyhow, bail};

use std::time::Duration;

use url::Url;

/// Parses a "key=value" string & returns a [(String, String)] tuple.
pub fn parse_key_value(s: &str) -> anyhow::Result<(String, String)> {
let mut parts = s.splitn(2, '=');
let key = parts.next().ok_or_else(|| anyhow::anyhow!("Invalid key-value pair"))?;
let value = parts.next().ok_or_else(|| anyhow::anyhow!("Invalid key-value pair"))?;
Ok((key.to_string(), value.to_string()))
}

/// Parse a string URL & returns it as [Url].
pub fn parse_url(s: &str) -> Result<Url, url::ParseError> {
s.parse()
}

/// Parses a string duration & return it as [Duration].
pub fn parse_duration(s: &str) -> anyhow::Result<Duration> {
let s = s.trim();
let split_index = s.find(|c: char| !c.is_ascii_digit()).ok_or_else(|| anyhow!("Invalid duration format: {}", s))?;

let (value_str, suffix) = s.split_at(split_index);
let value: u64 = value_str.parse().map_err(|_| anyhow!("Invalid duration value: {}", value_str))?;

match suffix.trim() {
"ms" => Ok(Duration::from_millis(value)),
"s" => Ok(Duration::from_secs(value)),
"min" => Ok(Duration::from_secs(value * 60)),
_ => bail!("Invalid duration suffix: {}. Expected 'ms', 's', or 'min'.", suffix),
}
}

#[cfg(test)]
mod tests {
use super::*;
use rstest::rstest;

#[rstest]
fn test_parse_duration() {
assert_eq!(parse_duration("2s").unwrap(), Duration::from_secs(2));
assert_eq!(parse_duration("200ms").unwrap(), Duration::from_millis(200));
assert_eq!(parse_duration("5min").unwrap(), Duration::from_secs(300));
assert_eq!(parse_duration("1 min").unwrap(), Duration::from_secs(60));
assert_eq!(parse_duration("10 s").unwrap(), Duration::from_secs(10));
assert!(parse_duration("2x").is_err());
assert!(parse_duration("200").is_err());
assert!(parse_duration("5h").is_err());
assert!(parse_duration("ms200").is_err());
assert!(parse_duration("-5s").is_err());
assert!(parse_duration("5.5s").is_err());
}
}
13 changes: 13 additions & 0 deletions crates/primitives/utils/src/serde.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
use std::time::Duration;

use serde::{Deserialize, Deserializer};

use crate::parsers::parse_duration;

pub fn deserialize_duration<'de, D>(deserializer: D) -> Result<Duration, D::Error>
where
D: Deserializer<'de>,
{
let s = String::deserialize(deserializer)?;
parse_duration(&s).map_err(serde::de::Error::custom)
}
Loading