diff --git a/crates/pool/proto/op_pool/op_pool.proto b/crates/pool/proto/op_pool/op_pool.proto index 629fecee9..d6f4fdea1 100644 --- a/crates/pool/proto/op_pool/op_pool.proto +++ b/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; +} diff --git a/crates/pool/src/server/remote/error.rs b/crates/pool/src/server/remote/error.rs index 3d115be84..8f3b732a5 100644 --- a/crates/pool/src/server/remote/error.rs +++ b/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 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 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") } diff --git a/crates/rpc/src/eth/error.rs b/crates/rpc/src/eth/error.rs index 5b77fc716..77dde57b4 100644 --- a/crates/rpc/src/eth/error.rs +++ b/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 for EthRpcError { ))) } SimulationViolation::AggregatorValidationFailed => Self::SignatureCheckFailed, + SimulationViolation::OutOfGas(entity) => Self::OutOfGas(entity), _ => Self::SimulationFailed(value), } } @@ -305,7 +309,7 @@ impl From 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) => { @@ -313,7 +317,7 @@ impl From for ErrorObjectOwned { } EthRpcError::OpcodeViolation(_, _) | EthRpcError::OpcodeViolationMap(_) - | EthRpcError::SimulationFailed(_) + | EthRpcError::OutOfGas(_) | EthRpcError::UnstakedAggregator | EthRpcError::MultipleRolesViolation(_) | EthRpcError::UnstakedPaymasterContext diff --git a/crates/sim/src/estimation/estimation.rs b/crates/sim/src/estimation/estimation.rs index 8dafd5513..a6388b14e 100644 --- a/crates/sim/src/estimation/estimation.rs +++ b/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 GasEstimator for GasEstimatorImpl { 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 diff --git a/crates/sim/src/simulation/mod.rs b/crates/sim/src/simulation/mod.rs index 79347ecda..6915b0b11 100644 --- a/crates/sim/src/simulation/mod.rs +++ b/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, diff --git a/crates/sim/src/simulation/simulation.rs b/crates/sim/src/simulation/simulation.rs index 19cb56c0c..28178c8d4 100644 --- a/crates/sim/src/simulation/simulation.rs +++ b/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,7 +281,9 @@ 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, @@ -286,7 +291,7 @@ where 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> { 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, 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 { #[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()) ] ); }