From 3eafd2c3589510626abaa32eac9de12e71439f53 Mon Sep 17 00:00:00 2001 From: dancoombs Date: Fri, 10 May 2024 14:14:48 -0500 Subject: [PATCH] fix(provider): use state overrides for gas used, not ctor method --- bin/rundler/chain_specs/arbitrum.toml | 2 +- bin/rundler/chain_specs/base.toml | 2 +- bin/rundler/chain_specs/optimism.toml | 2 +- bin/rundler/chain_specs/polygon.toml | 2 +- crates/builder/src/bundle_proposer.rs | 23 ++----- crates/provider/src/ethers/provider.rs | 25 ++++++++ crates/provider/src/traits/provider.rs | 12 +++- crates/rpc/src/eth/api.rs | 18 +++++- crates/rpc/src/task.rs | 7 +++ .../estimation/estimate_verification_gas.rs | 24 ++++---- crates/sim/src/estimation/v0_6.rs | 60 +++++++++---------- crates/sim/src/utils.rs | 25 +------- .../types/contracts/src/utils/GetGasUsed.sol | 25 +++++--- crates/types/src/chain.rs | 6 +- crates/types/src/user_operation/mod.rs | 29 +++++++++ crates/types/src/user_operation/v0_6.rs | 9 +++ crates/types/src/user_operation/v0_7.rs | 15 +++++ 17 files changed, 185 insertions(+), 101 deletions(-) diff --git a/bin/rundler/chain_specs/arbitrum.toml b/bin/rundler/chain_specs/arbitrum.toml index 96e2c1693..116d07fe7 100644 --- a/bin/rundler/chain_specs/arbitrum.toml +++ b/bin/rundler/chain_specs/arbitrum.toml @@ -6,4 +6,4 @@ l1_gas_oracle_contract_type = "ARBITRUM_NITRO" l1_gas_oracle_contract_address = "0x00000000000000000000000000000000000000C8" supports_eip1559 = false -max_bundle_size_bytes = 95000 +max_transaction_size_bytes = 95000 diff --git a/bin/rundler/chain_specs/base.toml b/bin/rundler/chain_specs/base.toml index f8eca2ba2..f038227f2 100644 --- a/bin/rundler/chain_specs/base.toml +++ b/bin/rundler/chain_specs/base.toml @@ -8,4 +8,4 @@ include_l1_gas_in_gas_limit = false priority_fee_oracle_type = "USAGE_BASED" min_max_priority_fee_per_gas = "0x0186A0" # 100_000 -max_bundle_size_bytes = 130000 +max_transaction_size_bytes = 130000 diff --git a/bin/rundler/chain_specs/optimism.toml b/bin/rundler/chain_specs/optimism.toml index b4ab38609..23a9608c1 100644 --- a/bin/rundler/chain_specs/optimism.toml +++ b/bin/rundler/chain_specs/optimism.toml @@ -8,4 +8,4 @@ include_l1_gas_in_gas_limit = false priority_fee_oracle_type = "USAGE_BASED" min_max_priority_fee_per_gas = "0x0186A0" # 100_000 -max_bundle_size_bytes = 90000 +max_transaction_size_bytes = 90000 diff --git a/bin/rundler/chain_specs/polygon.toml b/bin/rundler/chain_specs/polygon.toml index fe0d0b757..a541fd55f 100644 --- a/bin/rundler/chain_specs/polygon.toml +++ b/bin/rundler/chain_specs/polygon.toml @@ -4,4 +4,4 @@ id = 137 priority_fee_oracle_type = "USAGE_BASED" min_max_priority_fee_per_gas = "0x06FC23AC00" # 30_000_000_000 bloxroute_enabled = true -max_bundle_size_bytes = 300000 +max_transaction_size_bytes = 300000 diff --git a/crates/builder/src/bundle_proposer.rs b/crates/builder/src/bundle_proposer.rs index 8540f6130..4f777be47 100644 --- a/crates/builder/src/bundle_proposer.rs +++ b/crates/builder/src/bundle_proposer.rs @@ -40,7 +40,8 @@ use rundler_types::{ chain::ChainSpec, pool::{Pool, PoolOperation, SimulationViolation}, Entity, EntityInfo, EntityInfos, EntityType, EntityUpdate, EntityUpdateType, GasFees, - Timestamp, UserOperation, UserOperationVariant, UserOpsPerAggregator, + Timestamp, UserOperation, UserOperationVariant, UserOpsPerAggregator, BUNDLE_BYTE_OVERHEAD, + USER_OP_OFFSET_WORD_SIZE, }; use rundler_utils::{emit::WithEntryPoint, math}; use tokio::{sync::broadcast, try_join}; @@ -53,19 +54,6 @@ const TIME_RANGE_BUFFER: Duration = Duration::from_secs(60); /// 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; -/// Overhead for bytes required for each bundle -/// 4 bytes for function signature -/// 32 bytes for user op array offset -/// 32 bytes for beneficiary -/// 32 bytes for array count -/// Ontop of this offset there needs to be another 32 bytes for each -/// user operation in the bundle to store its offset within the array -const BUNDLE_BYTE_OVERHEAD: u64 = 4 + 32 + 32 + 32; - -/// Size of word that stores offset of user op location -/// within handleOps `ops` array -const USER_OP_OFFSET_WORD_SIZE: u64 = 32; - #[derive(Debug)] pub(crate) struct Bundle { pub(crate) ops_per_aggregator: Vec>, @@ -442,15 +430,12 @@ where continue; } - let op_size_bytes: u64 = op - .abi_encoded_size() - .try_into() - .expect("User operation size should fit within u64"); + 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_bundle_size_bytes + >= self.settings.chain_spec.max_transaction_size_bytes { continue; } diff --git a/crates/provider/src/ethers/provider.rs b/crates/provider/src/ethers/provider.rs index 8e700aeab..739d7ad3f 100644 --- a/crates/provider/src/ethers/provider.rs +++ b/crates/provider/src/ethers/provider.rs @@ -29,6 +29,9 @@ use ethers::{ }, }; use reqwest::Url; +use rundler_types::contracts::utils::get_gas_used::{ + GasUsedResult, GetGasUsed, GETGASUSED_DEPLOYED_BYTECODE, +}; use serde::{de::DeserializeOwned, Serialize}; use super::metrics_middleware::MetricsMiddleware; @@ -182,6 +185,28 @@ impl Provider for EthersProvider { async fn get_transaction_count(&self, address: Address) -> ProviderResult { Ok(Middleware::get_transaction_count(self, address, None).await?) } + + async fn get_gas_used( + self: &Arc, + target: Address, + value: U256, + data: Bytes, + mut state_overrides: spoof::State, + ) -> ProviderResult { + let helper_addr = Address::random(); + let helper = GetGasUsed::new(helper_addr, Arc::clone(self)); + + state_overrides + .account(helper_addr) + .code(GETGASUSED_DEPLOYED_BYTECODE.clone()); + + Ok(helper + .get_gas(target, value, data) + .call_raw() + .state(&state_overrides) + .await + .context("should get gas used")?) + } } impl From for ProviderError { diff --git a/crates/provider/src/traits/provider.rs b/crates/provider/src/traits/provider.rs index 4578194c4..86a313c8e 100644 --- a/crates/provider/src/traits/provider.rs +++ b/crates/provider/src/traits/provider.rs @@ -13,7 +13,7 @@ //! Trait for interacting with chain data and contracts. -use std::fmt::Debug; +use std::{fmt::Debug, sync::Arc}; use ethers::{ abi::{AbiDecode, AbiEncode}, @@ -25,6 +25,7 @@ use ethers::{ }; #[cfg(feature = "test-utils")] use mockall::automock; +use rundler_types::contracts::utils::get_gas_used::GasUsedResult; use serde::{de::DeserializeOwned, Serialize}; use super::error::ProviderError; @@ -127,4 +128,13 @@ pub trait Provider: Send + Sync + Debug + 'static { /// Get the logs matching a filter async fn get_logs(&self, filter: &Filter) -> ProviderResult>; + + /// Measures the gas used by a call to target with value and data. + async fn get_gas_used( + self: &Arc, + target: Address, + value: U256, + data: Bytes, + state_overrides: spoof::State, + ) -> ProviderResult; } diff --git a/crates/rpc/src/eth/api.rs b/crates/rpc/src/eth/api.rs index 6a209eb58..33067ff89 100644 --- a/crates/rpc/src/eth/api.rs +++ b/crates/rpc/src/eth/api.rs @@ -18,7 +18,9 @@ use ethers::{ utils::to_checksum, }; use futures_util::future; -use rundler_types::{chain::ChainSpec, pool::Pool, UserOperationOptionalGas, UserOperationVariant}; +use rundler_types::{ + chain::ChainSpec, pool::Pool, UserOperation, UserOperationOptionalGas, UserOperationVariant, +}; use rundler_utils::log::LogOnError; use tracing::Level; @@ -67,6 +69,13 @@ where op: UserOperationVariant, entry_point: Address, ) -> EthResult { + if op.single_uo_bundle_size_bytes() > self.chain_spec.max_transaction_size_bytes { + return Err(EthRpcError::InvalidParams(format!( + "User operation in bundle exceeds max transaciton size {}", + self.chain_spec.max_transaction_size_bytes + ))); + } + self.router.check_and_get_route(&entry_point, &op)?; self.pool @@ -82,6 +91,13 @@ where entry_point: Address, state_override: Option, ) -> EthResult { + if uo.single_uo_bundle_size_bytes() > self.chain_spec.max_transaction_size_bytes { + return Err(EthRpcError::InvalidParams(format!( + "User operation in bundle exceeds max transaciton size {}", + self.chain_spec.max_transaction_size_bytes + ))); + } + self.router .estimate_gas(&entry_point, uo, state_override) .await diff --git a/crates/rpc/src/task.rs b/crates/rpc/src/task.rs index ec5d79709..1e1a2771c 100644 --- a/crates/rpc/src/task.rs +++ b/crates/rpc/src/task.rs @@ -188,6 +188,13 @@ where .set_logger(RpcMetricsLogger) .set_middleware(service_builder) .max_connections(self.args.max_connections) + // Set max request body size to 2x the max transaction size as none of our + // APIs should require more than that. + .max_request_body_size( + (self.args.chain_spec.max_transaction_size_bytes * 2) + .try_into() + .expect("max_transaction_size_bytes * 2 overflowed u32"), + ) .http_only() .build(addr) .await?; diff --git a/crates/sim/src/estimation/estimate_verification_gas.rs b/crates/sim/src/estimation/estimate_verification_gas.rs index 7580926a9..1f6a3cae3 100644 --- a/crates/sim/src/estimation/estimate_verification_gas.rs +++ b/crates/sim/src/estimation/estimate_verification_gas.rs @@ -1,4 +1,4 @@ -use std::{ops::Deref, sync::Arc}; +use std::sync::Arc; use anyhow::{anyhow, Context}; use async_trait::async_trait; @@ -7,7 +7,7 @@ use rundler_provider::{EntryPoint, Provider, SimulateOpCallData, SimulationProvi use rundler_types::{chain::ChainSpec, UserOperation}; use super::Settings; -use crate::{utils, GasEstimationError}; +use crate::GasEstimationError; /// Gas estimation will stop when the binary search bounds are within /// `GAS_ESTIMATION_ERROR_MARGIN` of each other. @@ -111,15 +111,17 @@ where } = self .entry_point .get_simulate_op_call_data(initial_op, state_override); - let gas_used = utils::get_gas_used( - self.provider.deref(), - self.entry_point.address(), - U256::zero(), - call_data, - &spoofed_state, - ) - .await - .context("failed to run initial guess")?; + let gas_used = self + .provider + .get_gas_used( + self.entry_point.address(), + U256::zero(), + call_data, + spoofed_state.clone(), + ) + .await + .context("failed to run initial guess")?; + if gas_used.success { if self.entry_point.simulation_should_revert() { Err(anyhow!( diff --git a/crates/sim/src/estimation/v0_6.rs b/crates/sim/src/estimation/v0_6.rs index 0d04b63de..41e179d13 100644 --- a/crates/sim/src/estimation/v0_6.rs +++ b/crates/sim/src/estimation/v0_6.rs @@ -698,15 +698,15 @@ mod tests { })) }); - provider.expect_call_constructor().returning( - move |_a, _b: (Address, U256, Bytes), _c, _d| { + provider + .expect_get_gas_used() + .returning(move |_a, _b, _c, _d| { Ok(GasUsedResult { gas_used: gas_usage * 2, success: false, result: Bytes::new(), }) - }, - ); + }); let (estimator, _) = create_estimator(entry, provider); let optional_op = demo_user_op_optional_gas(Some(U256::from(10000))); @@ -757,15 +757,15 @@ mod tests { // this gas used number is larger than a u64 max number so we need to // check for this overflow - provider.expect_call_constructor().returning( - move |_a, _b: (Address, U256, Bytes), _c, _d| { + provider + .expect_get_gas_used() + .returning(move |_a, _b, _c, _d| { Ok(GasUsedResult { gas_used: U256::from(18446744073709551616_u128), success: false, result: Bytes::new(), }) - }, - ); + }); let (estimator, _) = create_estimator(entry, provider); let optional_op = demo_user_op_optional_gas(Some(U256::from(10000))); @@ -814,15 +814,15 @@ mod tests { // the success field should not be true as the // call should always revert - provider.expect_call_constructor().returning( - move |_a, _b: (Address, U256, Bytes), _c, _d| { + provider + .expect_get_gas_used() + .returning(move |_a, _b, _c, _d| { Ok(GasUsedResult { gas_used: U256::from(20000), success: true, result: Bytes::new(), }) - }, - ); + }); let (estimator, _) = create_estimator(entry, provider); let optional_op = demo_user_op_optional_gas(Some(U256::from(10000))); @@ -861,15 +861,15 @@ mod tests { })) }); - provider.expect_call_constructor().returning( - move |_a, _b: (Address, U256, Bytes), _c, _d| { + provider + .expect_get_gas_used() + .returning(move |_a, _b, _c, _d| { Ok(GasUsedResult { gas_used: U256::from(20000), success: false, result: Bytes::new(), }) - }, - ); + }); let (estimator, _) = create_estimator(entry, provider); let optional_op = demo_user_op_optional_gas(Some(U256::from(10000))); @@ -903,15 +903,15 @@ mod tests { .expect_call_spoofed_simulate_op() .returning(|_a, _b, _c, _d, _e, _f| Err(anyhow!("Invalid spoof error"))); - provider.expect_call_constructor().returning( - move |_a, _b: (Address, U256, Bytes), _c, _d| { + provider + .expect_get_gas_used() + .returning(move |_a, _b, _c, _d| { Ok(GasUsedResult { gas_used: U256::from(20000), success: false, result: Bytes::new(), }) - }, - ); + }); let (estimator, _) = create_estimator(entry, provider); let optional_op = demo_user_op_optional_gas(Some(U256::from(10000))); @@ -956,13 +956,11 @@ mod tests { })) }); - provider.expect_call_constructor().returning( - move |_a, _b: (Address, U256, Bytes), _c, _d| { - let ret: Result = - Err(anyhow::anyhow!("This should always revert")); - ret - }, - ); + provider + .expect_get_gas_used() + .returning(move |_a, _b, _c, _d| { + Err(anyhow::anyhow!("This should always revert").into()) + }); let (estimator, _) = create_estimator(entry, provider); let optional_op = demo_user_op_optional_gas(Some(U256::from(10000))); @@ -1146,15 +1144,15 @@ mod tests { provider .expect_get_latest_block_hash_and_number() .returning(|| Ok((H256::zero(), U64::zero()))); - provider.expect_call_constructor().returning( - move |_a, _b: (Address, U256, Bytes), _c, _d| { + provider + .expect_get_gas_used() + .returning(move |_a, _b, _c, _d| { Ok(GasUsedResult { gas_used: gas_usage, success: false, result: Bytes::new(), }) - }, - ); + }); provider.expect_get_base_fee().returning(|| Ok(TEST_FEE)); provider diff --git a/crates/sim/src/utils.rs b/crates/sim/src/utils.rs index 6b90a7ad3..73be29ecd 100644 --- a/crates/sim/src/utils.rs +++ b/crates/sim/src/utils.rs @@ -12,12 +12,9 @@ // If not, see https://www.gnu.org/licenses/. use anyhow::Context; -use ethers::types::{spoof, Address, BlockId, Bytes, H256, U256}; +use ethers::types::{spoof, Address, BlockId, H256}; use rundler_provider::Provider; -use rundler_types::contracts::utils::{ - get_code_hashes::{CodeHashesResult, GETCODEHASHES_BYTECODE}, - get_gas_used::{GasUsedResult, GETGASUSED_BYTECODE}, -}; +use rundler_types::contracts::utils::get_code_hashes::{CodeHashesResult, GETCODEHASHES_BYTECODE}; /// Hashes together the code from all the provided addresses. The order of the input addresses does /// not matter. @@ -38,21 +35,3 @@ pub(crate) async fn get_code_hash( .context("should compute code hashes")?; Ok(H256(out.hash)) } - -/// Measures the gas used by a call to target with value and data. -pub(crate) async fn get_gas_used( - provider: &P, - target: Address, - value: U256, - data: Bytes, - state_overrides: &spoof::State, -) -> anyhow::Result { - provider - .call_constructor( - &GETGASUSED_BYTECODE, - (target, value, data), - None, - state_overrides, - ) - .await -} diff --git a/crates/types/contracts/src/utils/GetGasUsed.sol b/crates/types/contracts/src/utils/GetGasUsed.sol index 743097680..5585abbfb 100644 --- a/crates/types/contracts/src/utils/GetGasUsed.sol +++ b/crates/types/contracts/src/utils/GetGasUsed.sol @@ -1,24 +1,33 @@ // SPDX-License-Identifier: GPL-3.0 pragma solidity ^0.8.19; -// Not intended to be deployed on-chain.. Instead, using a call to simulate -// deployment will revert with an error containing the desired result. +// Not intended to be deployed on-chain.. Instead use state overrides to deploy the bytecode and call it. contract GetGasUsed { - error GasUsedResult(uint256 gasUsed, bool success, bytes result); + struct GasUsedResult { + uint256 gasUsed; + bool success; + bytes result; + } - constructor(address target, uint256 value, bytes memory data) { - (uint256 gasUsed, bool success, bytes memory result) = getGas(target, value, data); - revert GasUsedResult(gasUsed, success, result); + /** + * Contract should not be deployed + */ + constructor() { + require(block.number < 100, "should not be deployed"); } function getGas( address target, uint256 value, bytes memory data - ) public returns (uint256, bool, bytes memory) { + ) public returns (GasUsedResult memory) { uint256 preGas = gasleft(); (bool success, bytes memory result) = target.call{value : value}(data); - return (preGas - gasleft(), success, result); + return GasUsedResult({ + gasUsed: preGas - gasleft(), + success: success, + result: result + }); } } diff --git a/crates/types/src/chain.rs b/crates/types/src/chain.rs index 1bbc1a983..fda6aaa44 100644 --- a/crates/types/src/chain.rs +++ b/crates/types/src/chain.rs @@ -41,6 +41,8 @@ pub struct ChainSpec { /// NOTE: This must take into account when the storage slot was originally 0 /// and is now non-zero, making the overhead slightly higher for most operations. pub deposit_transfer_overhead: U256, + /// The maximum size of a transaction in bytes + pub max_transaction_size_bytes: usize, /* * Gas estimation @@ -77,8 +79,6 @@ pub struct ChainSpec { /// This parameter is used to trigger the builder to send a bundle after a specified /// amount of time, before a new block is not received. pub bundle_max_send_interval_millis: u64, - /// The maximum size that an bundle can be in bytes. - pub max_bundle_size_bytes: u64, /* * Senders @@ -139,7 +139,7 @@ impl Default for ChainSpec { priority_fee_oracle_type: PriorityFeeOracleType::default(), min_max_priority_fee_per_gas: U256::zero(), max_max_priority_fee_per_gas: U256::MAX, - max_bundle_size_bytes: 100000, + max_transaction_size_bytes: 131072, // 128 KiB bundle_max_send_interval_millis: u64::MAX, flashbots_enabled: false, flashbots_relay_url: None, diff --git a/crates/types/src/user_operation/mod.rs b/crates/types/src/user_operation/mod.rs index cfb9f6f62..a9ef3ad0d 100644 --- a/crates/types/src/user_operation/mod.rs +++ b/crates/types/src/user_operation/mod.rs @@ -25,6 +25,19 @@ pub mod v0_7; use crate::Entity; +/// Overhead for bytes required for each bundle +/// 4 bytes for function signature +/// 32 bytes for user op array offset +/// 32 bytes for beneficiary +/// 32 bytes for array count +/// Ontop of this offset there needs to be another 32 bytes for each +/// user operation in the bundle to store its offset within the array +pub const BUNDLE_BYTE_OVERHEAD: usize = 4 + 32 + 32 + 32; + +/// Size of word that stores offset of user op location +/// within handleOps `ops` array +pub const USER_OP_OFFSET_WORD_SIZE: usize = 32; + /// ERC-4337 Entry point version #[derive(Debug, Clone, Copy, Eq, PartialEq)] pub enum EntryPointVersion { @@ -134,6 +147,11 @@ pub trait UserOperation: Debug + Clone + Send + Sync + 'static { /// Return the gas overheads for this user operation type fn gas_overheads() -> GasOverheads; + + /// Calculate the size of the user operation in single UO bundle in bytes + fn single_uo_bundle_size_bytes(&self) -> usize { + self.abi_encoded_size() + BUNDLE_BYTE_OVERHEAD + USER_OP_OFFSET_WORD_SIZE + } } /// User operation enum @@ -339,6 +357,17 @@ pub enum UserOperationOptionalGas { V0_7(v0_7::UserOperationOptionalGas), } +impl UserOperationOptionalGas { + /// Returns the user operation type + pub fn single_uo_bundle_size_bytes(&self) -> usize { + let abi_size = match self { + UserOperationOptionalGas::V0_6(op) => op.abi_encoded_size(), + UserOperationOptionalGas::V0_7(op) => op.abi_encoded_size(), + }; + abi_size + BUNDLE_BYTE_OVERHEAD + USER_OP_OFFSET_WORD_SIZE + } +} + /// Gas estimate #[derive(Debug, Clone)] pub struct GasEstimate { diff --git a/crates/types/src/user_operation/v0_6.rs b/crates/types/src/user_operation/v0_6.rs index 795cbcb86..a8abe5157 100644 --- a/crates/types/src/user_operation/v0_6.rs +++ b/crates/types/src/user_operation/v0_6.rs @@ -354,6 +354,15 @@ impl UserOperationOptionalGas { } } + /// Abi encoded size of the user operation (with its dummy fields) + pub fn abi_encoded_size(&self) -> usize { + ABI_ENCODED_USER_OPERATION_FIXED_LEN + + super::byte_array_abi_len(&self.init_code) + + super::byte_array_abi_len(&self.call_data) + + super::byte_array_abi_len(&self.paymaster_and_data) + + super::byte_array_abi_len(&self.signature) + } + fn random_bytes(len: usize) -> Bytes { let mut bytes = vec![0_u8; len]; rand::thread_rng().fill_bytes(&mut bytes); diff --git a/crates/types/src/user_operation/v0_7.rs b/crates/types/src/user_operation/v0_7.rs index 431760086..f1c01afc3 100644 --- a/crates/types/src/user_operation/v0_7.rs +++ b/crates/types/src/user_operation/v0_7.rs @@ -507,6 +507,21 @@ impl UserOperationOptionalGas { builder } + /// Abi encoded size of the user operation (with its dummy fields) + pub fn abi_encoded_size(&self) -> usize { + let mut base = ABI_ENCODED_USER_OPERATION_FIXED_LEN + + super::byte_array_abi_len(&self.call_data) + + super::byte_array_abi_len(&self.signature); + if self.factory.is_some() { + base += super::byte_array_abi_len(&self.factory_data) + 32; // account for factory address + } + if self.paymaster.is_some() { + base += super::byte_array_abi_len(&self.paymaster_data) + 64; // account for paymaster address and gas limits + } + + base + } + fn random_bytes(len: usize) -> Bytes { let mut bytes = vec![0_u8; len]; rand::thread_rng().fill_bytes(&mut bytes);