diff --git a/bin/rundler/src/cli/builder.rs b/bin/rundler/src/cli/builder.rs index 0e5f485a1..79d4f7984 100644 --- a/bin/rundler/src/cli/builder.rs +++ b/bin/rundler/src/cli/builder.rs @@ -361,6 +361,7 @@ impl BuilderArgs { remote_address, da_gas_tracking_enabled, provider_client_timeout_seconds, + max_expected_storage_slots: common.max_expected_storage_slots.unwrap_or(usize::MAX), }) } diff --git a/bin/rundler/src/cli/mod.rs b/bin/rundler/src/cli/mod.rs index 959c37773..7cab00982 100644 --- a/bin/rundler/src/cli/mod.rs +++ b/bin/rundler/src/cli/mod.rs @@ -354,6 +354,13 @@ pub struct CommonArgs { default_value = "10" )] pub provider_client_timeout_seconds: u64, + + #[arg( + long = "max_expected_storage_slots", + name = "max_expected_storage_slots", + env = "MAX_EXPECTED_STORAGE_SLOTS" + )] + pub max_expected_storage_slots: Option, } const SIMULATION_GAS_OVERHEAD: u64 = 100_000; diff --git a/bin/rundler/src/cli/pool.rs b/bin/rundler/src/cli/pool.rs index 50e8b2a4a..6fd9872d2 100644 --- a/bin/rundler/src/cli/pool.rs +++ b/bin/rundler/src/cli/pool.rs @@ -235,6 +235,7 @@ impl PoolArgs { drop_min_num_blocks: self.drop_min_num_blocks, da_gas_tracking_enabled, gas_limit_efficiency_reject_threshold: self.gas_limit_efficiency_reject_threshold, + max_expected_storage_slots: common.max_expected_storage_slots.unwrap_or(usize::MAX), }; let mut pool_configs = vec![]; diff --git a/crates/builder/src/bundle_proposer.rs b/crates/builder/src/bundle_proposer.rs index bbde98fba..f77cddfdd 100644 --- a/crates/builder/src/bundle_proposer.rs +++ b/crates/builder/src/bundle_proposer.rs @@ -156,6 +156,7 @@ pub(crate) struct Settings { pub(crate) bundle_priority_fee_overhead_percent: u32, pub(crate) priority_fee_mode: PriorityFeeMode, pub(crate) da_gas_tracking_enabled: bool, + pub(crate) max_expected_storage_slots: usize, } #[async_trait] @@ -303,15 +304,11 @@ where } } - 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, gas_fees: bundle_fees, - expected_storage, + expected_storage: context.expected_storage, rejected_ops: context.rejected_ops.iter().map(|po| po.0.clone()).collect(), entity_updates: context.entity_updates.into_values().collect(), }); @@ -528,6 +525,7 @@ where let mut gas_spent = rundler_types::bundle_shared_gas(&self.settings.chain_spec); let mut constructed_bundle_size = BUNDLE_BYTE_OVERHEAD; + for (po, simulation) in ops_with_simulations { let op = po.clone().uo; let simulation = match simulation { @@ -569,21 +567,49 @@ where continue; } + // Limit by transaction size let op_size_bytes: usize = op.abi_encoded_size(); - let op_size_with_offset_word = op_size_bytes.saturating_add(USER_OP_OFFSET_WORD_SIZE); - if op_size_with_offset_word.saturating_add(constructed_bundle_size) >= self.settings.chain_spec.max_transaction_size_bytes { + self.emit(BuilderEvent::skipped_op( + self.builder_index, + self.op_hash(&op), + SkipReason::TransactionSizeLimit, + )); continue; } // Skip this op if the bundle does not have enough remaining gas to execute it. let required_gas = gas_spent + op.execution_gas_limit(&self.settings.chain_spec, None); if required_gas > self.settings.max_bundle_gas { + self.emit(BuilderEvent::skipped_op( + self.builder_index, + self.op_hash(&op), + SkipReason::GasLimit, + )); + continue; + } + + // Merge the expected storage and skip if there is a conflict or if the storage is over max + let mut new_expected_storage = context.expected_storage.clone(); + if let Err(e) = new_expected_storage.merge(&simulation.expected_storage) { + self.emit(BuilderEvent::skipped_op( + self.builder_index, + self.op_hash(&op), + SkipReason::ExpectedStorageConflict(e.to_string()), + )); + continue; + } else if new_expected_storage.len() > self.settings.max_expected_storage_slots { + self.emit(BuilderEvent::skipped_op( + self.builder_index, + self.op_hash(&op), + SkipReason::ExpectedStorageLimit, + )); continue; } + context.expected_storage = new_expected_storage; if let Some(&other_sender) = simulation .accessed_addresses @@ -600,6 +626,7 @@ where )); continue; } + if let Some(paymaster) = op.paymaster() { let Some(balance) = balances_by_paymaster.get_mut(&paymaster) else { error!("Op had paymaster with unknown balance, but balances should have been loaded for all paymasters in bundle."); @@ -631,11 +658,14 @@ where simulation, }); } + for paymaster in paymasters_to_reject { // No need to update aggregator signatures because we haven't computed them yet. let _ = context.reject_entity(paymaster.entity, paymaster.is_staked); } + self.compute_all_aggregator_signatures(&mut context).await; + context } @@ -1203,6 +1233,7 @@ struct ProposalContext { rejected_ops: Vec<(UO, EntityInfos)>, // This is a BTreeMap so that the conversion to a Vec is deterministic, mainly for tests entity_updates: BTreeMap, + expected_storage: ExpectedStorage, } #[derive(Debug)] @@ -1226,6 +1257,7 @@ impl ProposalContext { groups_by_aggregator: LinkedHashMap::, AggregatorGroup>::new(), rejected_ops: Vec::<(UO, EntityInfos)>::new(), entity_updates: BTreeMap::new(), + expected_storage: ExpectedStorage::default(), } } @@ -2432,6 +2464,7 @@ mod tests { groups_by_aggregator, rejected_ops: vec![], entity_updates: BTreeMap::new(), + expected_storage: ExpectedStorage::default(), }; let expected_gas_limit = op1.gas_limit(&cs, None) @@ -2473,6 +2506,7 @@ mod tests { groups_by_aggregator, rejected_ops: vec![], entity_updates: BTreeMap::new(), + expected_storage: ExpectedStorage::default(), }; let gas_limit = context.get_bundle_gas_limit(&cs); @@ -2691,6 +2725,90 @@ mod tests { assert_eq!(bundle.rejected_ops, vec![op]); } + #[tokio::test] + async fn test_single_uo_max_expected_storage_slots() { + let op = default_op(); + let mut expected_storage = ExpectedStorage::default(); + for _ in 0..=MAX_EXPECTED_STORAGE_SLOTS { + expected_storage.insert(Address::random(), U256::ZERO, U256::ZERO); + } + + let bundle = mock_make_bundle( + vec![MockOp { + op, + simulation_result: Box::new(move || { + Ok(SimulationResult { + expected_storage: expected_storage.clone(), + ..Default::default() + }) + }), + }], + vec![], + vec![HandleOpsOut::Success], + vec![], + 0, + 0, + false, + ExpectedStorage::default(), + false, + ) + .await; + + assert!(bundle.is_empty()) + } + + #[tokio::test] + async fn test_skip_max_expected_storage_slots() { + let op0 = op_with_sender(address(1)); + let op1 = op_with_sender(address(2)); + let mut expected_storage0 = ExpectedStorage::default(); + for _ in 0..MAX_EXPECTED_STORAGE_SLOTS { + expected_storage0.insert(Address::random(), U256::ZERO, U256::ZERO); + } + let mut expected_storage1 = ExpectedStorage::default(); + expected_storage1.insert(Address::random(), U256::ZERO, U256::ZERO); + + let bundle = mock_make_bundle( + vec![ + MockOp { + op: op0.clone(), + simulation_result: Box::new(move || { + Ok(SimulationResult { + expected_storage: expected_storage0.clone(), + ..Default::default() + }) + }), + }, + MockOp { + op: op1.clone(), + simulation_result: Box::new(move || { + Ok(SimulationResult { + expected_storage: expected_storage1.clone(), + ..Default::default() + }) + }), + }, + ], + vec![], + vec![HandleOpsOut::Success], + vec![], + 0, + 0, + false, + ExpectedStorage::default(), + false, + ) + .await; + + assert_eq!( + bundle.ops_per_aggregator, + vec![UserOpsPerAggregator { + user_ops: vec![op0], + ..Default::default() + }] + ); + } + struct MockOp { op: UserOperation, simulation_result: Box Result + Send + Sync>, @@ -2716,6 +2834,8 @@ mod tests { .await } + const MAX_EXPECTED_STORAGE_SLOTS: usize = 100; + #[allow(clippy::too_many_arguments)] async fn mock_make_bundle( mock_ops: Vec, @@ -2866,6 +2986,7 @@ mod tests { bundle_base_fee_overhead_percent: 27, bundle_priority_fee_overhead_percent: 0, da_gas_tracking_enabled, + max_expected_storage_slots: MAX_EXPECTED_STORAGE_SLOTS, }, event_sender, ); diff --git a/crates/builder/src/emit.rs b/crates/builder/src/emit.rs index 4b2cee467..0855848c5 100644 --- a/crates/builder/src/emit.rs +++ b/crates/builder/src/emit.rs @@ -186,6 +186,12 @@ pub enum SkipReason { }, /// Bundle ran out of space by gas limit to include the operation GasLimit, + /// Expected storage conflict + ExpectedStorageConflict(String), + /// Expected storage limit reached + ExpectedStorageLimit, + /// Transaction size limit reached + TransactionSizeLimit, /// Other reason, typically internal errors Other { reason: Arc }, } diff --git a/crates/builder/src/task.rs b/crates/builder/src/task.rs index c67f8de8f..79064cd7b 100644 --- a/crates/builder/src/task.rs +++ b/crates/builder/src/task.rs @@ -91,6 +91,8 @@ pub struct Args { pub da_gas_tracking_enabled: bool, /// Provider client timeout pub provider_client_timeout_seconds: u64, + /// Maximum number of expected storage slots in a bundle + pub max_expected_storage_slots: usize, } /// Builder settings for an entrypoint @@ -355,6 +357,7 @@ where bundle_base_fee_overhead_percent: self.args.bundle_base_fee_overhead_percent, bundle_priority_fee_overhead_percent: self.args.bundle_priority_fee_overhead_percent, da_gas_tracking_enabled: self.args.da_gas_tracking_enabled, + max_expected_storage_slots: self.args.max_expected_storage_slots, }; let transaction_sender = self.args.sender_args.clone().into_sender( diff --git a/crates/pool/proto/op_pool/op_pool.proto b/crates/pool/proto/op_pool/op_pool.proto index f8b2020d8..94906b4be 100644 --- a/crates/pool/proto/op_pool/op_pool.proto +++ b/crates/pool/proto/op_pool/op_pool.proto @@ -532,6 +532,7 @@ message MempoolError { OperationDropTooSoon operation_drop_too_soon = 16; PreOpGasLimitEfficiencyTooLow pre_op_gas_limit_efficiency_too_low = 17; CallGasLimitEfficiencyTooLow call_gas_limit_efficiency_too_low = 18; + TooManyExpectedStorageSlots too_many_expected_storage_slots = 19; } } @@ -594,6 +595,11 @@ message CallGasLimitEfficiencyTooLow { float actual = 2; } +message TooManyExpectedStorageSlots { + uint64 max_slots = 1; + uint64 expected_slots = 2; +} + // PRECHECK VIOLATIONS message PrecheckViolationError { oneof violation { diff --git a/crates/pool/src/mempool/mod.rs b/crates/pool/src/mempool/mod.rs index 8307c5751..4395d5ffe 100644 --- a/crates/pool/src/mempool/mod.rs +++ b/crates/pool/src/mempool/mod.rs @@ -170,6 +170,8 @@ pub struct PoolConfig { /// Gas limit efficiency is defined as the ratio of the gas limit to the gas used. /// This applies to all the verification, call, and paymaster gas limits. pub gas_limit_efficiency_reject_threshold: f32, + /// The maximum number of storage slots that can be expected to be used by a user operation during validation + pub max_expected_storage_slots: usize, } /// Origin of an operation. diff --git a/crates/pool/src/mempool/uo_pool.rs b/crates/pool/src/mempool/uo_pool.rs index 658287615..b2f92ad09 100644 --- a/crates/pool/src/mempool/uo_pool.rs +++ b/crates/pool/src/mempool/uo_pool.rs @@ -554,6 +554,14 @@ where return Err(MempoolError::UnsupportedAggregator(agg.address)); } + // Check if op has more than the maximum allowed expected storage slots + if sim_result.expected_storage.len() > self.config.max_expected_storage_slots { + return Err(MempoolError::TooManyExpectedStorageSlots( + self.config.max_expected_storage_slots, + sim_result.expected_storage.len(), + )); + } + // Check if op violates the STO-041 spec rule self.state .read() @@ -1836,6 +1844,7 @@ mod tests { reputation_tracking_enabled: true, drop_min_num_blocks: 10, gas_limit_efficiency_reject_threshold: 0.0, + max_expected_storage_slots: usize::MAX, } } diff --git a/crates/pool/src/server/remote/error.rs b/crates/pool/src/server/remote/error.rs index 386038b97..43dea8a51 100644 --- a/crates/pool/src/server/remote/error.rs +++ b/crates/pool/src/server/remote/error.rs @@ -36,10 +36,10 @@ use super::protos::{ PaymasterIsNotContract, PreOpGasLimitEfficiencyTooLow, PreVerificationGasTooLow, PrecheckViolationError as ProtoPrecheckViolationError, ReplacementUnderpricedError, SenderAddressUsedAsAlternateEntity, SenderFundsTooLow, SenderIsNotContractAndNoInitCode, - SimulationViolationError as ProtoSimulationViolationError, TotalGasLimitTooHigh, - UnintendedRevert, UnintendedRevertWithMessage, UnknownEntryPointError, UnknownRevert, - UnstakedAggregator, UnstakedPaymasterContext, UnsupportedAggregatorError, UsedForbiddenOpcode, - UsedForbiddenPrecompile, ValidationRevert as ProtoValidationRevert, + SimulationViolationError as ProtoSimulationViolationError, TooManyExpectedStorageSlots, + TotalGasLimitTooHigh, UnintendedRevert, UnintendedRevertWithMessage, UnknownEntryPointError, + UnknownRevert, UnstakedAggregator, UnstakedPaymasterContext, UnsupportedAggregatorError, + UsedForbiddenOpcode, UsedForbiddenPrecompile, ValidationRevert as ProtoValidationRevert, VerificationGasLimitBufferTooLow, VerificationGasLimitTooHigh, WrongNumberOfPhases, }; @@ -133,6 +133,12 @@ impl TryFrom for MempoolError { Some(mempool_error::Error::CallGasLimitEfficiencyTooLow(e)) => { MempoolError::CallGasLimitEfficiencyTooLow(e.required, e.actual) } + Some(mempool_error::Error::TooManyExpectedStorageSlots(e)) => { + MempoolError::TooManyExpectedStorageSlots( + e.max_slots.try_into()?, + e.expected_slots.try_into()?, + ) + } None => bail!("unknown proto mempool error"), }) } @@ -247,6 +253,16 @@ impl From for ProtoMempoolError { CallGasLimitEfficiencyTooLow { required, actual }, )), }, + MempoolError::TooManyExpectedStorageSlots(max_slots, expected_slots) => { + ProtoMempoolError { + error: Some(mempool_error::Error::TooManyExpectedStorageSlots( + TooManyExpectedStorageSlots { + max_slots: max_slots as u64, + expected_slots: expected_slots as u64, + }, + )), + } + } } } } diff --git a/crates/rpc/src/eth/error.rs b/crates/rpc/src/eth/error.rs index f80baa3af..320dfa39a 100644 --- a/crates/rpc/src/eth/error.rs +++ b/crates/rpc/src/eth/error.rs @@ -317,6 +317,9 @@ impl From for EthRpcError { MempoolError::CallGasLimitEfficiencyTooLow(_, _) => { Self::InvalidParams(value.to_string()) } + MempoolError::TooManyExpectedStorageSlots(_, _) => { + Self::InvalidParams(value.to_string()) + } } } } diff --git a/crates/sim/src/types.rs b/crates/sim/src/types.rs index d8b6a0baf..5fcd700f6 100644 --- a/crates/sim/src/types.rs +++ b/crates/sim/src/types.rs @@ -54,6 +54,16 @@ impl ExpectedStorage { .or_default() .insert(B256::from(slot), B256::from(value)); } + + /// If the storage map is empty. + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } + + /// Size of the storage map. + pub fn len(&self) -> usize { + self.0.values().map(|slots| slots.len()).sum() + } } use std::fmt::{Display, Formatter}; diff --git a/crates/types/src/pool/error.rs b/crates/types/src/pool/error.rs index d070ca923..2889aeca4 100644 --- a/crates/types/src/pool/error.rs +++ b/crates/types/src/pool/error.rs @@ -100,6 +100,9 @@ pub enum MempoolError { /// Call gas limit efficiency too low #[error("Call gas limit efficiency too low. Required: {0}, Actual: {1}")] CallGasLimitEfficiencyTooLow(f32, f32), + /// Too many expected storage slots + #[error("Too many expected storage slots. Maximum: {0}, Actual: {1}")] + TooManyExpectedStorageSlots(usize, usize), } /// Precheck violation enumeration diff --git a/docs/cli.md b/docs/cli.md index e7da3292a..40d588a11 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -77,6 +77,8 @@ See [chain spec](./architecture/chain_spec.md) for a detailed description of cha - env: *NUM_BUILDERS_V0_7* - `--da_gas_tracking_enabled`: Enable the DA gas tracking feature of the mempool (default: `false`) - env: *DA_GAS_TRACKING_ENABLED* +- `--max_expected_storage_slots`: Optionally set the maximum number of expected storage slots to submit with a conditional transaction. (default: `None`) + - env: *MAX_EXPECTED_STORAGE_SLOTS* ## Metrics Options @@ -216,11 +218,11 @@ List of command line options for configuring the Builder. - `--builder.flashbots_relay_auth_key`: Only used/required if builder.sender == "flashbots." Authorization key to use with the flashbots relay. See [here](https://docs.flashbots.net/flashbots-auction/advanced/rpc-endpoint#authentication) for more info. (default: None) - env: *BUILDER_FLASHBOTS_RELAY_AUTH_KEY* - `--builder.bloxroute_auth_header`: Only used/required if builder.sender == "polygon_bloxroute." If using the bloxroute transaction sender on Polygon, this is the auth header to supply with the requests. (default: None) - - env: `BUILDER_BLOXROUTE_AUTH_HEADER` + - env: *BUILDER_BLOXROUTE_AUTH_HEADER* - `--builder.index_offset`: If running multiple builder processes, this is the index offset to assign unique indexes to each bundle sender. (default: 0) - - env: `BUILDER_INDEX_OFFSET` + - env: *BUILDER_INDEX_OFFSET* - `--builder.pool_url`: If running in distributed mode, the URL of the pool server to use. - - env: `BUILDER_POOL_URL` + - env: *BUILDER_POOL_URL* - *Only required when running in distributed mode* ### Key management