Skip to content

Commit

Permalink
chore: adds fmt and clippy checks on ci and fixes (#506)
Browse files Browse the repository at this point in the history
* chore: adds fmt and clippy checks on ci and fixes

* chore: clean up and cargo fmt
  • Loading branch information
dutterbutter authored Dec 19, 2024
1 parent 1f0a4b6 commit 3c5529f
Show file tree
Hide file tree
Showing 8 changed files with 83 additions and 40 deletions.
4 changes: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
9 changes: 6 additions & 3 deletions e2e-tests-rust/src/ext.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -80,7 +80,10 @@ pub trait ZksyncWalletProviderExt: WalletProvider<Zksync, Wallet = ZksyncWallet>
}

/// Registers provider signer.
fn register_signer<T: TxSigner<PrimitiveSignature> + Send + Sync + 'static>(&mut self, signer: T) {
fn register_signer<T: TxSigner<PrimitiveSignature> + Send + Sync + 'static>(
&mut self,
signer: T,
) {
self.wallet_mut().register_signer(signer);
}
}
Expand Down
5 changes: 4 additions & 1 deletion e2e-tests-rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
1 change: 1 addition & 0 deletions e2e-tests-rust/src/provider/anvil_zksync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T>: Provider<T, Zksync>
where
T: Transport + Clone,
Expand Down
4 changes: 2 additions & 2 deletions e2e-tests-rust/src/provider/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)?;

Expand Down
94 changes: 63 additions & 31 deletions e2e-tests-rust/tests/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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]
Expand Down Expand Up @@ -75,19 +73,19 @@ 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?;
assert!(receipt.status());

// 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
Expand All @@ -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
Expand Down Expand Up @@ -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());
Expand All @@ -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());
Expand All @@ -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")
Expand All @@ -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!(
Expand All @@ -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);
}
Expand All @@ -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")
Expand All @@ -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!(
Expand All @@ -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);
}
Expand Down
4 changes: 2 additions & 2 deletions spec-tests/src/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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",
)?)
Expand Down
2 changes: 1 addition & 1 deletion spec-tests/tests/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ fn resolve_method_spec(method_name: &str) -> anyhow::Result<Method> {
None
}
})
.expect(&format!("method '{method_name}' not found"));
.unwrap_or_else(|| panic!("method '{method_name}' not found"));
Ok(method)
}

Expand Down

0 comments on commit 3c5529f

Please sign in to comment.