fix(builder): add two phase UO gas limit filtering

crates/builder/src/bundle_proposer.rs

@@ -30,9 +30,7 @@ use mockall::automock;
 use rundler_pool::{PoolOperation, PoolServer};
 use rundler_provider::{EntryPoint, HandleOpsOut, Provider};
 use rundler_sim::{
-    gas::{
-        self, user_operation_gas_limit, user_operation_pre_verification_gas_limit, GasOverheads,
-    },
+    gas::{self, GasOverheads},
     ExpectedStorage, FeeEstimator, PriorityFeeMode, SimulationError, SimulationSuccess, Simulator,
 };
 use rundler_types::{Entity, EntityType, GasFees, Timestamp, UserOperation, UserOpsPerAggregator};
@@ -132,7 +130,9 @@ where
             self.builder_index,
             ops.len(),
         );
-        let (ops, gas_limit) = self.limit_gas_in_bundle(ops);
+
+        // Do an initial filtering of ops that we want to simulate.
+        let (ops, gas_limit) = self.limit_user_operations_for_simulation(ops);
         tracing::debug!(
             "Builder index: {}, bundle proposal after limit had {} ops and {:?} gas limit",
             self.builder_index,
@@ -189,17 +189,19 @@ where
             .await;
         while !context.is_empty() {
             let gas_estimate = self.estimate_gas_rejecting_failed_ops(&mut context).await?;
-            let mut expected_storage = ExpectedStorage::default();
-            for op in context.iter_ops_with_simulations() {
-                expected_storage.merge(&op.simulation.expected_storage)?;
-            }
             if let Some(gas_estimate) = gas_estimate {
                 tracing::debug!(
                     "Builder index: {}, bundle proposal succeeded with {} ops and {:?} gas limit",
                     self.builder_index,
                     context.iter_ops().count(),
                     gas_estimate
                 );
+
+                let mut expected_storage = ExpectedStorage::default();
+                for op in context.iter_ops_with_simulations() {
+                    expected_storage.merge(&op.simulation.expected_storage)?;
+                }
+
                 return Ok(Bundle {
                     ops_per_aggregator: context.to_ops_per_aggregator(),
                     gas_estimate,
@@ -284,6 +286,9 @@ where
         let mut groups_by_aggregator = LinkedHashMap::<Option<Address>, AggregatorGroup>::new();
         let mut rejected_ops = Vec::<UserOperation>::new();
         let mut paymasters_to_reject = Vec::<Address>::new();
+
+        let ov = GasOverheads::default();
+        let mut gas_spent = ov.transaction_gas_overhead;
         for (op, simulation) in ops_with_simulations {
             let simulation = match simulation {
                 Ok(simulation) => simulation,
@@ -314,6 +319,18 @@ where
                 continue;
             }
 
+            // Skip this op if the bundle does not have enough remaining gas to execute it.
+            let required_gas = get_gas_required_for_op(
+                gas_spent,
+                self.settings.chain_id,
+                ov,
+                &op,
+                simulation.requires_post_op,
+            );
+            if required_gas > self.settings.max_bundle_gas.into() {
+                continue;
+            }
+
             if let Some(&other_sender) = simulation
                 .accessed_addresses
                 .iter()
@@ -343,6 +360,15 @@ where
                     *balance -= max_cost;
                 }
             }
+
+            // Update the running gas that would need to be be spent to execute the bundle so far.
+            gas_spent += gas::user_operation_gas_limit(
+                &op,
+                self.settings.chain_id,
+                false,
+                simulation.requires_post_op,
+            );
+
             groups_by_aggregator
                 .entry(simulation.aggregator_address())
                 .or_default()
@@ -535,11 +561,17 @@ where
         Ok(())
     }
 
-    fn limit_gas_in_bundle(&self, ops: Vec<PoolOperation>) -> (Vec<PoolOperation>, u64) {
-        let mut gas_left = U256::from(self.settings.max_bundle_gas);
+    fn limit_user_operations_for_simulation(
+        &self,
+        ops: Vec<PoolOperation>,
+    ) -> (Vec<PoolOperation>, u64) {
+        // Make the bundle gas limit 10% higher here so that we simulate more UOs than we need in case that we end up dropping some UOs later so we can still pack a full bundle
+        let mut gas_left = math::increase_by_percent(U256::from(self.settings.max_bundle_gas), 10);
         let mut ops_in_bundle = Vec::new();
         for op in ops {
-            let gas = gas::user_operation_gas_limit(&op.uo, self.settings.chain_id, false, None);
+            // Here we use optimistic gas limits for the UOs by assuming none of the paymaster UOs use postOp calls.
+            // This way after simulation once we have determined if each UO actually uses a postOp call or not we can still pack a full bundle
+            let gas = gas::user_operation_gas_limit(&op.uo, self.settings.chain_id, false, false);
             if gas_left < gas {
                 self.emit(BuilderEvent::skipped_op(
                     self.builder_index,
@@ -741,30 +773,26 @@ impl ProposalContext {
 
     fn get_bundle_gas_limit(&self, chain_id: u64) -> U256 {
         let ov = GasOverheads::default();
+        let mut gas_spent = ov.transaction_gas_overhead;
         let mut max_gas = U256::zero();
-        let mut gas_spent = U256::zero();
         for op_with_sim in self.iter_ops_with_simulations() {
             let op = &op_with_sim.op;
-            let post_exec_req_gas = if op_with_sim.simulation.requires_post_op {
-                cmp::max(op.verification_gas_limit, ov.bundle_transaction_gas_buffer)
-            } else {
-                ov.bundle_transaction_gas_buffer
-            };
-            let required_gas = gas_spent
-                + user_operation_pre_verification_gas_limit(op, chain_id, false)
-                + op.verification_gas_limit * 2
-                + op.call_gas_limit
-                + post_exec_req_gas;
-            max_gas = cmp::max(required_gas, max_gas);
-            gas_spent += user_operation_gas_limit(
+            let required_gas = get_gas_required_for_op(
+                gas_spent,
+                chain_id,
+                ov,
+                op,
+                op_with_sim.simulation.requires_post_op,
+            );
+            max_gas = cmp::max(max_gas, required_gas);
+            gas_spent += gas::user_operation_gas_limit(
                 op,
                 chain_id,
                 false,
-                Some(op_with_sim.simulation.requires_post_op),
+                op_with_sim.simulation.requires_post_op,
             );
         }
-
-        max_gas + ov.transaction_gas_overhead
+        max_gas
     }
 
     fn iter_ops_with_simulations(&self) -> impl Iterator<Item = &OpWithSimulation> + '_ {
@@ -778,6 +806,26 @@ impl ProposalContext {
     }
 }
 
+fn get_gas_required_for_op(
+    gas_spent: U256,
+    chain_id: u64,
+    ov: GasOverheads,
+    op: &UserOperation,
+    requires_post_op: bool,
+) -> U256 {
+    let post_exec_req_gas = if requires_post_op {
+        cmp::max(op.verification_gas_limit, ov.bundle_transaction_gas_buffer)
+    } else {
+        ov.bundle_transaction_gas_buffer
+    };
+
+    gas_spent
+        + gas::user_operation_pre_verification_gas_limit(op, chain_id, false)
+        + op.verification_gas_limit * 2
+        + op.call_gas_limit
+        + post_exec_req_gas
+}
+
 #[cfg(test)]
 mod tests {
     use anyhow::anyhow;
@@ -1178,7 +1226,7 @@ mod tests {
     #[tokio::test]
     async fn test_bundle_gas_limit_simple() {
         let op1 = op_with_sender_call_gas_limit(address(1), U256::from(5_000_000));
-        let op2 = op_with_sender_call_gas_limit(address(2), U256::from(5_000_000));
+        let op2 = op_with_sender_call_gas_limit(address(2), U256::from(4_000_000));
         let op3 = op_with_sender_call_gas_limit(address(3), U256::from(10_000_000));
         let op4 = op_with_sender_call_gas_limit(address(4), U256::from(10_000_000));
         let deposit = parse_units("1", "ether").unwrap().into();
@@ -1222,7 +1270,7 @@ mod tests {
         assert_eq!(
             bundle.gas_estimate,
             U256::from(math::increase_by_percent(
-                10_000_000 + 5_000 + 21_000,
+                9_000_000 + 5_000 + 21_000,
                 BUNDLE_TRANSACTION_GAS_OVERHEAD_PERCENT
             ))
         );

crates/sim/src/gas/gas.rs

@@ -99,12 +99,12 @@ pub fn user_operation_gas_limit(
     uo: &UserOperation,
     chain_id: u64,
     assume_single_op_bundle: bool,
-    paymaster_post_op: Option<bool>,
+    paymaster_post_op: bool,
 ) -> U256 {
     user_operation_pre_verification_gas_limit(uo, chain_id, assume_single_op_bundle)
         + uo.call_gas_limit
         + uo.verification_gas_limit
-            * verification_gas_limit_multiplier(uo, assume_single_op_bundle, paymaster_post_op)
+            * verification_gas_limit_multiplier(assume_single_op_bundle, paymaster_post_op)
 }
 
 /// Returns the static pre-verification gas cost of a user operation
@@ -157,14 +157,13 @@ fn calc_static_pre_verification_gas(op: &UserOperation, include_fixed_gas_overhe
 }
 
 fn verification_gas_limit_multiplier(
-    uo: &UserOperation,
     assume_single_op_bundle: bool,
-    paymaster_post_op: Option<bool>,
+    paymaster_post_op: bool,
 ) -> u64 {
     // If using a paymaster that has a postOp, we need to account for potentially 2 postOp calls which can each use up to verification_gas_limit gas.
     // otherwise the entrypoint expects the gas for 1 postOp call that uses verification_gas_limit plus the actual verification call
     // we only add the additional verification_gas_limit only if we know for sure that this is a single op bundle, which what we do to get a worst-case upper bound
-    if uo.paymaster().is_some() && paymaster_post_op.unwrap_or(true) {
+    if paymaster_post_op {
         3
     } else if assume_single_op_bundle {
         2

crates/sim/src/gas/polygon.rs

@@ -66,12 +66,12 @@ where
 
     /// Estimates max and priority gas and converts to U256
     pub(crate) async fn estimate_priority_fee(&self) -> anyhow::Result<U256> {
-        let (provider_estiamte, fee_history_estimate) = try_join!(
+        let (provider_estimate, fee_history_estimate) = try_join!(
             self.provider.get_max_priority_fee().map_err(|e| e.into()),
             self.calculate_fees()
         )?;
 
-        Ok(cmp::max(provider_estiamte, fee_history_estimate))
+        Ok(cmp::max(provider_estimate, fee_history_estimate))
     }
 
     /// Perform a request to the gas price API and deserialize the response.

crates/sim/src/precheck.rs

@@ -180,7 +180,10 @@ impl<P: Provider, E: EntryPoint> PrecheckerImpl<P, E> {
                 max_verification_gas,
             ));
         }
-        let total_gas_limit = gas::user_operation_gas_limit(op, chain_id, true, None);
+
+        // compute the worst case total gas limit by assuming the UO is in its own bundle and has a postOp call.
+        // This is conservative and potentially may invalidate some very large UOs that would otherwise be valid.
+        let total_gas_limit = gas::user_operation_gas_limit(op, chain_id, true, true);
         if total_gas_limit > max_total_execution_gas {
             violations.push(PrecheckViolation::TotalGasLimitTooHigh(
                 total_gas_limit,
@@ -477,7 +480,7 @@ mod tests {
             res,
             ArrayVec::<PrecheckViolation, 6>::from([
                 PrecheckViolation::VerificationGasLimitTooHigh(10_000_000.into(), 5_000_000.into(),),
-                PrecheckViolation::TotalGasLimitTooHigh(20_009_000.into(), 10_000_000.into(),),
+                PrecheckViolation::TotalGasLimitTooHigh(30_009_000.into(), 10_000_000.into(),),
                 PrecheckViolation::PreVerificationGasTooLow(0.into(), 1_000.into(),),
                 PrecheckViolation::MaxFeePerGasTooLow(5_000.into(), 8_000.into(),),
                 PrecheckViolation::MaxPriorityFeePerGasTooLow(2_000.into(), 4_000.into(),),