From 3c5529f55ceb25fe758b00f5e02187c31a91bf1d Mon Sep 17 00:00:00 2001 From: Dustin Brickwood Date: Wed, 18 Dec 2024 21:58:37 -0600 Subject: [PATCH] chore: adds fmt and clippy checks on ci and fixes (#506) * chore: adds fmt and clippy checks on ci and fixes * chore: clean up and cargo fmt --- Makefile | 4 + e2e-tests-rust/src/ext.rs | 9 +- e2e-tests-rust/src/lib.rs | 5 +- e2e-tests-rust/src/provider/anvil_zksync.rs | 1 + e2e-tests-rust/src/provider/testing.rs | 4 +- e2e-tests-rust/tests/lib.rs | 94 ++++++++++++++------- spec-tests/src/process.rs | 4 +- spec-tests/tests/lib.rs | 2 +- 8 files changed, 83 insertions(+), 40 deletions(-) diff --git a/Makefile b/Makefile index e82692f2..c6b3e0b9 100644 --- a/Makefile +++ b/Makefile @@ -39,8 +39,12 @@ rust-doc: # Lint checks for Rust code lint: cd e2e-tests && yarn && yarn lint && yarn fmt && yarn typecheck + cd e2e-tests-rust && cargo fmt --all -- --check + cd spec-tests && cargo fmt --all -- --check cargo fmt --all -- --check cargo clippy --tests -p anvil-zksync -Zunstable-options -- -D warnings --allow clippy::unwrap_used + cd e2e-tests-rust && cargo clippy --tests -Zunstable-options -- -D warnings --allow clippy::unwrap_used + cd spec-tests && cargo clippy --tests -Zunstable-options -- -D warnings --allow clippy::unwrap_used # Fix lint errors for Rust code lint-fix: diff --git a/e2e-tests-rust/src/ext.rs b/e2e-tests-rust/src/ext.rs index 229fcc3e..2b56f16c 100644 --- a/e2e-tests-rust/src/ext.rs +++ b/e2e-tests-rust/src/ext.rs @@ -1,7 +1,7 @@ //! Module containing extensions of existing alloy abstractions. -use alloy::network::{ReceiptResponse,TxSigner}; -use alloy::primitives::{Address,BlockHash,PrimitiveSignature}; +use alloy::network::{ReceiptResponse, TxSigner}; +use alloy::primitives::{Address, BlockHash, PrimitiveSignature}; use alloy::providers::WalletProvider; use alloy::signers::local::PrivateKeySigner; use alloy_zksync::network::Zksync; @@ -80,7 +80,10 @@ pub trait ZksyncWalletProviderExt: WalletProvider } /// Registers provider signer. - fn register_signer + Send + Sync + 'static>(&mut self, signer: T) { + fn register_signer + Send + Sync + 'static>( + &mut self, + signer: T, + ) { self.wallet_mut().register_signer(signer); } } diff --git a/e2e-tests-rust/src/lib.rs b/e2e-tests-rust/src/lib.rs index 505f80de..f0423224 100644 --- a/e2e-tests-rust/src/lib.rs +++ b/e2e-tests-rust/src/lib.rs @@ -6,5 +6,8 @@ mod provider; mod utils; pub use ext::{ReceiptExt, ZksyncWalletProviderExt}; -pub use provider::{init_testing_provider, init_testing_provider_with_client, AnvilZKsyncApi, TestingProvider, DEFAULT_TX_VALUE}; +pub use provider::{ + init_testing_provider, init_testing_provider_with_client, AnvilZKsyncApi, TestingProvider, + DEFAULT_TX_VALUE, +}; pub use utils::get_node_binary_path; diff --git a/e2e-tests-rust/src/provider/anvil_zksync.rs b/e2e-tests-rust/src/provider/anvil_zksync.rs index b38e9836..75261e12 100644 --- a/e2e-tests-rust/src/provider/anvil_zksync.rs +++ b/e2e-tests-rust/src/provider/anvil_zksync.rs @@ -6,6 +6,7 @@ use alloy::transports::Transport; use alloy_zksync::network::Zksync; /// RPC interface that gives access to methods specific to anvil-zksync. +#[allow(clippy::type_complexity)] pub trait AnvilZKsyncApi: Provider where T: Transport + Clone, diff --git a/e2e-tests-rust/src/provider/testing.rs b/e2e-tests-rust/src/provider/testing.rs index 12328e1d..deed4936 100644 --- a/e2e-tests-rust/src/provider/testing.rs +++ b/e2e-tests-rust/src/provider/testing.rs @@ -20,7 +20,7 @@ use alloy_zksync::network::receipt_response::ReceiptResponse; use alloy_zksync::network::transaction_response::TransactionResponse; use alloy_zksync::network::Zksync; use alloy_zksync::node_bindings::{AnvilZKsync, AnvilZKsyncError::NoKeysAvailable}; -use alloy_zksync::provider::{zksync_provider, layers::anvil_zksync::AnvilZKsyncLayer}; +use alloy_zksync::provider::{layers::anvil_zksync::AnvilZKsyncLayer, zksync_provider}; use alloy_zksync::wallet::ZksyncWallet; use anyhow::Context as _; use async_trait::async_trait; @@ -105,7 +105,7 @@ pub async fn init_testing_provider_with_client( let http = HttpWithMiddleware::with_client(client, url.clone()); let rpc_client = RpcClient::new(http, true); - let rich_accounts = node_layer.instance().addresses().iter().cloned().collect(); + let rich_accounts = node_layer.instance().addresses().to_vec(); let default_keys = node_layer.instance().keys().to_vec(); let (default_key, remaining_keys) = default_keys.split_first().ok_or(NoKeysAvailable)?; diff --git a/e2e-tests-rust/tests/lib.rs b/e2e-tests-rust/tests/lib.rs index c0d78b72..9ecca27c 100644 --- a/e2e-tests-rust/tests/lib.rs +++ b/e2e-tests-rust/tests/lib.rs @@ -2,13 +2,11 @@ use alloy::network::ReceiptResponse; use alloy::providers::ext::AnvilApi; use alloy::providers::Provider; use alloy::{ - network::primitives::BlockTransactionsKind, - primitives::U256, - signers::local::PrivateKeySigner, + network::primitives::BlockTransactionsKind, primitives::U256, signers::local::PrivateKeySigner, }; use anvil_zksync_e2e_tests::{ - init_testing_provider, init_testing_provider_with_client, AnvilZKsyncApi, ReceiptExt, - ZksyncWalletProviderExt, DEFAULT_TX_VALUE, get_node_binary_path + get_node_binary_path, init_testing_provider, init_testing_provider_with_client, AnvilZKsyncApi, + ReceiptExt, ZksyncWalletProviderExt, DEFAULT_TX_VALUE, }; use http::header::{ HeaderMap, HeaderValue, ACCESS_CONTROL_ALLOW_HEADERS, ACCESS_CONTROL_ALLOW_METHODS, @@ -20,7 +18,7 @@ const OTHER_ORIGIN: HeaderValue = HeaderValue::from_static("http://other.origin" const ANY_ORIGIN: HeaderValue = HeaderValue::from_static("*"); use anvil_zksync_core::node::VersionedState; -use std::{ fs, convert::identity, thread::sleep, time::Duration }; +use std::{convert::identity, fs, thread::sleep, time::Duration}; use tempdir::TempDir; #[tokio::test] @@ -75,11 +73,11 @@ async fn no_sealing_timeout() -> anyhow::Result<()> { async fn dynamic_sealing_mode() -> anyhow::Result<()> { // Test that we can successfully switch between different sealing modes let provider = init_testing_provider(|node| node.no_mine()).await?; - assert_eq!(provider.anvil_get_auto_mine().await?, false); + assert!(!(provider.anvil_get_auto_mine().await?)); // Enable immediate block sealing provider.anvil_set_auto_mine(true).await?; - assert_eq!(provider.anvil_get_auto_mine().await?, true); + assert!(provider.anvil_get_auto_mine().await?); // Check that we can finalize transactions now let receipt = provider.tx().finalize().await?; @@ -87,7 +85,7 @@ async fn dynamic_sealing_mode() -> anyhow::Result<()> { // Enable interval block sealing provider.anvil_set_interval_mining(3).await?; - assert_eq!(provider.anvil_get_auto_mine().await?, false); + assert!(!(provider.anvil_get_auto_mine().await?)); // Check that we can finalize two txs in the same block now provider @@ -98,7 +96,7 @@ async fn dynamic_sealing_mode() -> anyhow::Result<()> { // Disable block sealing entirely provider.anvil_set_auto_mine(false).await?; - assert_eq!(provider.anvil_get_auto_mine().await?, false); + assert!(!(provider.anvil_get_auto_mine().await?)); // Check that transactions do not get finalized now provider @@ -467,13 +465,31 @@ async fn cli_allow_origin() -> anyhow::Result<()> { async fn pool_txs_order_fifo() -> anyhow::Result<()> { let provider_fifo = init_testing_provider(|node| node.no_mine()).await?; - let pending_tx0 = provider_fifo.tx().with_rich_from(0).with_max_fee_per_gas(50_000_000).register().await?; - let pending_tx1 = provider_fifo.tx().with_rich_from(1).with_max_fee_per_gas(100_000_000).register().await?; - let pending_tx2 = provider_fifo.tx().with_rich_from(2).with_max_fee_per_gas(150_000_000).register().await?; + let pending_tx0 = provider_fifo + .tx() + .with_rich_from(0) + .with_max_fee_per_gas(50_000_000) + .register() + .await?; + let pending_tx1 = provider_fifo + .tx() + .with_rich_from(1) + .with_max_fee_per_gas(100_000_000) + .register() + .await?; + let pending_tx2 = provider_fifo + .tx() + .with_rich_from(2) + .with_max_fee_per_gas(150_000_000) + .register() + .await?; provider_fifo.anvil_mine(Some(U256::from(1)), None).await?; - let block = provider_fifo.get_block(1.into(), BlockTransactionsKind::Hashes).await?.unwrap(); + let block = provider_fifo + .get_block(1.into(), BlockTransactionsKind::Hashes) + .await? + .unwrap(); let tx_hashes = block.transactions.as_hashes().unwrap(); assert_eq!(&tx_hashes[0], pending_tx0.tx_hash()); assert_eq!(&tx_hashes[1], pending_tx1.tx_hash()); @@ -485,13 +501,31 @@ async fn pool_txs_order_fifo() -> anyhow::Result<()> { async fn pool_txs_order_fees() -> anyhow::Result<()> { let provider_fees = init_testing_provider(|node| node.no_mine().arg("--order=fees")).await?; - let pending_tx0 = provider_fees.tx().with_rich_from(0).with_max_fee_per_gas(50_000_000).register().await?; - let pending_tx1 = provider_fees.tx().with_rich_from(1).with_max_fee_per_gas(100_000_000).register().await?; - let pending_tx2 = provider_fees.tx().with_rich_from(2).with_max_fee_per_gas(150_000_000).register().await?; + let pending_tx0 = provider_fees + .tx() + .with_rich_from(0) + .with_max_fee_per_gas(50_000_000) + .register() + .await?; + let pending_tx1 = provider_fees + .tx() + .with_rich_from(1) + .with_max_fee_per_gas(100_000_000) + .register() + .await?; + let pending_tx2 = provider_fees + .tx() + .with_rich_from(2) + .with_max_fee_per_gas(150_000_000) + .register() + .await?; provider_fees.anvil_mine(Some(U256::from(1)), None).await?; - let block = provider_fees.get_block(1.into(), BlockTransactionsKind::Hashes).await?.unwrap(); + let block = provider_fees + .get_block(1.into(), BlockTransactionsKind::Hashes) + .await? + .unwrap(); let tx_hashes = block.transactions.as_hashes().unwrap(); assert_eq!(&tx_hashes[0], pending_tx2.tx_hash()); assert_eq!(&tx_hashes[1], pending_tx1.tx_hash()); @@ -516,14 +550,13 @@ async fn transactions_have_index() -> anyhow::Result<()> { } #[tokio::test] -async fn dump_state_on_run() -> anyhow::Result<()> { +async fn dump_state_on_run() -> anyhow::Result<()> { let temp_dir = TempDir::new("state-test").expect("failed creating temporary dir"); let dump_path = temp_dir.path().join("state_dump.json"); let dump_path_clone = dump_path.clone(); - let provider = init_testing_provider(move |node| { - node - .path(get_node_binary_path()) + let provider = init_testing_provider(move |node| { + node.path(get_node_binary_path()) .arg("--state-interval") .arg("1") .arg("--dump-state") @@ -543,11 +576,11 @@ async fn dump_state_on_run() -> anyhow::Result<()> { "State dump file should exist at {:?}", dump_path ); - + let dumped_data = fs::read_to_string(&dump_path)?; let state: VersionedState = serde_json::from_str(&dumped_data) .map_err(|e| anyhow::anyhow!("Failed to deserialize state: {}", e))?; - + match state { VersionedState::V1 { version: _, state } => { assert!( @@ -558,7 +591,7 @@ async fn dump_state_on_run() -> anyhow::Result<()> { !state.transactions.is_empty(), "state_dump.json should contain at least one transaction" ); - }, + } VersionedState::Unknown { version } => { panic!("Encountered unknown state version: {}", version); } @@ -568,14 +601,13 @@ async fn dump_state_on_run() -> anyhow::Result<()> { } #[tokio::test] -async fn dump_state_on_fork() -> anyhow::Result<()> { +async fn dump_state_on_fork() -> anyhow::Result<()> { let temp_dir = TempDir::new("state-fork-test").expect("failed creating temporary dir"); let dump_path = temp_dir.path().join("state_dump_fork.json"); let dump_path_clone = dump_path.clone(); let provider = init_testing_provider(move |node| { - node - .path(get_node_binary_path()) + node.path(get_node_binary_path()) .arg("--state-interval") .arg("1") .arg("--dump-state") @@ -596,11 +628,11 @@ async fn dump_state_on_fork() -> anyhow::Result<()> { "State dump file should exist at {:?}", dump_path ); - + let dumped_data = fs::read_to_string(&dump_path)?; let state: VersionedState = serde_json::from_str(&dumped_data) .map_err(|e| anyhow::anyhow!("Failed to deserialize state: {}", e))?; - + match state { VersionedState::V1 { version: _, state } => { assert!( @@ -611,7 +643,7 @@ async fn dump_state_on_fork() -> anyhow::Result<()> { !state.transactions.is_empty(), "state_dump_fork.json should contain at least one transaction" ); - }, + } VersionedState::Unknown { version } => { panic!("Encountered unknown state version: {}", version); } diff --git a/spec-tests/src/process.rs b/spec-tests/src/process.rs index 6a9c1641..2dc4653d 100644 --- a/spec-tests/src/process.rs +++ b/spec-tests/src/process.rs @@ -128,7 +128,7 @@ impl AnvilZKsyncRunner { let path = match self.path { Some(path) => path, None => { - if let Some(path) = std::env::var("ANVIL_ZKSYNC_BINARY_PATH").ok() { + if let Ok(path) = std::env::var("ANVIL_ZKSYNC_BINARY_PATH") { path } else { // Default to the binary taken from the target directory @@ -140,7 +140,7 @@ impl AnvilZKsyncRunner { let rpc_port_lock = match self.rpc_port { Some(rpc_port) => LockedPort::acquire(rpc_port).await?, None => { - if let Some(rpc_port) = std::env::var("ANVIL_ZKSYNC_RPC_PORT").ok() { + if let Ok(rpc_port) = std::env::var("ANVIL_ZKSYNC_RPC_PORT") { LockedPort::acquire(rpc_port.parse().context( "failed to parse `ANVIL_ZKSYNC_RPC_PORT` var as a valid port number", )?) diff --git a/spec-tests/tests/lib.rs b/spec-tests/tests/lib.rs index c133426b..e25c068b 100644 --- a/spec-tests/tests/lib.rs +++ b/spec-tests/tests/lib.rs @@ -34,7 +34,7 @@ fn resolve_method_spec(method_name: &str) -> anyhow::Result { None } }) - .expect(&format!("method '{method_name}' not found")); + .unwrap_or_else(|| panic!("method '{method_name}' not found")); Ok(method) }