From 71425b3b6d89430d3874a385282de41aae8a60f4 Mon Sep 17 00:00:00 2001 From: Vid Kersic Date: Sat, 4 Mar 2023 18:31:34 -0700 Subject: [PATCH 1/5] fix: change RPC error string, fix call gas limit check --- src/types/simulation.rs | 4 +++- src/uopool/services/sanity_check.rs | 27 +++++++++++++++------------ 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/types/simulation.rs b/src/types/simulation.rs index ae765856..bffb25df 100644 --- a/src/types/simulation.rs +++ b/src/types/simulation.rs @@ -36,6 +36,8 @@ lazy_static! { set.insert("CREATE".to_string()); set.insert("COINBASE".to_string()); set.insert("SELFDESTRUCT".to_string()); + set.insert("RANDOM".to_string()); + set.insert("PREVRANDAO".to_string()); set }; } @@ -55,7 +57,7 @@ impl From> for SimulationError { } SimulateValidationError::OpcodeValidation { entity, opcode } => SimulationError::owned( OPCODE_VALIDATION_ERROR_CODE, - format!("{entity} uses opcode {opcode}"), + format!("{entity} uses banned opcode: {opcode}"), None::, ), SimulateValidationError::Middleware(_) => { diff --git a/src/uopool/services/sanity_check.rs b/src/uopool/services/sanity_check.rs index fd68e3b2..5b040ac1 100644 --- a/src/uopool/services/sanity_check.rs +++ b/src/uopool/services/sanity_check.rs @@ -116,18 +116,21 @@ where user_operation: &UserOperation, entry_point: &Address, ) -> Result<(), BadUserOperationError> { - let call_gas_estimation = self - .eth_provider - .estimate_gas( - &TransactionRequest::new() - .from(*entry_point) - .to(user_operation.sender) - .data(user_operation.call_data.clone()) - .into(), - None, - ) - .await - .map_err(|error| BadUserOperationError::Middleware(error))?; + let call_gas_estimation = if user_operation.call_data.is_empty() { + U256::zero() + } else { + self.eth_provider + .estimate_gas( + &TransactionRequest::new() + .from(*entry_point) + .to(user_operation.sender) + .data(user_operation.call_data.clone()) + .into(), + None, + ) + .await + .map_err(|error| BadUserOperationError::Middleware(error))? + }; if user_operation.call_gas_limit < call_gas_estimation { return Err(BadUserOperationError::LowCallGasLimit { From f19db08f9214ca5d8c92ed06ae646d2a8e803e8c Mon Sep 17 00:00:00 2001 From: Vid Kersic Date: Sat, 4 Mar 2023 18:33:21 -0700 Subject: [PATCH 2/5] chore: remove .gitkeep --- bundler-spec-test/keys/.gitkeep | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 bundler-spec-test/keys/.gitkeep diff --git a/bundler-spec-test/keys/.gitkeep b/bundler-spec-test/keys/.gitkeep deleted file mode 100644 index e69de29b..00000000 From 3656f2fe14abb937bcb128f9974b2e8649964090 Mon Sep 17 00:00:00 2001 From: Vid Kersic Date: Sun, 5 Mar 2023 00:28:38 -0700 Subject: [PATCH 3/5] feat: add check for opcode CREATE2 --- src/types/simulation.rs | 2 ++ src/uopool/services/simulation.rs | 12 +++++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/types/simulation.rs b/src/types/simulation.rs index bffb25df..e7aabde8 100644 --- a/src/types/simulation.rs +++ b/src/types/simulation.rs @@ -40,6 +40,8 @@ lazy_static! { set.insert("PREVRANDAO".to_string()); set }; + + pub static ref CREATE2_OPCODE: String = "CREATE2".to_string(); } #[derive(Debug)] diff --git a/src/uopool/services/simulation.rs b/src/uopool/services/simulation.rs index 1fe465eb..084034f8 100644 --- a/src/uopool/services/simulation.rs +++ b/src/uopool/services/simulation.rs @@ -6,7 +6,7 @@ use ethers::{ use crate::{ contracts::{tracer::JsTracerFrame, EntryPointErr, SimulateValidationResult}, types::{ - simulation::{SimulateValidationError, FORBIDDEN_OPCODES, LEVEL_TO_ENTITY}, + simulation::{SimulateValidationError, CREATE2_OPCODE, FORBIDDEN_OPCODES, LEVEL_TO_ENTITY}, user_operation::UserOperation, }, uopool::mempool_id, @@ -117,6 +117,16 @@ where }); } } + + if let Some(count) = trace.number_levels[index].opcodes.get(&*CREATE2_OPCODE) { + if LEVEL_TO_ENTITY[&index] == "factory" && *count == 1 { + continue; + } + return Err(SimulateValidationError::OpcodeValidation { + entity: LEVEL_TO_ENTITY[&index].to_string(), + opcode: CREATE2_OPCODE.to_string(), + }); + } } Ok(()) From 0b70237170836b3e886755bc23c110ce94dbb481 Mon Sep 17 00:00:00 2001 From: Vid Kersic Date: Sun, 5 Mar 2023 18:42:35 -0700 Subject: [PATCH 4/5] fix: RPC eth_estimateUserOperationGas --- src/contracts/entrypoint.rs | 41 +++++++++++- src/rpc/eth.rs | 5 +- src/types/sanity_check.rs | 25 +++++++- src/types/simulation.rs | 9 +++ src/types/user_operation.rs | 1 + src/uopool/mod.rs | 6 +- src/uopool/services/sanity_check.rs | 40 +++++++----- src/uopool/services/simulation.rs | 4 +- src/uopool/services/uopool.rs | 97 +++++++++++++++++------------ 9 files changed, 164 insertions(+), 64 deletions(-) diff --git a/src/contracts/entrypoint.rs b/src/contracts/entrypoint.rs index bced854f..4f64d483 100644 --- a/src/contracts/entrypoint.rs +++ b/src/contracts/entrypoint.rs @@ -6,7 +6,7 @@ use ethers::abi::AbiDecode; use ethers::providers::{FromErr, Middleware, ProviderError}; use ethers::types::{ Address, Bytes, GethDebugTracerType, GethDebugTracingCallOptions, GethDebugTracingOptions, - GethTrace, + GethTrace, TransactionRequest, U256, }; use regex::Regex; use serde::Deserialize; @@ -199,6 +199,44 @@ where } } + pub async fn estimate_call_gas>( + &self, + user_operation: U, + ) -> Result> { + let user_operation = user_operation.into(); + + if user_operation.call_data.is_empty() { + Ok(U256::zero()) + } else { + let result = self + .provider + .estimate_gas( + &TransactionRequest::new() + .from(self.address) + .to(user_operation.sender) + .data(user_operation.call_data.clone()) + .into(), + None, + ) + .await; + + match result { + Ok(gas) => Ok(gas), + Err(e) => { + let err_msg = e.to_string(); + + JsonRpcError::from_str(&err_msg) + .map_err(|_| { + EntryPointErr::DecodeErr(format!( + "{err_msg:?} is not a valid JsonRpcError message" + )) + }) + .and_then(|json_error| Err(EntryPointErr::JsonRpcError(json_error))) + } + } + } + } + pub async fn handle_aggregated_ops>( &self, _ops_per_aggregator: Vec, @@ -213,6 +251,7 @@ pub enum EntryPointErr { FailedOp(FailedOp), ProviderErr(ProviderError), MiddlewareErr(M::Error), + JsonRpcError(JsonRpcError), NetworkErr, // TODO DecodeErr(String), UnknownErr(String), // describe impossible error. We should fix the codes here(or contract codes) if this occurs. diff --git a/src/rpc/eth.rs b/src/rpc/eth.rs index be10ace3..ad1f63f8 100644 --- a/src/rpc/eth.rs +++ b/src/rpc/eth.rs @@ -113,8 +113,9 @@ impl EthApiServer for EthApiServerImpl { return Ok(user_operation_gas_estimation); } - Err(jsonrpsee::core::Error::Call(CallError::Failed( - anyhow::format_err!("failed to gas estimation for user operation"), + Err(jsonrpsee::core::Error::Call(CallError::Custom( + serde_json::from_str::(&response.data) + .map_err(|err| format_err!("error parsing error object: {}", err))?, ))) } diff --git a/src/types/sanity_check.rs b/src/types/sanity_check.rs index 33b90ef3..27d61e7d 100644 --- a/src/types/sanity_check.rs +++ b/src/types/sanity_check.rs @@ -5,6 +5,7 @@ use ethers::{ use jsonrpsee::types::{error::ErrorCode, ErrorObject}; const SANITY_CHECK_ERROR_CODE: i32 = -32602; +const SANITY_CHECK_EXECUTION_ERROR_CODE: i32 = -32521; pub type SanityCheckError = ErrorObject<'static>; @@ -47,7 +48,13 @@ pub enum BadUserOperationError { SenderVerification { sender: Address, }, + UserOperationExecution { + message: String, + }, Middleware(M::Error), + UnknownError { + error: String, + }, } impl From> for SanityCheckError { @@ -141,7 +148,23 @@ impl From> for SanityCheckError { format!("Sender {sender} is invalid (sender check)",), None::, ), - BadUserOperationError::Middleware(_) => SanityCheckError::from(ErrorCode::InternalError), + BadUserOperationError::UserOperationExecution { message } => { + SanityCheckError::owned( + SANITY_CHECK_EXECUTION_ERROR_CODE, + message, + None::, + ) + }, + BadUserOperationError::Middleware(_) => { + SanityCheckError::from(ErrorCode::InternalError) + }, + BadUserOperationError::UnknownError { error } => { + SanityCheckError::owned( + SANITY_CHECK_ERROR_CODE, + error, + None::, + ) + }, } } } diff --git a/src/types/simulation.rs b/src/types/simulation.rs index e7aabde8..03ade3e0 100644 --- a/src/types/simulation.rs +++ b/src/types/simulation.rs @@ -5,6 +5,7 @@ use std::collections::{HashMap, HashSet}; const SIMULATE_VALIDATION_ERROR_CODE: i32 = -32500; const OPCODE_VALIDATION_ERROR_CODE: i32 = -32502; +const SIMULATION_EXECUTION_ERROR_CODE: i32 = -32521; pub type SimulationError = ErrorObject<'static>; @@ -48,7 +49,9 @@ lazy_static! { pub enum SimulateValidationError { UserOperationRejected { message: String }, OpcodeValidation { entity: String, opcode: String }, + UserOperationExecution { message: String }, Middleware(M::Error), + UnknownError { error: String }, } impl From> for SimulationError { @@ -62,9 +65,15 @@ impl From> for SimulationError { format!("{entity} uses banned opcode: {opcode}"), None::, ), + SimulateValidationError::UserOperationExecution { message } => { + SimulationError::owned(SIMULATION_EXECUTION_ERROR_CODE, message, None::) + } SimulateValidationError::Middleware(_) => { SimulationError::from(ErrorCode::InternalError) } + SimulateValidationError::UnknownError { error } => { + SimulationError::owned(SIMULATE_VALIDATION_ERROR_CODE, error, None::) + } } } } diff --git a/src/types/user_operation.rs b/src/types/user_operation.rs index d1e7f48f..9649860a 100644 --- a/src/types/user_operation.rs +++ b/src/types/user_operation.rs @@ -248,6 +248,7 @@ impl From for UserOperation { #[serde(rename_all = "camelCase")] pub struct UserOperationGasEstimation { pub pre_verification_gas: U256, + #[serde(rename = "verificationGas")] pub verification_gas_limit: U256, pub call_gas_limit: U256, } diff --git a/src/uopool/mod.rs b/src/uopool/mod.rs index 4e52c2a8..91dc8c07 100644 --- a/src/uopool/mod.rs +++ b/src/uopool/mod.rs @@ -20,7 +20,7 @@ use ethers::{ abi::AbiEncode, providers::{Http, Middleware, Provider}, types::{Address, H256, U256}, - utils::keccak256, + utils::{keccak256, to_checksum}, }; use jsonrpsee::tracing::info; use parking_lot::RwLock; @@ -38,7 +38,9 @@ pub type MempoolBox = Box = Box + Send + Sync>; pub fn mempool_id(entry_point: &Address, chain_id: &U256) -> MempoolId { - H256::from_slice(keccak256([entry_point.encode(), chain_id.encode()].concat()).as_slice()) + H256::from_slice( + keccak256([to_checksum(entry_point, None).encode(), chain_id.encode()].concat()).as_slice(), + ) } pub trait Mempool: Debug { diff --git a/src/uopool/services/sanity_check.rs b/src/uopool/services/sanity_check.rs index 5b040ac1..d5ea93bc 100644 --- a/src/uopool/services/sanity_check.rs +++ b/src/uopool/services/sanity_check.rs @@ -10,7 +10,7 @@ use crate::{ }; use ethers::{ providers::Middleware, - types::{Address, TransactionRequest, U256}, + types::{Address, U256}, }; impl UoPoolService @@ -116,30 +116,36 @@ where user_operation: &UserOperation, entry_point: &Address, ) -> Result<(), BadUserOperationError> { - let call_gas_estimation = if user_operation.call_data.is_empty() { - U256::zero() - } else { - self.eth_provider - .estimate_gas( - &TransactionRequest::new() - .from(*entry_point) - .to(user_operation.sender) - .data(user_operation.call_data.clone()) - .into(), - None, - ) + let mempool_id = mempool_id(entry_point, &self.chain_id); + + if let Some(entry_point) = self.entry_points.get(&mempool_id) { + let call_gas_estimation = entry_point + .estimate_call_gas(user_operation.clone()) .await - .map_err(|error| BadUserOperationError::Middleware(error))? - }; + .map_err(|error| match error { + EntryPointErr::JsonRpcError(err) => { + BadUserOperationError::UserOperationExecution { + message: err.message, + } + } + _ => BadUserOperationError::UnknownError { + error: format!("{:?}", error), + }, + })?; + + if user_operation.call_gas_limit >= call_gas_estimation { + return Ok(()); + } - if user_operation.call_gas_limit < call_gas_estimation { return Err(BadUserOperationError::LowCallGasLimit { call_gas_limit: user_operation.call_gas_limit, call_gas_estimation, }); } - Ok(()) + Err(BadUserOperationError::UnknownError { + error: format!("Unknown entry point: {}", entry_point), + }) } async fn max_fee_per_gas( diff --git a/src/uopool/services/simulation.rs b/src/uopool/services/simulation.rs index 084034f8..2915ec71 100644 --- a/src/uopool/services/simulation.rs +++ b/src/uopool/services/simulation.rs @@ -136,7 +136,7 @@ where &self, user_operation: &UserOperation, entry_point: &Address, - ) -> Result<(), SimulateValidationError> { + ) -> Result> { let simulate_validation_result = self .simulate_validation(user_operation, entry_point) .await?; @@ -155,6 +155,6 @@ where self.forbidden_opcodes(&simulate_validation_result, &js_trace) .await?; - Ok(()) + Ok(simulate_validation_result) } } diff --git a/src/uopool/services/uopool.rs b/src/uopool/services/uopool.rs index ae37a54c..ece79e75 100644 --- a/src/uopool/services/uopool.rs +++ b/src/uopool/services/uopool.rs @@ -3,6 +3,7 @@ use crate::{ contracts::{EntryPoint, EntryPointErr, SimulateValidationResult}, types::{ reputation::ReputationEntry, + simulation::{SimulateValidationError, SimulationError}, user_operation::{UserOperation, UserOperationGasEstimation}, }, uopool::{ @@ -21,7 +22,7 @@ use crate::{ use async_trait::async_trait; use ethers::{ providers::Middleware, - types::{Address, TransactionRequest, U256}, + types::{Address, U256}, }; use jsonrpsee::types::ErrorObject; use parking_lot::RwLock; @@ -188,46 +189,64 @@ where let mempool_id = mempool_id(&entry_point, &self.chain_id); if let Some(entry_point) = self.entry_points.get(&mempool_id) { - // TODO: simulation - - let pre_verification_gas = - Overhead::default().calculate_pre_verification_gas(&user_operation); - - let simulate_validation_response = entry_point - .simulate_validation(user_operation.clone()) + match self + .simulate_user_operation(&user_operation, &entry_point.address()) .await - .map_err(|_| tonic::Status::internal("error estimating user operation gas"))?; - - let verification_gas_limit = match simulate_validation_response { - SimulateValidationResult::ValidationResult(validation_result) => { - validation_result.return_info.0 + { + Ok(simulate_validation_result) => { + let pre_verification_gas = + Overhead::default().calculate_pre_verification_gas(&user_operation); + + let verification_gas_limit = match simulate_validation_result { + SimulateValidationResult::ValidationResult(validation_result) => { + validation_result.return_info.0 + } + SimulateValidationResult::ValidationResultWithAggregation( + validation_result_with_aggregation, + ) => validation_result_with_aggregation.return_info.0, + }; + + match entry_point.estimate_call_gas(user_operation.clone()).await { + Ok(call_gas_limit) => { + res.set_result(EstimateUserOperationGasResult::Estimated); + res.data = serde_json::to_string(&UserOperationGasEstimation { + pre_verification_gas, + verification_gas_limit, + call_gas_limit, + }) + .map_err(|_| { + tonic::Status::internal("error estimating user operation gas") + })?; + } + Err(error) => { + res.set_result(EstimateUserOperationGasResult::NotEstimated); + res.data = + serde_json::to_string(&SimulationError::from(match error { + EntryPointErr::JsonRpcError(err) => { + SimulateValidationError::::UserOperationExecution { + message: err.message, + } + } + _ => SimulateValidationError::UnknownError { + error: format!("{:?}", error), + }, + })) + .map_err(|_| { + tonic::Status::internal( + "error estimating user operation gas", + ) + })?; + } + } } - SimulateValidationResult::ValidationResultWithAggregation( - validation_result_with_aggregation, - ) => validation_result_with_aggregation.return_info.0, - }; - - let call_gas_limit = self - .eth_provider - .estimate_gas( - &TransactionRequest::new() - .from(entry_point.address()) - .to(user_operation.sender) - .data(user_operation.call_data.clone()) - .into(), - None, - ) - .await - .map_err(|_| tonic::Status::internal("error estimating user operation gas"))?; - - res.set_result(EstimateUserOperationGasResult::Estimated); - res.data = serde_json::to_string(&UserOperationGasEstimation { - pre_verification_gas, - verification_gas_limit, - call_gas_limit, - }) - .map_err(|_| tonic::Status::internal("error estimating user operation gas"))?; - + Err(error) => { + res.set_result(EstimateUserOperationGasResult::NotEstimated); + res.data = + serde_json::to_string(&SimulationError::from(error)).map_err(|_| { + tonic::Status::internal("error estimating user operation gas") + })?; + } + } return Ok(tonic::Response::new(res)); } else { return Err(tonic::Status::invalid_argument("entry point not supported")); From 21c1df28b9b392b5f2699c20e2c5e724da3d0009 Mon Sep 17 00:00:00 2001 From: Vid Kersic Date: Sun, 5 Mar 2023 18:54:57 -0700 Subject: [PATCH 5/5] chore: fix lint --- src/uopool/services/sanity_check.rs | 4 ++-- src/uopool/services/uopool.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/uopool/services/sanity_check.rs b/src/uopool/services/sanity_check.rs index d5ea93bc..2070283c 100644 --- a/src/uopool/services/sanity_check.rs +++ b/src/uopool/services/sanity_check.rs @@ -129,7 +129,7 @@ where } } _ => BadUserOperationError::UnknownError { - error: format!("{:?}", error), + error: format!("{error:?}"), }, })?; @@ -144,7 +144,7 @@ where } Err(BadUserOperationError::UnknownError { - error: format!("Unknown entry point: {}", entry_point), + error: format!("Unknown entry point: {entry_point}"), }) } diff --git a/src/uopool/services/uopool.rs b/src/uopool/services/uopool.rs index ece79e75..30573ceb 100644 --- a/src/uopool/services/uopool.rs +++ b/src/uopool/services/uopool.rs @@ -228,7 +228,7 @@ where } } _ => SimulateValidationError::UnknownError { - error: format!("{:?}", error), + error: format!("{error:?}"), }, })) .map_err(|_| {