Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(gas): amortize evm transaction fixed cost over bundle #442

Merged
merged 3 commits into from
Oct 5, 2023
Merged

fix(gas): amortize evm transaction fixed cost over bundle #442

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d1bfebb
fix(gas): amortize evm transaction fixed cost over bundle
alex-miao Oct 3, 2023
10c13ed
fix(gas): refactor bundle gas calculation
alex-miao Oct 3, 2023
7bb3809
fix(gas): refactor gas overheads
alex-miao Oct 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/builder/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
26 changes: 15 additions & 11 deletions crates/builder/src/bundle_proposer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
dancoombs marked this conversation as resolved.
Show resolved Hide resolved

#[derive(Debug, Default)]
pub(crate) struct Bundle {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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<Item = &OpWithSimulation> + '_ {
Expand Down Expand Up @@ -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,
);

Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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
))
);
Expand All @@ -1216,7 +1220,7 @@ mod tests {
}

async fn simple_make_bundle(mock_ops: Vec<MockOp>) -> Bundle {
make_bundle(
mock_make_bundle(
mock_ops,
vec![],
vec![HandleOpsOut::Success],
Expand All @@ -1227,7 +1231,7 @@ mod tests {
.await
}

async fn make_bundle(
async fn mock_make_bundle(
mock_ops: Vec<MockOp>,
mock_aggregators: Vec<MockAggregator>,
mock_handle_ops_call_results: Vec<HandleOpsOut>,
Expand Down
22 changes: 14 additions & 8 deletions crates/sim/src/gas/gas.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ struct GasOverheads {
per_user_op_word: U256,
zero_byte: U256,
non_zero_byte: U256,
bundle_size: U256,
dancoombs marked this conversation as resolved.
Show resolved Hide resolved
}

impl Default for GasOverheads {
Expand All @@ -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(),
}
}
}
Expand All @@ -74,7 +72,7 @@ pub async fn calc_pre_verification_gas<P: Provider>(
provider: Arc<P>,
chain_id: u64,
) -> anyhow::Result<U256> {
let static_gas = calc_static_pre_verification_gas(full_op);
let static_gas = calc_static_pre_verification_gas(full_op, true);
dancoombs marked this conversation as resolved.
Show resolved Hide resolved
let dynamic_gas = match chain_id {
_ if ARBITRUM_CHAIN_IDS.contains(&chain_id) => {
provider
Expand All @@ -95,12 +93,16 @@ pub async fn calc_pre_verification_gas<P: Provider>(
}

/// 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
};
Expand All @@ -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
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion crates/sim/src/precheck.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ impl<P: Provider, E: EntryPoint> PrecheckerImpl<P, E> {
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);
dancoombs marked this conversation as resolved.
Show resolved Hide resolved
if total_gas_limit > max_total_execution_gas {
violations.push(PrecheckViolation::TotalGasLimitTooHigh(
total_gas_limit,
Expand Down
Loading