Skip to content

Commit

Permalink
feat(en): Add boxed L2 client and use it in DI (#1627)
Browse files Browse the repository at this point in the history
## What ❔

- Adds boxed L2 client implementation and uses it in DI instead of a
specific HTTP-based client.
- Adds a mock L2 client.
- Uses a mock L1 and L2 clients to test EN lifecycle in Rust, namely
that it can start and timely terminates when sent a SIGINT.

## Why ❔

- L2 client abstraction is better for the node framework.
- We have a frequent issue of newly added managed tasks being
non-responsive, which leads to unnecessary delays during EN shutdown
(30s in the worst case).
- Using TypeScript integration tests for this purpose is slower and
provides less control.

## Checklist

- [x] PR title corresponds to the body of PR (we generate changelog
entries from PRs).
- [x] Tests for the changes have been added / updated.
- [x] Documentation comments have been added / updated.
- [x] Code has been formatted via `zk fmt` and `zk lint`.
- [x] Spellcheck has been run via `zk spellcheck`.
- [x] Linkcheck has been run via `zk linkcheck`.

test(en): Add basic rust tests for EN
  • Loading branch information
slowli committed Apr 17, 2024
1 parent 02f7cd5 commit 9948187
Show file tree
Hide file tree
Showing 31 changed files with 967 additions and 261 deletions.
3 changes: 3 additions & 0 deletions Cargo.lock

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

5 changes: 5 additions & 0 deletions core/bin/external_node/Cargo.toml
Expand Up @@ -47,5 +47,10 @@ serde_json.workspace = true
semver.workspace = true
tracing.workspace = true

[dev-dependencies]
assert_matches.workspace = true
tempfile.workspace = true
test-casing.workspace = true

[build-dependencies]
rustc_version.workspace = true
84 changes: 76 additions & 8 deletions core/bin/external_node/src/config/mod.rs
@@ -1,6 +1,7 @@
use std::{
env,
env, fmt,
num::{NonZeroU32, NonZeroUsize},
str::FromStr,
time::Duration,
};

Expand All @@ -22,6 +23,8 @@ use zksync_core::{
},
temp_config_store::decode_yaml_repr,
};
#[cfg(test)]
use zksync_dal::{ConnectionPool, Core};
use zksync_protobuf_config::proto;
use zksync_types::{api::BridgeAddresses, fee_model::FeeParams};
use zksync_web3_decl::{
Expand Down Expand Up @@ -118,6 +121,26 @@ impl RemoteENConfig {
.unwrap_or_default(),
})
}

#[cfg(test)]
fn mock() -> Self {
Self {
bridgehub_proxy_addr: None,
state_transition_proxy_addr: None,
transparent_proxy_admin_addr: None,
diamond_proxy_addr: Address::repeat_byte(1),
l1_erc20_bridge_proxy_addr: Address::repeat_byte(2),
l2_erc20_bridge_addr: Address::repeat_byte(3),
l1_weth_bridge_proxy_addr: None,
l2_weth_bridge_addr: None,
l2_testnet_paymaster_addr: None,
l2_chain_id: L2ChainId::default(),
l1_chain_id: L1ChainId(9),
max_pubdata_per_batch: 1 << 17,
l1_batch_commit_data_generator_mode: L1BatchCommitDataGeneratorMode::Rollup,
dummy_verifier: true,
}
}
}

#[derive(Debug, Deserialize, Clone, PartialEq)]
Expand Down Expand Up @@ -513,6 +536,12 @@ impl OptionalENConfig {
pub fn mempool_cache_update_interval(&self) -> Duration {
Duration::from_millis(self.mempool_cache_update_interval)
}

#[cfg(test)]
fn mock() -> Self {
// Set all values to their defaults
serde_json::from_str("{}").unwrap()
}
}

/// This part of the external node config is required for its operation.
Expand All @@ -537,6 +566,24 @@ pub(crate) struct RequiredENConfig {
}

impl RequiredENConfig {
#[cfg(test)]
fn mock(temp_dir: &tempfile::TempDir) -> Self {
Self {
http_port: 0,
ws_port: 0,
healthcheck_port: 0,
eth_client_url: "unused".to_owned(), // L1 and L2 clients must be instantiated before accessing mocks
main_node_url: "unused".to_owned(),
state_cache_path: temp_dir
.path()
.join("state_keeper_cache")
.to_str()
.unwrap()
.to_owned(),
merkle_tree_path: temp_dir.path().join("tree").to_str().unwrap().to_owned(),
}
}

pub fn main_node_url(&self) -> anyhow::Result<String> {
Self::get_url(&self.main_node_url).context("Could not parse main node URL")
}
Expand Down Expand Up @@ -572,6 +619,14 @@ impl PostgresConfig {
.context("Unable to parse DATABASE_POOL_SIZE env variable")?,
})
}

#[cfg(test)]
fn mock(test_pool: &ConnectionPool<Core>) -> Self {
Self {
database_url: test_pool.database_url().to_owned(),
max_connections: test_pool.max_size(),
}
}
}

pub(crate) fn read_consensus_secrets() -> anyhow::Result<Option<ConsensusSecrets>> {
Expand Down Expand Up @@ -658,8 +713,8 @@ impl ExternalNodeConfig {
.await
.context("Unable to check L1 chain ID through the configured L1 client")?;

let l2_chain_id: L2ChainId = env_var("EN_L2_CHAIN_ID");
let l1_chain_id: u64 = env_var("EN_L1_CHAIN_ID");
let l2_chain_id: L2ChainId = env_var("EN_L2_CHAIN_ID")?;
let l1_chain_id: u64 = env_var("EN_L1_CHAIN_ID")?;
if l2_chain_id != remote.l2_chain_id {
anyhow::bail!(
"Configured L2 chain id doesn't match the one from main node.
Expand Down Expand Up @@ -698,17 +753,30 @@ impl ExternalNodeConfig {
api_component: api_component_config,
})
}

#[cfg(test)]
pub(crate) fn mock(temp_dir: &tempfile::TempDir, test_pool: &ConnectionPool<Core>) -> Self {
Self {
required: RequiredENConfig::mock(temp_dir),
postgres: PostgresConfig::mock(test_pool),
optional: OptionalENConfig::mock(),
remote: RemoteENConfig::mock(),
consensus: None,
api_component: ApiComponentConfig { tree_api_url: None },
tree_component: TreeComponentConfig { api_port: None },
}
}
}

fn env_var<T>(name: &str) -> T
fn env_var<T>(name: &str) -> anyhow::Result<T>
where
T: std::str::FromStr,
T::Err: std::fmt::Debug,
T: FromStr,
T::Err: fmt::Display,
{
env::var(name)
.unwrap_or_else(|_| panic!("{} env variable is not set", name))
.with_context(|| format!("`{name}` env variable is not set"))?
.parse()
.unwrap_or_else(|_| panic!("unable to parse {} env variable", name))
.map_err(|err| anyhow::anyhow!("unable to parse `{name}` env variable: {err}"))
}

impl From<ExternalNodeConfig> for InternalApiConfig {
Expand Down
8 changes: 4 additions & 4 deletions core/bin/external_node/src/helpers.rs
@@ -1,14 +1,14 @@
//! Miscellaneous helpers for the EN.

use zksync_health_check::{async_trait, CheckHealth, Health, HealthStatus};
use zksync_web3_decl::{client::L2Client, namespaces::EthNamespaceClient};
use zksync_web3_decl::{client::BoxedL2Client, namespaces::EthNamespaceClient};

/// Main node health check.
#[derive(Debug)]
pub(crate) struct MainNodeHealthCheck(L2Client);
pub(crate) struct MainNodeHealthCheck(BoxedL2Client);

impl From<L2Client> for MainNodeHealthCheck {
fn from(client: L2Client) -> Self {
impl From<BoxedL2Client> for MainNodeHealthCheck {
fn from(client: BoxedL2Client) -> Self {
Self(client.for_component("main_node_health_check"))
}
}
Expand Down
4 changes: 2 additions & 2 deletions core/bin/external_node/src/init.rs
Expand Up @@ -7,7 +7,7 @@ use zksync_dal::{ConnectionPool, Core, CoreDal};
use zksync_health_check::AppHealthCheck;
use zksync_object_store::ObjectStoreFactory;
use zksync_snapshots_applier::SnapshotsApplierConfig;
use zksync_web3_decl::client::L2Client;
use zksync_web3_decl::client::BoxedL2Client;

use crate::config::read_snapshots_recovery_config;

Expand All @@ -21,7 +21,7 @@ enum InitDecision {

pub(crate) async fn ensure_storage_initialized(
pool: &ConnectionPool<Core>,
main_node_client: L2Client,
main_node_client: BoxedL2Client,
app_health: &AppHealthCheck,
l2_chain_id: L2ChainId,
consider_snapshot_recovery: bool,
Expand Down

0 comments on commit 9948187

Please sign in to comment.