From 6e7cf2e45c6112fe8ba5e85c6ff6cb01d626d76a Mon Sep 17 00:00:00 2001 From: dancoombs Date: Mon, 14 Aug 2023 19:46:27 -0400 Subject: [PATCH] feat(builder): limit bundle size by max gas limit --- src/builder/bundle_proposer.rs | 94 +++++++++++++++++++++++++++------- src/cli/builder.rs | 2 +- src/common/types/mod.rs | 16 ++++++ 3 files changed, 93 insertions(+), 19 deletions(-) diff --git a/src/builder/bundle_proposer.rs b/src/builder/bundle_proposer.rs index c2431c5e0..659cabc43 100644 --- a/src/builder/bundle_proposer.rs +++ b/src/builder/bundle_proposer.rs @@ -1,5 +1,4 @@ use std::{ - cmp, collections::{HashMap, HashSet}, mem, sync::Arc, @@ -30,7 +29,7 @@ use crate::{ simulation::{SimulationError, SimulationSuccess, Simulator}, types::{ Entity, EntityType, EntryPointLike, ExpectedStorage, HandleOpsOut, ProviderLike, - Timestamp, UserOperation, OP_BEDROCK_CHAIN_IDS, + Timestamp, UserOperation, }, }, }; @@ -38,7 +37,8 @@ use crate::{ /// A user op must be valid for at least this long into the future to be included. const TIME_RANGE_BUFFER: Duration = Duration::from_secs(60); -const BUNDLE_TRANSACTION_GAS_OVERHEAD_PERCENT: u64 = 100; +const MAX_BUNDLE_GAS_LIMIT: u64 = 20_000_000; +const BUNDLE_TRANSACTION_GAS_OVERHEAD_PERCENT: u64 = 50; #[derive(Debug, Default)] pub struct Bundle { @@ -114,6 +114,9 @@ where self.fee_estimator.required_bundle_fees(required_fees) )?; + // Limit the amount of gas in the bundle + let ops = self.limit_gas_in_bundle(ops); + // Determine fees required for ops to be included in a bundle, and filter out ops that don't // meet the requirements. let required_op_fees = self.fee_estimator.required_op_fees(bundle_fees); @@ -479,6 +482,20 @@ where Ok(()) } + fn limit_gas_in_bundle(&self, ops: Vec) -> Vec { + let mut gas_left = U256::from(MAX_BUNDLE_GAS_LIMIT); + let mut ops_in_bundle = Vec::new(); + for op in ops { + let gas = op.op.total_execution_gas_limit(self.chain_id); + if gas_left < gas { + break; + } + gas_left -= gas; + ops_in_bundle.push(op); + } + ops_in_bundle + } + fn emit(&self, event: BuilderEvent) { let _ = self.event_sender.send(WithEntryPoint { entry_point: self.entry_point.address(), @@ -680,22 +697,8 @@ impl ProposalContext { } fn get_total_gas(&self, chain_id: u64) -> U256 { - // On some chains the L1 gas fee is charged via pre_verification_gas - // in the bundle, but is not part of the gas limit of the transaction. - // Thus, including it can cause the transaction to exceed the maximum limit - // of a block. This workaround caps the pre_verification_gas at 100k during - // bundle gas estimation. - let max_pre_verification_gas = if OP_BEDROCK_CHAIN_IDS.contains(&chain_id) { - U256::from(100_000) - } else { - U256::MAX - }; self.iter_ops() - .map(|op| { - cmp::min(op.pre_verification_gas, max_pre_verification_gas) - + op.verification_gas_limit - + op.call_gas_limit - }) + .map(|op| op.total_execution_gas_limit(chain_id)) .fold(U256::zero(), |acc, c| acc + c) } @@ -1108,6 +1111,53 @@ mod tests { ); } + #[tokio::test] + async fn test_bundle_gas_limit() { + let op1 = op_with_sender_call_gas_limit(address(1), U256::from(10_000_000)); + let op2 = op_with_sender_call_gas_limit(address(2), U256::from(10_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(); + + let bundle = make_bundle( + vec![ + MockOp { + op: op1.clone(), + simulation_result: Box::new(|| Ok(SimulationSuccess::default())), + }, + MockOp { + op: op2.clone(), + simulation_result: Box::new(|| Ok(SimulationSuccess::default())), + }, + MockOp { + op: op3.clone(), + simulation_result: Box::new(|| Ok(SimulationSuccess::default())), + }, + MockOp { + op: op4.clone(), + simulation_result: Box::new(|| Ok(SimulationSuccess::default())), + }, + ], + vec![], + vec![HandleOpsOut::Success], + vec![deposit, deposit, deposit], + U256::zero(), + U256::zero(), + ) + .await; + + assert_eq!(bundle.rejected_entities, vec![]); + assert_eq!(bundle.rejected_ops, vec![]); + assert_eq!( + bundle.ops_per_aggregator, + vec![UserOpsPerAggregator { + user_ops: vec![op1, op2], + ..Default::default() + }] + ); + assert_eq!(bundle.gas_estimate, U256::from(30_000_000)); + } + struct MockOp { op: UserOperation, simulation_result: @@ -1277,4 +1327,12 @@ mod tests { ..Default::default() } } + + fn op_with_sender_call_gas_limit(sender: Address, call_gas_limit: U256) -> UserOperation { + UserOperation { + sender, + call_gas_limit, + ..Default::default() + } + } } diff --git a/src/cli/builder.rs b/src/cli/builder.rs index ae987d646..fb74f2f44 100644 --- a/src/cli/builder.rs +++ b/src/cli/builder.rs @@ -79,7 +79,7 @@ pub struct BuilderArgs { long = "builder.max_bundle_size", name = "builder.max_bundle_size", env = "BUILDER_MAX_BUNDLE_SIZE", - default_value = "64" + default_value = "128" )] max_bundle_size: u64, diff --git a/src/common/types/mod.rs b/src/common/types/mod.rs index b33151c38..f7e06b7b6 100644 --- a/src/common/types/mod.rs +++ b/src/common/types/mod.rs @@ -101,6 +101,22 @@ impl UserOperation { let max_gas = self.call_gas_limit + self.verification_gas_limit + self.pre_verification_gas; max_gas * self.max_fee_per_gas } + + pub fn total_execution_gas_limit(&self, chain_id: u64) -> U256 { + // On some chains the L1 gas fee is charged via pre_verification_gas + // but this not part of the execution gas of the transaction. + // This workaround caps the pre_verification_gas at 100k during + // the execution gas calculation. + let max = if OP_BEDROCK_CHAIN_IDS.contains(&chain_id) { + U256::from(100_000) + } else { + U256::MAX + }; + + std::cmp::min(max, self.pre_verification_gas) + + self.call_gas_limit + + self.verification_gas_limit + } } #[derive(Display, Debug, Clone, Ord, Copy, Eq, PartialEq, EnumIter, PartialOrd, Deserialize)]