Skip to content

Commit

Permalink
fix: anvil-zksync able to start even if the port is busy (#542)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
dutterbutter authored Jan 16, 2025
1 parent 0668a24 commit bb90831
Show file tree
Hide file tree
Showing 13 changed files with 117 additions and 33 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/checks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/e2e-docker.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/e2e-rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
8 changes: 4 additions & 4 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -121,15 +121,15 @@ 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

# ==============================
# Download artifacts
# ==============================
- name: Download artifacts
uses: actions/download-artifact@v3
uses: actions/download-artifact@v4

# ==============================
# Create release draft
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/spec.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
22 changes: 13 additions & 9 deletions crates/api_server/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ impl NodeServerBuilder {
rpc
}

pub async fn build(self, addr: SocketAddr) -> NodeServer {
pub async fn build(self, addr: SocketAddr) -> Result<NodeServer, String> {
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.
Expand All @@ -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)),
}
}
}
Expand Down
41 changes: 37 additions & 4 deletions crates/cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())));
Expand Down
10 changes: 5 additions & 5 deletions crates/core/src/node/inner/in_memory_inner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand Down Expand Up @@ -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");
}
Expand Down Expand Up @@ -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 {
Expand Down
15 changes: 15 additions & 0 deletions e2e-tests-rust/Cargo.lock

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

2 changes: 1 addition & 1 deletion e2e-tests-rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
30 changes: 29 additions & 1 deletion e2e-tests-rust/tests/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(())
}

0 comments on commit bb90831

Please sign in to comment.