feat(spec-tests): enforce spec-tests in CI (#400)

.github/workflows/checks.yaml

@@ -62,4 +62,8 @@ jobs:
   e2e:
     needs: build
     uses: ./.github/workflows/e2e.yml
-    name: e2e-tests
+    name: e2e-tests
+  spec:
+    needs: build
+    uses: ./.github/workflows/spec.yml
+    name: spec-tests

.github/workflows/spec.yml

@@ -0,0 +1,36 @@
+name: Testing era_test_node against ETH spec
+on:
+  workflow_call:
+
+jobs:
+  spec:
+    runs-on: ubuntu-latest
+    name: spec
+    steps:
+      - uses: actions/checkout@v3
+        with:
+          submodules: "recursive"
+
+      - name: Download artifacts
+        uses: actions/download-artifact@v3
+        with:
+          name: era_test_node-ubuntu-latest.tar.gz
+
+      - name: Extract era_test_node
+        id: extract_node
+        run: |
+          echo "Extracting era_test_node binary"
+          tar -xzf era_test_node-ubuntu-latest.tar.gz
+          chmod +x era_test_node
+
+      - name: Build ETH spec
+        id: build_eth_spec
+        working-directory: ./execution-apis
+        run: npm install && npm run build
+
+      - name: Launch tests
+        id: launch
+        working-directory: ./spec-tests
+        run: cargo test
+        env:
+          ERA_TEST_NODE_BINARY_PATH: ../era_test_node

spec-tests/src/api.rs

@@ -63,7 +63,7 @@ impl EraApi {
     pub async fn transfer_eth_legacy(&self, value: U256) -> anyhow::Result<()> {
         // TODO: Make signer configurable, leave taking a random rich wallet as the default option
         let signer = PrivateKeySigner::from_str(
-            "0x3d3cbc973389cb26f657686445bcc75662b415b656078503592ac8c1abb8810e",
+            "0x7726827caac94a7f9e1b160f7ea819f172f7b6f9d2a97f992c38edeab82d4110",
         )?;
         let wallet = EthereumWallet::from(signer);
         let provider = ProviderBuilder::new()
@@ -74,7 +74,7 @@ impl EraApi {
         // TODO: Make `to` configurable, leave taking a random wallet as the default option
         let tx = TransactionRequest::default()
             .to(Address::from_str(
-                "0x55bE1B079b53962746B2e86d12f158a41DF294A6",
+                "0x36615Cf349d7F6344891B1e7CA7C72883F5dc049",
             )?)
             .value(value.to_string().parse()?)
             .with_gas_price(100000000000);

spec-tests/src/process.rs

@@ -2,12 +2,11 @@ use std::{ffi::OsStr, fmt::Display, path::Path, time::Duration};
 
 use anyhow::Context;
 use chrono::{DateTime, Local};
-use fs2::FileExt;
 use tokio::process::{Child, Command};
 
-use crate::utils;
+use crate::utils::LockedPort;
 
-const ERA_TEST_NODE_BINARY_PATH: &str = "../target/release/era_test_node";
+const ERA_TEST_NODE_BINARY_DEFAULT_PATH: &str = "../target/release/era_test_node";
 const ERA_TEST_NODE_SRC_PATH: &str = "../src";
 
 pub struct EraRunConfig {
@@ -51,19 +50,19 @@ pub fn run<S: AsRef<OsStr> + Clone + Display>(
 
 /// Ensures that the era-test-node binary was built after the last source file got modified.
 fn ensure_binary_is_fresh() -> anyhow::Result<()> {
-    if !Path::new(ERA_TEST_NODE_BINARY_PATH).exists() {
+    if !Path::new(ERA_TEST_NODE_BINARY_DEFAULT_PATH).exists() {
         anyhow::bail!(
             "Expected era-test-node binary to be built and present at '{}'. Please run `make all` in the root directory.",
-            ERA_TEST_NODE_BINARY_PATH
+            ERA_TEST_NODE_BINARY_DEFAULT_PATH
         );
     }
-    let metadata = std::fs::metadata(ERA_TEST_NODE_BINARY_PATH)?;
+    let metadata = std::fs::metadata(ERA_TEST_NODE_BINARY_DEFAULT_PATH)?;
     match metadata.modified() {
         Ok(binary_mod_time) => {
             let binary_mod_time = DateTime::<Local>::from(binary_mod_time);
             tracing::info!(
                 %binary_mod_time,
-                path = ERA_TEST_NODE_BINARY_PATH,
+                path = ERA_TEST_NODE_BINARY_DEFAULT_PATH,
                 "Resolved when binary file was last modified"
             );
             let source_mod_time = std::fs::read_dir(ERA_TEST_NODE_SRC_PATH)
@@ -99,7 +98,7 @@ fn ensure_binary_is_fresh() -> anyhow::Result<()> {
         Err(error) => {
             tracing::warn!(
                 %error,
-                path = ERA_TEST_NODE_BINARY_PATH,
+                path = ERA_TEST_NODE_BINARY_DEFAULT_PATH,
                 "Could not get modification time from file (your platform might not support it, refer to the attached error). \
                 Make sure that your binary has been built against the code you are working with."
             );
@@ -108,19 +107,58 @@ fn ensure_binary_is_fresh() -> anyhow::Result<()> {
     Ok(())
 }
 
-pub async fn run_default() -> anyhow::Result<EraRunHandle> {
-    ensure_binary_is_fresh()?;
-    let (rpc_port, rpc_port_lock) = utils::acquire_unused_port().await?;
-    let config = EraRunConfig { rpc_port };
+#[derive(Default)]
+pub struct EraTestNodeRunner {
+    path: Option<String>,
+    rpc_port: Option<u16>,
+}
+
+impl EraTestNodeRunner {
+    pub fn path(mut self, path: String) -> Self {
+        self.path = Some(path);
+        self
+    }
+
+    pub fn rpc_port(mut self, rpc_port: u16) -> Self {
+        self.rpc_port = Some(rpc_port);
+        self
+    }
 
-    let handle = run(ERA_TEST_NODE_BINARY_PATH, config)?;
+    pub async fn run(self) -> anyhow::Result<EraRunHandle> {
+        let path = match self.path {
+            Some(path) => path,
+            None => {
+                if let Some(path) = std::env::var("ERA_TEST_NODE_BINARY_PATH").ok() {
+                    path
+                } else {
+                    // Default to the binary taken from the target directory
+                    ensure_binary_is_fresh()?;
+                    ERA_TEST_NODE_BINARY_DEFAULT_PATH.to_string()
+                }
+            }
+        };
+        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("ERA_TEST_NODE_RPC_PORT").ok() {
+                    LockedPort::acquire(rpc_port.parse().context(
+                        "failed to parse `ERA_TEST_NODE_RPC_PORT` var as a valid port number",
+                    )?)
+                    .await?
+                } else {
+                    LockedPort::acquire_unused().await?
+                }
+            }
+        };
 
-    // TODO: Wait for era-test-node healthcheck instead
-    tokio::time::sleep(Duration::from_secs(1)).await;
+        let config = EraRunConfig {
+            rpc_port: rpc_port_lock.port,
+        };
+        let handle = run(path, config)?;
 
-    rpc_port_lock
-        .unlock()
-        .with_context(|| format!("failed to unlock lockfile for rpc_port={}", rpc_port))?;
+        // TODO: Wait for era-test-node healthcheck instead
+        tokio::time::sleep(Duration::from_secs(1)).await;
 
-    Ok(handle)
+        Ok(handle)
+    }
 }

spec-tests/src/utils.rs

@@ -1,36 +1,72 @@
+use anyhow::Context;
+use fs2::FileExt;
 use std::{
     fs::File,
     net::{Ipv4Addr, SocketAddrV4},
 };
-
-use anyhow::Context;
-use fs2::FileExt;
 use tokio::net::TcpListener;
 
-/// Request an unused port from the OS.
-async fn pick_unused_port() -> anyhow::Result<u16> {
-    // Port 0 means the OS gives us an unused port
-    let addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 0);
-    let listener = TcpListener::bind(addr)
-        .await
-        .context("failed to bind to random port")?;
-    let port = listener
-        .local_addr()
-        .context("failed to get local address for random port")?
-        .port();
-    Ok(port)
+pub struct LockedPort {
+    pub port: u16,
+    lockfile: File,
 }
 
-/// Acquire an unused port and lock it for the duration until the era-test-node server has
-/// been started.
-pub async fn acquire_unused_port() -> anyhow::Result<(u16, File)> {
-    loop {
-        let port = pick_unused_port().await?;
+impl LockedPort {
+    /// Checks if the requested port is free.
+    /// Returns the unused port (same value as input, except for `0`).
+    async fn check_port_is_unused(port: u16) -> anyhow::Result<u16> {
+        let addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, port);
+        let listener = TcpListener::bind(addr)
+            .await
+            .context("failed to bind to random port")?;
+        let port = listener
+            .local_addr()
+            .context("failed to get local address for random port")?
+            .port();
+        Ok(port)
+    }
+
+    /// Request an unused port from the OS.
+    async fn pick_unused_port() -> anyhow::Result<u16> {
+        // Port 0 means the OS gives us an unused port
+        Self::check_port_is_unused(0).await
+    }
+
+    /// Acquire an unused port and lock it (meaning no other competing callers of this method can
+    /// take this lock). Lock lasts until the returned `LockedPort` instance is dropped.
+    pub async fn acquire_unused() -> anyhow::Result<Self> {
+        loop {
+            let port = Self::pick_unused_port().await?;
+            let lockpath = std::env::temp_dir().join(format!("era-test-node-port{}.lock", port));
+            let lockfile = File::create(lockpath)
+                .with_context(|| format!("failed to create lockfile for port={}", port))?;
+            if lockfile.try_lock_exclusive().is_ok() {
+                break Ok(Self { port, lockfile });
+            }
+        }
+    }
+
+    /// Acquire the requested port and lock it (meaning no other competing callers of this method
+    /// can take this lock). Lock lasts until the returned `LockedPort` instance is dropped.
+    pub async fn acquire(port: u16) -> anyhow::Result<Self> {
+        let port = Self::check_port_is_unused(port).await?;
         let lockpath = std::env::temp_dir().join(format!("era-test-node-port{}.lock", port));
         let lockfile = File::create(lockpath)
-            .with_context(|| format!("failed to create lockfile for port {}", port))?;
-        if lockfile.try_lock_exclusive().is_ok() {
-            break Ok((port, lockfile));
-        }
+            .with_context(|| format!("failed to create lockfile for port={}", port))?;
+        lockfile
+            .try_lock_exclusive()
+            .with_context(|| format!("failed to lock the lockfile for port={}", port))?;
+        Ok(Self { port, lockfile })
+    }
+}
+
+/// Dropping `LockedPort` unlocks the port, caller needs to make sure the port is already bound to
+/// or is not needed anymore.
+impl Drop for LockedPort {
+    fn drop(&mut self) {
+        self.lockfile
+            .unlock()
+            .with_context(|| format!("failed to unlock lockfile for port={}", self.port))
+            .unwrap();
     }
 }

spec-tests/tests/lib.rs

@@ -1,6 +1,7 @@
 //! Validation that zkSync Era In-Memory Node conforms to the official Ethereum Spec
 
-use era_test_node_spec_tests::{process, EraApi, EthSpecPatch};
+use era_test_node_spec_tests::process::EraTestNodeRunner;
+use era_test_node_spec_tests::{EraApi, EthSpecPatch};
 use jsonschema::Validator;
 use openrpc_types::resolved::{Method, OpenRPC};
 use schemars::visit::Visitor;
@@ -59,7 +60,7 @@ fn validate_schema(validator: Validator, result: Value) {
 #[test_log::test(tokio::test)]
 async fn validate_eth_get_block_genesis() -> anyhow::Result<()> {
     // Start era-test-node as an OS process with a randomly selected RPC port
-    let node_handle = process::run_default().await?;
+    let node_handle = EraTestNodeRunner::default().run().await?;
     // Connect to it via JSON-RPC API
     let era_api = EraApi::local(node_handle.config.rpc_port)?;
 
@@ -85,8 +86,8 @@ async fn validate_eth_get_block_genesis() -> anyhow::Result<()> {
 }
 
 #[test_log::test(tokio::test)]
-async fn validate_eth_get_block_with_txs() -> anyhow::Result<()> {
-    let node_handle = process::run_default().await?;
+async fn validate_eth_get_block_with_txs_legacy() -> anyhow::Result<()> {
+    let node_handle = EraTestNodeRunner::default().run().await?;
     let era_api = EraApi::local(node_handle.config.rpc_port)?;
 
     era_api.transfer_eth_legacy(U256::from("100")).await?;