From b9d67e956baffad64b1c3e41993dc53fbc5aba95 Mon Sep 17 00:00:00 2001 From: sragss <65786432+sragss@users.noreply.github.com> Date: Fri, 11 Nov 2022 16:41:37 -0800 Subject: [PATCH] feat: surface revert errors to RPC (#106) * surface revert errors to RPC * remove temp generic errors from evm, node * merge resolution * cargo fmt --- Cargo.lock | 1 + client/Cargo.toml | 1 + client/src/client.rs | 14 ++++++++-- client/src/errors.rs | 59 +++++++++++++++++++++++++++++++++++++++++ client/src/lib.rs | 1 + client/src/node.rs | 51 ++++++++++++++++++++++------------- client/src/rpc.rs | 13 ++++++--- execution/src/errors.rs | 35 +++++++++++++++++++++++- execution/src/evm.rs | 56 ++++++++++++++++++-------------------- 9 files changed, 177 insertions(+), 54 deletions(-) create mode 100644 client/src/errors.rs diff --git a/Cargo.lock b/Cargo.lock index b83d96b..e308d8a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -511,6 +511,7 @@ dependencies = [ "revm", "serde", "ssz-rs", + "thiserror", "tokio", "toml", ] diff --git a/client/Cargo.toml b/client/Cargo.toml index d5cf9cb..efe0d91 100644 --- a/client/Cargo.toml +++ b/client/Cargo.toml @@ -21,6 +21,7 @@ futures = "0.3.23" toml = "0.5.9" log = "0.4.17" openssl = { version = "0.10", features = ["vendored"] } +thiserror = "1.0.37" common = { path = "../common" } consensus = { path = "../consensus" } diff --git a/client/src/client.rs b/client/src/client.rs index 21e2a95..45ef2c0 100644 --- a/client/src/client.rs +++ b/client/src/client.rs @@ -215,11 +215,21 @@ impl Client { } pub async fn call(&self, opts: &CallOpts, block: BlockTag) -> Result> { - self.node.read().await.call(opts, block).await + self.node + .read() + .await + .call(opts, block) + .await + .map_err(|err| err.into()) } pub async fn estimate_gas(&self, opts: &CallOpts) -> Result { - self.node.read().await.estimate_gas(opts).await + self.node + .read() + .await + .estimate_gas(opts) + .await + .map_err(|err| err.into()) } pub async fn get_balance(&self, address: &Address, block: BlockTag) -> Result { diff --git a/client/src/errors.rs b/client/src/errors.rs new file mode 100644 index 0000000..b0b0018 --- /dev/null +++ b/client/src/errors.rs @@ -0,0 +1,59 @@ +use common::errors::BlockNotFoundError; +use execution::errors::EvmError; +use eyre::Report; +use thiserror::Error; + +/// Errors that can occur during Node calls +#[derive(Debug, Error)] +pub enum NodeError { + #[error(transparent)] + ExecutionError(#[from] EvmError), + + #[error("out of sync: {0} slots behind")] + OutOfSync(u64), + + #[error("consensus payload error: {0}")] + ConsensusPayloadError(Report), + + #[error("execution payload error: {0}")] + ExecutionPayloadError(Report), + + #[error("consensus client creation error: {0}")] + ConsensusClientCreationError(Report), + + #[error("execution client creation error: {0}")] + ExecutionClientCreationError(Report), + + #[error("consensus advance error: {0}")] + ConsensusAdvanceError(Report), + + #[error("consensus sync error: {0}")] + ConsensusSyncError(Report), + + #[error(transparent)] + BlockNotFoundError(#[from] BlockNotFoundError), +} + +impl NodeError { + pub fn to_json_rpsee_error(self) -> jsonrpsee::core::Error { + match self { + NodeError::ExecutionError(evm_err) => match evm_err { + EvmError::Revert(data) => { + let mut msg = "execution reverted".to_string(); + if let Some(reason) = data.as_ref().and_then(EvmError::decode_revert_reason) { + msg = format!("{msg}: {reason}") + } + jsonrpsee::core::Error::Call(jsonrpsee::types::error::CallError::Custom( + jsonrpsee::types::error::ErrorObject::owned( + 3, + msg, + data.map(|data| format!("0x{}", hex::encode(data))), + ), + )) + } + _ => jsonrpsee::core::Error::Custom(evm_err.to_string()), + }, + _ => jsonrpsee::core::Error::Custom(self.to_string()), + } + } +} diff --git a/client/src/lib.rs b/client/src/lib.rs index 4f290ef..177f7c1 100644 --- a/client/src/lib.rs +++ b/client/src/lib.rs @@ -4,6 +4,7 @@ mod client; pub use crate::client::*; pub mod database; +pub mod errors; pub mod rpc; mod node; diff --git a/client/src/node.rs b/client/src/node.rs index 48c6966..b26e759 100644 --- a/client/src/node.rs +++ b/client/src/node.rs @@ -17,6 +17,8 @@ use execution::rpc::http_rpc::HttpRpc; use execution::types::{CallOpts, ExecutionBlock}; use execution::ExecutionClient; +use crate::errors::NodeError; + pub struct Node { consensus: ConsensusClient, execution: Arc>, @@ -27,13 +29,16 @@ pub struct Node { } impl Node { - pub fn new(config: Arc) -> Result { + pub fn new(config: Arc) -> Result { let consensus_rpc = &config.consensus_rpc; let checkpoint_hash = &config.checkpoint; let execution_rpc = &config.execution_rpc; - let consensus = ConsensusClient::new(consensus_rpc, checkpoint_hash, config.clone())?; - let execution = Arc::new(ExecutionClient::new(execution_rpc)?); + let consensus = ConsensusClient::new(consensus_rpc, checkpoint_hash, config.clone()) + .map_err(NodeError::ConsensusClientCreationError)?; + let execution = Arc::new( + ExecutionClient::new(execution_rpc).map_err(NodeError::ExecutionClientCreationError)?, + ); let payloads = BTreeMap::new(); let finalized_payloads = BTreeMap::new(); @@ -48,13 +53,19 @@ impl Node { }) } - pub async fn sync(&mut self) -> Result<()> { - self.consensus.sync().await?; + pub async fn sync(&mut self) -> Result<(), NodeError> { + self.consensus + .sync() + .await + .map_err(NodeError::ConsensusSyncError)?; self.update_payloads().await } - pub async fn advance(&mut self) -> Result<()> { - self.consensus.advance().await?; + pub async fn advance(&mut self) -> Result<(), NodeError> { + self.consensus + .advance() + .await + .map_err(NodeError::ConsensusAdvanceError)?; self.update_payloads().await } @@ -65,18 +76,20 @@ impl Node { .unwrap() } - async fn update_payloads(&mut self) -> Result<()> { + async fn update_payloads(&mut self) -> Result<(), NodeError> { let latest_header = self.consensus.get_header(); let latest_payload = self .consensus .get_execution_payload(&Some(latest_header.slot)) - .await?; + .await + .map_err(NodeError::ConsensusPayloadError)?; let finalized_header = self.consensus.get_finalized_header(); let finalized_payload = self .consensus .get_execution_payload(&Some(finalized_header.slot)) - .await?; + .await + .map_err(NodeError::ConsensusPayloadError)?; self.payloads .insert(latest_payload.block_number, latest_payload); @@ -98,7 +111,7 @@ impl Node { Ok(()) } - pub async fn call(&self, opts: &CallOpts, block: BlockTag) -> Result> { + pub async fn call(&self, opts: &CallOpts, block: BlockTag) -> Result, NodeError> { self.check_blocktag_age(&block)?; let payload = self.get_payload(block)?; @@ -108,10 +121,10 @@ impl Node { &self.payloads, self.chain_id(), ); - evm.call(opts).await + evm.call(opts).await.map_err(NodeError::ExecutionError) } - pub async fn estimate_gas(&self, opts: &CallOpts) -> Result { + pub async fn estimate_gas(&self, opts: &CallOpts) -> Result { self.check_head_age()?; let payload = self.get_payload(BlockTag::Latest)?; @@ -121,7 +134,9 @@ impl Node { &self.payloads, self.chain_id(), ); - evm.estimate_gas(opts).await + evm.estimate_gas(opts) + .await + .map_err(NodeError::ExecutionError) } pub async fn get_balance(&self, address: &Address, block: BlockTag) -> Result { @@ -257,7 +272,7 @@ impl Node { self.consensus.last_checkpoint.clone() } - fn get_payload(&self, block: BlockTag) -> Result<&ExecutionPayload> { + fn get_payload(&self, block: BlockTag) -> Result<&ExecutionPayload, BlockNotFoundError> { match block { BlockTag::Latest => { let payload = self.payloads.last_key_value(); @@ -276,19 +291,19 @@ impl Node { } } - fn check_head_age(&self) -> Result<()> { + fn check_head_age(&self) -> Result<(), NodeError> { let synced_slot = self.consensus.get_header().slot; let expected_slot = self.consensus.expected_current_slot(); let slot_delay = expected_slot - synced_slot; if slot_delay > 10 { - return Err(eyre!("out of sync")); + return Err(NodeError::OutOfSync(slot_delay)); } Ok(()) } - fn check_blocktag_age(&self, block: &BlockTag) -> Result<()> { + fn check_blocktag_age(&self, block: &BlockTag) -> Result<(), NodeError> { match block { BlockTag::Latest => self.check_head_age(), BlockTag::Finalized => Ok(()), diff --git a/client/src/rpc.rs b/client/src/rpc.rs index 3f21a13..21dc03b 100644 --- a/client/src/rpc.rs +++ b/client/src/rpc.rs @@ -13,7 +13,7 @@ use jsonrpsee::{ proc_macros::rpc, }; -use crate::node::Node; +use crate::{errors::NodeError, node::Node}; use common::{ types::BlockTag, @@ -134,14 +134,21 @@ impl EthRpcServer for RpcInner { async fn call(&self, opts: CallOpts, block: BlockTag) -> Result { let node = self.node.read().await; - let res = convert_err(node.call(&opts, block).await)?; + + let res = node + .call(&opts, block) + .await + .map_err(NodeError::to_json_rpsee_error)?; Ok(format!("0x{}", hex::encode(res))) } async fn estimate_gas(&self, opts: CallOpts) -> Result { let node = self.node.read().await; - let gas = convert_err(node.estimate_gas(&opts).await)?; + let gas = node + .estimate_gas(&opts) + .await + .map_err(NodeError::to_json_rpsee_error)?; Ok(u64_to_hex_string(gas)) } diff --git a/execution/src/errors.rs b/execution/src/errors.rs index 3fd7c33..65a121e 100644 --- a/execution/src/errors.rs +++ b/execution/src/errors.rs @@ -1,4 +1,9 @@ -use ethers::types::{Address, H256}; +use bytes::Bytes; +use ethers::{ + abi::AbiDecode, + types::{Address, H256}, +}; +use eyre::Report; use thiserror::Error; #[derive(Debug, Error)] @@ -14,3 +19,31 @@ pub enum ExecutionError { #[error("missing transaction for tx: {0}")] MissingTransaction(String), } + +/// Errors that can occur during evm.rs calls +#[derive(Debug, Error)] +pub enum EvmError { + #[error("execution reverted: {0:?}")] + Revert(Option), + + #[error("evm error: {0:?}")] + Generic(String), + + #[error("evm execution failed: {0:?}")] + Revm(revm::Return), + + #[error("rpc error: {0:?}")] + RpcError(Report), +} + +impl EvmError { + pub fn decode_revert_reason(data: impl AsRef<[u8]>) -> Option { + let data = data.as_ref(); + + // skip function selector + if data.len() < 4 { + return None; + } + String::decode(&data[4..]).ok() + } +} diff --git a/execution/src/evm.rs b/execution/src/evm.rs index a34b74c..02a9952 100644 --- a/execution/src/evm.rs +++ b/execution/src/evm.rs @@ -22,6 +22,7 @@ use tokio::runtime::Runtime; use consensus::types::ExecutionPayload; use crate::{ + errors::EvmError, rpc::ExecutionRpc, types::{Account, CallOpts}, }; @@ -47,7 +48,7 @@ impl<'a, R: ExecutionRpc> Evm<'a, R> { Evm { evm, chain_id } } - pub async fn call(&mut self, opts: &CallOpts) -> Result> { + pub async fn call(&mut self, opts: &CallOpts) -> Result, EvmError> { let account_map = self.batch_fetch_accounts(opts).await?; self.evm.db.as_mut().unwrap().set_accounts(account_map); @@ -55,32 +56,26 @@ impl<'a, R: ExecutionRpc> Evm<'a, R> { let tx = self.evm.transact().0; match tx.exit_reason { - revm::Return::Revert => Err(eyre::eyre!("execution reverted")), - revm::Return::OutOfGas => Err(eyre::eyre!("execution reverted: out of gas")), - revm::Return::OutOfFund => Err(eyre::eyre!("not enough funds")), - revm::Return::CallTooDeep => Err(eyre::eyre!("execution reverted: call too deep")), - revm::Return::InvalidJump => { - Err(eyre::eyre!("execution reverted: invalid jump destination")) - } - revm::Return::InvalidOpcode => Err(eyre::eyre!("execution reverted: invalid opcode")), - revm::Return::LackOfFundForGasLimit => Err(eyre::eyre!("not enough funds")), - revm::Return::GasPriceLessThenBasefee => Err(eyre::eyre!("gas price too low")), + revm::Return::Revert => match tx.out { + TransactOut::Call(bytes) => Err(EvmError::Revert(Some(bytes))), + _ => Err(EvmError::Revert(None)), + }, revm::Return::Return => { if let Some(err) = &self.evm.db.as_ref().unwrap().error { - return Err(eyre::eyre!(err.clone())); + return Err(EvmError::Generic(err.clone())); } match tx.out { - TransactOut::None => Err(eyre::eyre!("Invalid Call")), - TransactOut::Create(..) => Err(eyre::eyre!("Invalid Call")), + TransactOut::None => Err(EvmError::Generic("Invalid Call".to_string())), + TransactOut::Create(..) => Err(EvmError::Generic("Invalid Call".to_string())), TransactOut::Call(bytes) => Ok(bytes.to_vec()), } } - _ => Err(eyre::eyre!("call failed")), + _ => Err(EvmError::Revm(tx.exit_reason)), } } - pub async fn estimate_gas(&mut self, opts: &CallOpts) -> Result { + pub async fn estimate_gas(&mut self, opts: &CallOpts) -> Result { let account_map = self.batch_fetch_accounts(opts).await?; self.evm.db.as_mut().unwrap().set_accounts(account_map); @@ -89,30 +84,27 @@ impl<'a, R: ExecutionRpc> Evm<'a, R> { let gas = tx.gas_used; match tx.exit_reason { - revm::Return::Revert => Err(eyre::eyre!("execution reverted")), - revm::Return::OutOfGas => Err(eyre::eyre!("execution reverted: out of gas")), - revm::Return::OutOfFund => Err(eyre::eyre!("not enough funds")), - revm::Return::CallTooDeep => Err(eyre::eyre!("execution reverted: call too deep")), - revm::Return::InvalidJump => { - Err(eyre::eyre!("execution reverted: invalid jump destination")) - } - revm::Return::InvalidOpcode => Err(eyre::eyre!("execution reverted: invalid opcode")), - revm::Return::LackOfFundForGasLimit => Err(eyre::eyre!("not enough funds")), - revm::Return::GasPriceLessThenBasefee => Err(eyre::eyre!("gas price too low")), + revm::Return::Revert => match tx.out { + TransactOut::Call(bytes) => Err(EvmError::Revert(Some(bytes))), + _ => Err(EvmError::Revert(None)), + }, revm::Return::Return => { if let Some(err) = &self.evm.db.as_ref().unwrap().error { - return Err(eyre::eyre!(err.clone())); + return Err(EvmError::Generic(err.clone())); } // overestimate to avoid out of gas reverts let gas_scaled = (1.10 * gas as f64) as u64; Ok(gas_scaled) } - _ => Err(eyre::eyre!("call failed")), + _ => Err(EvmError::Revm(tx.exit_reason)), } } - async fn batch_fetch_accounts(&self, opts: &CallOpts) -> Result> { + async fn batch_fetch_accounts( + &self, + opts: &CallOpts, + ) -> Result, EvmError> { let db = self.evm.db.as_ref().unwrap(); let rpc = db.execution.rpc.clone(); let payload = db.current_payload.clone(); @@ -129,7 +121,11 @@ impl<'a, R: ExecutionRpc> Evm<'a, R> { }; let block_moved = block.clone(); - let mut list = rpc.create_access_list(&opts_moved, block_moved).await?.0; + let mut list = rpc + .create_access_list(&opts_moved, block_moved) + .await + .map_err(EvmError::RpcError)? + .0; let from_access_entry = AccessListItem { address: opts_moved.from.unwrap_or_default(),