feat(builder): only include paymaster gas if there is a postOp

crates/builder/src/bundle_proposer.rs

@@ -12,6 +12,7 @@
 // If not, see https://www.gnu.org/licenses/.
 
 use std::{
+    cmp,
     collections::{HashMap, HashSet},
     mem,
     sync::Arc,
@@ -29,8 +30,8 @@ use mockall::automock;
 use rundler_pool::{PoolOperation, PoolServer};
 use rundler_provider::{EntryPoint, HandleOpsOut, Provider};
 use rundler_sim::{
-    gas, ExpectedStorage, FeeEstimator, PriorityFeeMode, SimulationError, SimulationSuccess,
-    Simulator,
+    gas::{self, GasOverheads},
+    ExpectedStorage, FeeEstimator, PriorityFeeMode, SimulationError, SimulationSuccess, Simulator,
 };
 use rundler_types::{Entity, EntityType, GasFees, Timestamp, UserOperation, UserOpsPerAggregator};
 use rundler_utils::{emit::WithEntryPoint, math};
@@ -129,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,
@@ -186,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,
@@ -281,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,
@@ -311,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()
@@ -340,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()
@@ -409,7 +438,7 @@ where
         // sum up the gas needed for all the ops in the bundle
         // and apply an overhead multiplier
         let gas = math::increase_by_percent(
-            context.get_total_gas_limit(self.settings.chain_id),
+            context.get_bundle_gas_limit(self.settings.chain_id),
             BUNDLE_TRANSACTION_GAS_OVERHEAD_PERCENT,
         );
         let handle_ops_out = self
@@ -532,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);
+            // 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,
@@ -736,8 +771,28 @@ impl ProposalContext {
             .collect()
     }
 
-    fn get_total_gas_limit(&self, chain_id: u64) -> U256 {
-        gas::bundle_gas_limit(self.iter_ops(), chain_id)
+    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();
+        for op_with_sim in self.iter_ops_with_simulations() {
+            let op = &op_with_sim.op;
+            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,
+                op_with_sim.simulation.requires_post_op,
+            );
+        }
+        max_gas
     }
 
     fn iter_ops_with_simulations(&self) -> impl Iterator<Item = &OpWithSimulation> + '_ {
@@ -751,13 +806,33 @@ 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;
     use ethers::{types::H160, utils::parse_units};
     use rundler_pool::MockPoolServer;
     use rundler_provider::{AggregatorSimOut, MockEntryPoint, MockProvider};
-    use rundler_sim::{gas::GasOverheads, MockSimulator, SimulationViolation};
+    use rundler_sim::{MockSimulator, SimulationViolation};
     use rundler_types::ValidTimeRange;
 
     use super::*;
@@ -1149,9 +1224,9 @@ mod tests {
     }
 
     #[tokio::test]
-    async fn test_bundle_gas_limit() {
+    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();
@@ -1195,12 +1270,108 @@ 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
             ))
         );
     }
 
+    #[tokio::test]
+    async fn test_bundle_gas_limit() {
+        let op1 = op_with_gas(100_000.into(), 100_000.into(), 1_000_000.into(), false);
+        let op2 = op_with_gas(100_000.into(), 100_000.into(), 200_000.into(), false);
+        let chain_id = 1;
+        let mut groups_by_aggregator = LinkedHashMap::new();
+        groups_by_aggregator.insert(
+            None,
+            AggregatorGroup {
+                ops_with_simulations: vec![
+                    OpWithSimulation {
+                        op: op1.clone(),
+                        simulation: SimulationSuccess {
+                            requires_post_op: false,
+                            ..Default::default()
+                        },
+                    },
+                    OpWithSimulation {
+                        op: op2.clone(),
+                        simulation: SimulationSuccess {
+                            requires_post_op: false,
+                            ..Default::default()
+                        },
+                    },
+                ],
+                signature: Default::default(),
+            },
+        );
+        let context = ProposalContext {
+            groups_by_aggregator,
+            rejected_ops: vec![],
+            rejected_entities: vec![],
+        };
+
+        // The gas requirement from the execution of the first UO is: g >= p_1 + 2v_1 + c_1 + 5000
+        // The gas requirement from the execution of the second UO is: g >= p_1 + v_1 + c_1 + p_2 + 2v_2 + c_2 + 5000
+        // The first condition dominates and determines the expected gas limit
+        let expected_gas_limit = op1.pre_verification_gas
+            + op1.verification_gas_limit * 2
+            + op1.call_gas_limit
+            + 5_000
+            + 21_000;
+
+        assert_eq!(context.get_bundle_gas_limit(chain_id), expected_gas_limit);
+    }
+
+    #[tokio::test]
+    async fn test_bundle_gas_limit_with_paymaster_op() {
+        let op1 = op_with_gas(100_000.into(), 100_000.into(), 1_000_000.into(), true); // has paymaster
+        let op2 = op_with_gas(100_000.into(), 100_000.into(), 200_000.into(), false);
+        let chain_id = 1;
+        let mut groups_by_aggregator = LinkedHashMap::new();
+        groups_by_aggregator.insert(
+            None,
+            AggregatorGroup {
+                ops_with_simulations: vec![
+                    OpWithSimulation {
+                        op: op1.clone(),
+                        simulation: SimulationSuccess {
+                            requires_post_op: true, // uses postOp
+                            ..Default::default()
+                        },
+                    },
+                    OpWithSimulation {
+                        op: op2.clone(),
+                        simulation: SimulationSuccess {
+                            requires_post_op: false,
+                            ..Default::default()
+                        },
+                    },
+                ],
+                signature: Default::default(),
+            },
+        );
+        let context = ProposalContext {
+            groups_by_aggregator,
+            rejected_ops: vec![],
+            rejected_entities: vec![],
+        };
+        let gas_limit = context.get_bundle_gas_limit(chain_id);
+
+        // The gas requirement from the execution of the first UO is: g >= p_1 + 3v_1 + c_1
+        // The gas requirement from the execution of the second UO is: g >= p_1 + 3v_1 + c_1 + p_2 + 2v_2 + c_2 + 5000
+        // The first condition dominates and determines the expected gas limit
+        let expected_gas_limit = op1.pre_verification_gas
+            + op1.verification_gas_limit * 3
+            + op1.call_gas_limit
+            + op2.pre_verification_gas
+            + op2.verification_gas_limit * 2
+            + op2.call_gas_limit
+            + 21_000
+            + 5_000;
+
+        assert_eq!(gas_limit, expected_gas_limit);
+    }
+
     struct MockOp {
         op: UserOperation,
         simulation_result:
@@ -1380,4 +1551,23 @@ mod tests {
             ..Default::default()
         }
     }
+
+    fn op_with_gas(
+        pre_verification_gas: U256,
+        call_gas_limit: U256,
+        verification_gas_limit: U256,
+        with_paymaster: bool,
+    ) -> UserOperation {
+        UserOperation {
+            pre_verification_gas,
+            call_gas_limit,
+            verification_gas_limit,
+            paymaster_and_data: if with_paymaster {
+                Bytes::from(vec![0; 20])
+            } else {
+                Default::default()
+            },
+            ..Default::default()
+        }
+    }
 }