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

@@ -511,6 +511,7 @@ dependencies = [
  "revm",
  "serde",
  "ssz-rs",
+ "thiserror",
  "tokio",
  "toml",
 ]

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" }

client/src/client.rs

@@ -215,11 +215,21 @@ impl<DB: Database> Client<DB> {
     }
 
     pub async fn call(&self, opts: &CallOpts, block: BlockTag) -> Result<Vec<u8>> {
-        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<u64> {
-        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<U256> {

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()),
+        }
+    }
+}

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;

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<NimbusRpc>,
     execution: Arc<ExecutionClient<HttpRpc>>,
@@ -27,13 +29,16 @@ pub struct Node {
 }
 
 impl Node {
-    pub fn new(config: Arc<Config>) -> Result<Self> {
+    pub fn new(config: Arc<Config>) -> Result<Self, NodeError> {
         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 consensus = ConsensusClient::new(consensus_rpc, checkpoint_hash, config.clone())
-        let execution = Arc::new(ExecutionClient::new(execution_rpc)?);
+            .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<()> {
+    pub async fn sync(&mut self) -> Result<(), NodeError> {
-        self.consensus.sync().await?;
+        self.consensus
+            .sync()
+            .await
+            .map_err(NodeError::ConsensusSyncError)?;
         self.update_payloads().await
     }
 
-    pub async fn advance(&mut self) -> Result<()> {
+    pub async fn advance(&mut self) -> Result<(), NodeError> {
-        self.consensus.advance().await?;
+        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<Vec<u8>> {
+    pub async fn call(&self, opts: &CallOpts, block: BlockTag) -> Result<Vec<u8>, 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<u64> {
+    pub async fn estimate_gas(&self, opts: &CallOpts) -> Result<u64, NodeError> {
         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<U256> {
@@ -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(()),

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<String, Error> {
         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<String, Error> {
         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))
     }

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<Bytes>),
+
+    #[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<String> {
+        let data = data.as_ref();
+
+        // skip function selector
+        if data.len() < 4 {
+            return None;
+        }
+        String::decode(&data[4..]).ok()
+    }
+}

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<Vec<u8>> {
+    pub async fn call(&mut self, opts: &CallOpts) -> Result<Vec<u8>, 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::Revert => match tx.out {
-            revm::Return::OutOfGas => Err(eyre::eyre!("execution reverted: out of gas")),
+                TransactOut::Call(bytes) => Err(EvmError::Revert(Some(bytes))),
-            revm::Return::OutOfFund => Err(eyre::eyre!("not enough funds")),
+                _ => Err(EvmError::Revert(None)),
-            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::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::None => Err(EvmError::Generic("Invalid Call".to_string())),
-                    TransactOut::Create(..) => Err(eyre::eyre!("Invalid Call")),
+                    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<u64> {
+    pub async fn estimate_gas(&mut self, opts: &CallOpts) -> Result<u64, EvmError> {
         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::Revert => match tx.out {
-            revm::Return::OutOfGas => Err(eyre::eyre!("execution reverted: out of gas")),
+                TransactOut::Call(bytes) => Err(EvmError::Revert(Some(bytes))),
-            revm::Return::OutOfFund => Err(eyre::eyre!("not enough funds")),
+                _ => Err(EvmError::Revert(None)),
-            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::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<HashMap<Address, Account>> {
+    async fn batch_fetch_accounts(
+        &self,
+        opts: &CallOpts,
+    ) -> Result<HashMap<Address, Account>, 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(),