From bf800cc77384b32bf853b8f155c277d7154d88b0 Mon Sep 17 00:00:00 2001 From: dancoombs Date: Thu, 5 Oct 2023 11:12:32 -0400 Subject: [PATCH 1/9] fix(pool): remove insert dup, scope write lock --- crates/pool/src/mempool/uo_pool.rs | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/crates/pool/src/mempool/uo_pool.rs b/crates/pool/src/mempool/uo_pool.rs index 0b8c224e0..bfa9fe20e 100644 --- a/crates/pool/src/mempool/uo_pool.rs +++ b/crates/pool/src/mempool/uo_pool.rs @@ -258,12 +258,15 @@ where }; // Add op to pool - let mut state = self.state.write(); - let hash = state.pool.add_operation(pool_op.clone())?; - let bn = state.block_number; - if throttled { - state.throttled_ops.insert(hash, bn); - } + let (hash, bn) = { + let mut state = self.state.write(); + let hash = state.pool.add_operation(pool_op.clone())?; + let bn = state.block_number; + if throttled { + state.throttled_ops.insert(hash, bn); + } + (hash, bn) + }; // Update reputation pool_op @@ -272,11 +275,6 @@ where .unique() .for_each(|a| self.reputation.add_seen(a)); - // If an entity was throttled, track with throttled ops - if throttled { - state.throttled_ops.insert(hash, bn); - } - let op_hash = pool_op.uo.op_hash(self.entry_point, self.chain_id); let valid_after = pool_op.valid_time_range.valid_after; let valid_until = pool_op.valid_time_range.valid_until; From 2fa968879da9a03463949f8b4ab8b3eb8a258024 Mon Sep 17 00:00:00 2001 From: Alex Miao Date: Mon, 2 Oct 2023 21:09:32 -0700 Subject: [PATCH 2/9] fix(gas): amortize evm transaction fixed cost over bundle --- crates/builder/Cargo.toml | 1 + crates/builder/src/bundle_proposer.rs | 26 +++++++++++++++----------- crates/sim/src/gas/gas.rs | 22 ++++++++++++++-------- crates/sim/src/precheck.rs | 2 +- 4 files changed, 31 insertions(+), 20 deletions(-) diff --git a/crates/builder/Cargo.toml b/crates/builder/Cargo.toml index 872db3942..833a6be58 100644 --- a/crates/builder/Cargo.toml +++ b/crates/builder/Cargo.toml @@ -48,6 +48,7 @@ mockall = {workspace = true, optional = true } mockall.workspace = true rundler-pool = { path = "../pool", features = ["test-utils"] } rundler-provider = { path = "../provider", features = ["test-utils"] } +rundler-sim = { path = "../sim", features = ["test-utils"] } [build-dependencies] tonic-build.workspace = true diff --git a/crates/builder/src/bundle_proposer.rs b/crates/builder/src/bundle_proposer.rs index 5976e686a..ac9dcdbc0 100644 --- a/crates/builder/src/bundle_proposer.rs +++ b/crates/builder/src/bundle_proposer.rs @@ -45,6 +45,8 @@ const TIME_RANGE_BUFFER: Duration = Duration::from_secs(60); const BUNDLE_TRANSACTION_GAS_OVERHEAD_BUFFER: u64 = 5000; /// Extra buffer percent to add on the bundle transaction gas estimate to be sure it will be enough const BUNDLE_TRANSACTION_GAS_OVERHEAD_PERCENT: u64 = 5; +/// The fixed gas overhead for any EVM transaction +const EVM_TRANSACTION_GAS_OVERHEAD: u64 = 21000; #[derive(Debug, Default)] pub(crate) struct Bundle { @@ -538,7 +540,7 @@ where let mut gas_left = U256::from(self.settings.max_bundle_gas); let mut ops_in_bundle = Vec::new(); for op in ops { - let gas = gas::user_operation_gas_limit(&op.uo, self.settings.chain_id); + let gas = gas::user_operation_gas_limit(&op.uo, self.settings.chain_id, false); if gas_left < gas { self.emit(BuilderEvent::skipped_op( self.builder_index, @@ -740,9 +742,10 @@ impl ProposalContext { fn get_total_gas_limit(&self, chain_id: u64) -> U256 { self.iter_ops() - .map(|op| gas::user_operation_gas_limit(op, chain_id)) + .map(|op| gas::user_operation_gas_limit(op, chain_id, false)) .fold(U256::zero(), |acc, c| acc + c) + BUNDLE_TRANSACTION_GAS_OVERHEAD_BUFFER + + EVM_TRANSACTION_GAS_OVERHEAD } fn iter_ops_with_simulations(&self) -> impl Iterator + '_ { @@ -786,7 +789,8 @@ mod tests { op.pre_verification_gas + op.verification_gas_limit * 2 + op.call_gas_limit - + BUNDLE_TRANSACTION_GAS_OVERHEAD_BUFFER, + + BUNDLE_TRANSACTION_GAS_OVERHEAD_BUFFER + + EVM_TRANSACTION_GAS_OVERHEAD, BUNDLE_TRANSACTION_GAS_OVERHEAD_PERCENT, ); @@ -909,7 +913,7 @@ mod tests { let max_priority_fee_per_gas = U256::from(50); let op1 = op_with_sender_and_fees(address(1), 2054.into(), 54.into()); let op2 = op_with_sender_and_fees(address(2), 2055.into(), 55.into()); - let bundle = make_bundle( + let bundle = mock_make_bundle( vec![ MockOp { op: op1.clone(), @@ -943,7 +947,7 @@ mod tests { let max_priority_fee_per_gas = U256::from(50); let op1 = op_with_sender_and_fees(address(1), 1054.into(), 55.into()); let op2 = op_with_sender_and_fees(address(2), 1055.into(), 55.into()); - let bundle = make_bundle( + let bundle = mock_make_bundle( vec![ MockOp { op: op1.clone(), @@ -993,7 +997,7 @@ mod tests { let op_b_aggregated_sig = 21; let aggregator_a_signature = 101; let aggregator_b_signature = 102; - let bundle = make_bundle( + let bundle = mock_make_bundle( vec![ MockOp { op: unaggregated_op.clone(), @@ -1098,7 +1102,7 @@ mod tests { let op6 = op_with_sender_factory(address(6), address(4)); let deposit = parse_units("1", "ether").unwrap().into(); - let bundle = make_bundle( + let bundle = mock_make_bundle( vec![ MockOp { op: op1.clone(), @@ -1159,7 +1163,7 @@ mod tests { 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( + let bundle = mock_make_bundle( vec![ MockOp { op: op1.clone(), @@ -1198,7 +1202,7 @@ mod tests { assert_eq!( bundle.gas_estimate, U256::from(math::increase_by_percent( - 10_000_000 + BUNDLE_TRANSACTION_GAS_OVERHEAD_BUFFER, + 10_000_000 + BUNDLE_TRANSACTION_GAS_OVERHEAD_BUFFER + EVM_TRANSACTION_GAS_OVERHEAD, BUNDLE_TRANSACTION_GAS_OVERHEAD_PERCENT )) ); @@ -1216,7 +1220,7 @@ mod tests { } async fn simple_make_bundle(mock_ops: Vec) -> Bundle { - make_bundle( + mock_make_bundle( mock_ops, vec![], vec![HandleOpsOut::Success], @@ -1227,7 +1231,7 @@ mod tests { .await } - async fn make_bundle( + async fn mock_make_bundle( mock_ops: Vec, mock_aggregators: Vec, mock_handle_ops_call_results: Vec, diff --git a/crates/sim/src/gas/gas.rs b/crates/sim/src/gas/gas.rs index 470988075..c15896cae 100644 --- a/crates/sim/src/gas/gas.rs +++ b/crates/sim/src/gas/gas.rs @@ -38,7 +38,6 @@ struct GasOverheads { per_user_op_word: U256, zero_byte: U256, non_zero_byte: U256, - bundle_size: U256, } impl Default for GasOverheads { @@ -49,7 +48,6 @@ impl Default for GasOverheads { per_user_op_word: 4.into(), zero_byte: 4.into(), non_zero_byte: 16.into(), - bundle_size: 1.into(), } } } @@ -74,7 +72,7 @@ pub async fn calc_pre_verification_gas( provider: Arc

, chain_id: u64, ) -> anyhow::Result { - let static_gas = calc_static_pre_verification_gas(full_op); + let static_gas = calc_static_pre_verification_gas(full_op, true); let dynamic_gas = match chain_id { _ if ARBITRUM_CHAIN_IDS.contains(&chain_id) => { provider @@ -95,12 +93,16 @@ pub async fn calc_pre_verification_gas( } /// Returns the gas limit for the user operation that applies to bundle transaction's limit -pub fn user_operation_gas_limit(uo: &UserOperation, chain_id: u64) -> U256 { +pub fn user_operation_gas_limit( + uo: &UserOperation, + chain_id: u64, + include_fixed_gas_overhead: bool, +) -> U256 { // On some chains (OP bedrock, Arbitrum) the L1 gas fee is charged via pre_verification_gas // but this not part of the execution gas limit of the transaction. // In such cases we only consider the static portion of the pre_verification_gas in the gas limit. let pvg = if OP_BEDROCK_CHAIN_IDS.contains(&chain_id) | ARBITRUM_CHAIN_IDS.contains(&chain_id) { - calc_static_pre_verification_gas(uo) + calc_static_pre_verification_gas(uo, include_fixed_gas_overhead) } else { uo.pre_verification_gas }; @@ -115,7 +117,7 @@ pub fn user_operation_max_gas_cost(uo: &UserOperation) -> U256 { * (uo.pre_verification_gas + uo.call_gas_limit + uo.verification_gas_limit * mul) } -fn calc_static_pre_verification_gas(op: &UserOperation) -> U256 { +fn calc_static_pre_verification_gas(op: &UserOperation, include_fixed_gas_overhead: bool) -> U256 { let ov = GasOverheads::default(); let encoded_op = op.clone().encode(); let length_in_words = encoded_op.len() / 32; // size of packed user op is always a multiple of 32 bytes @@ -131,10 +133,14 @@ fn calc_static_pre_verification_gas(op: &UserOperation) -> U256 { .reduce(|a, b| a + b) .unwrap_or_default(); - ov.fixed / ov.bundle_size - + call_data_cost + call_data_cost + ov.per_user_op + ov.per_user_op_word * length_in_words + + (if include_fixed_gas_overhead { + ov.fixed + } else { + 0.into() + }) } fn verification_gas_limit_multiplier(uo: &UserOperation) -> u64 { diff --git a/crates/sim/src/precheck.rs b/crates/sim/src/precheck.rs index 95acec561..eef954f71 100644 --- a/crates/sim/src/precheck.rs +++ b/crates/sim/src/precheck.rs @@ -180,7 +180,7 @@ impl PrecheckerImpl { max_verification_gas, )); } - let total_gas_limit = gas::user_operation_gas_limit(op, chain_id); + let total_gas_limit = gas::user_operation_gas_limit(op, chain_id, false); if total_gas_limit > max_total_execution_gas { violations.push(PrecheckViolation::TotalGasLimitTooHigh( total_gas_limit, From dd81f93e4126682a6ada3999fa5c6df848bbbb63 Mon Sep 17 00:00:00 2001 From: Alex Miao Date: Tue, 3 Oct 2023 11:39:52 -0700 Subject: [PATCH 3/9] fix(gas): refactor bundle gas calculation --- crates/builder/src/bundle_proposer.rs | 16 +++--------- crates/sim/src/gas/gas.rs | 35 +++++++++++++++++++++------ crates/sim/src/precheck.rs | 2 +- 3 files changed, 32 insertions(+), 21 deletions(-) diff --git a/crates/builder/src/bundle_proposer.rs b/crates/builder/src/bundle_proposer.rs index ac9dcdbc0..48cd3230d 100644 --- a/crates/builder/src/bundle_proposer.rs +++ b/crates/builder/src/bundle_proposer.rs @@ -41,12 +41,8 @@ use crate::emit::{BuilderEvent, OpRejectionReason, SkipReason}; /// 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); -/// Entrypoint requires a buffer over the user operation gas limits in the bundle transaction -const BUNDLE_TRANSACTION_GAS_OVERHEAD_BUFFER: u64 = 5000; /// Extra buffer percent to add on the bundle transaction gas estimate to be sure it will be enough const BUNDLE_TRANSACTION_GAS_OVERHEAD_PERCENT: u64 = 5; -/// The fixed gas overhead for any EVM transaction -const EVM_TRANSACTION_GAS_OVERHEAD: u64 = 21000; #[derive(Debug, Default)] pub(crate) struct Bundle { @@ -741,11 +737,7 @@ impl ProposalContext { } fn get_total_gas_limit(&self, chain_id: u64) -> U256 { - self.iter_ops() - .map(|op| gas::user_operation_gas_limit(op, chain_id, false)) - .fold(U256::zero(), |acc, c| acc + c) - + BUNDLE_TRANSACTION_GAS_OVERHEAD_BUFFER - + EVM_TRANSACTION_GAS_OVERHEAD + gas::bundle_gas_limit(self.iter_ops(), chain_id) } fn iter_ops_with_simulations(&self) -> impl Iterator + '_ { @@ -789,8 +781,8 @@ mod tests { op.pre_verification_gas + op.verification_gas_limit * 2 + op.call_gas_limit - + BUNDLE_TRANSACTION_GAS_OVERHEAD_BUFFER - + EVM_TRANSACTION_GAS_OVERHEAD, + + U256::from(5_000) + + U256::from(21_000), BUNDLE_TRANSACTION_GAS_OVERHEAD_PERCENT, ); @@ -1202,7 +1194,7 @@ mod tests { assert_eq!( bundle.gas_estimate, U256::from(math::increase_by_percent( - 10_000_000 + BUNDLE_TRANSACTION_GAS_OVERHEAD_BUFFER + EVM_TRANSACTION_GAS_OVERHEAD, + 10_000_000 + 5_000 + 21_000, BUNDLE_TRANSACTION_GAS_OVERHEAD_PERCENT )) ); diff --git a/crates/sim/src/gas/gas.rs b/crates/sim/src/gas/gas.rs index c15896cae..3988a7ccd 100644 --- a/crates/sim/src/gas/gas.rs +++ b/crates/sim/src/gas/gas.rs @@ -33,7 +33,8 @@ use super::polygon::Polygon; // see: https://github.com/eth-infinitism/bundler/blob/main/packages/sdk/src/calcPreVerificationGas.ts #[derive(Clone, Copy, Debug)] struct GasOverheads { - fixed: U256, + bundle_transaction_gas_buffer: U256, + transaction_gas_overhead: U256, per_user_op: U256, per_user_op_word: U256, zero_byte: U256, @@ -43,8 +44,9 @@ struct GasOverheads { impl Default for GasOverheads { fn default() -> Self { Self { - fixed: 21000.into(), - per_user_op: 18300.into(), + bundle_transaction_gas_buffer: 5_000.into(), // Entrypoint requires a buffer over the user operation gas limits in the bundle transaction + transaction_gas_overhead: 21_000.into(), // The fixed gas overhead for any EVM transaction + per_user_op: 18_300.into(), per_user_op_word: 4.into(), zero_byte: 4.into(), non_zero_byte: 16.into(), @@ -72,7 +74,7 @@ pub async fn calc_pre_verification_gas( provider: Arc

, chain_id: u64, ) -> anyhow::Result { - let static_gas = calc_static_pre_verification_gas(full_op, true); + let static_gas = calc_static_pre_verification_gas(full_op, GasOverheads::default(), true); let dynamic_gas = match chain_id { _ if ARBITRUM_CHAIN_IDS.contains(&chain_id) => { provider @@ -92,6 +94,20 @@ pub async fn calc_pre_verification_gas( Ok(static_gas + dynamic_gas) } +/// Compute the gas limit for the bundle composed of the given user operations +pub fn bundle_gas_limit<'a, I>(iter_ops: I, chain_id: u64) -> U256 +where + I: Iterator, +{ + let ov = GasOverheads::default(); + iter_ops + .map(|op| user_operation_gas_limit(op, chain_id, false)) + .fold( + ov.bundle_transaction_gas_buffer + ov.transaction_gas_overhead, + |acc, c| acc + c, + ) +} + /// Returns the gas limit for the user operation that applies to bundle transaction's limit pub fn user_operation_gas_limit( uo: &UserOperation, @@ -102,7 +118,7 @@ pub fn user_operation_gas_limit( // but this not part of the execution gas limit of the transaction. // In such cases we only consider the static portion of the pre_verification_gas in the gas limit. let pvg = if OP_BEDROCK_CHAIN_IDS.contains(&chain_id) | ARBITRUM_CHAIN_IDS.contains(&chain_id) { - calc_static_pre_verification_gas(uo, include_fixed_gas_overhead) + calc_static_pre_verification_gas(uo, GasOverheads::default(), include_fixed_gas_overhead) } else { uo.pre_verification_gas }; @@ -117,8 +133,11 @@ pub fn user_operation_max_gas_cost(uo: &UserOperation) -> U256 { * (uo.pre_verification_gas + uo.call_gas_limit + uo.verification_gas_limit * mul) } -fn calc_static_pre_verification_gas(op: &UserOperation, include_fixed_gas_overhead: bool) -> U256 { - let ov = GasOverheads::default(); +fn calc_static_pre_verification_gas( + op: &UserOperation, + ov: GasOverheads, + include_fixed_gas_overhead: bool, +) -> U256 { let encoded_op = op.clone().encode(); let length_in_words = encoded_op.len() / 32; // size of packed user op is always a multiple of 32 bytes let call_data_cost: U256 = encoded_op @@ -137,7 +156,7 @@ fn calc_static_pre_verification_gas(op: &UserOperation, include_fixed_gas_overhe + ov.per_user_op + ov.per_user_op_word * length_in_words + (if include_fixed_gas_overhead { - ov.fixed + ov.transaction_gas_overhead } else { 0.into() }) diff --git a/crates/sim/src/precheck.rs b/crates/sim/src/precheck.rs index eef954f71..74fddf1f2 100644 --- a/crates/sim/src/precheck.rs +++ b/crates/sim/src/precheck.rs @@ -180,7 +180,7 @@ impl PrecheckerImpl { max_verification_gas, )); } - let total_gas_limit = gas::user_operation_gas_limit(op, chain_id, false); + let total_gas_limit = gas::user_operation_gas_limit(op, chain_id, true); if total_gas_limit > max_total_execution_gas { violations.push(PrecheckViolation::TotalGasLimitTooHigh( total_gas_limit, From e220e0a2118b33bfcf374448e8943b6667273bb8 Mon Sep 17 00:00:00 2001 From: Alex Miao Date: Wed, 4 Oct 2023 14:56:29 -0700 Subject: [PATCH 4/9] fix(gas): refactor gas overheads --- crates/builder/src/bundle_proposer.rs | 7 ++++--- crates/sim/src/gas/gas.rs | 27 ++++++++++++--------------- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/crates/builder/src/bundle_proposer.rs b/crates/builder/src/bundle_proposer.rs index 48cd3230d..c23f7db0b 100644 --- a/crates/builder/src/bundle_proposer.rs +++ b/crates/builder/src/bundle_proposer.rs @@ -757,7 +757,7 @@ mod tests { use ethers::{types::H160, utils::parse_units}; use rundler_pool::MockPoolServer; use rundler_provider::{AggregatorSimOut, MockEntryPoint, MockProvider}; - use rundler_sim::{MockSimulator, SimulationViolation}; + use rundler_sim::{gas::GasOverheads, MockSimulator, SimulationViolation}; use rundler_types::ValidTimeRange; use super::*; @@ -777,12 +777,13 @@ mod tests { }]) .await; + let ov = GasOverheads::default(); let expected_gas = math::increase_by_percent( op.pre_verification_gas + op.verification_gas_limit * 2 + op.call_gas_limit - + U256::from(5_000) - + U256::from(21_000), + + ov.bundle_transaction_gas_buffer + + ov.transaction_gas_overhead, BUNDLE_TRANSACTION_GAS_OVERHEAD_PERCENT, ); diff --git a/crates/sim/src/gas/gas.rs b/crates/sim/src/gas/gas.rs index 3988a7ccd..50fcdd771 100644 --- a/crates/sim/src/gas/gas.rs +++ b/crates/sim/src/gas/gas.rs @@ -28,13 +28,13 @@ use tokio::try_join; use super::polygon::Polygon; -// Gas overheads for user operations -// used in calculating the pre-verification gas -// see: https://github.com/eth-infinitism/bundler/blob/main/packages/sdk/src/calcPreVerificationGas.ts +/// Gas overheads for user operations used in calculating the pre-verification gas. See: https://github.com/eth-infinitism/bundler/blob/main/packages/sdk/src/calcPreVerificationGas.ts #[derive(Clone, Copy, Debug)] -struct GasOverheads { - bundle_transaction_gas_buffer: U256, - transaction_gas_overhead: U256, +pub struct GasOverheads { + /// The Entrypoint requires a gas buffer for the bundle to account for the gas spent outside of the major steps in the processing of UOs + pub bundle_transaction_gas_buffer: U256, + /// The fixed gas overhead for any EVM transaction + pub transaction_gas_overhead: U256, per_user_op: U256, per_user_op_word: U256, zero_byte: U256, @@ -44,8 +44,8 @@ struct GasOverheads { impl Default for GasOverheads { fn default() -> Self { Self { - bundle_transaction_gas_buffer: 5_000.into(), // Entrypoint requires a buffer over the user operation gas limits in the bundle transaction - transaction_gas_overhead: 21_000.into(), // The fixed gas overhead for any EVM transaction + bundle_transaction_gas_buffer: 5_000.into(), + transaction_gas_overhead: 21_000.into(), per_user_op: 18_300.into(), per_user_op_word: 4.into(), zero_byte: 4.into(), @@ -74,7 +74,7 @@ pub async fn calc_pre_verification_gas( provider: Arc

, chain_id: u64, ) -> anyhow::Result { - let static_gas = calc_static_pre_verification_gas(full_op, GasOverheads::default(), true); + let static_gas = calc_static_pre_verification_gas(full_op, true); let dynamic_gas = match chain_id { _ if ARBITRUM_CHAIN_IDS.contains(&chain_id) => { provider @@ -118,7 +118,7 @@ pub fn user_operation_gas_limit( // but this not part of the execution gas limit of the transaction. // In such cases we only consider the static portion of the pre_verification_gas in the gas limit. let pvg = if OP_BEDROCK_CHAIN_IDS.contains(&chain_id) | ARBITRUM_CHAIN_IDS.contains(&chain_id) { - calc_static_pre_verification_gas(uo, GasOverheads::default(), include_fixed_gas_overhead) + calc_static_pre_verification_gas(uo, include_fixed_gas_overhead) } else { uo.pre_verification_gas }; @@ -133,11 +133,8 @@ pub fn user_operation_max_gas_cost(uo: &UserOperation) -> U256 { * (uo.pre_verification_gas + uo.call_gas_limit + uo.verification_gas_limit * mul) } -fn calc_static_pre_verification_gas( - op: &UserOperation, - ov: GasOverheads, - include_fixed_gas_overhead: bool, -) -> U256 { +fn calc_static_pre_verification_gas(op: &UserOperation, include_fixed_gas_overhead: bool) -> U256 { + let ov = GasOverheads::default(); let encoded_op = op.clone().encode(); let length_in_words = encoded_op.len() / 32; // size of packed user op is always a multiple of 32 bytes let call_data_cost: U256 = encoded_op From 758a96b1aada1d3a7765d5a00cbd33a818ca8ee0 Mon Sep 17 00:00:00 2001 From: Alex Miao Date: Thu, 5 Oct 2023 11:36:50 -0700 Subject: [PATCH 5/9] fix(gas): improve bundle gas estimation calculation --- crates/sim/src/gas/gas.rs | 145 ++++++++++++++++++++++++++++++++++---- 1 file changed, 133 insertions(+), 12 deletions(-) diff --git a/crates/sim/src/gas/gas.rs b/crates/sim/src/gas/gas.rs index 50fcdd771..26d2225d9 100644 --- a/crates/sim/src/gas/gas.rs +++ b/crates/sim/src/gas/gas.rs @@ -100,16 +100,42 @@ where I: Iterator, { let ov = GasOverheads::default(); - iter_ops - .map(|op| user_operation_gas_limit(op, chain_id, false)) - .fold( - ov.bundle_transaction_gas_buffer + ov.transaction_gas_overhead, - |acc, c| acc + c, - ) + let mut max_gas = U256::zero(); + let mut gas_spent = U256::zero(); + for op in iter_ops { + let post_exec_req_gas = if op.paymaster().is_some() + && (op.verification_gas_limit).gt(&ov.bundle_transaction_gas_buffer) + { + op.verification_gas_limit + } else { + ov.bundle_transaction_gas_buffer + }; + let required_gas = gas_spent + + user_operation_pre_verification_gas_limit(op, chain_id, false) + + U256::from(2) * op.verification_gas_limit + + op.call_gas_limit + + post_exec_req_gas; + if required_gas > max_gas { + max_gas = required_gas; + } + gas_spent += user_operation_gas_limit(op, chain_id, false); + } + + max_gas + ov.transaction_gas_overhead } /// Returns the gas limit for the user operation that applies to bundle transaction's limit pub fn user_operation_gas_limit( + uo: &UserOperation, + chain_id: u64, + assume_single_op_bundle: 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) +} + +fn user_operation_pre_verification_gas_limit( uo: &UserOperation, chain_id: u64, include_fixed_gas_overhead: bool, @@ -117,13 +143,11 @@ pub fn user_operation_gas_limit( // On some chains (OP bedrock, Arbitrum) the L1 gas fee is charged via pre_verification_gas // but this not part of the execution gas limit of the transaction. // In such cases we only consider the static portion of the pre_verification_gas in the gas limit. - let pvg = if OP_BEDROCK_CHAIN_IDS.contains(&chain_id) | ARBITRUM_CHAIN_IDS.contains(&chain_id) { + if OP_BEDROCK_CHAIN_IDS.contains(&chain_id) | ARBITRUM_CHAIN_IDS.contains(&chain_id) { calc_static_pre_verification_gas(uo, include_fixed_gas_overhead) } else { uo.pre_verification_gas - }; - - pvg + uo.call_gas_limit + uo.verification_gas_limit * verification_gas_limit_multiplier(uo) + } } /// Returns the maximum cost, in wei, of this user operation @@ -159,15 +183,18 @@ fn calc_static_pre_verification_gas(op: &UserOperation, include_fixed_gas_overhe }) } -fn verification_gas_limit_multiplier(uo: &UserOperation) -> u64 { +fn verification_gas_limit_multiplier(uo: &UserOperation, assume_single_op_bundle: bool) -> u64 { // If using a paymaster, we need to account for potentially 2 postOp // calls (even though it won't be called). // Else 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() { 3 - } else { + } else if assume_single_op_bundle { 2 + } else { + 1 } } @@ -306,3 +333,97 @@ const NON_EIP_1559_CHAIN_IDS: &[u64] = &[ fn is_known_non_eip_1559_chain(chain_id: u64) -> bool { NON_EIP_1559_CHAIN_IDS.contains(&chain_id) } + +#[cfg(test)] +mod tests { + use ethers::types::Bytes; + + use super::*; + + #[tokio::test] + async fn test_bundle_gas_limit() { + let op1 = UserOperation { + pre_verification_gas: 100_000.into(), + call_gas_limit: 100_000.into(), + verification_gas_limit: 1_000_000.into(), + max_fee_per_gas: Default::default(), + sender: Default::default(), + nonce: Default::default(), + init_code: Default::default(), + call_data: Default::default(), + max_priority_fee_per_gas: Default::default(), + paymaster_and_data: Default::default(), + signature: Default::default(), + }; + let op2 = UserOperation { + pre_verification_gas: 100_000.into(), + call_gas_limit: 100_000.into(), + verification_gas_limit: 200_000.into(), + max_fee_per_gas: Default::default(), + sender: Default::default(), + nonce: Default::default(), + init_code: Default::default(), + call_data: Default::default(), + max_priority_fee_per_gas: Default::default(), + paymaster_and_data: Default::default(), + signature: Default::default(), + }; + let ops = vec![op1.clone(), op2.clone()]; + let chain_id = 1; + let gas_limit = bundle_gas_limit(ops.iter(), chain_id); + + // The gas requirement in the first user operation dominates and determines the expected gas limit + let expected_gas_limit = U256::from(21_000) + + U256::from(5_000) + + op1.pre_verification_gas + + U256::from(2) * op1.verification_gas_limit + + op1.call_gas_limit; + + assert_eq!(gas_limit, expected_gas_limit); + } + + #[tokio::test] + async fn test_bundle_gas_limit_with_paymaster_op() { + let op1 = UserOperation { + pre_verification_gas: 100_000.into(), + call_gas_limit: 100_000.into(), + verification_gas_limit: 1_000_000.into(), + max_fee_per_gas: Default::default(), + sender: Default::default(), + nonce: Default::default(), + init_code: Default::default(), + call_data: Default::default(), + max_priority_fee_per_gas: Default::default(), + paymaster_and_data: Bytes::from(vec![0; 20]), // has paymaster + signature: Default::default(), + }; + let op2 = UserOperation { + pre_verification_gas: 100_000.into(), + call_gas_limit: 100_000.into(), + verification_gas_limit: 200_000.into(), + max_fee_per_gas: Default::default(), + sender: Default::default(), + nonce: Default::default(), + init_code: Default::default(), + call_data: Default::default(), + max_priority_fee_per_gas: Default::default(), + paymaster_and_data: Default::default(), + signature: Default::default(), + }; + let ops = vec![op1.clone(), op2.clone()]; + let chain_id = 1; + let gas_limit = bundle_gas_limit(ops.iter(), chain_id); + + // The gas requirement in the second user operation dominates and determines the expected gas limit + let expected_gas_limit = U256::from(21_000) + + U256::from(5_000) + + op1.pre_verification_gas + + U256::from(3) * op1.verification_gas_limit + + op1.call_gas_limit + + op2.pre_verification_gas + + U256::from(2) * op2.verification_gas_limit + + op2.call_gas_limit; + + assert_eq!(gas_limit, expected_gas_limit); + } +} From 2776b2518321397da1cb40956297df68d5d17d93 Mon Sep 17 00:00:00 2001 From: Alex Miao Date: Wed, 4 Oct 2023 15:26:36 -0700 Subject: [PATCH 6/9] fix(gas): clean up arithmetic --- crates/sim/src/gas/gas.rs | 42 ++++++++++++++++++--------------------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/crates/sim/src/gas/gas.rs b/crates/sim/src/gas/gas.rs index 26d2225d9..64f10dd58 100644 --- a/crates/sim/src/gas/gas.rs +++ b/crates/sim/src/gas/gas.rs @@ -11,7 +11,7 @@ // You should have received a copy of the GNU General Public License along with Rundler. // If not, see https://www.gnu.org/licenses/. -use std::sync::Arc; +use std::{cmp, sync::Arc}; use ethers::{ abi::AbiEncode, @@ -103,21 +103,17 @@ where let mut max_gas = U256::zero(); let mut gas_spent = U256::zero(); for op in iter_ops { - let post_exec_req_gas = if op.paymaster().is_some() - && (op.verification_gas_limit).gt(&ov.bundle_transaction_gas_buffer) - { - op.verification_gas_limit - } else { - ov.bundle_transaction_gas_buffer - }; + 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 required_gas = gas_spent + user_operation_pre_verification_gas_limit(op, chain_id, false) - + U256::from(2) * op.verification_gas_limit + + op.verification_gas_limit * 2 + op.call_gas_limit + post_exec_req_gas; - if required_gas > max_gas { - max_gas = required_gas; - } + max_gas = cmp::max(required_gas, max_gas); gas_spent += user_operation_gas_limit(op, chain_id, false); } @@ -373,11 +369,11 @@ mod tests { let gas_limit = bundle_gas_limit(ops.iter(), chain_id); // The gas requirement in the first user operation dominates and determines the expected gas limit - let expected_gas_limit = U256::from(21_000) - + U256::from(5_000) - + op1.pre_verification_gas - + U256::from(2) * op1.verification_gas_limit - + op1.call_gas_limit; + let expected_gas_limit = op1.pre_verification_gas + + op1.verification_gas_limit * 2 + + op1.call_gas_limit + + 21_000 + + 5_000; assert_eq!(gas_limit, expected_gas_limit); } @@ -415,14 +411,14 @@ mod tests { let gas_limit = bundle_gas_limit(ops.iter(), chain_id); // The gas requirement in the second user operation dominates and determines the expected gas limit - let expected_gas_limit = U256::from(21_000) - + U256::from(5_000) - + op1.pre_verification_gas - + U256::from(3) * op1.verification_gas_limit + let expected_gas_limit = op1.pre_verification_gas + + op1.verification_gas_limit * 3 + op1.call_gas_limit + op2.pre_verification_gas - + U256::from(2) * op2.verification_gas_limit - + op2.call_gas_limit; + + op2.verification_gas_limit * 2 + + op2.call_gas_limit + + 21_000 + + 5_000; assert_eq!(gas_limit, expected_gas_limit); } From b337dcb090c2ec26418878b3a4d3eb82f452257f Mon Sep 17 00:00:00 2001 From: Alex Miao Date: Wed, 4 Oct 2023 15:41:45 -0700 Subject: [PATCH 7/9] fix(gas): clean up op creation in gas tests --- crates/sim/src/gas/gas.rs | 75 ++++++++++++--------------------------- 1 file changed, 23 insertions(+), 52 deletions(-) diff --git a/crates/sim/src/gas/gas.rs b/crates/sim/src/gas/gas.rs index 64f10dd58..fa0393184 100644 --- a/crates/sim/src/gas/gas.rs +++ b/crates/sim/src/gas/gas.rs @@ -336,34 +336,29 @@ mod tests { use super::*; + fn create_test_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() + } + } + #[tokio::test] async fn test_bundle_gas_limit() { - let op1 = UserOperation { - pre_verification_gas: 100_000.into(), - call_gas_limit: 100_000.into(), - verification_gas_limit: 1_000_000.into(), - max_fee_per_gas: Default::default(), - sender: Default::default(), - nonce: Default::default(), - init_code: Default::default(), - call_data: Default::default(), - max_priority_fee_per_gas: Default::default(), - paymaster_and_data: Default::default(), - signature: Default::default(), - }; - let op2 = UserOperation { - pre_verification_gas: 100_000.into(), - call_gas_limit: 100_000.into(), - verification_gas_limit: 200_000.into(), - max_fee_per_gas: Default::default(), - sender: Default::default(), - nonce: Default::default(), - init_code: Default::default(), - call_data: Default::default(), - max_priority_fee_per_gas: Default::default(), - paymaster_and_data: Default::default(), - signature: Default::default(), - }; + let op1 = create_test_op_with_gas(100_000.into(), 100_000.into(), 1_000_000.into(), false); + let op2 = create_test_op_with_gas(100_000.into(), 100_000.into(), 200_000.into(), false); let ops = vec![op1.clone(), op2.clone()]; let chain_id = 1; let gas_limit = bundle_gas_limit(ops.iter(), chain_id); @@ -380,32 +375,8 @@ mod tests { #[tokio::test] async fn test_bundle_gas_limit_with_paymaster_op() { - let op1 = UserOperation { - pre_verification_gas: 100_000.into(), - call_gas_limit: 100_000.into(), - verification_gas_limit: 1_000_000.into(), - max_fee_per_gas: Default::default(), - sender: Default::default(), - nonce: Default::default(), - init_code: Default::default(), - call_data: Default::default(), - max_priority_fee_per_gas: Default::default(), - paymaster_and_data: Bytes::from(vec![0; 20]), // has paymaster - signature: Default::default(), - }; - let op2 = UserOperation { - pre_verification_gas: 100_000.into(), - call_gas_limit: 100_000.into(), - verification_gas_limit: 200_000.into(), - max_fee_per_gas: Default::default(), - sender: Default::default(), - nonce: Default::default(), - init_code: Default::default(), - call_data: Default::default(), - max_priority_fee_per_gas: Default::default(), - paymaster_and_data: Default::default(), - signature: Default::default(), - }; + let op1 = create_test_op_with_gas(100_000.into(), 100_000.into(), 1_000_000.into(), true); // has paymaster + let op2 = create_test_op_with_gas(100_000.into(), 100_000.into(), 200_000.into(), false); let ops = vec![op1.clone(), op2.clone()]; let chain_id = 1; let gas_limit = bundle_gas_limit(ops.iter(), chain_id); From f1c3f5d8e1919be8c0c07bdf474373a72f79d750 Mon Sep 17 00:00:00 2001 From: dancoombs Date: Fri, 6 Oct 2023 15:01:40 -0400 Subject: [PATCH 8/9] fix: rust 1.73.0 fixes --- Cargo.toml | 1 + crates/pool/src/chain.rs | 3 +-- crates/sim/src/estimation/estimation.rs | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index e13584830..8eb8123e7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,6 +4,7 @@ members = [ "crates/*", ] default-members = ["bin/rundler"] +resolver = "2" [workspace.package] version = "0.1.0-beta" diff --git a/crates/pool/src/chain.rs b/crates/pool/src/chain.rs index 7e2f4a34f..8b95f0c70 100644 --- a/crates/pool/src/chain.rs +++ b/crates/pool/src/chain.rs @@ -508,8 +508,7 @@ mod tests { fn get_block_by_hash(&self, hash: H256) -> Option> { let blocks = self.blocks.read(); - let number = blocks.iter().position(|block| block.hash == hash); - let Some(number) = number else { return None }; + let number = blocks.iter().position(|block| block.hash == hash)?; let parent_hash = if number > 0 { blocks[number - 1].hash } else { diff --git a/crates/sim/src/estimation/estimation.rs b/crates/sim/src/estimation/estimation.rs index 2c96fb3ad..f095057a0 100644 --- a/crates/sim/src/estimation/estimation.rs +++ b/crates/sim/src/estimation/estimation.rs @@ -256,7 +256,6 @@ impl GasEstimatorImpl { guess = (max_failure_gas + min_success_gas) / 2; } - let mut min_success_gas = min_success_gas; if op.paymaster().is_none() { // If not using a paymaster, add the gas for the gas fee transfer. min_success_gas += GAS_FEE_TRANSFER_COST; From 440c096ed60a62c495603b189fa3901708c79564 Mon Sep 17 00:00:00 2001 From: 0xfourzerofour Date: Mon, 9 Oct 2023 14:45:03 -0400 Subject: [PATCH 9/9] chore(spectests): fix timeout issues and add to CI --- .github/workflows/compliance.yaml | 40 +++++++++++ Cargo.lock | 1 + crates/builder/proto/builder/builder.proto | 1 + crates/builder/src/bundle_sender.rs | 70 +++++++++---------- crates/builder/src/server/local.rs | 10 +-- crates/builder/src/server/mod.rs | 2 +- crates/builder/src/server/remote/client.rs | 4 +- crates/builder/src/server/remote/server.rs | 3 +- crates/builder/src/task.rs | 1 - crates/builder/src/transaction_tracker.rs | 2 +- crates/pool/src/server/mod.rs | 9 +++ crates/rpc/Cargo.toml | 1 + crates/rpc/src/debug.rs | 31 +++++++- test/spec-tests/bundler-spec-tests | 2 +- test/spec-tests/ci/run-spec-tests.sh | 8 ++- .../rundler-launcher/docker-compose.yml | 6 +- .../rundler-launcher/rundler-launcher.sh | 5 +- .../rundler-launcher/waitForServices.sh | 57 +++++++++++++++ 18 files changed, 192 insertions(+), 61 deletions(-) create mode 100644 .github/workflows/compliance.yaml create mode 100755 test/spec-tests/launchers/rundler-launcher/waitForServices.sh diff --git a/.github/workflows/compliance.yaml b/.github/workflows/compliance.yaml new file mode 100644 index 000000000..2d9cb08af --- /dev/null +++ b/.github/workflows/compliance.yaml @@ -0,0 +1,40 @@ +on: + push: + branches: + - main + pull_request: + +name: compliance +jobs: + compliance: + runs-on: ubuntu-latest + steps: + - name: Checkout sources + uses: actions/checkout@v3 + with: + submodules: recursive + - name: Cache Docker layers + uses: actions/cache@v2 + with: + path: /tmp/.buildx-cache + key: ${{ runner.os }}-buildx-${{ github.sha }} + restore-keys: | + ${{ runner.os }}-buildx- + - uses: actions/setup-node@v3 + with: + node-version: 18 + cache: 'yarn' + cache-dependency-path: test/spec-tests/bundler-spec-tests/@account-abstraction/yarn.lock + - name: Install Foundry + uses: foundry-rs/foundry-toolchain@v1 + - name: Setup PDM + uses: pdm-project/setup-pdm@v3 + with: + cache: true + cache-dependency-path: '**/pdm.lock' + - name: Run spec tests + run: ./test/spec-tests/ci/run-spec-tests.sh + - name: Move cache + run: | + rm -rf /tmp/.buildx-cache + mv /tmp/.buildx-cache-new /tmp/.buildx-cache diff --git a/Cargo.lock b/Cargo.lock index 5c08ff6e9..18682ef2f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4070,6 +4070,7 @@ dependencies = [ "anyhow", "async-trait", "ethers", + "futures-util", "jsonrpsee", "metrics", "rundler-builder", diff --git a/crates/builder/proto/builder/builder.proto b/crates/builder/proto/builder/builder.proto index 115216769..db32615b1 100644 --- a/crates/builder/proto/builder/builder.proto +++ b/crates/builder/proto/builder/builder.proto @@ -55,6 +55,7 @@ message DebugSendBundleNowResponse { } message DebugSendBundleNowSuccess { bytes transaction_hash = 1; + uint64 block_number = 2; } message DebugSetBundlingModeRequest { diff --git a/crates/builder/src/bundle_sender.rs b/crates/builder/src/bundle_sender.rs index f1864fdc3..f0d4af553 100644 --- a/crates/builder/src/bundle_sender.rs +++ b/crates/builder/src/bundle_sender.rs @@ -11,12 +11,9 @@ // You should have received a copy of the GNU General Public License along with Rundler. // If not, see https://www.gnu.org/licenses/. -use std::{ - sync::{ - atomic::{AtomicBool, Ordering}, - Arc, - }, - time::Duration, +use std::sync::{ + atomic::{AtomicBool, Ordering}, + Arc, }; use anyhow::{bail, Context}; @@ -31,7 +28,6 @@ use rundler_utils::emit::WithEntryPoint; use tokio::{ join, sync::{broadcast, mpsc, oneshot}, - time, }; use tracing::{error, info, trace, warn}; @@ -65,7 +61,6 @@ where send_bundle_receiver: mpsc::Receiver, chain_id: u64, beneficiary: Address, - eth_poll_interval: Duration, proposer: P, entry_point: E, transaction_tracker: T, @@ -141,44 +136,45 @@ where loop { let mut send_bundle_response: Option> = None; + let mut last_block = None; if self.manual_bundling_mode.load(Ordering::Relaxed) { - tokio::select! { - Some(r) = self.send_bundle_receiver.recv() => { - send_bundle_response = Some(r.responder); - } - _ = time::sleep(self.eth_poll_interval) => { - continue; - } + if let Some(r) = self.send_bundle_receiver.recv().await { + send_bundle_response = Some(r.responder); + } else { + error!("Bundle stream closed in manual mode"); + bail!("Bundle stream closed in manual mode"); } - } + } else { + // Wait for new block. Block number doesn't matter as the pool will only notify of new blocks + // after the pool has updated its state. The bundle will be formed using the latest pool state + // and can land in the next block + last_block = rx.recv().await; - // Wait for new block. Block number doesn't matter as the pool will only notify of new blocks - // after the pool has updated its state. The bundle will be formed using the latest pool state - // and can land in the next block - let mut last_block = match rx.recv().await { - Some(b) => b, - None => { + if last_block.is_none() { error!("Block stream closed"); bail!("Block stream closed"); } - }; - // Consume any other blocks that may have been buffered up - loop { - match rx.try_recv() { - Ok(b) => { - last_block = b; - } - Err(mpsc::error::TryRecvError::Empty) => { - break; - } - Err(mpsc::error::TryRecvError::Disconnected) => { - error!("Block stream closed"); - bail!("Block stream closed"); + // Consume any other blocks that may have been buffered up + loop { + match rx.try_recv() { + Ok(b) => { + last_block = Some(b); + } + Err(mpsc::error::TryRecvError::Empty) => { + break; + } + Err(mpsc::error::TryRecvError::Disconnected) => { + error!("Block stream closed"); + bail!("Block stream closed"); + } } } } + // Wait for new block. Block number doesn't matter as the pool will only notify of new blocks + // after the pool has updated its state. The bundle will be formed using the latest pool state + // and can land in the next block self.check_for_and_log_transaction_update().await; let result = self.send_bundle_with_increasing_gas_fees().await; match &result { @@ -192,7 +188,7 @@ where } else { info!("Bundle with hash {tx_hash:?} landed in block {block_number} after increasing gas fees {attempt_number} time(s)"); } - SendBundleResult::NoOperationsInitially => trace!("No ops to send at block {}", last_block.block_number), + SendBundleResult::NoOperationsInitially => trace!("No ops to send at block {}", last_block.unwrap_or_default().block_number), SendBundleResult::NoOperationsAfterFeeIncreases { initial_op_count, attempt_number, @@ -227,7 +223,6 @@ where send_bundle_receiver: mpsc::Receiver, chain_id: u64, beneficiary: Address, - eth_poll_interval: Duration, proposer: P, entry_point: E, transaction_tracker: T, @@ -241,7 +236,6 @@ where send_bundle_receiver, chain_id, beneficiary, - eth_poll_interval, proposer, entry_point, transaction_tracker, diff --git a/crates/builder/src/server/local.rs b/crates/builder/src/server/local.rs index 8dadbb7c6..ce620b5b8 100644 --- a/crates/builder/src/server/local.rs +++ b/crates/builder/src/server/local.rs @@ -113,11 +113,11 @@ impl BuilderServer for LocalBuilderHandle { } } - async fn debug_send_bundle_now(&self) -> BuilderResult { + async fn debug_send_bundle_now(&self) -> BuilderResult<(H256, u64)> { let req = ServerRequestKind::DebugSendBundleNow; let resp = self.send(req).await?; match resp { - ServerResponse::DebugSendBundleNow { hash } => Ok(hash), + ServerResponse::DebugSendBundleNow { hash, block_number } => Ok((hash, block_number)), _ => Err(BuilderServerError::UnexpectedResponse), } } @@ -197,8 +197,8 @@ impl LocalBuilderServerRunner { }; match result { - SendBundleResult::Success { tx_hash, .. } => { - Ok(ServerResponse::DebugSendBundleNow { hash: tx_hash }) + SendBundleResult::Success { tx_hash, block_number, .. } => { + Ok(ServerResponse::DebugSendBundleNow { hash: tx_hash, block_number }) }, SendBundleResult::NoOperationsInitially => { Err(anyhow::anyhow!("no ops to send").into()) @@ -242,6 +242,6 @@ struct ServerRequest { #[derive(Clone, Debug)] enum ServerResponse { GetSupportedEntryPoints { entry_points: Vec

}, - DebugSendBundleNow { hash: H256 }, + DebugSendBundleNow { hash: H256, block_number: u64 }, DebugSetBundlingMode, } diff --git a/crates/builder/src/server/mod.rs b/crates/builder/src/server/mod.rs index 1f4ba1695..4193e3ad3 100644 --- a/crates/builder/src/server/mod.rs +++ b/crates/builder/src/server/mod.rs @@ -48,7 +48,7 @@ pub trait BuilderServer: Send + Sync + 'static { /// Trigger the builder to send a bundle now, used for debugging. /// /// Bundling mode must be set to `Manual`, or this will error - async fn debug_send_bundle_now(&self) -> BuilderResult; + async fn debug_send_bundle_now(&self) -> BuilderResult<(H256, u64)>; /// Set the bundling mode async fn debug_set_bundling_mode(&self, mode: BundlingMode) -> BuilderResult<()>; diff --git a/crates/builder/src/server/remote/client.rs b/crates/builder/src/server/remote/client.rs index b8ff964e0..34d23e372 100644 --- a/crates/builder/src/server/remote/client.rs +++ b/crates/builder/src/server/remote/client.rs @@ -69,7 +69,7 @@ impl BuilderServer for RemoteBuilderClient { .collect::>()?) } - async fn debug_send_bundle_now(&self) -> BuilderResult { + async fn debug_send_bundle_now(&self) -> BuilderResult<(H256, u64)> { let res = self .grpc_client .clone() @@ -80,7 +80,7 @@ impl BuilderServer for RemoteBuilderClient { match res { Some(debug_send_bundle_now_response::Result::Success(s)) => { - Ok(H256::from_slice(&s.transaction_hash)) + Ok((H256::from_slice(&s.transaction_hash), s.block_number)) } Some(debug_send_bundle_now_response::Result::Failure(f)) => Err(f.try_into()?), None => Err(BuilderServerError::Other(anyhow::anyhow!( diff --git a/crates/builder/src/server/remote/server.rs b/crates/builder/src/server/remote/server.rs index a2ae3b82f..6c25be149 100644 --- a/crates/builder/src/server/remote/server.rs +++ b/crates/builder/src/server/remote/server.rs @@ -102,10 +102,11 @@ impl GrpcBuilder for GrpcBuilderServerImpl { _request: Request, ) -> tonic::Result> { let resp = match self.local_builder.debug_send_bundle_now().await { - Ok(hash) => DebugSendBundleNowResponse { + Ok((hash, block_number)) => DebugSendBundleNowResponse { result: Some(debug_send_bundle_now_response::Result::Success( DebugSendBundleNowSuccess { transaction_hash: hash.as_bytes().to_vec(), + block_number, }, )), }, diff --git a/crates/builder/src/task.rs b/crates/builder/src/task.rs index 5fe468afa..de6735ccc 100644 --- a/crates/builder/src/task.rs +++ b/crates/builder/src/task.rs @@ -326,7 +326,6 @@ where send_bundle_rx, self.args.chain_id, beneficiary, - self.args.eth_poll_interval, proposer, entry_point, transaction_tracker, diff --git a/crates/builder/src/transaction_tracker.rs b/crates/builder/src/transaction_tracker.rs index 2a7ad0d75..254181aa5 100644 --- a/crates/builder/src/transaction_tracker.rs +++ b/crates/builder/src/transaction_tracker.rs @@ -180,7 +180,7 @@ where let nonce = provider .get_transaction_count(sender.address()) .await - .context("tracker should load initial nonce on construction")?; + .unwrap_or(U256::zero()); Ok(Self { provider, sender, diff --git a/crates/pool/src/server/mod.rs b/crates/pool/src/server/mod.rs index c70e9aa6a..ad4737f20 100644 --- a/crates/pool/src/server/mod.rs +++ b/crates/pool/src/server/mod.rs @@ -39,6 +39,15 @@ pub struct NewHead { pub block_number: u64, } +impl Default for NewHead { + fn default() -> NewHead { + NewHead { + block_hash: H256::zero(), + block_number: 0, + } + } +} + /// Pool server trait #[cfg_attr(feature = "test-utils", automock)] #[async_trait] diff --git a/crates/rpc/Cargo.toml b/crates/rpc/Cargo.toml index 1d7903998..067d78a90 100644 --- a/crates/rpc/Cargo.toml +++ b/crates/rpc/Cargo.toml @@ -29,3 +29,4 @@ tracing.workspace = true serde.workspace = true strum.workspace = true url.workspace = true +futures-util.workspace = true diff --git a/crates/rpc/src/debug.rs b/crates/rpc/src/debug.rs index f4d127587..e722fceef 100644 --- a/crates/rpc/src/debug.rs +++ b/crates/rpc/src/debug.rs @@ -13,6 +13,7 @@ use async_trait::async_trait; use ethers::types::{Address, H256}; +use futures_util::StreamExt; use jsonrpsee::{core::RpcResult, proc_macros::rpc, types::error::INTERNAL_ERROR_CODE}; use rundler_builder::{BuilderServer, BundlingMode}; use rundler_pool::PoolServer; @@ -95,10 +96,36 @@ where } async fn bundler_send_bundle_now(&self) -> RpcResult { - self.builder + let mut new_heads = self + .pool + .subscribe_new_heads() + .await + .map_err(|e| rpc_err(INTERNAL_ERROR_CODE, e.to_string()))?; + + let (tx, block_number) = self + .builder .debug_send_bundle_now() .await - .map_err(|e| rpc_err(INTERNAL_ERROR_CODE, e.to_string())) + .map_err(|e| rpc_err(INTERNAL_ERROR_CODE, e.to_string()))?; + + // After the bundle is sent, we need to make sure that the mempool + // has processes the same block that the transaction was mined on. + // This removes the potential for an incorrect response from `debug_bundler_dumpMempool` + // method. + loop { + match new_heads.next().await { + Some(b) => { + if b.block_number.eq(&block_number) { + break; + } + } + None => { + return Err(rpc_err(INTERNAL_ERROR_CODE, "Next block not available")); + } + } + } + + Ok(tx) } async fn bundler_set_bundling_mode(&self, mode: BundlingMode) -> RpcResult { diff --git a/test/spec-tests/bundler-spec-tests b/test/spec-tests/bundler-spec-tests index 2e81f94f1..b8a738db7 160000 --- a/test/spec-tests/bundler-spec-tests +++ b/test/spec-tests/bundler-spec-tests @@ -1 +1 @@ -Subproject commit 2e81f94f1ee21a4a30a90f84b2a11beaef733965 +Subproject commit b8a738db703b9a135fdf21be9d377f381a33bc7d diff --git a/test/spec-tests/ci/run-spec-tests.sh b/test/spec-tests/ci/run-spec-tests.sh index 2cc4e0e52..ae7eea2ad 100755 --- a/test/spec-tests/ci/run-spec-tests.sh +++ b/test/spec-tests/ci/run-spec-tests.sh @@ -1,11 +1,13 @@ #!/bin/bash -set -x +set -e cd "$(dirname "$0")" (cd ../bundler-spec-tests && pdm install && pdm run update-deps) -docker build ../../.. -t alchemy-platform/rundler:latest -(cd ../bundler-spec-tests && pdm run test --launcher-script=../launchers/rundler-launcher/rundler-launcher.sh $@) +docker buildx create --use +docker buildx build --load --cache-from type=local,src=/tmp/.buildx-cache --cache-to type=local,mode=max,dest=/tmp/.buildx-cache-new -t alchemy-platform/rundler:latest ../../.. + +(cd ../bundler-spec-tests && pdm run test --url http://127.0.0.1:3000 --launcher-script=../launchers/rundler-launcher/rundler-launcher.sh $@) ../launchers/rundler-launcher/rundler-launcher.sh stop diff --git a/test/spec-tests/launchers/rundler-launcher/docker-compose.yml b/test/spec-tests/launchers/rundler-launcher/docker-compose.yml index be0b88e5b..09f931a59 100644 --- a/test/spec-tests/launchers/rundler-launcher/docker-compose.yml +++ b/test/spec-tests/launchers/rundler-launcher/docker-compose.yml @@ -3,12 +3,10 @@ version: "3.8" services: rundler: image: alchemy-platform/rundler:$TAG - depends_on: - - geth ports: - "3000:3000" - "8080:8080" - command: bash -c "sleep 10; /usr/local/bin/rundler node" + command: bash -c "/usr/local/bin/rundler node" environment: - RUST_LOG=debug - ENTRY_POINTS=0x5FF137D4b0FDCD49DcA30c7CF57E578a026d2789 @@ -17,7 +15,7 @@ services: - MIN_UNSTAKE_DELAY=2 - BUILDER_PRIVATE_KEY=0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80 healthcheck: - test: curl --fail http://localhost:3000/health || exit 1 + test: curl --fail http://rundler:3000/health || exit 1 interval: 1s timeout: 1s retries: 60 diff --git a/test/spec-tests/launchers/rundler-launcher/rundler-launcher.sh b/test/spec-tests/launchers/rundler-launcher/rundler-launcher.sh index 9ff4d7dc7..69a736afe 100755 --- a/test/spec-tests/launchers/rundler-launcher/rundler-launcher.sh +++ b/test/spec-tests/launchers/rundler-launcher/rundler-launcher.sh @@ -10,8 +10,9 @@ case $1 in ;; start) - docker-compose up -d --wait - cast send --from $(cast rpc eth_accounts | tail -n 1 | tr -d '[]"') --value 1ether 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266 > /dev/null + docker-compose up -d + ./waitForServices.sh + cast send --from $(cast rpc eth_accounts | tail -n 1 | tr -d '[]"') --unlocked --value 1ether 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266 > /dev/null cd ../../bundler-spec-tests/@account-abstraction && yarn deploy --network localhost ;; stop) diff --git a/test/spec-tests/launchers/rundler-launcher/waitForServices.sh b/test/spec-tests/launchers/rundler-launcher/waitForServices.sh new file mode 100755 index 000000000..c037a9c65 --- /dev/null +++ b/test/spec-tests/launchers/rundler-launcher/waitForServices.sh @@ -0,0 +1,57 @@ +#!/bin/bash + +# Define the service and port combinations to check +services=("rundler" "geth") +endpoints=("/health" "") # Specify the endpoint for service1 and leave service2 empty +ports=(3000 8545) + +# Set the total duration in seconds +total_duration=30 + +# Initialize flags to track service status +rundler_active=false +geth_active=false + +# Loop for the total duration with a 1-second interval +for ((i=0; i/dev/null; then + echo "${service} on port ${port} is active." + + # Set the flag for the corresponding service to true + if [ "${service}" == "rundler" ]; then + rundler_active=true + elif [ "${service}" == "geth" ]; then + geth_active=true + fi + fi + done + + # Sleep for 1 second before the next iteration + sleep 1 +done + +# If we reach this point, it means both services were not active within the 30-second window +echo "Both services were not active within the 30-second window. Exiting with failure." +exit 1