From 14888f9767eabfe0b01f83155be5dc5c8456ba94 Mon Sep 17 00:00:00 2001 From: Omar Date: Tue, 15 Jul 2025 14:19:17 +0300 Subject: [PATCH 1/3] Update the async runtime (#42) * Update the async runtime with syntactic sugar. * Fix doc test * Update crates/node-interaction/src/blocking_executor.rs Co-authored-by: xermicus * Update crates/node-interaction/src/blocking_executor.rs Co-authored-by: xermicus * Update crates/node-interaction/src/blocking_executor.rs Co-authored-by: xermicus * Update crates/node-interaction/src/blocking_executor.rs Co-authored-by: xermicus * Update crates/node-interaction/src/blocking_executor.rs Co-authored-by: xermicus * Update crates/node-interaction/src/blocking_executor.rs Co-authored-by: xermicus * Update crates/node-interaction/src/blocking_executor.rs Co-authored-by: xermicus * Update crates/node-interaction/src/blocking_executor.rs Co-authored-by: xermicus * Update crates/node-interaction/src/blocking_executor.rs Co-authored-by: xermicus * Update crates/node-interaction/src/blocking_executor.rs Co-authored-by: xermicus * Improve the comments * Update the release profile --------- Co-authored-by: xermicus --- Cargo.lock | 1 + Cargo.toml | 1 + crates/node-interaction/Cargo.toml | 1 + .../node-interaction/src/blocking_executor.rs | 221 ++++++++++++++++++ crates/node-interaction/src/lib.rs | 7 +- crates/node-interaction/src/nonce.rs | 55 ----- crates/node-interaction/src/tokio_runtime.rs | 87 ------- crates/node-interaction/src/trace.rs | 43 ---- crates/node-interaction/src/transaction.rs | 46 ---- crates/node/src/geth.rs | 23 +- crates/node/src/kitchensink.rs | 23 +- 11 files changed, 254 insertions(+), 254 deletions(-) create mode 100644 crates/node-interaction/src/blocking_executor.rs delete mode 100644 crates/node-interaction/src/nonce.rs delete mode 100644 crates/node-interaction/src/tokio_runtime.rs delete mode 100644 crates/node-interaction/src/trace.rs delete mode 100644 crates/node-interaction/src/transaction.rs diff --git a/Cargo.lock b/Cargo.lock index 42b6164..eebc456 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4012,6 +4012,7 @@ version = "0.1.0" dependencies = [ "alloy", "anyhow", + "futures", "once_cell", "tokio", "tracing", diff --git a/Cargo.toml b/Cargo.toml index 5d1d6a7..9f3c875 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,6 +25,7 @@ alloy-primitives = "1.2.1" alloy-sol-types = "1.2.1" anyhow = "1.0" clap = { version = "4", features = ["derive"] } +futures = { version = "0.3.31" } hex = "0.4.3" reqwest = { version = "0.12.15", features = ["blocking", "json"] } once_cell = "1.21" diff --git a/crates/node-interaction/Cargo.toml b/crates/node-interaction/Cargo.toml index b044456..84ea315 100644 --- a/crates/node-interaction/Cargo.toml +++ b/crates/node-interaction/Cargo.toml @@ -11,6 +11,7 @@ rust-version.workspace = true [dependencies] alloy = { workspace = true } anyhow = { workspace = true } +futures = { workspace = true } tracing = { workspace = true } once_cell = { workspace = true } tokio = { workspace = true } diff --git a/crates/node-interaction/src/blocking_executor.rs b/crates/node-interaction/src/blocking_executor.rs new file mode 100644 index 0000000..043dd19 --- /dev/null +++ b/crates/node-interaction/src/blocking_executor.rs @@ -0,0 +1,221 @@ +//! The alloy crate __requires__ a tokio runtime. +//! We contain any async rust right here. + +use std::{any::Any, panic::AssertUnwindSafe, pin::Pin, thread}; + +use futures::FutureExt; +use once_cell::sync::Lazy; +use tokio::{ + runtime::Builder, + sync::{mpsc::UnboundedSender, oneshot}, +}; + +/// A blocking async executor. +/// +/// This struct exposes the abstraction of a blocking async executor. It is a global and static +/// executor which means that it doesn't require for new instances of it to be created, it's a +/// singleton and can be accessed by any thread that wants to perform some async computation on the +/// blocking executor thread. +/// +/// The API of the blocking executor is created in a way so that it's very natural, simple to use, +/// and unbounded to specific tasks or return types. The following is an example of using this +/// executor to drive an async computation: +/// +/// ```rust +/// use revive_dt_node_interaction::*; +/// +/// fn blocking_function() { +/// let result = BlockingExecutor::execute(async move { +/// tokio::time::sleep(std::time::Duration::from_secs(1)).await; +/// 0xFFu8 +/// }) +/// .expect("Computation failed"); +/// +/// assert_eq!(result, 0xFF); +/// } +/// ``` +/// +/// Users get to pass in their async tasks without needing to worry about putting them in a [`Box`], +/// [`Pin`], needing to perform down-casting, or the internal channel mechanism used by the runtime. +/// To the user, it just looks like a function that converts some async code into sync code. +/// +/// This struct also handled panics that occur in the passed futures and converts them into errors +/// that can be handled by the user. This is done to allow the executor to be robust. +/// +/// Internally, the executor communicates with the tokio runtime thread through channels which carry +/// the [`TaskMessage`] and the results of the execution. +pub struct BlockingExecutor; + +impl BlockingExecutor { + pub fn execute(future: impl Future + Send + 'static) -> Result + where + R: Send + 'static, + { + // Note: The blocking executor is a singleton and therefore we store its state in a static + // so that it's assigned only once. Additionally, when we set the state of the executor we + // spawn the thread where the async runtime runs. + static STATE: Lazy = Lazy::new(|| { + tracing::trace!("Initializing the BlockingExecutor state"); + + // All communication with the tokio runtime thread happens over mspc channels where the + // producers here are the threads that want to run async tasks and the consumer here is + // the tokio runtime thread. + let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel::(); + + thread::spawn(move || { + let runtime = Builder::new_current_thread() + .enable_all() + .build() + .expect("Failed to create the async runtime"); + + runtime.block_on(async move { + while let Some(TaskMessage { + future: task, + response_tx: response_channel, + }) = rx.recv().await + { + tracing::trace!("Received a new future to execute"); + tokio::spawn(async move { + // One of the things that the blocking executor does is that it allows + // us to catch panics if they occur. By wrapping the given future in an + // AssertUnwindSafe::catch_unwind we are able to catch all panic unwinds + // in the given future and convert them into errors. + let task = AssertUnwindSafe(task).catch_unwind(); + + let result = task.await; + let _ = response_channel.send(result); + }); + } + }) + }); + + ExecutorState { tx } + }); + + // We need to perform blocking synchronous communication between the current thread and the + // tokio runtime thread with the result of the async computation and the oneshot channels + // from tokio allows us to do that. The sender side of the channel will be given to the + // tokio runtime thread to send the result when the computation is completed and the receive + // side of the channel will be kept with this thread to await for the response of the async + // task to come back. + let (response_tx, response_rx) = + oneshot::channel::, Box>>(); + + // The tokio runtime thread expects a Future> + Send to be + // sent to it to execute. However, this function has a typed Future + Send and + // therefore we need to change the type of the future to fit what the runtime thread expects + // in the task message. In doing this conversion, we lose some of the type information since + // we're converting R => dyn Any. However, we will perform down-casting on the result to + // convert it back into R. + let future = Box::pin(async move { Box::new(future.await) as Box }); + + let task = TaskMessage::new(future, response_tx); + if let Err(error) = STATE.tx.send(task) { + tracing::error!(?error, "Failed to send the task to the blocking executor"); + anyhow::bail!("Failed to send the task to the blocking executor: {error:?}") + } + + let result = match response_rx.blocking_recv() { + Ok(result) => result, + Err(error) => { + tracing::error!( + ?error, + "Failed to get the response from the blocking executor" + ); + anyhow::bail!("Failed to get the response from the blocking executor: {error:?}") + } + }; + + match result.map(|result| { + *result + .downcast::() + .expect("Type mismatch in the downcast") + }) { + Ok(result) => Ok(result), + Err(error) => { + tracing::error!( + ?error, + "Failed to downcast the returned result into the expected type" + ); + anyhow::bail!( + "Failed to downcast the returned result into the expected type: {error:?}" + ) + } + } + } +} + +/// Represents the state of the async runtime. This runtime is designed to be a singleton runtime +/// which means that in the current running program there's just a single thread that has an async +/// runtime. +struct ExecutorState { + /// The sending side of the task messages channel. This is used by all of the other threads to + /// communicate with the async runtime thread. + tx: UnboundedSender, +} + +/// Represents a message that contains an asynchronous task that's to be executed by the runtime +/// as well as a way for the runtime to report back on the result of the execution. +struct TaskMessage { + /// The task that's being requested to run. This is a future that returns an object that does + /// implement [`Any`] and [`Send`] to allow it to be sent between the requesting thread and the + /// async thread. + future: Pin> + Send>>, + + /// A one shot sender channel where the sender of the task is expecting to hear back on the + /// result of the task. + response_tx: oneshot::Sender, Box>>, +} + +impl TaskMessage { + pub fn new( + future: Pin> + Send>>, + response_tx: oneshot::Sender, Box>>, + ) -> Self { + Self { + future, + response_tx, + } + } +} + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn simple_future_works() { + // Act + let result = BlockingExecutor::execute(async move { + tokio::time::sleep(std::time::Duration::from_secs(1)).await; + 0xFFu8 + }) + .unwrap(); + + // Assert + assert_eq!(result, 0xFFu8); + } + + #[test] + #[allow(unreachable_code, clippy::unreachable)] + fn panics_in_futures_are_caught() { + // Act + let result = BlockingExecutor::execute(async move { + panic!("This is a panic!"); + 0xFFu8 + }); + + // Assert + assert!(result.is_err()); + + // Act + let result = BlockingExecutor::execute(async move { + tokio::time::sleep(std::time::Duration::from_secs(1)).await; + 0xFFu8 + }) + .unwrap(); + + // Assert + assert_eq!(result, 0xFFu8) + } +} diff --git a/crates/node-interaction/src/lib.rs b/crates/node-interaction/src/lib.rs index 2006d1b..2c2eef5 100644 --- a/crates/node-interaction/src/lib.rs +++ b/crates/node-interaction/src/lib.rs @@ -3,12 +3,9 @@ use alloy::primitives::Address; use alloy::rpc::types::trace::geth::{DiffMode, GethTrace}; use alloy::rpc::types::{TransactionReceipt, TransactionRequest}; -use tokio_runtime::TO_TOKIO; -pub mod nonce; -mod tokio_runtime; -pub mod trace; -pub mod transaction; +mod blocking_executor; +pub use blocking_executor::*; /// An interface for all interactions with Ethereum compatible nodes. pub trait EthereumNode { diff --git a/crates/node-interaction/src/nonce.rs b/crates/node-interaction/src/nonce.rs deleted file mode 100644 index 53b73c9..0000000 --- a/crates/node-interaction/src/nonce.rs +++ /dev/null @@ -1,55 +0,0 @@ -use std::pin::Pin; - -use alloy::{ - primitives::Address, - providers::{Provider, ProviderBuilder}, -}; -use tokio::sync::oneshot; - -use crate::{TO_TOKIO, tokio_runtime::AsyncNodeInteraction}; - -pub type Task = Pin> + Send>>; - -pub(crate) struct Nonce { - sender: oneshot::Sender>, - task: Task, -} - -impl AsyncNodeInteraction for Nonce { - type Output = anyhow::Result; - - fn split( - self, - ) -> ( - std::pin::Pin + Send>>, - oneshot::Sender, - ) { - (self.task, self.sender) - } -} - -/// This is like `trace_transaction`, just for nonces. -pub fn fetch_onchain_nonce( - connection: String, - wallet: alloy::network::EthereumWallet, - address: Address, -) -> anyhow::Result { - let sender = TO_TOKIO.lock().unwrap().nonce_sender.clone(); - - let (tx, rx) = oneshot::channel(); - let task: Task = Box::pin(async move { - let provider = ProviderBuilder::new() - .wallet(wallet) - .connect(&connection) - .await?; - let onchain = provider.get_transaction_count(address).await?; - Ok(onchain) - }); - - sender - .blocking_send(Nonce { task, sender: tx }) - .expect("not in async context"); - - rx.blocking_recv() - .unwrap_or_else(|err| anyhow::bail!("nonce fetch failed: {err}")) -} diff --git a/crates/node-interaction/src/tokio_runtime.rs b/crates/node-interaction/src/tokio_runtime.rs deleted file mode 100644 index 20a3ae3..0000000 --- a/crates/node-interaction/src/tokio_runtime.rs +++ /dev/null @@ -1,87 +0,0 @@ -//! The alloy crate __requires__ a tokio runtime. -//! We contain any async rust right here. - -use once_cell::sync::Lazy; -use std::pin::Pin; -use std::sync::Mutex; -use std::thread; -use tokio::runtime::Runtime; -use tokio::spawn; -use tokio::sync::{mpsc, oneshot}; -use tokio::task::JoinError; - -use crate::nonce::Nonce; -use crate::trace::Trace; -use crate::transaction::Transaction; - -pub(crate) static TO_TOKIO: Lazy> = - Lazy::new(|| Mutex::new(TokioRuntime::spawn())); - -/// Common interface for executing async node interactions from a non-async context. -#[allow(clippy::type_complexity)] -pub(crate) trait AsyncNodeInteraction: Send + 'static { - type Output: Send; - - //// Returns the task and the output sender. - fn split( - self, - ) -> ( - Pin + Send>>, - oneshot::Sender, - ); -} - -pub(crate) struct TokioRuntime { - pub(crate) transaction_sender: mpsc::Sender, - pub(crate) trace_sender: mpsc::Sender, - pub(crate) nonce_sender: mpsc::Sender, -} - -impl TokioRuntime { - fn spawn() -> Self { - let rt = Runtime::new().expect("should be able to create the tokio runtime"); - let (transaction_sender, transaction_receiver) = mpsc::channel::(1024); - let (trace_sender, trace_receiver) = mpsc::channel::(1024); - let (nonce_sender, nonce_receiver) = mpsc::channel::(1024); - - thread::spawn(move || { - rt.block_on(async move { - let transaction_task = spawn(interaction::(transaction_receiver)); - let trace_task = spawn(interaction::(trace_receiver)); - let nonce_task = spawn(interaction::(nonce_receiver)); - - if let Err(error) = transaction_task.await { - tracing::error!("tokio transaction task failed: {error}"); - } - if let Err(error) = trace_task.await { - tracing::error!("tokio trace transaction task failed: {error}"); - } - if let Err(error) = nonce_task.await { - tracing::error!("tokio nonce task failed: {error}"); - } - }); - }); - - Self { - transaction_sender, - trace_sender, - nonce_sender, - } - } -} - -async fn interaction(mut receiver: mpsc::Receiver) -> Result<(), JoinError> -where - T: AsyncNodeInteraction, -{ - while let Some(task) = receiver.recv().await { - spawn(async move { - let (task, sender) = task.split(); - sender - .send(task.await) - .unwrap_or_else(|_| panic!("failed to send task output")); - }); - } - - Ok(()) -} diff --git a/crates/node-interaction/src/trace.rs b/crates/node-interaction/src/trace.rs deleted file mode 100644 index 9255d00..0000000 --- a/crates/node-interaction/src/trace.rs +++ /dev/null @@ -1,43 +0,0 @@ -//! Trace transactions in a sync context. - -use std::pin::Pin; - -use alloy::rpc::types::trace::geth::GethTrace; -use tokio::sync::oneshot; - -use crate::TO_TOKIO; -use crate::tokio_runtime::AsyncNodeInteraction; - -pub type Task = Pin> + Send>>; - -pub(crate) struct Trace { - sender: oneshot::Sender>, - task: Task, -} - -impl AsyncNodeInteraction for Trace { - type Output = anyhow::Result; - - fn split( - self, - ) -> ( - std::pin::Pin + Send>>, - oneshot::Sender, - ) { - (self.task, self.sender) - } -} - -/// Execute some [Task] that return a [GethTrace] result. -pub fn trace_transaction(task: Task) -> anyhow::Result { - let task_sender = TO_TOKIO.lock().unwrap().trace_sender.clone(); - let (sender, receiver) = oneshot::channel(); - - task_sender - .blocking_send(Trace { task, sender }) - .expect("we are not calling this from an async context"); - - receiver - .blocking_recv() - .unwrap_or_else(|error| anyhow::bail!("no trace received: {error}")) -} diff --git a/crates/node-interaction/src/transaction.rs b/crates/node-interaction/src/transaction.rs deleted file mode 100644 index b5af221..0000000 --- a/crates/node-interaction/src/transaction.rs +++ /dev/null @@ -1,46 +0,0 @@ -//! Execute transactions in a sync context. - -use std::pin::Pin; - -use alloy::rpc::types::TransactionReceipt; -use tokio::sync::oneshot; - -use crate::TO_TOKIO; -use crate::tokio_runtime::AsyncNodeInteraction; - -pub type Task = Pin> + Send>>; - -pub(crate) struct Transaction { - receipt_sender: oneshot::Sender>, - task: Task, -} - -impl AsyncNodeInteraction for Transaction { - type Output = anyhow::Result; - - fn split( - self, - ) -> ( - Pin + Send>>, - oneshot::Sender, - ) { - (self.task, self.receipt_sender) - } -} - -/// Execute some [Task] that returns a [TransactionReceipt]. -pub fn execute_transaction(task: Task) -> anyhow::Result { - let request_sender = TO_TOKIO.lock().unwrap().transaction_sender.clone(); - let (receipt_sender, receipt_receiver) = oneshot::channel(); - - request_sender - .blocking_send(Transaction { - receipt_sender, - task, - }) - .expect("we are not calling this from an async context"); - - receipt_receiver - .blocking_recv() - .unwrap_or_else(|error| anyhow::bail!("no receipt received: {error}")) -} diff --git a/crates/node/src/geth.rs b/crates/node/src/geth.rs index 99893a0..57cb2db 100644 --- a/crates/node/src/geth.rs +++ b/crates/node/src/geth.rs @@ -23,10 +23,7 @@ use alloy::{ }, }; use revive_dt_config::Arguments; -use revive_dt_node_interaction::{ - EthereumNode, nonce::fetch_onchain_nonce, trace::trace_transaction, - transaction::execute_transaction, -}; +use revive_dt_node_interaction::{BlockingExecutor, EthereumNode}; use tracing::Level; use crate::Node; @@ -205,7 +202,7 @@ impl EthereumNode for Instance { let connection_string = self.connection_string(); let wallet = self.wallet.clone(); - execute_transaction(Box::pin(async move { + BlockingExecutor::execute(async move { let outer_span = tracing::debug_span!("Submitting transaction", ?transaction,); let _outer_guard = outer_span.enter(); @@ -284,7 +281,7 @@ impl EthereumNode for Instance { } } } - })) + })? } #[tracing::instrument(skip_all, fields(geth_node_id = self.id))] @@ -300,14 +297,14 @@ impl EthereumNode for Instance { }); let wallet = self.wallet.clone(); - trace_transaction(Box::pin(async move { + BlockingExecutor::execute(async move { Ok(ProviderBuilder::new() .wallet(wallet) .connect(&connection_string) .await? .debug_trace_transaction(transaction.transaction_hash, trace_options) .await?) - })) + })? } #[tracing::instrument(skip_all, fields(geth_node_id = self.id))] @@ -329,7 +326,15 @@ impl EthereumNode for Instance { let connection_string = self.connection_string.clone(); let wallet = self.wallet.clone(); - let onchain_nonce = fetch_onchain_nonce(connection_string, wallet, address)?; + let onchain_nonce = BlockingExecutor::execute::>(async move { + ProviderBuilder::new() + .wallet(wallet) + .connect(&connection_string) + .await? + .get_transaction_count(address) + .await + .map_err(Into::into) + })??; let mut nonces = self.nonces.lock().unwrap(); let current = nonces.entry(address).or_insert(onchain_nonce); diff --git a/crates/node/src/kitchensink.rs b/crates/node/src/kitchensink.rs index c05f54c..768c1aa 100644 --- a/crates/node/src/kitchensink.rs +++ b/crates/node/src/kitchensink.rs @@ -27,10 +27,7 @@ use sp_runtime::AccountId32; use tracing::Level; use revive_dt_config::Arguments; -use revive_dt_node_interaction::{ - EthereumNode, nonce::fetch_onchain_nonce, trace::trace_transaction, - transaction::execute_transaction, -}; +use revive_dt_node_interaction::{BlockingExecutor, EthereumNode}; use crate::Node; @@ -340,7 +337,7 @@ impl EthereumNode for KitchensinkNode { tracing::debug!("Submitting transaction: {transaction:#?}"); tracing::info!("Submitting tx to kitchensink"); - let receipt = execute_transaction(Box::pin(async move { + let receipt = BlockingExecutor::execute(async move { Ok(ProviderBuilder::new() .wallet(wallet) .connect(&url) @@ -349,7 +346,7 @@ impl EthereumNode for KitchensinkNode { .await? .get_receipt() .await?) - })); + })?; tracing::info!(?receipt, "Submitted tx to kitchensink"); receipt } @@ -368,14 +365,14 @@ impl EthereumNode for KitchensinkNode { let wallet = self.wallet.clone(); - trace_transaction(Box::pin(async move { + BlockingExecutor::execute(async move { Ok(ProviderBuilder::new() .wallet(wallet) .connect(&url) .await? .debug_trace_transaction(transaction.transaction_hash, trace_options) .await?) - })) + })? } #[tracing::instrument(skip_all, fields(kitchensink_node_id = self.id))] @@ -394,7 +391,15 @@ impl EthereumNode for KitchensinkNode { let url = self.rpc_url.clone(); let wallet = self.wallet.clone(); - let onchain_nonce = fetch_onchain_nonce(url, wallet, address)?; + let onchain_nonce = BlockingExecutor::execute::>(async move { + ProviderBuilder::new() + .wallet(wallet) + .connect(&url) + .await? + .get_transaction_count(address) + .await + .map_err(Into::into) + })??; let mut nonces = self.nonces.lock().unwrap(); let current = nonces.entry(address).or_insert(onchain_nonce); From c2e65f9e3333c14e8085ddf304175affa741e515 Mon Sep 17 00:00:00 2001 From: Omar Date: Tue, 15 Jul 2025 23:00:10 +0300 Subject: [PATCH 2/3] Fix function selector & argument encoding (#39) * Fix function selector and argument encoding * Avoid extra buffer allocation * Remove reliance on the web3 crate * Fix tests --- crates/format/src/input.rs | 338 +++++++++++++------------------------ 1 file changed, 117 insertions(+), 221 deletions(-) diff --git a/crates/format/src/input.rs b/crates/format/src/input.rs index 34a7614..270d908 100644 --- a/crates/format/src/input.rs +++ b/crates/format/src/input.rs @@ -1,16 +1,13 @@ use std::collections::HashMap; use alloy::{ - hex, - json_abi::{Function, JsonAbi}, + json_abi::JsonAbi, network::TransactionBuilder, - primitives::{Address, Bytes}, + primitives::{Address, Bytes, U256}, rpc::types::TransactionRequest, }; -use alloy_primitives::U256; -use alloy_sol_types::SolValue; use semver::VersionReq; -use serde::{Deserialize, de::Deserializer}; +use serde::Deserialize; use serde_json::Value; #[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq)] @@ -20,7 +17,6 @@ pub struct Input { pub comment: Option, #[serde(default = "default_instance")] pub instance: String, - #[serde(deserialize_with = "deserialize_method")] pub method: Method, pub calldata: Option, pub expected: Option, @@ -48,58 +44,28 @@ pub struct ExpectedOutput { #[serde(untagged)] pub enum Calldata { Single(String), - Compound(Vec), -} - -#[derive(Clone, Debug, Deserialize, Eq, PartialEq)] -#[serde(untagged)] -pub enum CalldataArg { - Literal(String), - /// For example: `Contract.address` - AddressRef(String), + Compound(Vec), } /// Specify how the contract is called. -#[derive(Debug, Default, Clone, Eq, PartialEq)] +#[derive(Debug, Default, Deserialize, Clone, Eq, PartialEq)] pub enum Method { /// Initiate a deploy transaction, calling contracts constructor. /// /// Indicated by `#deployer`. + #[serde(rename = "#deployer")] Deployer, + /// Does not calculate and insert a function selector. /// /// Indicated by `#fallback`. #[default] + #[serde(rename = "#fallback")] Fallback, - /// Call the public function with this selector. - /// - /// Calculates the selector if neither deployer or fallback matches. - Function([u8; 4]), -} -fn deserialize_method<'de, D>(deserializer: D) -> Result -where - D: Deserializer<'de>, -{ - Ok(match String::deserialize(deserializer)?.as_str() { - "#deployer" => Method::Deployer, - "#fallback" => Method::Fallback, - signature => { - let signature = if signature.ends_with(')') { - signature.to_string() - } else { - format!("{signature}()") - }; - match Function::parse(&signature) { - Ok(function) => Method::Function(function.selector().0), - Err(error) => { - return Err(serde::de::Error::custom(format!( - "parsing function signature '{signature}' error: {error}" - ))); - } - } - } - }) + /// Call the public function with the given name. + #[serde(untagged)] + FunctionName(String), } impl Input { @@ -119,7 +85,7 @@ impl Input { deployed_abis: &HashMap, deployed_contracts: &HashMap, ) -> anyhow::Result { - let Method::Function(selector) = self.method else { + let Method::FunctionName(ref function_name) = self.method else { return Ok(Bytes::default()); // fallback or deployer — no input }; @@ -129,14 +95,17 @@ impl Input { tracing::trace!("ABI found for instance: {}", &self.instance); - // Find function by selector + // We follow the same logic that's implemented in the matter-labs-tester where they resolve + // the function name into a function selector and they assume that he function doesn't have + // any existing overloads. + // https://github.com/matter-labs/era-compiler-tester/blob/1dfa7d07cba0734ca97e24704f12dd57f6990c2c/compiler_tester/src/test/case/input/mod.rs#L158-L190 let function = abi .functions() - .find(|f| f.selector().0 == selector) + .find(|function| function.name.starts_with(function_name)) .ok_or_else(|| { anyhow::anyhow!( - "Function with selector {:?} not found in ABI for the instance {:?}", - selector, + "Function with name {:?} not found in ABI for the instance {:?}", + function_name, &self.instance ) })?; @@ -161,61 +130,28 @@ impl Input { &self.instance ); - let mut encoded = selector.to_vec(); + // Allocating a vector that we will be using for the calldata. The vector size will be: + // 4 bytes for the function selector. + // function.inputs.len() * 32 bytes for the arguments (each argument is a U256). + // + // We're using indices in the following code in order to avoid the need for us to allocate + // a new buffer for each one of the resolved arguments. + let mut calldata = Vec::::with_capacity(4 + calldata_args.len() * 32); + calldata.extend(function.selector().0); - for (i, param) in function.inputs.iter().enumerate() { - let arg = calldata_args.get(i).unwrap(); - let encoded_arg = match arg { - CalldataArg::Literal(value) => match param.ty.as_str() { - "uint256" | "uint" => { - let val: U256 = value.parse()?; - val.abi_encode() - } - "uint24" => { - let val: u32 = value.parse()?; - (val & 0xFFFFFF).abi_encode() - } - "bool" => { - let val: bool = value.parse()?; - val.abi_encode() - } - "address" => { - let addr: Address = value.parse()?; - addr.abi_encode() - } - "string" => value.abi_encode(), - "bytes32" => { - let val = hex::decode(value.trim_start_matches("0x"))?; - let mut fixed = [0u8; 32]; - fixed[..val.len()].copy_from_slice(&val); - fixed.abi_encode() - } - "uint256[]" | "uint[]" => { - let nums: Vec = serde_json::from_str(value)?; - nums.abi_encode() - } - "bytes" => { - let val = hex::decode(value.trim_start_matches("0x"))?; - val.abi_encode() - } - _ => anyhow::bail!("Unsupported type: {}", param.ty), - }, - CalldataArg::AddressRef(name) => { - let contract_name = name.trim_end_matches(".address"); - let addr = deployed_contracts - .get(contract_name) - .copied() - .ok_or_else(|| { - anyhow::anyhow!("Address for '{}' not found", contract_name) - })?; - addr.abi_encode() + for (arg_idx, arg) in calldata_args.iter().enumerate() { + match resolve_argument(arg, deployed_contracts) { + Ok(resolved) => { + calldata.extend(resolved.to_be_bytes::<32>()); + } + Err(error) => { + tracing::error!(arg, arg_idx, ?error, "Failed to resolve argument"); + return Err(error); } }; - - encoded.extend(encoded_arg); } - Ok(Bytes::from(encoded)) + Ok(calldata.into()) } /// Parse this input into a legacy transaction. @@ -244,12 +180,72 @@ fn default_caller() -> Address { "90F8bf6A479f320ead074411a4B0e7944Ea8c9C1".parse().unwrap() } +/// This function takes in the string calldata argument provided in the JSON input and resolves it +/// into a [`U256`] which is later used to construct the calldata. +/// +/// # Note +/// +/// This piece of code is taken from the matter-labs-tester repository which is licensed under MIT +/// or Apache. The original source code can be found here: +/// https://github.com/matter-labs/era-compiler-tester/blob/0ed598a27f6eceee7008deab3ff2311075a2ec69/compiler_tester/src/test/case/input/value.rs#L43-L146 +fn resolve_argument( + value: &str, + deployed_contracts: &HashMap, +) -> anyhow::Result { + if let Some(instance) = value.strip_suffix(".address") { + Ok(U256::from_be_slice( + deployed_contracts + .get(instance) + .ok_or_else(|| anyhow::anyhow!("Instance `{}` not found", instance))? + .as_ref(), + )) + } else if let Some(value) = value.strip_prefix('-') { + let value = U256::from_str_radix(value, 10) + .map_err(|error| anyhow::anyhow!("Invalid decimal literal after `-`: {}", error))?; + if value > U256::ONE << 255u8 { + anyhow::bail!("Decimal literal after `-` is too big"); + } + let value = value + .checked_sub(U256::ONE) + .ok_or_else(|| anyhow::anyhow!("`-0` is invalid literal"))?; + Ok(U256::MAX.checked_sub(value).expect("Always valid")) + } else if let Some(value) = value.strip_prefix("0x") { + Ok(U256::from_str_radix(value, 16) + .map_err(|error| anyhow::anyhow!("Invalid hexadecimal literal: {}", error))?) + } else { + // TODO: This is a set of "variables" that we need to be able to resolve to be fully in + // compliance with the matter labs tester but we currently do not resolve them. We need to + // add logic that does their resolution in the future, perhaps through some kind of system + // context API that we pass down to the resolution function that allows it to make calls to + // the node to perform these resolutions. + let is_unsupported = [ + "$CHAIN_ID", + "$GAS_LIMIT", + "$COINBASE", + "$DIFFICULTY", + "$BLOCK_HASH", + "$BLOCK_TIMESTAMP", + ] + .iter() + .any(|var| value.starts_with(var)); + + if is_unsupported { + tracing::error!(value, "Unsupported variable used"); + anyhow::bail!("Encountered {value} which is currently unsupported by the framework"); + } else { + Ok(U256::from_str_radix(value, 10) + .map_err(|error| anyhow::anyhow!("Invalid decimal literal: {}", error))?) + } + } +} + #[cfg(test)] mod tests { use super::*; use alloy::json_abi::JsonAbi; - use alloy_primitives::{address, keccak256}; + use alloy_primitives::address; + use alloy_sol_types::SolValue; use std::collections::HashMap; #[test] @@ -267,16 +263,18 @@ mod tests { "#; let parsed_abi: JsonAbi = serde_json::from_str(raw_metadata).unwrap(); - let selector = keccak256("store(uint256)".as_bytes())[0..4] - .try_into() - .unwrap(); + let selector = parsed_abi + .function("store") + .unwrap() + .first() + .unwrap() + .selector() + .0; let input = Input { instance: "Contract".to_string(), - method: Method::Function(selector), - calldata: Some(Calldata::Compound(vec![CalldataArg::Literal( - "42".to_string(), - )])), + method: Method::FunctionName("store".to_owned()), + calldata: Some(Calldata::Compound(vec!["42".into()])), ..Default::default() }; @@ -294,112 +292,6 @@ mod tests { assert_eq!(decoded.0, 42); } - #[test] - fn test_encoded_input_bool() { - let raw_abi = r#"[ - { - "inputs": [{"name": "flag", "type": "bool"}], - "name": "toggle", - "outputs": [], - "stateMutability": "nonpayable", - "type": "function" - } - ]"#; - - let parsed_abi: JsonAbi = serde_json::from_str(raw_abi).unwrap(); - let selector = keccak256("toggle(bool)".as_bytes())[0..4] - .try_into() - .unwrap(); - - let input = Input { - instance: "Contract".to_string(), - method: Method::Function(selector), - calldata: Some(Calldata::Compound(vec![CalldataArg::Literal( - "true".to_string(), - )])), - ..Default::default() - }; - - let mut abis = HashMap::new(); - abis.insert("Contract".to_string(), parsed_abi); - let contracts = HashMap::new(); - - let encoded = input.encoded_input(&abis, &contracts).unwrap(); - assert!(encoded.0.starts_with(&selector)); - - type T = (bool,); - let decoded: T = T::abi_decode(&encoded.0[4..]).unwrap(); - assert_eq!(decoded.0, true); - } - - #[test] - fn test_encoded_input_string() { - let raw_abi = r#"[ - { - "inputs": [{"name": "msg", "type": "string"}], - "name": "echo", - "outputs": [], - "stateMutability": "nonpayable", - "type": "function" - } - ]"#; - - let parsed_abi: JsonAbi = serde_json::from_str(raw_abi).unwrap(); - let selector = keccak256("echo(string)".as_bytes())[0..4] - .try_into() - .unwrap(); - - let input = Input { - instance: "Contract".to_string(), - method: Method::Function(selector), - calldata: Some(Calldata::Compound(vec![CalldataArg::Literal( - "hello".to_string(), - )])), - ..Default::default() - }; - - let mut abis = HashMap::new(); - abis.insert("Contract".to_string(), parsed_abi); - let contracts = HashMap::new(); - - let encoded = input.encoded_input(&abis, &contracts).unwrap(); - assert!(encoded.0.starts_with(&selector)); - } - - #[test] - fn test_encoded_input_uint256_array() { - let raw_abi = r#"[ - { - "inputs": [{"name": "arr", "type": "uint256[]"}], - "name": "sum", - "outputs": [], - "stateMutability": "nonpayable", - "type": "function" - } - ]"#; - - let parsed_abi: JsonAbi = serde_json::from_str(raw_abi).unwrap(); - let selector = keccak256("sum(uint256[])".as_bytes())[0..4] - .try_into() - .unwrap(); - - let input = Input { - instance: "Contract".to_string(), - method: Method::Function(selector), - calldata: Some(Calldata::Compound(vec![CalldataArg::Literal( - "[1,2,3]".to_string(), - )])), - ..Default::default() - }; - - let mut abis = HashMap::new(); - abis.insert("Contract".to_string(), parsed_abi); - let contracts = HashMap::new(); - - let encoded = input.encoded_input(&abis, &contracts).unwrap(); - assert!(encoded.0.starts_with(&selector)); - } - #[test] fn test_encoded_input_address() { let raw_abi = r#"[ @@ -413,16 +305,20 @@ mod tests { ]"#; let parsed_abi: JsonAbi = serde_json::from_str(raw_abi).unwrap(); - let selector = keccak256("send(address)".as_bytes())[0..4] - .try_into() - .unwrap(); + let selector = parsed_abi + .function("send") + .unwrap() + .first() + .unwrap() + .selector() + .0; - let input = Input { + let input: Input = Input { instance: "Contract".to_string(), - method: Method::Function(selector), - calldata: Some(Calldata::Compound(vec![CalldataArg::Literal( + method: Method::FunctionName("send".to_owned()), + calldata: Some(Calldata::Compound(vec![ "0x1000000000000000000000000000000000000001".to_string(), - )])), + ])), ..Default::default() }; From baa11ad28f48cd8c30541caf0b649880dba630a0 Mon Sep 17 00:00:00 2001 From: Omar Date: Wed, 16 Jul 2025 14:52:40 +0300 Subject: [PATCH 3/3] Correctly identify which contracts to compile (#44) * Compile all contracts for a test file * Fix compilation errors related to paths * Set the base path if specified --- crates/compiler/src/lib.rs | 12 ++-- crates/compiler/src/revive_resolc.rs | 33 ++++++++-- crates/compiler/src/solc.rs | 41 ++++++++++-- crates/core/src/driver/mod.rs | 98 ++++++++++++++++++++++++---- crates/format/src/input.rs | 11 +++- 5 files changed, 168 insertions(+), 27 deletions(-) diff --git a/crates/compiler/src/lib.rs b/crates/compiler/src/lib.rs index 7e92cf0..e43a527 100644 --- a/crates/compiler/src/lib.rs +++ b/crates/compiler/src/lib.rs @@ -44,6 +44,8 @@ pub trait SolidityCompiler { pub struct CompilerInput { pub extra_options: T, pub input: SolcStandardJsonInput, + pub allow_paths: Vec, + pub base_path: Option, } /// The generic compilation output configuration. @@ -83,8 +85,8 @@ where pub struct Compiler { input: SolcStandardJsonInput, extra_options: T::Options, - allow_paths: Vec, - base_path: Option, + allow_paths: Vec, + base_path: Option, } impl Default for Compiler { @@ -145,12 +147,12 @@ where self } - pub fn allow_path(mut self, path: String) -> Self { + pub fn allow_path(mut self, path: PathBuf) -> Self { self.allow_paths.push(path); self } - pub fn base_path(mut self, base_path: String) -> Self { + pub fn base_path(mut self, base_path: PathBuf) -> Self { self.base_path = Some(base_path); self } @@ -159,6 +161,8 @@ where T::new(solc_path).build(CompilerInput { extra_options: self.extra_options, input: self.input, + allow_paths: self.allow_paths, + base_path: self.base_path, }) } diff --git a/crates/compiler/src/revive_resolc.rs b/crates/compiler/src/revive_resolc.rs index eb63719..501a2f9 100644 --- a/crates/compiler/src/revive_resolc.rs +++ b/crates/compiler/src/revive_resolc.rs @@ -23,13 +23,27 @@ impl SolidityCompiler for Resolc { &self, input: CompilerInput, ) -> anyhow::Result> { - let mut child = Command::new(&self.resolc_path) - .arg("--standard-json") - .args(&input.extra_options) + let mut command = Command::new(&self.resolc_path); + command .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) - .spawn()?; + .arg("--standard-json"); + + if let Some(ref base_path) = input.base_path { + command.arg("--base-path").arg(base_path); + } + if !input.allow_paths.is_empty() { + command.arg("--allow-paths").arg( + input + .allow_paths + .iter() + .map(|path| path.display().to_string()) + .collect::>() + .join(","), + ); + } + let mut child = command.spawn()?; let stdin_pipe = child.stdin.as_mut().expect("stdin must be piped"); serde_json::to_writer(stdin_pipe, &input.input)?; @@ -55,13 +69,22 @@ impl SolidityCompiler for Resolc { }); } - let parsed: SolcStandardJsonOutput = serde_json::from_slice(&stdout).map_err(|e| { + let parsed = serde_json::from_slice::(&stdout).map_err(|e| { anyhow::anyhow!( "failed to parse resolc JSON output: {e}\nstderr: {}", String::from_utf8_lossy(&stderr) ) })?; + // Detecting if the compiler output contained errors and reporting them through logs and + // errors instead of returning the compiler output that might contain errors. + for error in parsed.errors.iter().flatten() { + if error.severity == "error" { + tracing::error!(?error, ?input, "Encountered an error in the compilation"); + anyhow::bail!("Encountered an error in the compilation: {error}") + } + } + Ok(CompilerOutput { input, output: parsed, diff --git a/crates/compiler/src/solc.rs b/crates/compiler/src/solc.rs index b40d18f..653dd33 100644 --- a/crates/compiler/src/solc.rs +++ b/crates/compiler/src/solc.rs @@ -9,6 +9,7 @@ use std::{ use crate::{CompilerInput, CompilerOutput, SolidityCompiler}; use revive_dt_config::Arguments; use revive_dt_solc_binaries::download_solc; +use revive_solc_json_interface::SolcStandardJsonOutput; pub struct Solc { solc_path: PathBuf, @@ -21,12 +22,27 @@ impl SolidityCompiler for Solc { &self, input: CompilerInput, ) -> anyhow::Result> { - let mut child = Command::new(&self.solc_path) + let mut command = Command::new(&self.solc_path); + command .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) - .arg("--standard-json") - .spawn()?; + .arg("--standard-json"); + + if let Some(ref base_path) = input.base_path { + command.arg("--base-path").arg(base_path); + } + if !input.allow_paths.is_empty() { + command.arg("--allow-paths").arg( + input + .allow_paths + .iter() + .map(|path| path.display().to_string()) + .collect::>() + .join(","), + ); + } + let mut child = command.spawn()?; let stdin = child.stdin.as_mut().expect("should be piped"); serde_json::to_writer(stdin, &input.input)?; @@ -42,9 +58,26 @@ impl SolidityCompiler for Solc { }); } + let parsed = + serde_json::from_slice::(&output.stdout).map_err(|e| { + anyhow::anyhow!( + "failed to parse resolc JSON output: {e}\nstderr: {}", + String::from_utf8_lossy(&output.stdout) + ) + })?; + + // Detecting if the compiler output contained errors and reporting them through logs and + // errors instead of returning the compiler output that might contain errors. + for error in parsed.errors.iter().flatten() { + if error.severity == "error" { + tracing::error!(?error, ?input, "Encountered an error in the compilation"); + anyhow::bail!("Encountered an error in the compilation: {error}") + } + } + Ok(CompilerOutput { input, - output: serde_json::from_slice(&output.stdout)?, + output: parsed, error: None, }) } diff --git a/crates/core/src/driver/mod.rs b/crates/core/src/driver/mod.rs index ba7020d..36ef0e4 100644 --- a/crates/core/src/driver/mod.rs +++ b/crates/core/src/driver/mod.rs @@ -69,14 +69,13 @@ where anyhow::bail!("unsupported solc version: {:?}", &mode.solc_version); }; - let mut compiler = Compiler::::new() - .base_path(metadata.directory()?.display().to_string()) + let compiler = Compiler::::new() + .allow_path(metadata.directory()?) .solc_optimizer(mode.solc_optimize()); - for (file, _contract) in metadata.contract_sources()?.values() { - tracing::debug!("contract source {}", file.display()); - compiler = compiler.with_source(file)?; - } + let compiler = FilesWithExtensionIterator::new(metadata.directory()?) + .with_allowed_extension("sol") + .try_fold(compiler, |compiler, path| compiler.with_source(&path))?; let mut task = CompilationTask { json_input: compiler.input(), @@ -180,12 +179,15 @@ where } pub fn deploy_contracts(&mut self, input: &Input, node: &T::Blockchain) -> anyhow::Result<()> { - tracing::debug!( - "Deploying contracts {}, having address {} on node: {}", - &input.instance, - &input.caller, - std::any::type_name::() + let tracing_span = tracing::debug_span!( + "Deploying contracts", + ?input, + node = std::any::type_name::() ); + let _guard = tracing_span.enter(); + + tracing::debug!(number_of_contracts_to_deploy = self.contracts.len()); + for output in self.contracts.values() { let Some(contract_map) = &output.contracts else { tracing::debug!( @@ -476,3 +478,77 @@ where Ok(()) } } + +/// An iterator that finds files of a certain extension in the provided directory. You can think of +/// this a glob pattern similar to: `${path}/**/*.md` +struct FilesWithExtensionIterator { + /// The set of allowed extensions that that match the requirement and that should be returned + /// when found. + allowed_extensions: std::collections::HashSet>, + + /// The set of directories to visit next. This iterator does BFS and so these directories will + /// only be visited if we can't find any files in our state. + directories_to_search: Vec, + + /// The set of files matching the allowed extensions that were found. If there are entries in + /// this vector then they will be returned when the [`Iterator::next`] method is called. If not + /// then we visit one of the next directories to visit. + /// + /// [`Iterator`]: std::iter::Iterator + files_matching_allowed_extensions: Vec, +} + +impl FilesWithExtensionIterator { + fn new(root_directory: std::path::PathBuf) -> Self { + Self { + allowed_extensions: Default::default(), + directories_to_search: vec![root_directory], + files_matching_allowed_extensions: Default::default(), + } + } + + fn with_allowed_extension( + mut self, + allowed_extension: impl Into>, + ) -> Self { + self.allowed_extensions.insert(allowed_extension.into()); + self + } +} + +impl Iterator for FilesWithExtensionIterator { + type Item = std::path::PathBuf; + + fn next(&mut self) -> Option { + if let Some(file_path) = self.files_matching_allowed_extensions.pop() { + return Some(file_path); + }; + + let directory_to_search = self.directories_to_search.pop()?; + + // Read all of the entries in the directory. If we failed to read this dir's entires then we + // elect to just ignore it and look in the next directory, we do that by calling the next + // method again on the iterator, which is an intentional decision that we made here instead + // of panicking. + let Ok(dir_entries) = std::fs::read_dir(directory_to_search) else { + return self.next(); + }; + + for entry in dir_entries.flatten() { + let entry_path = entry.path(); + if entry_path.is_dir() { + self.directories_to_search.push(entry_path) + } else if entry_path.is_file() + && entry_path.extension().is_some_and(|ext| { + self.allowed_extensions + .iter() + .any(|allowed| ext.eq_ignore_ascii_case(allowed.as_ref())) + }) + { + self.files_matching_allowed_extensions.push(entry_path) + } + } + + self.next() + } +} diff --git a/crates/format/src/input.rs b/crates/format/src/input.rs index 270d908..9275dca 100644 --- a/crates/format/src/input.rs +++ b/crates/format/src/input.rs @@ -89,9 +89,14 @@ impl Input { return Ok(Bytes::default()); // fallback or deployer — no input }; - let abi = deployed_abis - .get(&self.instance) - .ok_or_else(|| anyhow::anyhow!("ABI for instance '{}' not found", &self.instance))?; + let Some(abi) = deployed_abis.get(&self.instance) else { + tracing::error!( + contract_name = self.instance, + available_abis = ?deployed_abis.keys().collect::>(), + "Attempted to lookup ABI of contract but it wasn't found" + ); + anyhow::bail!("ABI for instance '{}' not found", &self.instance); + }; tracing::trace!("ABI found for instance: {}", &self.instance);