diff --git a/src/bitcoin.rs b/src/bitcoin.rs index 4f61478..f7e9b4a 100644 --- a/src/bitcoin.rs +++ b/src/bitcoin.rs @@ -1,6 +1,7 @@ use std::{ collections::HashSet, - path::PathBuf, + fs::File, + process::Stdio, sync::Arc, time::{Duration, Instant}, }; @@ -17,10 +18,10 @@ use super::{ config::BitcoinConfig, docker::DockerEnv, framework::TestContext, - traits::{LogProvider, NodeT, Restart, SpawnOutput}, + traits::{NodeT, Restart, SpawnOutput}, Result, }; -use crate::node::NodeKind; +use crate::{log_provider::LogPathProvider, node::NodeKind}; pub const FINALITY_DEPTH: u64 = 8; @@ -185,10 +186,19 @@ impl NodeT for BitcoinNode { let args = config.args(); debug!("Running bitcoind with args : {args:?}"); + info!( + "Bitcoin debug.log available at : {}", + config.log_path().display() + ); + + let stderr_path = config.stderr_path(); + let stderr_file = File::create(stderr_path).context("Failed to create stderr file")?; + Command::new("bitcoind") .args(&args) .kill_on_drop(true) .envs(config.env.clone()) + .stderr(Stdio::from(stderr_file)) .spawn() .context("Failed to spawn bitcoind process") .map(SpawnOutput::Child) @@ -258,7 +268,7 @@ impl Restart for BitcoinNode { async fn start(&mut self, config: Option) -> Result<()> { if let Some(config) = config { - self.config = config + self.config = config; } self.spawn_output = Self::spawn(&self.config, &self.docker_env).await?; @@ -271,16 +281,6 @@ impl Restart for BitcoinNode { } } -impl LogProvider for BitcoinNode { - fn kind(&self) -> NodeKind { - NodeKind::Bitcoin - } - - fn log_path(&self) -> PathBuf { - self.config.data_dir.join("regtest").join("debug.log") - } -} - pub struct BitcoinNodeCluster { inner: Vec, } @@ -361,7 +361,7 @@ async fn wait_for_rpc_ready(client: &Client, timeout: Option) -> Resul Ok(_) => return Ok(()), Err(e) => { trace!("[wait_for_rpc_ready] error {e}"); - sleep(Duration::from_millis(500)).await + sleep(Duration::from_millis(500)).await; } } } diff --git a/src/citrea_config/rollup.rs b/src/citrea_config/rollup.rs index cc8fcf9..23bad87 100644 --- a/src/citrea_config/rollup.rs +++ b/src/citrea_config/rollup.rs @@ -86,7 +86,7 @@ const fn default_max_subscriptions_per_connection() -> u32 { pub struct StorageConfig { /// Path that can be utilized by concrete rollup implementation pub path: PathBuf, - /// File descriptor limit for RocksDB + /// File descriptor limit for `RocksDB` pub db_max_open_files: Option, } diff --git a/src/citrea_config/sequencer.rs b/src/citrea_config/sequencer.rs index 03babd8..412527e 100644 --- a/src/citrea_config/sequencer.rs +++ b/src/citrea_config/sequencer.rs @@ -29,7 +29,7 @@ impl Default for SequencerConfig { deposit_mempool_fetch_limit: 10, block_production_interval_ms: 100, da_update_interval_ms: 100, - mempool_conf: Default::default(), + mempool_conf: SequencerMempoolConfig::default(), } } } @@ -57,11 +57,11 @@ pub struct SequencerMempoolConfig { impl Default for SequencerMempoolConfig { fn default() -> Self { Self { - pending_tx_limit: 100000, + pending_tx_limit: 100_000, pending_tx_size: 200, - queue_tx_limit: 100000, + queue_tx_limit: 100_000, queue_tx_size: 200, - base_fee_tx_limit: 100000, + base_fee_tx_limit: 100_000, base_fee_tx_size: 200, max_account_slots: 16, } diff --git a/src/client.rs b/src/client.rs index 06feefb..2b70ca0 100644 --- a/src/client.rs +++ b/src/client.rs @@ -86,7 +86,7 @@ impl Client { self.client .get_sequencer_commitments_on_slot_by_hash(hash) .await - .map_err(|e| e.into()) + .map_err(Into::into) } pub async fn ledger_get_head_soft_confirmation_height(&self) -> Result { diff --git a/src/config/bitcoin.rs b/src/config/bitcoin.rs index 2410934..47c2ede 100644 --- a/src/config/bitcoin.rs +++ b/src/config/bitcoin.rs @@ -3,6 +3,8 @@ use std::path::PathBuf; use bitcoin::Network; use tempfile::TempDir; +use crate::{log_provider::LogPathProvider, node::NodeKind}; + #[derive(Debug, Clone)] pub struct BitcoinConfig { pub p2p_port: u16, @@ -62,3 +64,17 @@ impl BitcoinConfig { .concat() } } + +impl LogPathProvider for BitcoinConfig { + fn kind() -> NodeKind { + NodeKind::Bitcoin + } + + fn log_path(&self) -> PathBuf { + self.data_dir.join("regtest").join("debug.log") + } + + fn stderr_path(&self) -> PathBuf { + self.data_dir.join("stderr.log") + } +} diff --git a/src/config/mod.rs b/src/config/mod.rs index 49badb2..e93473d 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -24,7 +24,10 @@ pub use crate::citrea_config::{ rollup::{FullNodeConfig, RollupPublicKeys, RpcConfig, RunnerConfig, StorageConfig}, sequencer::SequencerConfig, }; -use crate::node::{Config, NodeKind}; +use crate::{ + log_provider::LogPathProvider, + node::{Config, NodeKind}, +}; #[derive(Clone, Debug)] pub struct FullL2NodeConfig { @@ -98,3 +101,20 @@ where &self.rollup } } + +impl LogPathProvider for FullL2NodeConfig +where + FullL2NodeConfig: Config, +{ + fn kind() -> NodeKind { + Self::node_kind() + } + + fn log_path(&self) -> PathBuf { + self.dir().join("stdout.log") + } + + fn stderr_path(&self) -> PathBuf { + self.dir().join("stderr.log") + } +} diff --git a/src/config/rollup.rs b/src/config/rollup.rs index 8e75183..cc6066d 100644 --- a/src/config/rollup.rs +++ b/src/config/rollup.rs @@ -66,7 +66,7 @@ impl From for BitcoinServiceConfig { node_password: v.rpc_password, network: v.network, da_private_key: None, - tx_backup_dir: "".to_string(), + tx_backup_dir: String::new(), } } } diff --git a/src/framework.rs b/src/framework.rs index db39187..d667b6d 100644 --- a/src/framework.rs +++ b/src/framework.rs @@ -4,16 +4,15 @@ use bitcoincore_rpc::RpcApi; use tracing::{debug, info}; use super::{ - bitcoin::BitcoinNodeCluster, - config::TestConfig, - docker::DockerEnv, - full_node::FullNode, - node::NodeKind, - sequencer::Sequencer, - traits::{LogProvider, LogProviderErased, NodeT}, - Result, + bitcoin::BitcoinNodeCluster, config::TestConfig, docker::DockerEnv, full_node::FullNode, + node::NodeKind, sequencer::Sequencer, traits::NodeT, Result, +}; +use crate::{ + batch_prover::BatchProver, + light_client_prover::LightClientProver, + log_provider::{LogPathProvider, LogPathProviderErased}, + utils::tail_file, }; -use crate::{batch_prover::BatchProver, light_client_prover::LightClientProver, utils::tail_file}; pub struct TestContext { pub config: TestConfig, @@ -41,7 +40,6 @@ pub struct TestFramework { pub batch_prover: Option, pub light_client_prover: Option, pub full_node: Option, - show_logs: bool, pub initial_da_height: u64, } @@ -64,7 +62,6 @@ impl TestFramework { let bitcoin_nodes = BitcoinNodeCluster::new(&ctx).await?; - // tokio::time::sleep(std::time::Duration::from_secs(30)).await; Ok(Self { bitcoin_nodes, sequencer: None, @@ -72,7 +69,6 @@ impl TestFramework { light_client_prover: None, full_node: None, ctx, - show_logs: true, initial_da_height: 0, }) } @@ -103,36 +99,31 @@ impl TestFramework { Ok(()) } - fn get_nodes_as_log_provider(&self) -> Vec<&dyn LogProviderErased> { - vec![ - self.bitcoin_nodes.get(0).map(LogProvider::as_erased), - self.sequencer.as_ref().map(LogProvider::as_erased), - self.full_node.as_ref().map(LogProvider::as_erased), - self.batch_prover.as_ref().map(LogProvider::as_erased), - self.light_client_prover - .as_ref() - .map(LogProvider::as_erased), - ] - .into_iter() - .flatten() - .collect() - } - - pub fn show_log_paths(&self) { - if self.show_logs { - info!( - "Logs available at {}", - self.ctx.config.test_case.dir.display() - ); - - for node in self.get_nodes_as_log_provider() { - info!( - "{} logs available at : {}", - node.kind(), - node.log_path().display() - ); - } - } + fn get_nodes_as_log_provider(&self) -> Vec<&dyn LogPathProviderErased> { + let test_case = &self.ctx.config.test_case; + + self.ctx + .config + .bitcoin + .iter() + .map(LogPathProvider::as_erased) + .map(Option::Some) + .chain(vec![ + test_case + .with_sequencer + .then(|| LogPathProvider::as_erased(&self.ctx.config.sequencer)), + test_case + .with_full_node + .then(|| LogPathProvider::as_erased(&self.ctx.config.full_node)), + test_case + .with_batch_prover + .then(|| LogPathProvider::as_erased(&self.ctx.config.batch_prover)), + test_case + .with_light_client_prover + .then(|| LogPathProvider::as_erased(&self.ctx.config.light_client_prover)), + ]) + .flatten() + .collect() } pub fn dump_log(&self) -> Result<()> { @@ -142,11 +133,11 @@ impl TestFramework { .ok() .and_then(|v| v.parse::().ok()) .unwrap_or(25); - for node in self.get_nodes_as_log_provider() { - debug!("{} logs (last {n_lines} lines):", node.kind()); - if let Err(e) = tail_file(&node.log_path(), n_lines) { - eprint!("{e}"); - } + for provider in self.get_nodes_as_log_provider() { + println!("{} logs (last {n_lines} lines):", provider.kind()); + let _ = tail_file(&provider.log_path(), n_lines); + println!("{} stderr logs (last {n_lines} lines):", provider.kind()); + let _ = tail_file(&provider.stderr_path(), n_lines); } Ok(()) } diff --git a/src/lib.rs b/src/lib.rs index 16b28ac..bd64896 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -7,6 +7,7 @@ mod docker; pub mod framework; pub mod full_node; pub mod light_client_prover; +mod log_provider; pub mod node; pub mod sequencer; pub mod test_case; diff --git a/src/log_provider.rs b/src/log_provider.rs new file mode 100644 index 0000000..0266888 --- /dev/null +++ b/src/log_provider.rs @@ -0,0 +1,35 @@ +use std::path::PathBuf; + +use crate::node::NodeKind; + +pub trait LogPathProvider { + fn kind() -> NodeKind; + fn log_path(&self) -> PathBuf; + fn stderr_path(&self) -> PathBuf; + fn as_erased(&self) -> &dyn LogPathProviderErased + where + Self: Sized, + { + self + } +} + +pub trait LogPathProviderErased { + fn kind(&self) -> NodeKind; + fn log_path(&self) -> PathBuf; + fn stderr_path(&self) -> PathBuf; +} + +impl LogPathProviderErased for T { + fn kind(&self) -> NodeKind { + T::kind() + } + + fn log_path(&self) -> PathBuf { + LogPathProvider::log_path(self) + } + + fn stderr_path(&self) -> PathBuf { + LogPathProvider::stderr_path(self) + } +} diff --git a/src/node.rs b/src/node.rs index f0be166..3c9bdf5 100644 --- a/src/node.rs +++ b/src/node.rs @@ -13,13 +13,14 @@ use tokio::{ process::Command, time::{sleep, Instant}, }; -use tracing::trace; +use tracing::{info, trace}; use crate::{ client::Client, config::{config_to_file, RollupConfig}, - traits::{LogProvider, NodeT, Restart, SpawnOutput}, - utils::{get_citrea_path, get_genesis_path, get_stderr_path, get_stdout_path}, + log_provider::LogPathProvider, + traits::{NodeT, Restart, SpawnOutput}, + utils::{get_citrea_path, get_genesis_path}, Result, }; @@ -58,13 +59,13 @@ pub trait Config: Clone { fn rollup_config(&self) -> &RollupConfig; } -pub struct Node { +pub struct Node { spawn_output: SpawnOutput, config: C, pub client: Client, } -impl Node { +impl Node { pub async fn new(config: &C) -> Result { let spawn_output = Self::spawn(config)?; @@ -102,10 +103,16 @@ impl Node { let kind = C::node_kind(); - let stdout_file = - File::create(get_stdout_path(dir)).context("Failed to create stdout file")?; - let stderr_file = - File::create(get_stderr_path(dir)).context("Failed to create stderr file")?; + let stdout_path = config.log_path(); + let stdout_file = File::create(&stdout_path).context("Failed to create stdout file")?; + info!( + "{} stdout logs available at : {}", + kind, + stdout_path.display() + ); + + let stderr_path = config.stderr_path(); + let stderr_file = File::create(stderr_path).context("Failed to create stderr file")?; // Handle full node not having any node config let node_config_args = Self::get_node_config_args(config)?; @@ -160,7 +167,7 @@ impl Node { #[async_trait] impl NodeT for Node where - C: Config + Send + Sync, + C: Config + LogPathProvider + Send + Sync, { type Config = C; type Client = Client; @@ -210,20 +217,10 @@ where } } -impl LogProvider for Node { - fn kind(&self) -> NodeKind { - C::node_kind() - } - - fn log_path(&self) -> PathBuf { - get_stdout_path(self.config.dir()) - } -} - #[async_trait] impl Restart for Node where - C: Config + Send + Sync, + C: Config + LogPathProvider + Send + Sync, { async fn wait_until_stopped(&mut self) -> Result<()> { self.stop().await?; diff --git a/src/test_case.rs b/src/test_case.rs index a553cc2..dc04c67 100644 --- a/src/test_case.rs +++ b/src/test_case.rs @@ -46,7 +46,6 @@ impl TestCaseRunner { async fn prepare(&self, f: &mut TestFramework) -> Result<()> { f.fund_da_wallets().await?; f.init_nodes().await?; - f.show_log_paths(); f.bitcoin_nodes.connect_nodes().await?; if let Some(sequencer) = &f.sequencer { diff --git a/src/traits.rs b/src/traits.rs index 7942401..c5fff5f 100644 --- a/src/traits.rs +++ b/src/traits.rs @@ -1,4 +1,4 @@ -use std::{path::PathBuf, time::Duration}; +use std::time::Duration; use anyhow::Context; use async_trait::async_trait; @@ -7,7 +7,6 @@ use tokio::process::Child; use tracing::info; use super::Result; -use crate::node::NodeKind; #[derive(Debug)] pub struct ContainerSpawnOutput { @@ -82,29 +81,3 @@ pub trait Restart: NodeT + Send { self.start(new_config).await } } - -pub trait LogProvider: NodeT { - fn kind(&self) -> NodeKind; - fn log_path(&self) -> PathBuf; - fn as_erased(&self) -> &dyn LogProviderErased - where - Self: Sized, - { - self - } -} - -pub trait LogProviderErased { - fn kind(&self) -> NodeKind; - fn log_path(&self) -> PathBuf; -} - -impl LogProviderErased for T { - fn kind(&self) -> NodeKind { - LogProvider::kind(self) - } - - fn log_path(&self) -> PathBuf { - LogProvider::log_path(self) - } -} diff --git a/src/utils.rs b/src/utils.rs index 66f0b02..e913c9d 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -27,21 +27,13 @@ pub fn get_workspace_root() -> PathBuf { .to_path_buf() } -/// Get citrea path from CITREA_E2E_TEST_BINARY env +/// Get citrea path from `CITREA_E2E_TEST_BINARY` env pub fn get_citrea_path() -> Result { std::env::var("CITREA_E2E_TEST_BINARY") .map(PathBuf::from) .map_err(|_| anyhow!("CITREA_E2E_TEST_BINARY is not set. Cannot resolve citrea path")) } -pub fn get_stdout_path(dir: &Path) -> PathBuf { - dir.join("stdout.log") -} - -pub fn get_stderr_path(dir: &Path) -> PathBuf { - dir.join("stderr.log") -} - /// Get genesis path from resources /// TODO: assess need for customable genesis path in e2e tests pub fn get_default_genesis_path() -> PathBuf {