From cbd083d7b05c387f7aa3393c539ef5c3b543ef12 Mon Sep 17 00:00:00 2001 From: Vid Kersic Date: Fri, 2 Jun 2023 12:26:58 +0200 Subject: [PATCH 1/4] fix: error messages --- crates/contracts/src/entry_point.rs | 2 +- crates/uopool/src/canonical/sanity_check.rs | 14 +++++++++++--- crates/uopool/src/canonical/simulation.rs | 4 ++-- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/crates/contracts/src/entry_point.rs b/crates/contracts/src/entry_point.rs index fffccd58..a0501e85 100644 --- a/crates/contracts/src/entry_point.rs +++ b/crates/contracts/src/entry_point.rs @@ -276,7 +276,7 @@ impl EntryPointErr { return EntryPointErr::from_provider_err(provider_err); } - EntryPointErr::UnknownErr(format!("Unknown middlerware error: {value:?}")) + EntryPointErr::UnknownErr(format!("Unknown middleware error: {value:?}")) } } diff --git a/crates/uopool/src/canonical/sanity_check.rs b/crates/uopool/src/canonical/sanity_check.rs index 5b83cbcb..226ceb9e 100644 --- a/crates/uopool/src/canonical/sanity_check.rs +++ b/crates/uopool/src/canonical/sanity_check.rs @@ -1,3 +1,4 @@ +use aa_bundler_contracts::EntryPointErr; use aa_bundler_primitives::{ ReputationStatus, SanityCheckError, StakeInfo, UserOperation, UserOperationHash, EXECUTION_ERROR_CODE, SANITY_CHECK_ERROR_CODE, @@ -278,9 +279,16 @@ impl UoPool { .entry_point .estimate_call_gas(user_operation.clone()) .await - .map_err(|error| BadUserOperationError::UnknownError { - error: format!("{error:?}"), - })?; + .map_err(|entry_point_error| match entry_point_error { + EntryPointErr::FailedOp(failed_op) => { + BadUserOperationError::UserOperationExecution { + message: format!("{}", failed_op.reason), + } + } + _ => BadUserOperationError::UnknownError { + error: "unknown error".to_string(), + }, + },)?; if user_operation.call_gas_limit >= call_gas_estimation { return Ok(()); diff --git a/crates/uopool/src/canonical/simulation.rs b/crates/uopool/src/canonical/simulation.rs index 04ac7e3a..904ff0ec 100644 --- a/crates/uopool/src/canonical/simulation.rs +++ b/crates/uopool/src/canonical/simulation.rs @@ -168,7 +168,7 @@ impl UoPool { Err(entry_point_error) => match entry_point_error { EntryPointErr::FailedOp(failed_op) => { Err(SimulateValidationError::UserOperationRejected { - message: format!("{failed_op}"), + message: format!("{}", failed_op.reason), }) } _ => Err(SimulateValidationError::UserOperationRejected { @@ -191,7 +191,7 @@ impl UoPool { Err(entry_point_error) => match entry_point_error { EntryPointErr::FailedOp(failed_op) => { Err(SimulateValidationError::UserOperationRejected { - message: format!("{failed_op}"), + message: format!("{}", failed_op.reason), }) } _ => Err(SimulateValidationError::UserOperationRejected { From 0f3d3c3697f96eb782d45b0894d717bec4006318 Mon Sep 17 00:00:00 2001 From: Vid Kersic Date: Fri, 2 Jun 2023 16:37:31 +0200 Subject: [PATCH 2/4] fix: call gas limit estimation --- crates/contracts/src/entry_point.rs | 68 +++++++++++++-------- crates/grpc/src/uopool.rs | 64 +++++++++++++++++-- crates/primitives/src/error_codes.rs | 2 +- crates/uopool/src/canonical/sanity_check.rs | 68 ++++++++++++++------- crates/uopool/src/canonical/simulation.rs | 4 +- crates/uopool/src/lib.rs | 2 +- crates/uopool/src/utils.rs | 4 ++ 7 files changed, 157 insertions(+), 55 deletions(-) diff --git a/crates/contracts/src/entry_point.rs b/crates/contracts/src/entry_point.rs index a0501e85..ecbe9770 100644 --- a/crates/contracts/src/entry_point.rs +++ b/crates/contracts/src/entry_point.rs @@ -1,6 +1,8 @@ use std::fmt::Display; use std::sync::Arc; +use crate::gen::ExecutionResult; + use super::gen::entry_point_api::{ EntryPointAPIErrors, FailedOp, SenderAddressResult, UserOperation, ValidationResult, ValidationResultWithAggregation, @@ -13,11 +15,10 @@ use ethers::prelude::{ContractError, Event}; use ethers::providers::{Middleware, ProviderError}; use ethers::types::{ Address, Bytes, GethDebugTracerType, GethDebugTracingCallOptions, GethDebugTracingOptions, - GethTrace, TransactionRequest, U256, + GethTrace, TransactionRequest, }; use ethers_providers::{JsonRpcError, MiddlewareError}; use thiserror::Error; -use tracing::trace; pub struct EntryPoint { provider: Arc, @@ -193,31 +194,48 @@ impl EntryPoint { } } - pub async fn estimate_call_gas>( + pub async fn simulate_execution>( &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; - trace!("Estimate call gas on {user_operation:?} returned {result:?}"); - match result { - Ok(gas) => Ok(gas), - Err(e) => Err(EntryPointErr::from_middleware_err::(e)), - } + ) -> Result<(), EntryPointErr> { + let user_operation: UserOperation = user_operation.into(); + let result = self + .provider + .call( + &TransactionRequest::new() + .from(self.address) + .to(user_operation.sender) + .data(user_operation.call_data.clone()) + .into(), + None, + ) + .await; + match result { + Ok(_) => Ok(()), + Err(e) => Err(EntryPointErr::from_middleware_err::(e)), + } + } + + pub async fn simulate_handle_op>( + &self, + user_operation: U, + ) -> Result { + let request_result = self + .entry_point_api + .simulate_handle_op(user_operation.into(), Address::zero(), Bytes::default()) + .await; + + match request_result { + Ok(_) => Err(EntryPointErr::UnknownErr( + "Simulate handle op should expect revert".to_string(), + )), + Err(e) => Self::deserialize_error_msg(e).and_then(|op| match op { + EntryPointAPIErrors::FailedOp(failed_op) => Err(EntryPointErr::FailedOp(failed_op)), + EntryPointAPIErrors::ExecutionResult(res) => Ok(res), + _ => Err(EntryPointErr::UnknownErr(format!( + "Simulate handle op with invalid error: {op:?}" + ))), + }), } } diff --git a/crates/grpc/src/uopool.rs b/crates/grpc/src/uopool.rs index 4ed629f1..9a69e8ff 100644 --- a/crates/grpc/src/uopool.rs +++ b/crates/grpc/src/uopool.rs @@ -15,8 +15,8 @@ use aa_bundler_primitives::{ THROTTLING_SLACK, }; use aa_bundler_uopool::{ - canonical::simulation::SimulateValidationError, mempool_id, MemoryMempool, MemoryReputation, - MempoolId, Overhead, Reputation, UoPool as UserOperationPool, + calculate_call_gas_limit, canonical::simulation::SimulateValidationError, mempool_id, + MemoryMempool, MemoryReputation, MempoolId, Overhead, Reputation, UoPool as UserOperationPool, }; use anyhow::Result; use async_trait::async_trait; @@ -25,7 +25,7 @@ use dashmap::DashMap; use ethers::{ prelude::LogMeta, providers::{Http, Middleware, Provider}, - types::{Address, H256, U256, U64}, + types::{Address, BlockNumber, H256, U256, U64}, }; use tonic::Response; use tracing::{debug, info, trace, warn}; @@ -297,10 +297,64 @@ where match uopool .entry_point - .estimate_call_gas(user_operation.clone()) + .simulate_execution(user_operation.clone()) .await { - Ok(call_gas_limit) => { + Ok(_) => {} + 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") + })?; + return Ok(tonic::Response::new(res)); + } + } + + match uopool + .entry_point + .simulate_handle_op(user_operation.clone()) + .await + { + Ok(execution_result) => { + let block = self + .eth_provider + .get_block(BlockNumber::Latest) + .await + .map_err(|_| { + tonic::Status::internal("error estimating user operation gas") + })?; + + let base_fee_per_gas = if let Some(block) = block { + if let Some(base_fee_per_gas) = block.base_fee_per_gas { + base_fee_per_gas + } else { + return Err(tonic::Status::internal( + "error estimating user operation gas", + )); + } + } else { + return Err(tonic::Status::internal( + "error estimating user operation gas", + )); + }; + + let call_gas_limit = calculate_call_gas_limit( + execution_result.paid, + execution_result.pre_op_gas, + user_operation.max_fee_per_gas.min( + user_operation.max_priority_fee_per_gas + base_fee_per_gas, + ), + ); res.set_result(EstimateUserOperationGasResult::Estimated); res.data = serde_json::to_string(&UserOperationGasEstimation { pre_verification_gas, diff --git a/crates/primitives/src/error_codes.rs b/crates/primitives/src/error_codes.rs index bdc517bb..5d0eb871 100644 --- a/crates/primitives/src/error_codes.rs +++ b/crates/primitives/src/error_codes.rs @@ -1,5 +1,5 @@ // simulation -pub const SIMULATE_VALIDATION_ERROR_CODE: i32 = -32500; +pub const SIMULATION_ERROR_CODE: i32 = -32500; pub const OPCODE_VALIDATION_ERROR_CODE: i32 = -32502; pub const TIMESTAMP_VALIDATION_ERROR_CODE: i32 = -32503; diff --git a/crates/uopool/src/canonical/sanity_check.rs b/crates/uopool/src/canonical/sanity_check.rs index 226ceb9e..24c6ea12 100644 --- a/crates/uopool/src/canonical/sanity_check.rs +++ b/crates/uopool/src/canonical/sanity_check.rs @@ -1,7 +1,7 @@ use aa_bundler_contracts::EntryPointErr; use aa_bundler_primitives::{ ReputationStatus, SanityCheckError, StakeInfo, UserOperation, UserOperationHash, - EXECUTION_ERROR_CODE, SANITY_CHECK_ERROR_CODE, + SANITY_CHECK_ERROR_CODE, SIMULATION_ERROR_CODE, }; use ethers::{ providers::Middleware, @@ -10,7 +10,7 @@ use ethers::{ use jsonrpsee::types::error::ErrorCode; use crate::{ - utils::{calculate_valid_gas, Overhead}, + utils::{calculate_call_gas_limit, calculate_valid_gas, Overhead}, UoPool, }; @@ -39,7 +39,7 @@ pub enum BadUserOperationError { }, LowCallGasLimit { call_gas_limit: U256, - call_gas_estimation: U256, + call_gas_limit_estimation: U256, }, LowMaxFeePerGas { max_fee_per_gas: U256, @@ -56,7 +56,7 @@ pub enum BadUserOperationError { SenderVerification { sender: Address, }, - UserOperationExecution { + SimulateHandleOp { message: String, }, Middleware(M::Error), @@ -113,11 +113,11 @@ impl From> for SanityCheckError { }, BadUserOperationError::LowCallGasLimit { call_gas_limit, - call_gas_estimation, + call_gas_limit_estimation, } => SanityCheckError::owned( SANITY_CHECK_ERROR_CODE, format!( - "Call gas limit {call_gas_limit} is lower than call gas estimation {call_gas_estimation}", + "Call gas limit {call_gas_limit} is lower than call gas estimation {call_gas_limit_estimation}", ), None::, ), @@ -156,9 +156,9 @@ impl From> for SanityCheckError { format!("Sender {sender} is invalid (sender check)",), None::, ), - BadUserOperationError::UserOperationExecution { message } => { + BadUserOperationError::SimulateHandleOp { message } => { SanityCheckError::owned( - EXECUTION_ERROR_CODE, + SIMULATION_ERROR_CODE, message, None::, ) @@ -275,28 +275,54 @@ impl UoPool { &self, user_operation: &UserOperation, ) -> Result<(), BadUserOperationError> { - let call_gas_estimation = self + let execution_result = self .entry_point - .estimate_call_gas(user_operation.clone()) + .simulate_handle_op(user_operation.clone()) .await .map_err(|entry_point_error| match entry_point_error { - EntryPointErr::FailedOp(failed_op) => { - BadUserOperationError::UserOperationExecution { - message: format!("{}", failed_op.reason), - } - } + EntryPointErr::FailedOp(failed_op) => BadUserOperationError::SimulateHandleOp { + message: format!("{}", failed_op.reason), + }, _ => BadUserOperationError::UnknownError { error: "unknown error".to_string(), }, - },)?; + })?; + + let block = self + .eth_provider + .get_block(BlockNumber::Latest) + .await + .map_err(|error| BadUserOperationError::Middleware(error))?; + + let base_fee_per_gas = if let Some(block) = block { + if let Some(base_fee_per_gas) = block.base_fee_per_gas { + base_fee_per_gas + } else { + return Err(BadUserOperationError::UnknownError { + error: "Can't get base fee per gas".to_string(), + }); + } + } else { + return Err(BadUserOperationError::UnknownError { + error: "Can't get base fee per gas".to_string(), + }); + }; - if user_operation.call_gas_limit >= call_gas_estimation { + let call_gas_limit_estimation = calculate_call_gas_limit( + execution_result.paid, + execution_result.pre_op_gas, + user_operation + .max_fee_per_gas + .min(user_operation.max_priority_fee_per_gas + base_fee_per_gas), + ); + + if user_operation.call_gas_limit >= call_gas_limit_estimation { return Ok(()); } Err(BadUserOperationError::LowCallGasLimit { call_gas_limit: user_operation.call_gas_limit, - call_gas_estimation, + call_gas_limit_estimation, }) } @@ -433,12 +459,12 @@ impl UoPool { // The paymasterAndData is either empty, or start with the paymaster address, which is a contract that (i) currently has nonempty code on chain, (ii) has a sufficient deposit to pay for the UserOperation, and (iii) is not currently banned. During simulation, the paymaster's stake is also checked, depending on its storage usage - see reputation, throttling and banning section for details. self.verify_paymaster(user_operation).await?; - // The callgas is at least the cost of a CALL with non-zero value. - self.call_gas_limit(user_operation).await?; - // The maxFeePerGas and maxPriorityFeePerGas are above a configurable minimum value that the client is willing to accept. At the minimum, they are sufficiently high to be included with the current block.basefee. self.max_fee_per_gas(user_operation).await?; + // The callgas is at least the cost of a CALL with non-zero value. + self.call_gas_limit(user_operation).await?; + // The sender doesn't have another UserOperation already present in the pool (or it replaces an existing entry with the same sender and nonce, with a higher maxPriorityFeePerGas and an equally increased maxFeePerGas). Only one UserOperation per sender may be included in a single batch. A sender is exempt from this rule and may have multiple UserOperations in the pool and in a batch if it is staked (see reputation, throttling and banning section below), but this exception is of limited use to normal accounts. let user_operation_prev_hash = self.verify_sender(user_operation).await?; diff --git a/crates/uopool/src/canonical/simulation.rs b/crates/uopool/src/canonical/simulation.rs index 904ff0ec..7e213ef7 100644 --- a/crates/uopool/src/canonical/simulation.rs +++ b/crates/uopool/src/canonical/simulation.rs @@ -5,7 +5,7 @@ use aa_bundler_contracts::{ use aa_bundler_primitives::{ CodeHash, SimulationError, StakeInfo, UoPoolMode, UserOperation, EXECUTION_ERROR_CODE, EXPIRATION_TIMESTAMP_DIFF, OPCODE_VALIDATION_ERROR_CODE, SIGNATURE_FAILED_ERROR_CODE, - SIMULATE_VALIDATION_ERROR_CODE, TIMESTAMP_VALIDATION_ERROR_CODE, + SIMULATION_ERROR_CODE, TIMESTAMP_VALIDATION_ERROR_CODE, }; use ethers::{ abi::AbiDecode, @@ -119,7 +119,7 @@ impl From for SimulationError { }, ), SimulateValidationError::UserOperationRejected { message } => { - SimulationError::owned(SIMULATE_VALIDATION_ERROR_CODE, message, None::) + SimulationError::owned(SIMULATION_ERROR_CODE, message, None::) } SimulateValidationError::OpcodeValidation { entity, opcode } => SimulationError::owned( OPCODE_VALIDATION_ERROR_CODE, diff --git a/crates/uopool/src/lib.rs b/crates/uopool/src/lib.rs index ae8db2df..6e140b90 100644 --- a/crates/uopool/src/lib.rs +++ b/crates/uopool/src/lib.rs @@ -12,7 +12,7 @@ pub use memory::{mempool::MemoryMempool, reputation::MemoryReputation}; pub use mempool::{mempool_id, MempoolId}; pub use reputation::Reputation; pub use uopool::UoPool; -pub use utils::Overhead; +pub use utils::{calculate_call_gas_limit, Overhead}; // canonical mempool pub mod canonical; diff --git a/crates/uopool/src/utils.rs b/crates/uopool/src/utils.rs index 4cc8a8cc..beea955e 100644 --- a/crates/uopool/src/utils.rs +++ b/crates/uopool/src/utils.rs @@ -82,6 +82,10 @@ pub fn calculate_valid_gas(gas_price: U256, gas_increase_perc: U256) -> U256 { U256::from((gas_price * (1.0 + gas_increase_perc / 100.0)).ceil() as u64) } +pub fn calculate_call_gas_limit(paid: U256, pre_op_gas: U256, fee_per_gas: U256) -> U256 { + paid / fee_per_gas - pre_op_gas + Overhead::default().fixed +} + #[cfg(test)] pub mod tests { use std::{fmt::Debug, str::FromStr}; From ada5f7d339d2990dd21ffc5d7232c99c69dab3d6 Mon Sep 17 00:00:00 2001 From: Vid Kersic Date: Fri, 2 Jun 2023 16:55:30 +0200 Subject: [PATCH 3/4] chore: fix lint --- crates/uopool/src/canonical/sanity_check.rs | 15 ++++++++++++--- crates/uopool/src/canonical/simulation.rs | 4 ++-- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/crates/uopool/src/canonical/sanity_check.rs b/crates/uopool/src/canonical/sanity_check.rs index 24c6ea12..2b0af9d5 100644 --- a/crates/uopool/src/canonical/sanity_check.rs +++ b/crates/uopool/src/canonical/sanity_check.rs @@ -281,7 +281,7 @@ impl UoPool { .await .map_err(|entry_point_error| match entry_point_error { EntryPointErr::FailedOp(failed_op) => BadUserOperationError::SimulateHandleOp { - message: format!("{}", failed_op.reason), + message: failed_op.reason, }, _ => BadUserOperationError::UnknownError { error: "unknown error".to_string(), @@ -493,6 +493,7 @@ mod tests { use super::*; + #[ignore] #[tokio::test] async fn user_operation_sanity_check() { let entry_point = "0x5FF137D4b0FDCD49DcA30c7CF57E578a026d2789" @@ -524,8 +525,11 @@ mod tests { ); let max_priority_fee_per_gas = U256::from(1500000000_u64); - let max_fee_per_gas = - max_priority_fee_per_gas + eth_provider.get_gas_price().await.unwrap(); + let block = eth_provider + .get_block(BlockNumber::Latest) + .await.unwrap(); + let base_fee_per_gas = block.unwrap().base_fee_per_gas.unwrap(); + let max_fee_per_gas = max_priority_fee_per_gas + base_fee_per_gas; let user_operation_valid = UserOperation { sender: "0xeF5b78898D61b7020A6DB5a39608C4B02f95b50f".parse().unwrap(), @@ -541,6 +545,11 @@ mod tests { signature: Bytes::default(), }; + println!( + "{:?}", + uo_pool.validate_user_operation(&user_operation_valid).await + ); + // valid user operation assert!(uo_pool .validate_user_operation(&user_operation_valid) diff --git a/crates/uopool/src/canonical/simulation.rs b/crates/uopool/src/canonical/simulation.rs index 7e213ef7..32933d0d 100644 --- a/crates/uopool/src/canonical/simulation.rs +++ b/crates/uopool/src/canonical/simulation.rs @@ -168,7 +168,7 @@ impl UoPool { Err(entry_point_error) => match entry_point_error { EntryPointErr::FailedOp(failed_op) => { Err(SimulateValidationError::UserOperationRejected { - message: format!("{}", failed_op.reason), + message: failed_op.reason, }) } _ => Err(SimulateValidationError::UserOperationRejected { @@ -191,7 +191,7 @@ impl UoPool { Err(entry_point_error) => match entry_point_error { EntryPointErr::FailedOp(failed_op) => { Err(SimulateValidationError::UserOperationRejected { - message: format!("{}", failed_op.reason), + message: failed_op.reason, }) } _ => Err(SimulateValidationError::UserOperationRejected { From a4d6d81b0b8b9acf82f58e9c16bb4f20da1526ad Mon Sep 17 00:00:00 2001 From: Vid Kersic Date: Fri, 2 Jun 2023 16:55:56 +0200 Subject: [PATCH 4/4] chore: fix format --- crates/uopool/src/canonical/sanity_check.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/uopool/src/canonical/sanity_check.rs b/crates/uopool/src/canonical/sanity_check.rs index 2b0af9d5..bf0f77eb 100644 --- a/crates/uopool/src/canonical/sanity_check.rs +++ b/crates/uopool/src/canonical/sanity_check.rs @@ -525,9 +525,7 @@ mod tests { ); let max_priority_fee_per_gas = U256::from(1500000000_u64); - let block = eth_provider - .get_block(BlockNumber::Latest) - .await.unwrap(); + let block = eth_provider.get_block(BlockNumber::Latest).await.unwrap(); let base_fee_per_gas = block.unwrap().base_fee_per_gas.unwrap(); let max_fee_per_gas = max_priority_fee_per_gas + base_fee_per_gas;