From 64d0a7f995864e710facf00e9516433c26d60900 Mon Sep 17 00:00:00 2001 From: Omar Abdulla Date: Fri, 15 Aug 2025 09:57:05 +0300 Subject: [PATCH] Resolve the OS problems --- crates/common/src/cached_fs/mod.rs | 24 +++++++++++++ crates/compiler/src/revive_resolc.rs | 29 ++++++++++------ crates/compiler/src/solc.rs | 50 +++++++++++++++++++--------- crates/core/src/driver/mod.rs | 9 +---- crates/core/src/main.rs | 43 ++++++++++++++++++++---- crates/node/src/geth.rs | 26 +++++++-------- 6 files changed, 126 insertions(+), 55 deletions(-) diff --git a/crates/common/src/cached_fs/mod.rs b/crates/common/src/cached_fs/mod.rs index 17a1fa5..492c302 100644 --- a/crates/common/src/cached_fs/mod.rs +++ b/crates/common/src/cached_fs/mod.rs @@ -47,3 +47,27 @@ pub fn read_dir(path: impl AsRef) -> Result Result; +} + +impl PathExt for T +where + T: AsRef, +{ + fn cached_canonicalize(&self) -> Result { + static CANONICALIZATION_CACHE: Lazy> = + Lazy::new(|| Cache::new(10_000)); + + let path = self.as_ref().to_path_buf(); + match CANONICALIZATION_CACHE.get(&path) { + Some(canonicalized) => Ok(canonicalized), + None => { + let canonicalized = path.canonicalize()?; + CANONICALIZATION_CACHE.insert(path, canonicalized.clone()); + Ok(canonicalized) + } + } + } +} diff --git a/crates/compiler/src/revive_resolc.rs b/crates/compiler/src/revive_resolc.rs index 0261de8..6f0246e 100644 --- a/crates/compiler/src/revive_resolc.rs +++ b/crates/compiler/src/revive_resolc.rs @@ -2,6 +2,7 @@ //! compiling contracts to PolkaVM (PVM) bytecode. use std::{ + os::unix::process::CommandExt, path::PathBuf, process::{Command, Stdio}, }; @@ -92,11 +93,14 @@ impl SolidityCompiler for Resolc { }; let mut command = AsyncCommand::new(&self.resolc_path); - command - .stdin(Stdio::piped()) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .arg("--standard-json"); + unsafe { + command + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .arg("--standard-json") + .pre_exec(|| Ok(())) + }; if let Some(ref base_path) = base_path { command.arg("--base-path").arg(base_path); @@ -215,12 +219,15 @@ impl SolidityCompiler for Resolc { // Logic for parsing the resolc version from the following string: // Solidity frontend for the revive compiler version 0.3.0+commit.b238913.llvm-18.1.8 - let output = Command::new(self.resolc_path.as_path()) - .arg("--version") - .stdout(Stdio::piped()) - .spawn()? - .wait_with_output()? - .stdout; + let output = unsafe { + Command::new(self.resolc_path.as_path()) + .arg("--version") + .stdout(Stdio::piped()) + .pre_exec(|| Ok(())) + .spawn()? + .wait_with_output()? + .stdout + }; let output = String::from_utf8_lossy(&output); let version_string = output .split("version ") diff --git a/crates/compiler/src/solc.rs b/crates/compiler/src/solc.rs index f714857..a3ae816 100644 --- a/crates/compiler/src/solc.rs +++ b/crates/compiler/src/solc.rs @@ -2,6 +2,7 @@ //! compiling contracts to EVM bytecode. use std::{ + os::unix::process::CommandExt, path::PathBuf, process::{Command, Stdio}, }; @@ -102,11 +103,14 @@ impl SolidityCompiler for Solc { }; let mut command = AsyncCommand::new(&self.solc_path); - command - .stdin(Stdio::piped()) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .arg("--standard-json"); + unsafe { + command + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .arg("--standard-json") + .pre_exec(|| Ok(())) + }; if let Some(ref base_path) = base_path { command.arg("--base-path").arg(base_path); @@ -120,12 +124,20 @@ impl SolidityCompiler for Solc { .join(","), ); } - let mut child = command.spawn()?; + let mut child = command + .spawn() + .inspect_err(|err| tracing::error!(%err, "Failed to spawn the solc command"))?; let stdin = child.stdin.as_mut().expect("should be piped"); let serialized_input = serde_json::to_vec(&input)?; - stdin.write_all(&serialized_input).await?; - let output = child.wait_with_output().await?; + stdin + .write_all(&serialized_input) + .await + .inspect_err(|err| tracing::error!(%err, "Failed to write standard JSON to stdin"))?; + let output = child + .wait_with_output() + .await + .inspect_err(|err| tracing::error!(%err, "Failed to get the output of solc"))?; if !output.status.success() { let json_in = serde_json::to_string_pretty(&input)?; @@ -162,10 +174,13 @@ impl SolidityCompiler for Solc { let mut compiler_output = CompilerOutput::default(); for (contract_path, contracts) in parsed.contracts { - let map = compiler_output - .contracts - .entry(contract_path.canonicalize()?) - .or_default(); + let map = + compiler_output + .contracts + .entry(contract_path.canonicalize().inspect_err( + |err| tracing::error!(%err, "Canonicalization of path failed"), + )?) + .or_default(); for (contract_name, contract_info) in contracts.into_iter() { let source_code = contract_info .evm @@ -205,10 +220,13 @@ impl SolidityCompiler for Solc { // Version: 0.8.30+commit.73712a01.Darwin.appleclang // ``` - let child = Command::new(self.solc_path.as_path()) - .arg("--version") - .stdout(Stdio::piped()) - .spawn()?; + let child = unsafe { + Command::new(self.solc_path.as_path()) + .arg("--version") + .stdout(Stdio::piped()) + .pre_exec(|| Ok(())) + .spawn()? + }; let output = child.wait_with_output()?; let output = String::from_utf8_lossy(&output.stdout); let version_line = output diff --git a/crates/core/src/driver/mod.rs b/crates/core/src/driver/mod.rs index ab61ed2..a8e908e 100644 --- a/crates/core/src/driver/mod.rs +++ b/crates/core/src/driver/mod.rs @@ -35,7 +35,7 @@ use revive_dt_format::metadata::{ContractInstance, ContractPathAndIdent}; use revive_dt_format::{input::Step, metadata::Metadata}; use revive_dt_node::Node; use revive_dt_node_interaction::EthereumNode; -use tracing::{Instrument, instrument}; +use tracing::Instrument; use crate::Platform; @@ -152,7 +152,6 @@ where } /// Handles the contract deployment for a given input performing it if it needs to be performed. - #[instrument(level = "info", skip_all, ret)] async fn handle_input_contract_deployment( &mut self, metadata: &Metadata, @@ -209,7 +208,6 @@ where } /// Handles the execution of the input in terms of the calls that need to be made. - #[instrument(level = "info", skip_all, ret)] async fn handle_input_execution( &mut self, input: &Input, @@ -254,7 +252,6 @@ where } } - #[instrument(level = "info", skip_all, ret)] async fn handle_input_call_frame_tracing( &self, execution_receipt: &TransactionReceipt, @@ -282,7 +279,6 @@ where }) } - #[instrument(level = "info", skip_all, ret)] fn handle_input_variable_assignment( &mut self, input: &Input, @@ -313,7 +309,6 @@ where Ok(()) } - #[instrument(level = "info", skip_all, ret)] async fn handle_input_expectations( &mut self, input: &Input, @@ -365,7 +360,6 @@ where Ok(()) } - #[instrument(level = "info", skip_all, ret)] async fn handle_input_expectation_item( &mut self, execution_receipt: &TransactionReceipt, @@ -508,7 +502,6 @@ where Ok(()) } - #[instrument(level = "info", skip_all, ret)] async fn handle_input_diff( &mut self, _: CaseIdx, diff --git a/crates/core/src/main.rs b/crates/core/src/main.rs index 6860792..87063bd 100644 --- a/crates/core/src/main.rs +++ b/crates/core/src/main.rs @@ -19,7 +19,7 @@ use revive_dt_node_interaction::EthereumNode; use semver::Version; use temp_dir::TempDir; use tokio::sync::{Mutex, RwLock, mpsc}; -use tracing::{Instrument, Level}; +use tracing::{Instrument, Level, instrument}; use tracing_subscriber::{EnvFilter, FmtSubscriber}; use revive_dt_compiler::SolidityCompiler; @@ -298,7 +298,7 @@ where "Running driver", metadata_file_path = %test.path.display(), case_idx = ?test.case_idx, - solc_mode = ?test.mode, + solc_mode = %test.mode, ); let result = handle_case_driver::( @@ -694,6 +694,16 @@ async fn get_or_build_contracts( Ok(compiled_contracts.clone()) } +#[instrument( + level = "info", + skip_all, + fields( + metadata_file_path = %metadata_file_path.display(), + mode = %mode, + deployed_libraries = deployed_libraries.len(), + ), + err +)] async fn compile_contracts( metadata: &Metadata, metadata_file_path: &Path, @@ -704,17 +714,23 @@ async fn compile_contracts( let compiler_version_or_requirement = mode.compiler_version_to_use(config.solc.clone()); let compiler_path = P::Compiler::get_compiler_executable(config, compiler_version_or_requirement).await?; - let compiler_version = P::Compiler::new(compiler_path.clone()).version()?; + let compiler_version = P::Compiler::new(compiler_path.clone()) + .version() + .inspect_err(|err| tracing::error!(%err, "Failed to get compiler version"))?; tracing::info!( %compiler_version, metadata_file_path = %metadata_file_path.display(), - mode = ?mode, + mode = %mode, "Compiling contracts" ); let compiler = Compiler::::new() - .with_allow_path(metadata.directory()?) + .with_allow_path( + metadata + .directory() + .inspect_err(|err| tracing::error!(%err, "Failed to get the metadata directory"))?, + ) .with_optimization(mode.optimize) .with_via_ir(mode.via_ir); let mut compiler = metadata @@ -733,7 +749,10 @@ async fn compile_contracts( // yet more compute intensive route, of telling solc that all of the files need to link the // library and it will only perform the linking for the files that do actually need the // library. - compiler = FilesWithExtensionIterator::new(metadata.directory()?) + compiler = + FilesWithExtensionIterator::new(metadata.directory().inspect_err( + |err| tracing::error!(%err, "Failed to get the metadata directory"), + )?) .with_allowed_extension("sol") .with_use_cached_fs(true) .fold(compiler, |compiler, path| { @@ -741,7 +760,17 @@ async fn compile_contracts( }); } - let compiler_output = compiler.try_build(compiler_path).await?; + let compiler_output = compiler + .try_build(compiler_path) + .await + .inspect_err(|err| tracing::error!(%err, "Contract compilation failed"))?; + + tracing::info!( + %compiler_version, + metadata_file_path = %metadata_file_path.display(), + mode = %mode, + "Compiled contracts" + ); Ok((compiler_version, compiler_output)) } diff --git a/crates/node/src/geth.rs b/crates/node/src/geth.rs index 09cf6c6..819b5a0 100644 --- a/crates/node/src/geth.rs +++ b/crates/node/src/geth.rs @@ -264,6 +264,7 @@ impl GethNode { .connect(&connection_string) .await .map_err(Into::into) + .inspect_err(|err| tracing::error!(%err, "Failed to create the alloy provider")) }) } } @@ -280,24 +281,23 @@ impl EthereumNode for GethNode { let provider = Arc::new(self.provider().await?); let transaction_hash = *provider.send_transaction(transaction).await?.tx_hash(); - // The following is a fix for the "transaction indexing is in progress" error that we - // used to get. You can find more information on this in the following GH issue in geth + // The following is a fix for the "transaction indexing is in progress" error that we used + // to get. You can find more information on this in the following GH issue in geth // https://github.com/ethereum/go-ethereum/issues/28877. To summarize what's going on, // before we can get the receipt of the transaction it needs to have been indexed by the - // node's indexer. Just because the transaction has been confirmed it doesn't mean that - // it has been indexed. When we call alloy's `get_receipt` it checks if the transaction - // was confirmed. If it has been, then it will call `eth_getTransactionReceipt` method - // which _might_ return the above error if the tx has not yet been indexed yet. So, we - // need to implement a retry mechanism for the receipt to keep retrying to get it until - // it eventually works, but we only do that if the error we get back is the "transaction + // node's indexer. Just because the transaction has been confirmed it doesn't mean that it + // has been indexed. When we call alloy's `get_receipt` it checks if the transaction was + // confirmed. If it has been, then it will call `eth_getTransactionReceipt` method which + // _might_ return the above error if the tx has not yet been indexed yet. So, we need to + // implement a retry mechanism for the receipt to keep retrying to get it until it + // eventually works, but we only do that if the error we get back is the "transaction // indexing is in progress" error or if the receipt is None. // // Getting the transaction indexed and taking a receipt can take a long time especially - // when a lot of transactions are being submitted to the node. Thus, while initially we - // only allowed for 60 seconds of waiting with a 1 second delay in polling, we need to - // allow for a larger wait time. Therefore, in here we allow for 5 minutes of waiting - // with exponential backoff each time we attempt to get the receipt and find that it's - // not available. + // when a lot of transactions are being submitted to the node. Thus, while initially we only + // allowed for 60 seconds of waiting with a 1 second delay in polling, we need to allow for + // a larger wait time. Therefore, in here we allow for 5 minutes of waiting with exponential + // backoff each time we attempt to get the receipt and find that it's not available. poll( Self::RECEIPT_POLLING_DURATION, Default::default(),