Merge pull request #138 from Vid201/fix/call_gas_limit

Fix call gas limit

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<M: Middleware> {
     provider: Arc<M>,
@@ -193,31 +194,48 @@ impl<M: Middleware + 'static> EntryPoint<M> {
         }
     }
 
-    pub async fn estimate_call_gas<U: Into<UserOperation>>(
+    pub async fn simulate_execution<U: Into<UserOperation>>(
         &self,
         user_operation: U,
-    ) -> Result<U256, EntryPointErr> {
-        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::<M>(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::<M>(e)),
+        }
+    }
+
+    pub async fn simulate_handle_op<U: Into<UserOperation>>(
+        &self,
+        user_operation: U,
+    ) -> Result<ExecutionResult, EntryPointErr> {
+        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:?}"
+                ))),
+            }),
         }
     }
 
@@ -276,7 +294,7 @@ impl EntryPointErr {
             return EntryPointErr::from_provider_err(provider_err);
         }
 
-        EntryPointErr::UnknownErr(format!("Unknown middlerware error: {value:?}"))
+        EntryPointErr::UnknownErr(format!("Unknown middleware error: {value:?}"))
     }
 }
 

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,

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;
 

crates/uopool/src/canonical/sanity_check.rs

@@ -1,6 +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,
@@ -9,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,
 };
 
@@ -38,7 +39,7 @@ pub enum BadUserOperationError<M: Middleware> {
     },
     LowCallGasLimit {
         call_gas_limit: U256,
-        call_gas_estimation: U256,
+        call_gas_limit_estimation: U256,
     },
     LowMaxFeePerGas {
         max_fee_per_gas: U256,
@@ -55,7 +56,7 @@ pub enum BadUserOperationError<M: Middleware> {
     SenderVerification {
         sender: Address,
     },
-    UserOperationExecution {
+    SimulateHandleOp {
         message: String,
     },
     Middleware(M::Error),
@@ -112,11 +113,11 @@ impl<M: Middleware> From<BadUserOperationError<M>> 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::<bool>,
             ),
@@ -155,9 +156,9 @@ impl<M: Middleware> From<BadUserOperationError<M>> for SanityCheckError {
                 format!("Sender {sender} is invalid (sender check)",),
                 None::<bool>,
             ),
-            BadUserOperationError::UserOperationExecution { message } => {
+            BadUserOperationError::SimulateHandleOp { message } => {
                 SanityCheckError::owned(
-                    EXECUTION_ERROR_CODE,
+                    SIMULATION_ERROR_CODE,
                     message,
                     None::<bool>,
                 )
@@ -274,21 +275,54 @@ impl<M: Middleware + 'static> UoPool<M> {
         &self,
         user_operation: &UserOperation,
     ) -> Result<(), BadUserOperationError<M>> {
-        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(|error| BadUserOperationError::UnknownError {
-                error: format!("{error:?}"),
+            .map_err(|entry_point_error| match entry_point_error {
+                EntryPointErr::FailedOp(failed_op) => BadUserOperationError::SimulateHandleOp {
+                    message: failed_op.reason,
+                },
+                _ => BadUserOperationError::UnknownError {
+                    error: "unknown error".to_string(),
+                },
             })?;
 
-        if user_operation.call_gas_limit >= call_gas_estimation {
+        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(),
+            });
+        };
+
+        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,
         })
     }
 
@@ -425,12 +459,12 @@ impl<M: Middleware + 'static> UoPool<M> {
         // 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?;
 
@@ -459,6 +493,7 @@ mod tests {
 
     use super::*;
 
+    #[ignore]
     #[tokio::test]
     async fn user_operation_sanity_check() {
         let entry_point = "0x5FF137D4b0FDCD49DcA30c7CF57E578a026d2789"
@@ -490,8 +525,9 @@ 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(),
@@ -507,6 +543,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)