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

Jump to bottom

fix(anvil): clean up eth_estimateGas #7515

Merged
merged 5 commits into from
Mar 28, 2024
Merged

fix(anvil): clean up eth_estimateGas #7515

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a52a53d
fix(anvil): clean up eth_estimateGas
klkvr Mar 28, 2024
dc3f627
fix doc
klkvr Mar 28, 2024
9937e2e
fix doc
klkvr Mar 28, 2024
96a9457
fix doc
klkvr Mar 28, 2024
b088f96
review fixes
klkvr Mar 28, 2024
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
229 changes: 88 additions & 141 deletions crates/anvil/src/eth/api.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::{
backend::mem::{state, BlockRequest},
backend::mem::{state, BlockRequest, State},
sign::build_typed_transaction,
};
use crate::{
Expand Down Expand Up @@ -2214,10 +2214,10 @@ impl EthApi {
// configured gas limit
let mut highest_gas_limit = request.gas.unwrap_or(block_env.gas_limit);

// check with the funds of the sender
if let Some(from) = request.from {
let gas_price = fees.gas_price.unwrap_or_default();
if gas_price > U256::ZERO {
let gas_price = fees.gas_price.unwrap_or_default();
// If we have non-zero gas price, cap gas limit by sender balance
if !gas_price.is_zero() {
if let Some(from) = request.from {
let mut available_funds = self.backend.get_balance_with_state(&state, from)?;
if let Some(value) = request.value {
if value > available_funds {
Expand All @@ -2228,86 +2228,42 @@ impl EthApi {
}
// amount of gas the sender can afford with the `gas_price`
let allowance = available_funds.checked_div(gas_price).unwrap_or_default();
if highest_gas_limit > allowance {
trace!(target: "node", "eth_estimateGas capped by limited user funds");
highest_gas_limit = allowance;
}
highest_gas_limit = std::cmp::min(highest_gas_limit, allowance);
}
}

// if the provided gas limit is less than computed cap, use that
let gas_limit = std::cmp::min(request.gas.unwrap_or(highest_gas_limit), highest_gas_limit);
let mut call_to_estimate = request.clone();
call_to_estimate.gas = Some(gas_limit);
call_to_estimate.gas = Some(highest_gas_limit);

// execute the call without writing to db
let ethres =
self.backend.call_with_state(&state, call_to_estimate, fees.clone(), block_env.clone());

// Exceptional case: init used too much gas, we need to increase the gas limit and try
// again
if let Err(BlockchainError::InvalidTransaction(InvalidTransactionError::GasTooHigh(_))) =
ethres
{
// if price or limit was included in the request then we can execute the request
// again with the block's gas limit to check if revert is gas related or not
if request.gas.is_some() || request.gas_price.is_some() {
return Err(map_out_of_gas_err(
request,
state,
self.backend.clone(),
block_env,
fees,
gas_limit,
));
let gas_used = match ethres.try_into()? {
GasEstimationCallResult::Success(gas) => Ok(U256::from(gas)),
GasEstimationCallResult::OutOfGas => {
Err(InvalidTransactionError::BasicOutOfGas(highest_gas_limit).into())
}
}

let (exit, out, gas, _) = ethres?;
match exit {
return_ok!() => {
// succeeded
}
InstructionResult::OutOfGas | InstructionResult::OutOfFunds => {
return Err(InvalidTransactionError::BasicOutOfGas(gas_limit).into())
}
// need to check if the revert was due to lack of gas or unrelated reason
// we're also checking for InvalidFEOpcode here because this can be used to trigger an error <https://github.com/foundry-rs/foundry/issues/6138> common usage in openzeppelin <https://github.com/OpenZeppelin/openzeppelin-contracts/blob/94697be8a3f0dfcd95dfb13ffbd39b5973f5c65d/contracts/metatx/ERC2771Forwarder.sol#L360-L367>
return_revert!() | InstructionResult::InvalidFEOpcode => {
// if price or limit was included in the request then we can execute the request
// again with the max gas limit to check if revert is gas related or not
return if request.gas.is_some() || request.gas_price.is_some() {
Err(map_out_of_gas_err(
request,
state,
self.backend.clone(),
block_env,
fees,
gas_limit,
))
} else {
// the transaction did fail due to lack of gas from the user
Err(InvalidTransactionError::Revert(Some(convert_transact_out(&out).0.into()))
.into())
};
GasEstimationCallResult::Revert(output) => {
Err(InvalidTransactionError::Revert(output).into())
mattsse marked this conversation as resolved.
Show resolved Hide resolved
}
reason => {
warn!(target: "node", "estimation failed due to {:?}", reason);
return Err(BlockchainError::EvmError(reason));
GasEstimationCallResult::EvmError(err) => {
warn!(target: "node", "estimation failed due to {:?}", err);
Err(BlockchainError::EvmError(err))
}
}
}?;

// at this point we know the call succeeded but want to find the _best_ (lowest) gas the
// transaction succeeds with. we find this by doing a binary search over the
// possible range NOTE: this is the gas the transaction used, which is less than the
// transaction requires to succeed
let gas: U256 = U256::from(gas);

// Get the starting lowest gas needed depending on the transaction kind.
let mut lowest_gas_limit = determine_base_gas_by_kind(&request);

// pick a point that's close to the estimated gas
let mut mid_gas_limit = std::cmp::min(
gas * U256::from(3),
gas_used * U256::from(3),
(highest_gas_limit + lowest_gas_limit) / U256::from(2),
);

Expand All @@ -2321,52 +2277,25 @@ impl EthApi {
block_env.clone(),
);

// Exceptional case: init used too much gas, we need to increase the gas limit and try
// again
if let Err(BlockchainError::InvalidTransaction(InvalidTransactionError::GasTooHigh(
_,
))) = ethres
{
// increase the lowest gas limit
lowest_gas_limit = mid_gas_limit;

// new midpoint
mid_gas_limit = (highest_gas_limit + lowest_gas_limit) / U256::from(2);
continue;
}

match ethres {
Ok((exit, _, _gas, _)) => match exit {
match ethres.try_into()? {
GasEstimationCallResult::Success(_) => {
// If the transaction succeeded, we can set a ceiling for the highest gas limit
// at the current midpoint, as spending any more gas would
// make no sense (as the TX would still succeed).
return_ok!() => {
highest_gas_limit = mid_gas_limit;
}
// If the transaction failed due to lack of gas, we can set a floor for the
// lowest gas limit at the current midpoint, as spending any
// less gas would make no sense (as the TX would still revert due to lack of
// gas).
InstructionResult::Revert |
InstructionResult::OutOfGas |
InstructionResult::OutOfFunds |
// we're also checking for InvalidFEOpcode here because this can be used to trigger an error <https://github.com/foundry-rs/foundry/issues/6138> common usage in openzeppelin <https://github.com/OpenZeppelin/openzeppelin-contracts/blob/94697be8a3f0dfcd95dfb13ffbd39b5973f5c65d/contracts/metatx/ERC2771Forwarder.sol#L360-L367>
mattsse marked this conversation as resolved.
Show resolved Hide resolved
InstructionResult::InvalidFEOpcode => {
lowest_gas_limit = mid_gas_limit;
}
// The tx failed for some other reason.
reason => {
warn!(target: "node", "estimation failed due to {:?}", reason);
return Err(BlockchainError::EvmError(reason))
}
},
// We've already checked for the exceptional GasTooHigh case above, so this is a
// real error.
Err(reason) => {
warn!(target: "node", "estimation failed due to {:?}", reason);
return Err(reason);
highest_gas_limit = mid_gas_limit;
}
}
GasEstimationCallResult::OutOfGas |
GasEstimationCallResult::Revert(_) |
GasEstimationCallResult::EvmError(_) => {
// If the transaction failed, we can set a floor for the lowest gas limit at the
// current midpoint, as spending any less gas would make no
// sense (as the TX would still revert due to lack of gas).
//
// We don't care about the reason here, as we known that trasaction is correct
// as it succeeded earlier
lowest_gas_limit = mid_gas_limit;
}
};
// new midpoint
mid_gas_limit = (highest_gas_limit + lowest_gas_limit) / U256::from(2);
}
Expand Down Expand Up @@ -2631,42 +2560,6 @@ fn ensure_return_ok(exit: InstructionResult, out: &Option<Output>) -> Result<Byt
}
}

/// Executes the requests again after an out of gas error to check if the error is gas related or
/// not
#[inline]
fn map_out_of_gas_err<D>(
mut request: TransactionRequest,
state: D,
backend: Arc<backend::mem::Backend>,
block_env: BlockEnv,
fees: FeeDetails,
gas_limit: U256,
) -> BlockchainError
where
D: DatabaseRef<Error = DatabaseError>,
{
request.gas = Some(backend.gas_limit());
let (exit, out, _, _) = match backend.call_with_state(&state, request, fees, block_env) {
Ok(res) => res,
Err(err) => return err,
};
match exit {
return_ok!() => {
// transaction succeeded by manually increasing the gas limit to
// highest, which means the caller lacks funds to pay for the tx
InvalidTransactionError::BasicOutOfGas(gas_limit).into()
}
return_revert!() => {
// reverted again after bumping the limit
InvalidTransactionError::Revert(Some(convert_transact_out(&out).0.into())).into()
}
reason => {
warn!(target: "node", "estimation failed due to {:?}", reason);
BlockchainError::EvmError(reason)
}
}
}

/// Determines the minimum gas needed for a transaction depending on the transaction kind.
#[inline]
fn determine_base_gas_by_kind(request: &TransactionRequest) -> U256 {
Expand Down Expand Up @@ -2698,3 +2591,57 @@ fn determine_base_gas_by_kind(request: &TransactionRequest) -> U256 {
_ => MIN_CREATE_GAS,
}
}

/// Keeps result of a call to revm EVM used for gas estimation
enum GasEstimationCallResult {
Success(u64),
OutOfGas,
Revert(Option<Bytes>),
EvmError(InstructionResult),
}

/// Converts the result of a call to revm EVM into a [GasEstimationCallRes].
impl TryFrom<Result<(InstructionResult, Option<Output>, u64, State)>> for GasEstimationCallResult {
mattsse marked this conversation as resolved.
Show resolved Hide resolved
type Error = BlockchainError;

fn try_from(res: Result<(InstructionResult, Option<Output>, u64, State)>) -> Result<Self> {
match res {
// Exceptional case: init used too much gas, treated as out of gas error
Err(BlockchainError::InvalidTransaction(InvalidTransactionError::GasTooHigh(_))) => {
Ok(Self::OutOfGas)
}
Err(err) => Err(err),
Ok((exit, output, gas, _)) => match exit {
return_ok!() | InstructionResult::CallOrCreate => Ok(Self::Success(gas)),

InstructionResult::Revert => Ok(Self::Revert(output.map(|o| o.into_data()))),

InstructionResult::OutOfGas |
InstructionResult::MemoryOOG |
InstructionResult::MemoryLimitOOG |
InstructionResult::PrecompileOOG |
InstructionResult::InvalidOperandOOG => Ok(Self::OutOfGas),

InstructionResult::OpcodeNotFound |
InstructionResult::CallNotAllowedInsideStatic |
InstructionResult::StateChangeDuringStaticCall |
InstructionResult::InvalidFEOpcode |
InstructionResult::InvalidJump |
InstructionResult::NotActivated |
InstructionResult::StackUnderflow |
InstructionResult::StackOverflow |
InstructionResult::OutOfOffset |
InstructionResult::CreateCollision |
InstructionResult::OverflowPayment |
InstructionResult::PrecompileError |
InstructionResult::NonceOverflow |
InstructionResult::CreateContractSizeLimit |
InstructionResult::CreateContractStartingWithEF |
InstructionResult::CreateInitCodeSizeLimit |
InstructionResult::FatalExternalError |
InstructionResult::OutOfFunds |
InstructionResult::CallTooDeep => Ok(Self::EvmError(exit)),
},
}
}
}
Loading