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

refactor(katana): move forked blockchain creation logic to core #2545

Merged
merged 4 commits into from
Oct 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion Cargo.lock

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

47 changes: 27 additions & 20 deletions bin/katana/src/cli/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,13 @@
use katana_core::service::messaging::MessagingConfig;
use katana_node::config::db::DbConfig;
use katana_node::config::dev::DevConfig;
use katana_node::config::fork::ForkingConfig;
use katana_node::config::metrics::MetricsConfig;
use katana_node::config::rpc::{
ApiKind, RpcConfig, DEFAULT_RPC_ADDR, DEFAULT_RPC_MAX_CONNECTIONS, DEFAULT_RPC_PORT,
};
use katana_node::config::{Config, SequencingConfig};
use katana_primitives::block::BlockHashOrNumber;
use katana_primitives::chain::ChainId;
use katana_primitives::chain_spec::{self, ChainSpec};
use katana_primitives::class::ClassHash;
Expand All @@ -47,7 +49,7 @@
use tracing_subscriber::{fmt, EnvFilter};
use url::Url;

use crate::utils::{parse_genesis, parse_seed};
use crate::utils::{parse_block_hash_or_number, parse_genesis, parse_seed};

#[derive(Parser, Debug)]
pub struct NodeArgs {
Expand All @@ -73,10 +75,16 @@
initialized Katana database.")]
pub db_dir: Option<PathBuf>,

#[arg(long)]
#[arg(value_name = "URL")]
#[arg(long = "fork.rpc-url", value_name = "URL", alias = "rpc-url")]
#[arg(help = "The Starknet RPC provider to fork the network from.")]
pub rpc_url: Option<Url>,
pub fork_rpc_url: Option<Url>,

#[arg(long = "fork.block", value_name = "BLOCK_ID", alias = "fork-block-number")]
#[arg(requires = "fork_rpc_url")]
#[arg(help = "Fork the network at a specific block id, can either be a hash (0x-prefixed) \
or number.")]
#[arg(value_parser = parse_block_hash_or_number)]
pub fork_block: Option<BlockHashOrNumber>,

#[arg(long)]
pub dev: bool,
Expand All @@ -91,12 +99,6 @@
#[arg(long, value_name = "SOCKET", value_parser = parse_socket_address, help_heading = "Metrics")]
pub metrics: Option<SocketAddr>,

#[arg(long)]
#[arg(requires = "rpc_url")]
#[arg(value_name = "BLOCK_NUMBER")]
#[arg(help = "Fork the network at a specific block.")]
pub fork_block_number: Option<u64>,

#[arg(long)]
#[arg(value_name = "PATH")]
#[arg(value_parser = katana_core::service::messaging::MessagingConfig::parse)]
Expand Down Expand Up @@ -170,7 +172,7 @@

#[arg(long)]
#[arg(value_parser = parse_genesis)]
#[arg(conflicts_with_all(["rpc_url", "seed", "total_accounts"]))]
#[arg(conflicts_with_all(["fork_rpc_url", "seed", "total_accounts"]))]
pub genesis: Option<Genesis>,
}

Expand Down Expand Up @@ -255,10 +257,9 @@
}

fn init_logging(&self) -> Result<()> {
const DEFAULT_LOG_FILTER: &str = "tasks=debug,info,executor=trace,forking::backend=trace,\
server=debug,katana_core=trace,blockifier=off,\
jsonrpsee_server=off,hyper=off,messaging=debug,\
node=error";
const DEFAULT_LOG_FILTER: &str = "info,tasks=debug,executor=trace,forking::backend=trace,\
server=debug,blockifier=off,jsonrpsee_server=off,\
hyper=off,messaging=debug,node=error";

Check warning on line 262 in bin/katana/src/cli/node.rs

View check run for this annotation

Codecov / codecov/patch

bin/katana/src/cli/node.rs#L260-L262

Added lines #L260 - L262 were not covered by tests

LogTracer::init()?;

Expand All @@ -282,10 +283,11 @@
let chain = self.chain_spec()?;
let metrics = self.metrics_config();
let starknet = self.starknet_config()?;
let forking = self.forking_config()?;
let sequencing = self.sequencer_config();
let messaging = self.messaging.clone();

Ok(Config { metrics, db, dev, rpc, chain, starknet, sequencing, messaging })
Ok(Config { metrics, db, dev, rpc, chain, starknet, sequencing, messaging, forking })
}

fn sequencer_config(&self) -> SequencingConfig {
Expand Down Expand Up @@ -347,15 +349,21 @@

fn starknet_config(&self) -> Result<StarknetConfig> {
Ok(StarknetConfig {
fork_rpc_url: self.rpc_url.clone(),
fork_block_number: self.fork_block_number,
env: Environment {
invoke_max_steps: self.starknet.environment.invoke_max_steps,
validate_max_steps: self.starknet.environment.validate_max_steps,
},
})
}

fn forking_config(&self) -> Result<Option<ForkingConfig>> {
if let Some(url) = self.fork_rpc_url.clone() {
Ok(Some(ForkingConfig { url, block: self.fork_block }))

Check warning on line 361 in bin/katana/src/cli/node.rs

View check run for this annotation

Codecov / codecov/patch

bin/katana/src/cli/node.rs#L361

Added line #L361 was not covered by tests
} else {
Ok(None)
}
}

fn db_config(&self) -> DbConfig {
DbConfig { dir: self.db_dir.clone() }
}
Expand Down Expand Up @@ -488,8 +496,7 @@

assert!(config.dev.fee);
assert!(config.dev.account_validation);
assert_eq!(config.starknet.fork_rpc_url, None);
assert_eq!(config.starknet.fork_block_number, None);
assert!(config.forking.is_none());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Ohayo sensei, consider adding tests for forking configuration

Currently, the tests verify that config.forking is None by default. It would be beneficial to add test cases that validate the forking configuration when fork_rpc_url and fork_block are provided. This ensures the new forking functionality is properly tested and helps prevent regressions.

assert_eq!(config.starknet.env.invoke_max_steps, DEFAULT_INVOKE_MAX_STEPS);
assert_eq!(config.starknet.env.validate_max_steps, DEFAULT_VALIDATE_MAX_STEPS);
assert_eq!(config.db.dir, None);
Expand Down
14 changes: 13 additions & 1 deletion bin/katana/src/utils.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use std::path::PathBuf;

use anyhow::{Context, Result};
use katana_primitives::block::{BlockHash, BlockHashOrNumber, BlockNumber};
use katana_primitives::genesis::json::GenesisJson;
use katana_primitives::genesis::Genesis;

Expand All @@ -16,12 +18,22 @@
}

/// Used as clap value parser for [Genesis].
pub fn parse_genesis(value: &str) -> Result<Genesis, anyhow::Error> {
pub fn parse_genesis(value: &str) -> Result<Genesis> {
let path = PathBuf::from(shellexpand::full(value)?.into_owned());
let genesis = Genesis::try_from(GenesisJson::load(path)?)?;
Ok(genesis)
}

/// If the value starts with `0x`, it is parsed as a [`BlockHash`], otherwise as a [`BlockNumber`].
pub fn parse_block_hash_or_number(value: &str) -> Result<BlockHashOrNumber> {
if value.starts_with("0x") {
Ok(BlockHashOrNumber::Hash(BlockHash::from_hex(value)?))

Check warning on line 30 in bin/katana/src/utils.rs

View check run for this annotation

Codecov / codecov/patch

bin/katana/src/utils.rs#L28-L30

Added lines #L28 - L30 were not covered by tests
} else {
let num = value.parse::<BlockNumber>().context("could not parse block number")?;
Ok(BlockHashOrNumber::Num(num))

Check warning on line 33 in bin/katana/src/utils.rs

View check run for this annotation

Codecov / codecov/patch

bin/katana/src/utils.rs#L32-L33

Added lines #L32 - L33 were not covered by tests
}
}

Check warning on line 35 in bin/katana/src/utils.rs

View check run for this annotation

Codecov / codecov/patch

bin/katana/src/utils.rs#L35

Added line #L35 was not covered by tests

#[cfg(test)]
mod tests {
use super::*;
Expand Down
1 change: 1 addition & 0 deletions crates/katana/core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ dojo-metrics.workspace = true
futures.workspace = true
lazy_static.workspace = true
metrics.workspace = true
num-traits.workspace = true
parking_lot.workspace = true
reqwest.workspace = true
serde.workspace = true
Expand Down
5 changes: 0 additions & 5 deletions crates/katana/core/src/backend/config.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,9 @@
use katana_primitives::block::BlockNumber;
use url::Url;

use crate::constants::{DEFAULT_INVOKE_MAX_STEPS, DEFAULT_VALIDATE_MAX_STEPS};
use crate::env::BlockContextGenerator;

#[derive(Debug, Clone, Default)]
pub struct StarknetConfig {
pub env: Environment,
pub fork_rpc_url: Option<Url>,
pub fork_block_number: Option<BlockNumber>,
}

impl StarknetConfig {
Expand Down
Loading
Loading