-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feat(forge): loosen tx gas limit restrictions ahead of Osaka + make enforceable w/ --enable-tx-gas-limit
#11427
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.
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
feat(forge): loosen tx gas limit restrictions ahead of Osaka + make enforceable w/ --enable-tx-gas-limit
#11427
Conversation
--enable-tx-gas-limit
@@ -265,6 +265,7 @@ impl CallArgs { | |||
|
|||
// modify settings that usually set in eth_call | |||
env.evm_env.cfg_env.disable_block_gas_limit = true; | |||
env.evm_env.cfg_env.tx_gas_limit_cap = Some(u64::MAX); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this makes sense to me, do not enforce tx gas limit when simulating the behavior of an eth_call
@@ -1558,6 +1559,7 @@ impl Backend { | |||
// we want to disable this in eth_call, since this is common practice used by other node | |||
// impls and providers <https://github.com/foundry-rs/foundry/issues/4388> | |||
env.evm_env.cfg_env.disable_block_gas_limit = true; | |||
env.evm_env.cfg_env.tx_gas_limit_cap = Some(u64::MAX); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here, makes sense to avoid enforcing a tx gas limit cap here (and not introduce an option to configure it)
} | ||
|
||
// check that we comply with the transaction's gas limit as imposed by Osaka (EIP-7825) | ||
if env.evm_env.cfg_env.tx_gas_limit_cap.is_none() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spec's before Osaka have this set to Some(U64::max)
by default
it is debatable if it is really necessary to catch it here but could give user a richer error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you see any issue if we infer / enable tx gas limit automatically if osaka evm version instead adding yet another enable_tx_gas_limit
arg?
So by default REVM enables the tx gas limit with Osaka. This causes problems for users that have specified a high gas limit in their foundry.toml (say 100M gas) whose transactions will fail as it exceeds ~16M or if they have extremely large transactions in their tests / scripts. So in Foundry we need to always disable this behavior REVM enables by default. If users do want to accurately simulate the tx gas limit their gas set this flag. |
Ah, I see, makes sense then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm! pending one more reviewer before merge
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the changes make sense to me!
Motivation
Closes: #11348
Solution
Enforcing of tx gas limit is disabled by default as this is what users would expect, allow users to enforce the tx gas limit imposed by Osaka (EIP-7825). If there are further changes that change this tx gas limit we can make expand this to be EVM version specific.
Adds clarifying alias to
block_gas_limit
/disable_block_gas_limit
to beblock
specific, retaining original alias for backwards compatibilityBreaking changes
Renamed
TransactionExecutionOutcome::Exhausted
=>TransactionExecutionOutcome::BlockGasExhausted
for clarity, expecting this to have no impact on regular usersPR Checklist