From bb908317cd1c28ff23aebdd764c108f16a1fe370 Mon Sep 17 00:00:00 2001 From: Dustin Brickwood Date: Thu, 16 Jan 2025 11:55:59 -0600 Subject: [PATCH] fix: anvil-zksync able to start even if the port is busy (#542) * fix: closes #512 - start instance even if port is busy * chore: apply make lint:fix * chore: refactor test to make use of anvil-zksync instance * chore: remove no longer needed deps * chore: remove no longer needed deps * chore: bump actions/upload-artifact: v3 to v4 due to CI fail * chore: bump actions/upload-artifact / download: v3 to v4 due to CI fail --- .github/workflows/checks.yaml | 6 +-- .github/workflows/e2e-docker.yml | 2 +- .github/workflows/e2e-rust.yml | 4 +- .github/workflows/e2e.yml | 2 +- .github/workflows/release.yml | 8 ++-- .github/workflows/spec.yml | 4 +- Makefile | 4 ++ crates/api_server/src/server.rs | 22 ++++++---- crates/cli/src/main.rs | 41 +++++++++++++++++-- crates/core/src/node/inner/in_memory_inner.rs | 10 ++--- e2e-tests-rust/Cargo.lock | 15 +++++++ e2e-tests-rust/src/lib.rs | 2 +- e2e-tests-rust/tests/lib.rs | 30 +++++++++++++- 13 files changed, 117 insertions(+), 33 deletions(-) diff --git a/.github/workflows/checks.yaml b/.github/workflows/checks.yaml index 340b2d08..900a51df 100644 --- a/.github/workflows/checks.yaml +++ b/.github/workflows/checks.yaml @@ -15,7 +15,7 @@ jobs: name: lint runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Install Rust uses: actions-rust-lang/setup-rust-toolchain@v1 with: @@ -39,7 +39,7 @@ jobs: matrix: os: [ubuntu-latest, macos-latest] steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Install Rust uses: actions-rust-lang/setup-rust-toolchain@v1 @@ -55,7 +55,7 @@ jobs: tar -czf anvil-zksync-${{ matrix.os }}.tar.gz ./anvil-zksync* - name: Upload artifact - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: anvil-zksync-${{ matrix.os }}.tar.gz path: ./target/release/anvil-zksync-${{ matrix.os }}.tar.gz diff --git a/.github/workflows/e2e-docker.yml b/.github/workflows/e2e-docker.yml index 68b8a4b9..fc791833 100644 --- a/.github/workflows/e2e-docker.yml +++ b/.github/workflows/e2e-docker.yml @@ -14,7 +14,7 @@ jobs: steps: - name: Checkout code - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Set up Docker Buildx uses: docker/setup-buildx-action@v3 diff --git a/.github/workflows/e2e-rust.yml b/.github/workflows/e2e-rust.yml index 4a1576f9..69bfc55f 100644 --- a/.github/workflows/e2e-rust.yml +++ b/.github/workflows/e2e-rust.yml @@ -11,12 +11,12 @@ jobs: os: [ ubuntu-latest, macos-latest ] name: e2e-rust steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: fetch-depth: 0 - name: Download artifacts - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 with: name: anvil-zksync-${{ matrix.os }}.tar.gz diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index ef45f8a7..b92d326b 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -22,7 +22,7 @@ jobs: cache-dependency-path: 'e2e-tests/yarn.lock' - name: Download artifacts - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 with: name: anvil-zksync-${{ matrix.os }}.tar.gz diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 3f74d1e6..553eef17 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -53,7 +53,7 @@ jobs: needs: [extract-version] steps: - name: Checkout sources - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Install Rust uses: actions-rust-lang/setup-rust-toolchain@v1 @@ -99,7 +99,7 @@ jobs: # This is required to share artifacts between different jobs # ======================================================================= - name: Upload artifact - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: anvil-zksync-${{ needs.extract-version.outputs.VERSION }}-${{ matrix.arch }}.tar.gz path: anvil-zksync-${{ needs.extract-version.outputs.VERSION }}-${{ matrix.arch }}.tar.gz @@ -121,7 +121,7 @@ jobs: steps: # This is necessary for generating the changelog. It has to come before "Download Artifacts" or else it deletes the artifacts. - name: Checkout sources - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: fetch-depth: 0 @@ -129,7 +129,7 @@ jobs: # Download artifacts # ============================== - name: Download artifacts - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 # ============================== # Create release draft diff --git a/.github/workflows/spec.yml b/.github/workflows/spec.yml index 87a50ac0..3d8118a2 100644 --- a/.github/workflows/spec.yml +++ b/.github/workflows/spec.yml @@ -7,12 +7,12 @@ jobs: runs-on: ubuntu-latest name: spec steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: "recursive" - name: Download artifacts - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 with: name: anvil-zksync-ubuntu-latest.tar.gz diff --git a/Makefile b/Makefile index 630f32d9..c445e687 100644 --- a/Makefile +++ b/Makefile @@ -55,6 +55,10 @@ lint-fix: cd e2e-tests && yarn && yarn lint:fix && yarn fmt:fix cargo clippy --fix cargo fmt + cd e2e-tests-rust && cargo fmt --all + cd e2e-tests-rust && cargo clippy --fix + cd spec-tests && cargo fmt --all + cd spec-tests && cargo clippy --fix # Run unit tests for Rust code test: diff --git a/crates/api_server/src/server.rs b/crates/api_server/src/server.rs index 39488a5d..fa7cd21a 100644 --- a/crates/api_server/src/server.rs +++ b/crates/api_server/src/server.rs @@ -62,7 +62,7 @@ impl NodeServerBuilder { rpc } - pub async fn build(self, addr: SocketAddr) -> NodeServer { + pub async fn build(self, addr: SocketAddr) -> Result { let cors_layers = tower::util::option_layer(self.cors_enabled.then(|| { // `CorsLayer` adds CORS-specific headers to responses but does not do filtering by itself. // CORS relies on browsers respecting server's access list response headers. @@ -86,14 +86,18 @@ impl NodeServerBuilder { ) .set_rpc_middleware(RpcServiceBuilder::new().rpc_logger(100)); - let server = server_builder.build(addr).await.unwrap(); - let local_addr = server.local_addr().unwrap(); - let rpc = Self::default_rpc(self.node); - // `jsonrpsee` does `tokio::spawn` within `start` method, so we cannot invoke it here, as this method - // should only build the server. This way we delay the launch until the `NodeServer::run` is invoked. - NodeServer { - local_addr, - run_fn: Box::new(move || server.start(rpc)), + match server_builder.build(addr).await { + Ok(server) => { + let local_addr = server.local_addr().unwrap(); + let rpc = Self::default_rpc(self.node); + // `jsonrpsee` does `tokio::spawn` within `start` method, so we cannot invoke it here, as this method + // should only build the server. This way we delay the launch until the `NodeServer::run` is invoked. + Ok(NodeServer { + local_addr, + run_fn: Box::new(move || server.start(rpc)), + }) + } + Err(e) => Err(format!("Failed to bind to address {}: {}", addr, e)), } } } diff --git a/crates/cli/src/main.rs b/crates/cli/src/main.rs index 40b87a13..cb1d7f80 100644 --- a/crates/cli/src/main.rs +++ b/crates/cli/src/main.rs @@ -317,10 +317,43 @@ async fn main() -> anyhow::Result<()> { } let mut server_handles = Vec::with_capacity(config.host.len()); for host in &config.host { - let addr = SocketAddr::new(*host, config.port); - let server = server_builder.clone().build(addr).await; - config.port = server.local_addr().port(); - server_handles.push(server.run()); + let mut addr = SocketAddr::new(*host, config.port); + + match server_builder.clone().build(addr).await { + Ok(server) => { + config.port = server.local_addr().port(); + server_handles.push(server.run()); + } + Err(err) => { + tracing::info!( + "Failed to bind to address {}:{}: {}. Retrying with a different port...", + host, + config.port, + err + ); + + // Attempt to bind to a dynamic port + addr.set_port(0); + match server_builder.clone().build(addr).await { + Ok(server) => { + config.port = server.local_addr().port(); + tracing::info!( + "Successfully started server on port {} for host {}", + config.port, + host + ); + server_handles.push(server.run()); + } + Err(err) => { + return Err(anyhow!( + "Failed to start server on host {} with port: {}", + host, + err + )); + } + } + } + } } let any_server_stopped = futures::future::select_all(server_handles.into_iter().map(|h| Box::pin(h.stopped()))); diff --git a/crates/core/src/node/inner/in_memory_inner.rs b/crates/core/src/node/inner/in_memory_inner.rs index d6a87acf..7b0d0e11 100644 --- a/crates/core/src/node/inner/in_memory_inner.rs +++ b/crates/core/src/node/inner/in_memory_inner.rs @@ -1628,7 +1628,7 @@ mod tests { let system_contracts = node .system_contracts .system_contracts_for_initiator(&node.impersonation, &tx.initiator_account()); - let (block_ctx, batch_env, mut vm) = test_vm(&mut *node, system_contracts).await; + let (block_ctx, batch_env, mut vm) = test_vm(&mut node, system_contracts).await; let err = node .run_l2_tx(tx, 0, &block_ctx, &batch_env, &mut vm) .unwrap_err(); @@ -1647,7 +1647,7 @@ mod tests { let system_contracts = node .system_contracts .system_contracts_for_initiator(&node.impersonation, &tx.initiator_account()); - let (block_ctx, batch_env, mut vm) = test_vm(&mut *node, system_contracts).await; + let (block_ctx, batch_env, mut vm) = test_vm(&mut node, system_contracts).await; let err = node .run_l2_tx(tx, 0, &block_ctx, &batch_env, &mut vm) .unwrap_err(); @@ -1670,7 +1670,7 @@ mod tests { let system_contracts = node .system_contracts .system_contracts_for_initiator(&node.impersonation, &tx.initiator_account()); - let (block_ctx, batch_env, mut vm) = test_vm(&mut *node, system_contracts).await; + let (block_ctx, batch_env, mut vm) = test_vm(&mut node, system_contracts).await; let err = node .run_l2_tx(tx, 0, &block_ctx, &batch_env, &mut vm) .unwrap_err(); @@ -1740,7 +1740,7 @@ mod tests { let system_contracts = node .system_contracts .system_contracts_for_initiator(&node.impersonation, &tx.initiator_account()); - let (_, _, mut vm) = test_vm(&mut *node, system_contracts).await; + let (_, _, mut vm) = test_vm(&mut node, system_contracts).await; node.run_l2_tx_raw(tx, &mut vm) .expect("transaction must pass with external storage"); } @@ -1790,7 +1790,7 @@ mod tests { let system_contracts = node .system_contracts .system_contracts_for_initiator(&node.impersonation, &tx.initiator_account()); - let (_, _, mut vm) = test_vm(&mut *node, system_contracts).await; + let (_, _, mut vm) = test_vm(&mut node, system_contracts).await; let TxExecutionOutput { result, .. } = node.run_l2_tx_raw(tx, &mut vm).expect("failed tx"); match result.result { diff --git a/e2e-tests-rust/Cargo.lock b/e2e-tests-rust/Cargo.lock index d704cb5c..20e2dc5f 100644 --- a/e2e-tests-rust/Cargo.lock +++ b/e2e-tests-rust/Cargo.lock @@ -956,6 +956,7 @@ dependencies = [ "tempdir", "tokio", "tower 0.5.1", + "tower-http", ] [[package]] @@ -6285,6 +6286,20 @@ dependencies = [ "tower-service", ] +[[package]] +name = "tower-http" +version = "0.6.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "403fa3b783d4b626a8ad51d766ab03cb6d2dbfc46b1c5d4448395e6628dc9697" +dependencies = [ + "bitflags 2.6.0", + "bytes", + "http 1.1.0", + "pin-project-lite", + "tower-layer", + "tower-service", +] + [[package]] name = "tower-layer" version = "0.3.3" diff --git a/e2e-tests-rust/src/lib.rs b/e2e-tests-rust/src/lib.rs index f0423224..596ff525 100644 --- a/e2e-tests-rust/src/lib.rs +++ b/e2e-tests-rust/src/lib.rs @@ -10,4 +10,4 @@ pub use provider::{ init_testing_provider, init_testing_provider_with_client, AnvilZKsyncApi, TestingProvider, DEFAULT_TX_VALUE, }; -pub use utils::get_node_binary_path; +pub use utils::{get_node_binary_path, LockedPort}; diff --git a/e2e-tests-rust/tests/lib.rs b/e2e-tests-rust/tests/lib.rs index 23fc97f5..9ae3d38f 100644 --- a/e2e-tests-rust/tests/lib.rs +++ b/e2e-tests-rust/tests/lib.rs @@ -4,11 +4,12 @@ use alloy::providers::Provider; use alloy::{ network::primitives::BlockTransactionsKind, primitives::U256, signers::local::PrivateKeySigner, }; +use alloy_zksync::node_bindings::AnvilZKsync; use anvil_zksync_core::node::VersionedState; use anvil_zksync_core::utils::write_json_file; use anvil_zksync_e2e_tests::{ get_node_binary_path, init_testing_provider, init_testing_provider_with_client, AnvilZKsyncApi, - ReceiptExt, ZksyncWalletProviderExt, DEFAULT_TX_VALUE, + LockedPort, ReceiptExt, ZksyncWalletProviderExt, DEFAULT_TX_VALUE, }; use anyhow::Context; use flate2::read::GzDecoder; @@ -778,3 +779,30 @@ async fn load_state_on_fork() -> anyhow::Result<()> { Ok(()) } + +#[tokio::test] +async fn test_server_port_fallback() -> anyhow::Result<()> { + let locked_port = LockedPort::acquire_unused().await?; + + let node1 = AnvilZKsync::new() + .path(get_node_binary_path()) + .port(locked_port.port) + .spawn(); + let port1 = node1.port(); + + let node2 = AnvilZKsync::new() + .path(get_node_binary_path()) + .port(locked_port.port) + .spawn(); + let port2 = node2.port(); + + assert_ne!( + port1, port2, + "The second instance should have a different port due to fallback" + ); + + drop(node1); + drop(node2); + + Ok(()) +}