From eaa88430c4d83a8de85ce7050023cc07b278d023 Mon Sep 17 00:00:00 2001 From: Alex Miao Date: Mon, 9 Oct 2023 11:43:07 -0700 Subject: [PATCH] feat(builder): fix postOp condition and move tests --- crates/builder/src/bundle_proposer.rs | 127 ++++++++++++++++++++++++-- crates/sim/src/gas/gas.rs | 2 +- 2 files changed, 122 insertions(+), 7 deletions(-) diff --git a/crates/builder/src/bundle_proposer.rs b/crates/builder/src/bundle_proposer.rs index 94132fdec..0e8d2d883 100644 --- a/crates/builder/src/bundle_proposer.rs +++ b/crates/builder/src/bundle_proposer.rs @@ -745,11 +745,11 @@ impl ProposalContext { 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 = op - .paymaster() - .map_or(ov.bundle_transaction_gas_buffer, |_| { - cmp::max(op.verification_gas_limit, ov.bundle_transaction_gas_buffer) - }); + 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 @@ -1176,7 +1176,7 @@ 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 op3 = op_with_sender_call_gas_limit(address(3), U256::from(10_000_000)); @@ -1228,6 +1228,102 @@ mod tests { ); } + #[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: @@ -1407,4 +1503,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() + } + } } diff --git a/crates/sim/src/gas/gas.rs b/crates/sim/src/gas/gas.rs index 646df51e7..d564fd452 100644 --- a/crates/sim/src/gas/gas.rs +++ b/crates/sim/src/gas/gas.rs @@ -164,7 +164,7 @@ fn verification_gas_limit_multiplier( // 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.map_or(true, |x| x) { + if uo.paymaster().is_some() && paymaster_post_op.unwrap_or(true) { 3 } else if assume_single_op_bundle { 2