feat(sim): reject UOs that don't have a buffer on verification gas li…

…mit (#599)

crates/pool/proto/op_pool/op_pool.proto

@@ -558,6 +558,7 @@ message SimulationViolationError {
     AggregatorValidationFailed aggregator_validation_failed = 16;
     UnstakedPaymasterContext unstaked_paymaster_context = 17;
     UnstakedAggregator unstaked_aggregator = 18;
+    VerificationGasLimitBufferTooLow verification_gas_limit_buffer_too_low = 19;
   }
 }
 
@@ -634,3 +635,7 @@ message CodeHashChanged {}
 
 message AggregatorValidationFailed {}
 
+message VerificationGasLimitBufferTooLow {
+  bytes limit = 1;
+  bytes needed = 2;
+}

crates/pool/src/server/remote/error.rs

@@ -32,7 +32,8 @@ use super::protos::{
     SimulationViolationError as ProtoSimulationViolationError, TotalGasLimitTooHigh,
     UnintendedRevert, UnintendedRevertWithMessage, UnknownEntryPointError, UnstakedAggregator,
     UnstakedPaymasterContext, UnsupportedAggregatorError, UsedForbiddenOpcode,
-    UsedForbiddenPrecompile, VerificationGasLimitTooHigh, WrongNumberOfPhases,
+    UsedForbiddenPrecompile, VerificationGasLimitBufferTooLow, VerificationGasLimitTooHigh,
+    WrongNumberOfPhases,
 };
 use crate::{mempool::MempoolError, server::error::PoolServerError};
 
@@ -599,6 +600,18 @@ impl From<SimulationViolation> for ProtoSimulationViolationError {
                     ),
                 ),
             },
+            SimulationViolation::VerificationGasLimitBufferTooLow(limit, needed) => {
+                ProtoSimulationViolationError {
+                    violation: Some(
+                        simulation_violation_error::Violation::VerificationGasLimitBufferTooLow(
+                            VerificationGasLimitBufferTooLow {
+                                limit: to_le_bytes(limit),
+                                needed: to_le_bytes(needed),
+                            },
+                        ),
+                    ),
+                }
+            }
         }
     }
 }
@@ -723,6 +736,12 @@ impl TryFrom<ProtoSimulationViolationError> for SimulationViolation {
             Some(simulation_violation_error::Violation::AggregatorValidationFailed(_)) => {
                 SimulationViolation::AggregatorValidationFailed
             }
+            Some(simulation_violation_error::Violation::VerificationGasLimitBufferTooLow(e)) => {
+                SimulationViolation::VerificationGasLimitBufferTooLow(
+                    from_bytes(&e.limit)?,
+                    from_bytes(&e.needed)?,
+                )
+            }
             None => {
                 bail!("unknown proto mempool simulation violation")
             }

crates/rpc/src/eth/error.rs

@@ -69,6 +69,9 @@ pub enum EthRpcError {
     /// Sender address used as different entity in another UserOperation currently in the mempool.
     #[error("The sender address {0} is used as a different entity in another UserOperation currently in mempool")]
     SenderAddressUsedAsAlternateEntity(Address),
+    /// Simulation ran out of gas
+    #[error("Simulation ran out of gas for entity: {0}")]
+    OutOfGas(Entity),
     /// Opcode violation
     #[error("{0} uses banned opcode: {1:?}")]
     OpcodeViolation(EntityType, Opcode),
@@ -293,6 +296,7 @@ impl From<SimulationViolation> for EthRpcError {
                 )))
             }
             SimulationViolation::AggregatorValidationFailed => Self::SignatureCheckFailed,
+            SimulationViolation::OutOfGas(entity) => Self::OutOfGas(entity),
             _ => Self::SimulationFailed(value),
         }
     }
@@ -305,15 +309,15 @@ impl From<EthRpcError> for ErrorObjectOwned {
         match error {
             EthRpcError::Internal(_) => rpc_err(INTERNAL_ERROR_CODE, msg),
             EthRpcError::InvalidParams(_) => rpc_err(INVALID_PARAMS_CODE, msg),
-            EthRpcError::EntryPointValidationRejected(_) => {
+            EthRpcError::EntryPointValidationRejected(_) | EthRpcError::SimulationFailed(_) => {
                 rpc_err(ENTRYPOINT_VALIDATION_REJECTED_CODE, msg)
             }
             EthRpcError::PaymasterValidationRejected(data) => {
                 rpc_err_with_data(PAYMASTER_VALIDATION_REJECTED_CODE, msg, data)
             }
             EthRpcError::OpcodeViolation(_, _)
             | EthRpcError::OpcodeViolationMap(_)
-            | EthRpcError::SimulationFailed(_)
+            | EthRpcError::OutOfGas(_)
             | EthRpcError::UnstakedAggregator
             | EthRpcError::MultipleRolesViolation(_)
             | EthRpcError::UnstakedPaymasterContext

crates/sim/src/estimation/estimation.rs

@@ -39,7 +39,7 @@ use rundler_utils::{eth, math};
 use tokio::join;
 
 use super::types::{GasEstimate, Settings, UserOperationOptionalGas};
-use crate::{gas, precheck::MIN_CALL_GAS_LIMIT, utils, FeeEstimator};
+use crate::{gas, precheck::MIN_CALL_GAS_LIMIT, simulation, utils, FeeEstimator};
 
 /// Gas estimates will be rounded up to the next multiple of this. Increasing
 /// this value reduces the number of rounds of `eth_call` needed in binary
@@ -150,13 +150,17 @@ impl<P: Provider, E: EntryPoint> GasEstimator for GasEstimatorImpl<P, E> {
             return Err(GasEstimationError::RevertInValidation(err));
         }
 
+        // Add a buffer to the verification gas limit. Add 10% or 2000 gas, whichever is larger
+        // to ensure we get at least a 2000 gas buffer. Cap at the max verification gas.
+        let verification_gas_limit = cmp::max(
+            math::increase_by_percent(verification_gas_limit, VERIFICATION_GAS_BUFFER_PERCENT),
+            verification_gas_limit + simulation::REQUIRED_VERIFICATION_GAS_LIMIT_BUFFER,
+        )
+        .min(settings.max_verification_gas.into());
+
         Ok(GasEstimate {
             pre_verification_gas,
-            verification_gas_limit: math::increase_by_percent(
-                verification_gas_limit,
-                VERIFICATION_GAS_BUFFER_PERCENT,
-            )
-            .min(settings.max_verification_gas.into()),
+            verification_gas_limit,
             call_gas_limit: call_gas_limit.clamp(MIN_CALL_GAS_LIMIT, settings.max_call_gas.into()),
         })
     }
@@ -1210,7 +1214,10 @@ mod tests {
         // gas used increased by 10%
         assert_eq!(
             estimation.verification_gas_limit,
-            math::increase_by_percent(gas_usage, 10)
+            cmp::max(
+                math::increase_by_percent(gas_usage, 10),
+                gas_usage + simulation::REQUIRED_VERIFICATION_GAS_LIMIT_BUFFER
+            )
         );
 
         // input gas limit clamped with the set limit in settings and constant MIN

crates/sim/src/simulation/mod.rs

@@ -15,6 +15,7 @@
 mod simulation;
 #[cfg(feature = "test-utils")]
 pub use simulation::MockSimulator;
+pub(crate) use simulation::REQUIRED_VERIFICATION_GAS_LIMIT_BUFFER;
 pub use simulation::{
     EntityInfo, EntityInfos, NeedsStakeInformation, Settings, SimulationError, SimulationResult,
     SimulationViolation, Simulator, SimulatorImpl, ViolationOpCode,

crates/sim/src/simulation/simulation.rs

@@ -47,6 +47,9 @@ use crate::{
     utils,
 };
 
+/// Required buffer for verification gas limit when targeting the 0.6 entrypoint contract
+pub(crate) const REQUIRED_VERIFICATION_GAS_LIMIT_BUFFER: U256 = U256([2000, 0, 0, 0]);
+
 /// The result of a successful simulation
 #[derive(Clone, Debug, Default)]
 pub struct SimulationResult {
@@ -278,15 +281,17 @@ where
 
         let associated_addresses = tracer_out.associated_slots_by_address.addresses();
 
+        let initcode_length = op.init_code.len();
         Ok(ValidationContext {
+            op,
             block_id,
             entity_infos,
             tracer_out,
             entry_point_out,
             associated_addresses,
             entities_needing_stake: vec![],
             accessed_addresses: HashSet::new(),
-            initcode_length: op.init_code.len(),
+            initcode_length,
         })
     }
 
@@ -315,6 +320,7 @@ where
         context: &mut ValidationContext,
     ) -> anyhow::Result<Vec<SimulationViolation>> {
         let &mut ValidationContext {
+            ref op,
             ref entity_infos,
             ref tracer_out,
             ref entry_point_out,
@@ -331,8 +337,7 @@ where
         }
 
         let sender_address = entity_infos.sender_address();
-        let mut entity_types_needing_stake: HashMap<Entity, (Address, Option<EntityType>, U256)> =
-            HashMap::new();
+        let mut entity_types_needing_stake = HashMap::new();
 
         for (index, phase) in tracer_out.phases.iter().enumerate().take(3) {
             let kind = entity_type_from_simulation_phase(index).unwrap();
@@ -420,7 +425,6 @@ where
                 violations.push(SimulationViolation::CalledBannedEntryPointMethod(entity));
             }
 
-            // These violations are not allowlistable but we need to collect them here
             if phase.ran_out_of_gas {
                 violations.push(SimulationViolation::OutOfGas(entity));
             }
@@ -470,6 +474,23 @@ where
             }
         }
 
+        // This is a special case to cover a bug in the 0.6 entrypoint contract where a specially
+        // crafted UO can use extra verification gas that isn't caught during simulation, but when
+        // it runs on chain causes the transaction to revert.
+        let verification_gas_used = entry_point_out
+            .return_info
+            .pre_op_gas
+            .saturating_sub(op.pre_verification_gas);
+        let verification_buffer = op
+            .verification_gas_limit
+            .saturating_sub(verification_gas_used);
+        if verification_buffer < REQUIRED_VERIFICATION_GAS_LIMIT_BUFFER {
+            violations.push(SimulationViolation::VerificationGasLimitBufferTooLow(
+                op.verification_gas_limit,
+                verification_gas_used + REQUIRED_VERIFICATION_GAS_LIMIT_BUFFER,
+            ));
+        }
+
         Ok(violations)
     }
 
@@ -693,6 +714,9 @@ pub enum SimulationViolation {
     /// The user operation aggregator signature validation failed
     #[display("aggregator signature validation failed")]
     AggregatorValidationFailed,
+    /// Verification gas limit doesn't have the required buffer on the measured gas
+    #[display("verification gas limit doesn't have the required buffer on the measured gas, limit: {0}, needed: {1}")]
+    VerificationGasLimitBufferTooLow(U256, U256),
 }
 
 /// A wrapper around Opcode that implements extra traits
@@ -726,6 +750,7 @@ fn entity_type_from_simulation_phase(i: usize) -> Option<EntityType> {
 
 #[derive(Debug)]
 struct ValidationContext {
+    op: UserOperation,
     block_id: BlockId,
     entity_infos: EntityInfos,
     tracer_out: SimulationTracerOutput,
@@ -1271,6 +1296,11 @@ mod tests {
         );
 
         let mut validation_context = ValidationContext {
+            op: UserOperation {
+                verification_gas_limit: U256::from(2000),
+                pre_verification_gas: U256::from(1000),
+                ..Default::default()
+            },
             initcode_length: 10,
             associated_addresses: HashSet::new(),
             block_id: BlockId::Number(BlockNumber::Latest),
@@ -1297,7 +1327,7 @@ mod tests {
             tracer_out: tracer_output,
             entry_point_out: ValidationOutput {
                 return_info: ValidationReturnInfo::from((
-                    U256::default(),
+                    3000.into(),
                     U256::default(),
                     true,
                     0,
@@ -1362,6 +1392,7 @@ mod tests {
                         .unwrap()
                     }
                 ),
+                SimulationViolation::VerificationGasLimitBufferTooLow(2000.into(), 4000.into())
             ]
         );
     }