Skip to content

Commit

Permalink
feat: add maximum number of expected storage slots param
Browse files Browse the repository at this point in the history
  • Loading branch information
dancoombs committed Jan 10, 2025
1 parent f88136c commit d7a2a7e
Show file tree
Hide file tree
Showing 14 changed files with 200 additions and 14 deletions.
1 change: 1 addition & 0 deletions bin/rundler/src/cli/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
})
}

Expand Down
7 changes: 7 additions & 0 deletions bin/rundler/src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<usize>,
}

const SIMULATION_GAS_OVERHEAD: u64 = 100_000;
Expand Down
1 change: 1 addition & 0 deletions bin/rundler/src/cli/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ impl PoolArgs {
da_gas_tracking_enabled,
gas_limit_efficiency_reject_threshold: self.gas_limit_efficiency_reject_threshold,
max_time_in_pool: self.max_time_in_pool_secs.map(Duration::from_secs),
max_expected_storage_slots: common.max_expected_storage_slots.unwrap_or(usize::MAX),
};

let mut pool_configs = vec![];
Expand Down
135 changes: 128 additions & 7 deletions crates/builder/src/bundle_proposer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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(),
});
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -569,22 +567,50 @@ 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.computation_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.num_slots() > 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
Expand All @@ -601,6 +627,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.");
Expand Down Expand Up @@ -632,11 +659,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
}

Expand Down Expand Up @@ -1204,6 +1234,7 @@ struct ProposalContext<UO> {
rejected_ops: Vec<(UO, EntityInfos)>,
// This is a BTreeMap so that the conversion to a Vec<EntityUpdate> is deterministic, mainly for tests
entity_updates: BTreeMap<Address, EntityUpdate>,
expected_storage: ExpectedStorage,
}

#[derive(Debug)]
Expand All @@ -1227,6 +1258,7 @@ impl<UO: UserOperation> ProposalContext<UO> {
groups_by_aggregator: LinkedHashMap::<Option<Address>, AggregatorGroup<UO>>::new(),
rejected_ops: Vec::<(UO, EntityInfos)>::new(),
entity_updates: BTreeMap::new(),
expected_storage: ExpectedStorage::default(),
}
}

Expand Down Expand Up @@ -2433,6 +2465,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)
Expand Down Expand Up @@ -2474,6 +2507,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);

Expand Down Expand Up @@ -2692,6 +2726,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<dyn Fn() -> Result<SimulationResult, SimulationError> + Send + Sync>,
Expand All @@ -2717,6 +2835,8 @@ mod tests {
.await
}

const MAX_EXPECTED_STORAGE_SLOTS: usize = 100;

#[allow(clippy::too_many_arguments)]
async fn mock_make_bundle(
mock_ops: Vec<MockOp>,
Expand Down Expand Up @@ -2867,6 +2987,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,
);
Expand Down
6 changes: 6 additions & 0 deletions crates/builder/src/emit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> },
}
Expand Down
3 changes: 3 additions & 0 deletions crates/builder/src/task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down
6 changes: 6 additions & 0 deletions crates/pool/proto/op_pool/op_pool.proto
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,7 @@ message MempoolError {
OperationDropTooSoon operation_drop_too_soon = 16;
PreOpGasLimitEfficiencyTooLow pre_op_gas_limit_efficiency_too_low = 17;
ExecutionGasLimitEfficiencyTooLow execution_gas_limit_efficiency_too_low = 18;
TooManyExpectedStorageSlots too_many_expected_storage_slots = 19;
}
}

Expand Down Expand Up @@ -594,6 +595,11 @@ message ExecutionGasLimitEfficiencyTooLow {
float actual = 2;
}

message TooManyExpectedStorageSlots {
uint64 max_slots = 1;
uint64 expected_slots = 2;
}

// PRECHECK VIOLATIONS
message PrecheckViolationError {
oneof violation {
Expand Down
2 changes: 2 additions & 0 deletions crates/pool/src/mempool/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,8 @@ pub struct PoolConfig {
pub gas_limit_efficiency_reject_threshold: f32,
/// Maximum time a UO is allowed in the pool before being dropped
pub max_time_in_pool: Option<Duration>,
/// 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.
Expand Down
10 changes: 10 additions & 0 deletions crates/pool/src/mempool/uo_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,15 @@ where
return Err(MempoolError::UnsupportedAggregator(agg.address));
}

// Check if op has more than the maximum allowed expected storage slots
let expected_slots = sim_result.expected_storage.num_slots();
if expected_slots > self.config.max_expected_storage_slots {
return Err(MempoolError::TooManyExpectedStorageSlots(
self.config.max_expected_storage_slots,
expected_slots,
));
}

// Check if op violates the STO-041 spec rule
self.state
.read()
Expand Down Expand Up @@ -1907,6 +1916,7 @@ mod tests {
drop_min_num_blocks: 10,
gas_limit_efficiency_reject_threshold: 0.0,
max_time_in_pool: None,
max_expected_storage_slots: usize::MAX,
}
}

Expand Down
Loading

0 comments on commit d7a2a7e

Please sign in to comment.