feat: improve node bindings (#1279)

* add run_with_tempdir

* update tests, now picks random instance by default

* document new random port picking

* fix clippy

* add port customization

* reset instance when configuring custom port as this is incompatible in reth

* additional tests for http_port(0)

* fix comment

crates/node-bindings/Cargo.toml

@@ -22,11 +22,9 @@ workspace = true
 alloy-primitives = { workspace = true, features = ["std", "k256", "serde"] }
 alloy-genesis.workspace = true
 k256.workspace = true
+rand.workspace = true
 serde_json = { workspace = true, features = ["std"] }
 tempfile.workspace = true
 thiserror.workspace = true
 tracing.workspace = true
 url.workspace = true
-
-[dev-dependencies]
-rand.workspace = true

crates/node-bindings/src/lib.rs

@@ -21,8 +21,7 @@ pub use nodes::{
 mod node;
 pub use node::*;
 
-mod utils;
-use utils::*;
+pub mod utils;
 
 /// 1 Ether = 1e18 Wei == 0x0de0b6b3a7640000 Wei
 pub const WEI_IN_ETHER: U256 = U256::from_limbs([0x0de0b6b3a7640000, 0x0, 0x0, 0x0]);

crates/node-bindings/src/node.rs

@@ -1,5 +1,6 @@
 //! Node-related types and constants.
 
+use alloy_primitives::hex;
 use std::time::Duration;
 use thiserror::Error;
 
@@ -9,31 +10,9 @@ pub const NODE_STARTUP_TIMEOUT: Duration = Duration::from_secs(10);
 /// Timeout for waiting for the node to add a peer.
 pub const NODE_DIAL_LOOP_TIMEOUT: Duration = Duration::from_secs(20);
 
-/// Errors that can occur when working with a node instance.
-#[derive(Debug)]
-pub enum NodeInstanceError {
- /// Timed out waiting for a message from node's stderr.
- Timeout(String),
-
- /// A line could not be read from the node's stderr.
- ReadLineError(std::io::Error),
-
- /// The child node process's stderr was not captured.
- NoStderr,
-
- /// The child node process's stdout was not captured.
- NoStdout,
-}
-
-/// Errors that can occur when working with the node.
+/// Errors that can occur when working with the node instance.
 #[derive(Debug, Error)]
 pub enum NodeError {
- /// The chain id was not set.
- #[error("the chain ID was not set")]
- ChainIdNotSet,
- /// Could not create the data directory.
- #[error("could not create directory: {0}")]
- CreateDirError(std::io::Error),
  /// No stderr was captured from the child process.
  #[error("no stderr was captured from the process")]
  NoStderr,
@@ -49,6 +28,14 @@ pub enum NodeError {
  /// A line could not be read from the node stderr.
  #[error("could not read line from node stderr: {0}")]
  ReadLineError(std::io::Error),
+
+ /// The chain id was not set.
+ #[error("the chain ID was not set")]
+ ChainIdNotSet,
+ /// Could not create the data directory.
+ #[error("could not create directory: {0}")]
+ CreateDirError(std::io::Error),
+
  /// Genesis error
  #[error("genesis error occurred: {0}")]
  GenesisError(String),
@@ -65,4 +52,17 @@ pub enum NodeError {
  /// Clique private key error
  #[error("clique address error: {0}")]
  CliqueAddressError(String),
+
+ /// The private key could not be parsed.
+ #[error("could not parse private key")]
+ ParsePrivateKeyError,
+ /// An error occurred while deserializing a private key.
+ #[error("could not deserialize private key from bytes")]
+ DeserializePrivateKeyError,
+ /// An error occurred while parsing a hex string.
+ #[error(transparent)]
+ FromHexError(#[from] hex::FromHexError),
+ /// No keys available this node instance.
+ #[error("no keys available in this node instance")]
+ NoKeysAvailable,
 }

crates/node-bindings/src/nodes/anvil.rs

@@ -10,9 +10,10 @@ use std::{
  str::FromStr,
  time::{Duration, Instant},
 };
-use thiserror::Error;
 use url::Url;
 
+use crate::NodeError;
+
 /// How long we will wait for anvil to indicate that it is ready.
 const ANVIL_STARTUP_TIMEOUT_MILLIS: u64 = 10_000;
 
@@ -89,42 +90,6 @@ impl Drop for AnvilInstance {
  }
 }
 
-/// Errors that can occur when working with the [`Anvil`].
-#[derive(Debug, Error)]
-pub enum AnvilError {
- /// Spawning the anvil process failed.
- #[error("could not start anvil: {0}")]
- SpawnError(std::io::Error),
-
- /// Timed out waiting for a message from anvil's stderr.
- #[error("timed out waiting for anvil to spawn; is anvil installed?")]
- Timeout,
-
- /// A line could not be read from the geth stderr.
- #[error("could not read line from anvil stderr: {0}")]
- ReadLineError(std::io::Error),
-
- /// The child anvil process's stderr was not captured.
- #[error("could not get stderr for anvil child process")]
- NoStderr,
-
- /// The private key could not be parsed.
- #[error("could not parse private key")]
- ParsePrivateKeyError,
-
- /// An error occurred while deserializing a private key.
- #[error("could not deserialize private key from bytes")]
- DeserializePrivateKeyError,
-
- /// An error occurred while parsing a hex string.
- #[error(transparent)]
- FromHexError(#[from] hex::FromHexError),
-
- /// No keys available in anvil instance.
- #[error("no keys available in anvil instance")]
- NoKeysAvailable,
-}
-
 /// Builder for launching `anvil`.
 ///
 /// # Panics
@@ -288,7 +253,7 @@ impl Anvil {
  }
 
  /// Consumes the builder and spawns `anvil`. If spawning fails, returns an error.
- pub fn try_spawn(self) -> Result<AnvilInstance, AnvilError> {
+ pub fn try_spawn(self) -> Result<AnvilInstance, NodeError> {
  let mut cmd = self.program.as_ref().map_or_else(|| Command::new("anvil"), Command::new);
  cmd.stdout(std::process::Stdio::piped()).stderr(std::process::Stdio::inherit());
  let mut port = self.port.unwrap_or_default();
@@ -316,9 +281,9 @@ impl Anvil {
 
  cmd.args(self.args);
 
- let mut child = cmd.spawn().map_err(AnvilError::SpawnError)?;
+ let mut child = cmd.spawn().map_err(NodeError::SpawnError)?;
 
- let stdout = child.stdout.take().ok_or(AnvilError::NoStderr)?;
+ let stdout = child.stdout.take().ok_or(NodeError::NoStderr)?;
 
  let start = Instant::now();
  let mut reader = BufReader::new(stdout);
@@ -331,11 +296,11 @@ impl Anvil {
  if start + Duration::from_millis(self.timeout.unwrap_or(ANVIL_STARTUP_TIMEOUT_MILLIS))
  <= Instant::now()
  {
- return Err(AnvilError::Timeout);
+ return Err(NodeError::Timeout);
  }
 
  let mut line = String::new();
- reader.read_line(&mut line).map_err(AnvilError::ReadLineError)?;
+ reader.read_line(&mut line).map_err(NodeError::ReadLineError)?;
  trace!(target: "anvil", line);
  if let Some(addr) = line.strip_prefix("Listening on") {
  // <Listening on 127.0.0.1:8545>
@@ -352,10 +317,10 @@ impl Anvil {
 
  if is_private_key && line.starts_with('(') {
  let key_str =
- line.split("0x").last().ok_or(AnvilError::ParsePrivateKeyError)?.trim();
- let key_hex = hex::decode(key_str).map_err(AnvilError::FromHexError)?;
+ line.split("0x").last().ok_or(NodeError::ParsePrivateKeyError)?.trim();
+ let key_hex = hex::decode(key_str).map_err(NodeError::FromHexError)?;
  let key = K256SecretKey::from_bytes((&key_hex[..]).into())
- .map_err(|_| AnvilError::DeserializePrivateKeyError)?;
+ .map_err(|_| NodeError::DeserializePrivateKeyError)?;
  addresses.push(Address::from_public_key(SigningKey::from(&key).verifying_key()));
  private_keys.push(key);
  }

crates/node-bindings/src/nodes/geth.rs

@@ -1,8 +1,8 @@
 //! Utilities for launching a Geth dev-mode instance.
 
 use crate::{
- extract_endpoint, extract_value, unused_port, NodeError, NodeInstanceError,
- NODE_DIAL_LOOP_TIMEOUT, NODE_STARTUP_TIMEOUT,
+ utils::{extract_endpoint, extract_value, unused_port},
+ NodeError, NODE_DIAL_LOOP_TIMEOUT, NODE_STARTUP_TIMEOUT,
 };
 use alloy_genesis::{CliqueConfig, Genesis};
 use alloy_primitives::Address;
@@ -139,30 +139,30 @@ impl GethInstance {
  ///
  /// This leaves a `None` in its place, so calling methods that require a stderr to be present
  /// will fail if called after this.
- pub fn stderr(&mut self) -> Result<ChildStderr, NodeInstanceError> {
- self.pid.stderr.take().ok_or(NodeInstanceError::NoStderr)
+ pub fn stderr(&mut self) -> Result<ChildStderr, NodeError> {
+ self.pid.stderr.take().ok_or(NodeError::NoStderr)
  }
 
  /// Blocks until geth adds the specified peer, using 20s as the timeout.
  ///
  /// Requires the stderr to be present in the `GethInstance`.
- pub fn wait_to_add_peer(&mut self, id: &str) -> Result<(), NodeInstanceError> {
- let mut stderr = self.pid.stderr.as_mut().ok_or(NodeInstanceError::NoStderr)?;
+ pub fn wait_to_add_peer(&mut self, id: &str) -> Result<(), NodeError> {
+ let mut stderr = self.pid.stderr.as_mut().ok_or(NodeError::NoStderr)?;
  let mut err_reader = BufReader::new(&mut stderr);
  let mut line = String::new();
  let start = Instant::now();
 
  while start.elapsed() < NODE_DIAL_LOOP_TIMEOUT {
  line.clear();
- err_reader.read_line(&mut line).map_err(NodeInstanceError::ReadLineError)?;
+ err_reader.read_line(&mut line).map_err(NodeError::ReadLineError)?;
 
  // geth ids are truncated
  let truncated_id = if id.len() > 16 { &id[..16] } else { id };
  if line.contains("Adding p2p peer") && line.contains(truncated_id) {
  return Ok(());
  }
  }
- Err(NodeInstanceError::Timeout("Timed out waiting for geth to add a peer".into()))
+ Err(NodeError::Timeout)
  }
 }
 
@@ -622,33 +622,20 @@ impl Geth {
 // These tests should use a different datadir for each `geth` spawned.
 #[cfg(test)]
 mod tests {
+ use crate::utils::run_with_tempdir_sync;
+
  use super::*;
- use std::path::Path;
 
  #[test]
  fn port_0() {
- run_with_tempdir(|_| {
+ run_with_tempdir_sync("geth-test-", |_| {
  let _geth = Geth::new().disable_discovery().port(0u16).spawn();
  });
  }
 
- /// Allows running tests with a temporary directory, which is cleaned up after the function is
- /// called.
- ///
- /// Helps with tests that spawn a helper instance, which has to be dropped before the temporary
- /// directory is cleaned up.
- #[track_caller]
- fn run_with_tempdir(f: impl Fn(&Path)) {
- let temp_dir = tempfile::tempdir().unwrap();
- let temp_dir_path = temp_dir.path();
- f(temp_dir_path);
- #[cfg(not(windows))]
- temp_dir.close().unwrap();
- }
-
  #[test]
  fn p2p_port() {
- run_with_tempdir(|temp_dir_path| {
+ run_with_tempdir_sync("geth-test-", |temp_dir_path| {
  let geth = Geth::new().disable_discovery().data_dir(temp_dir_path).spawn();
  let p2p_port = geth.p2p_port();
  assert!(p2p_port.is_some());
@@ -657,7 +644,7 @@ mod tests {
 
  #[test]
  fn explicit_p2p_port() {
- run_with_tempdir(|temp_dir_path| {
+ run_with_tempdir_sync("geth-test-", |temp_dir_path| {
  // if a p2p port is explicitly set, it should be used
  let geth = Geth::new().p2p_port(1234).data_dir(temp_dir_path).spawn();
  let p2p_port = geth.p2p_port();
@@ -667,7 +654,7 @@ mod tests {
 
  #[test]
  fn dev_mode() {
- run_with_tempdir(|temp_dir_path| {
+ run_with_tempdir_sync("geth-test-", |temp_dir_path| {
  // dev mode should not have a p2p port, and dev should be the default
  let geth = Geth::new().data_dir(temp_dir_path).spawn();
  let p2p_port = geth.p2p_port();
@@ -679,7 +666,7 @@ mod tests {
  #[ignore = "fails on geth >=1.14"]
  #[allow(deprecated)]
  fn clique_correctly_configured() {
- run_with_tempdir(|temp_dir_path| {
+ run_with_tempdir_sync("geth-test-", |temp_dir_path| {
  let private_key = SigningKey::random(&mut rand::thread_rng());
  let geth = Geth::new()
  .set_clique_private_key(private_key)