-
Notifications
You must be signed in to change notification settings - Fork 199
chore(l1): sync EIP-8037 with EIPs PR 11573 #6546
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
base: bal-devnet-5
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,27 @@ pub const MEMORY_EXPANSION_QUOTIENT: u64 = 512; | |
| // Dedicated gas limit for system calls according to EIPs 2935, 4788, 7002 and 7251 | ||
| pub const SYS_CALL_GAS_LIMIT: u64 = 30000000; | ||
|
|
||
| // EIP-8037 (ethereum/EIPs#11573): per-system-call upper bound on new storage slots | ||
| // written. Matches MAX_WITHDRAWAL_REQUESTS_PER_BLOCK (EIP-7002), the largest per-block | ||
| // bound across the existing system contracts. | ||
| pub const SYSTEM_MAX_SSTORES_PER_CALL: u64 = 16; | ||
|
|
||
| // EIP-8037: extra state-gas budget reserved for system contracts that perform up to | ||
| // SYSTEM_MAX_SSTORES_PER_CALL new SSTOREs per invocation. Derived symbolically from | ||
| // the EIP-8037 byte constant so the bound stays in lockstep with | ||
| // SYSTEM_MAX_SSTORES_PER_CALL. The CPSB factor (1174) is inlined here because | ||
| // `cost_per_state_byte` is a runtime fn; if CPSB ever changes, this constant and | ||
| // `cost_per_state_byte` must move together. | ||
| pub const SYS_CALL_STATE_GAS_RESERVOIR: u64 = | ||
| crate::gas_cost::STATE_BYTES_PER_STORAGE_SET * 1174 * SYSTEM_MAX_SSTORES_PER_CALL; | ||
|
|
||
| // EIP-8037: From Amsterdam, system calls receive the legacy 30M execution budget plus | ||
| // SYS_CALL_STATE_GAS_RESERVOIR placed in `state_gas_reservoir`. This preserves the | ||
| // existing 30M execution margin under the higher state-creation costs. | ||
| // | ||
| // 30_000_000 + 32 * 1174 * 16 = 30_601_088 | ||
| pub const SYS_CALL_GAS_LIMIT_AMSTERDAM: u64 = SYS_CALL_GAS_LIMIT + SYS_CALL_STATE_GAS_RESERVOIR; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| // Transaction costs in gas | ||
| pub const TX_BASE_COST: u64 = 21000; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -164,22 +164,15 @@ pub const CREATE_BASE_COST: u64 = 32000; | |
| // EIP-8037: Multidimensional gas for state creation (Amsterdam only) | ||
| pub const STATE_BYTES_PER_NEW_ACCOUNT: u64 = 112; | ||
| pub const STATE_BYTES_PER_STORAGE_SET: u64 = 32; | ||
| pub const STATE_BYTES_PER_AUTH_TOTAL: u64 = 135; // 112 account + 23 auth-specific | ||
| pub const STATE_BYTES_PER_AUTH_ONLY: u64 = 23; // auth-specific delta when authority pre-existed (downgrade) | ||
| pub const STATE_BYTES_PER_AUTH_BASE: u64 = 23; | ||
|
|
||
| // EIP-8037: Dynamic cost_per_state_byte formula constants (execution-specs#2687) | ||
| pub const BLOCKS_PER_YEAR: u64 = 2_628_000; | ||
| pub const TARGET_STATE_GROWTH_PER_YEAR: u64 = 100 * (1u64 << 30); // 100 GiB | ||
| pub const CPSB_SIGNIFICANT_BITS: u32 = 5; | ||
| pub const CPSB_OFFSET: u64 = 9578; | ||
|
|
||
| /// Compute cost_per_state_byte from the block gas limit (EIP-8037, execution-specs#2687). | ||
| /// Cost per state byte for EIP-8037 (ethereum/EIPs#11573). | ||
| /// | ||
| /// TEMPORARY for bal-devnet-4: returns the fixed value 1174 used by bal-devnet-3 | ||
| /// regardless of `block_gas_limit`. The dynamic formula (BLOCKS_PER_YEAR / | ||
| /// TARGET_STATE_GROWTH_PER_YEAR / CPSB_SIGNIFICANT_BITS / CPSB_OFFSET) is preserved | ||
| /// in the consts above so this commit can be reverted with a single `git revert` to | ||
| /// restore the formula body. See execution-specs#2687. | ||
| /// Pinned at 1174, the value derived from a 100 GiB/year state-growth target at a | ||
| /// 96M-gas reference block limit. The original draft computed this dynamically from | ||
| /// the block gas limit; PR 11573 collapses that to a single fixed constant, so | ||
| /// `block_gas_limit` is no longer an input. The argument is retained to keep call | ||
| /// sites stable; if a future EIP re-derives CPSB, change the body. | ||
| pub fn cost_per_state_byte(_block_gas_limit: u64) -> u64 { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we still need this function if the value will be left fixed? |
||
| 1174 | ||
| } | ||
|
Comment on lines
176
to
178
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The Prompt To Fix With AIThis is a comment left during a code review.
Path: crates/vm/levm/src/gas_cost.rs
Line: 176-178
Comment:
**Silently-ignored parameter may mislead callers**
The `_block_gas_limit` argument is now permanently unused but still accepted. Any caller that constructs its own `block_gas_limit` value and passes it expecting it to influence the cost will silently get `1174` regardless. Consider making the contract explicit by changing the signature to `cost_per_state_byte() -> u64` (a breaking change at call sites, but honest) or at least adding an inline note to make it clear the call sites don't need to supply a meaningful value.
How can I resolve this? If you propose a fix, please make it concise. |
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,7 +8,7 @@ use crate::{ | |
| gas_cost::{ | ||
| self, ACCESS_LIST_ADDRESS_COST, ACCESS_LIST_STORAGE_KEY_COST, BLOB_GAS_PER_BLOB, | ||
| COLD_ADDRESS_ACCESS_COST, CREATE_BASE_COST, REGULAR_GAS_CREATE, STANDARD_TOKEN_COST, | ||
| STATE_BYTES_PER_AUTH_TOTAL, STATE_BYTES_PER_NEW_ACCOUNT, WARM_ADDRESS_ACCESS_COST, | ||
| STATE_BYTES_PER_AUTH_BASE, STATE_BYTES_PER_NEW_ACCOUNT, WARM_ADDRESS_ACCESS_COST, | ||
| cost_per_state_byte, floor_tokens_in_access_list, total_cost_floor_per_token, | ||
| }, | ||
| vm::{Substate, VM}, | ||
|
|
@@ -284,8 +284,9 @@ impl<'a> VM<'a> { | |
| // or any other validation in this function never reach the | ||
| // `record_auth_downgrade_to_only` call below. They remain in `auth_total` in | ||
| // state_diff_intrinsic_seed (seeded by add_intrinsic_gas), matching legacy behavior | ||
| // where invalid tuples still cost the full STATE_BYTES_PER_AUTH_TOTAL (135 bytes) | ||
| // intrinsic state gas. | ||
| // where invalid tuples still cost the full new-account + auth-base charge | ||
| // (STATE_BYTES_PER_NEW_ACCOUNT + STATE_BYTES_PER_AUTH_BASE = 135 bytes) of intrinsic | ||
| // state gas. | ||
| let mut refunded_gas: u64 = 0; | ||
| // IMPORTANT: | ||
| // If any of the below steps fail, immediately stop processing that tuple and continue to the next tuple in the list. It will in the case of multiple tuples for the same authority, set the code using the address in the last valid occurrence. | ||
|
|
@@ -451,14 +452,36 @@ impl<'a> VM<'a> { | |
| // execution_gas = what remains after all intrinsic gas; regular_gas_budget = how much | ||
| // regular execution gas is allowed (capped at TX_MAX_GAS_LIMIT_AMSTERDAM); the difference becomes | ||
| // the reservoir for drawing state gas without consuming regular gas_remaining. | ||
| // | ||
| // System calls (EIPs 2935, 4788, 7002, 7251) bypass EIP-7825's TX_MAX_GAS_LIMIT cap and | ||
| // instead receive a fixed split per ethereum/EIPs#11573: | ||
| // gas_left = SYS_CALL_GAS_LIMIT (the legacy 30M execution budget) | ||
| // state_gas_reservoir = STATE_BYTES_PER_STORAGE_SET * CPSB * SYSTEM_MAX_SSTORES_PER_CALL | ||
| // The caller (`generic_system_contract_levm`) sets `env.gas_limit` to the sum of both | ||
| // plus TX_BASE_COST, so we just split `execution_gas` here. | ||
| if self.env.config.fork >= Fork::Amsterdam { | ||
| let gas_limit = self.tx.gas_limit(); | ||
| // System calls have `tx.gas_limit() == 0` (the tx is built from | ||
| // `EIP1559Transaction::default()` in `generic_system_contract_levm`); their | ||
| // budget lives in `env.gas_limit` instead. For ordinary user txs the two | ||
| // agree (set together at the env-builder), but for `eth_call`/simulation | ||
| // paths `env.gas_limit` may default to the block max while the wrapping | ||
| // `tx` has `gas_limit = 0`. So we read whichever source carries the budget | ||
| // for the path we're on. | ||
| let gas_limit = if self.env.is_system_call { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doc/impl mismatch: the surrounding comment says "For ordinary user txs the two agree (set together at the env-builder), but for Two possible fixes — pick whichever matches your intent:
|
||
| self.env.gas_limit | ||
| } else { | ||
| self.tx.gas_limit() | ||
| }; | ||
| let execution_gas = gas_limit.saturating_sub(total_gas); | ||
| let regular_gas_budget = TX_MAX_GAS_LIMIT_AMSTERDAM.saturating_sub(regular_gas); | ||
| let gas_left = regular_gas_budget.min(execution_gas); | ||
| let reservoir = execution_gas.saturating_sub(gas_left); | ||
| let reservoir = if self.env.is_system_call { | ||
| execution_gas.saturating_sub(SYS_CALL_GAS_LIMIT) | ||
| } else { | ||
| let regular_gas_budget = TX_MAX_GAS_LIMIT_AMSTERDAM.saturating_sub(regular_gas); | ||
| let gas_left = regular_gas_budget.min(execution_gas); | ||
| execution_gas.saturating_sub(gas_left) | ||
| }; | ||
| if reservoir > 0 { | ||
| // Pre-consume reservoir from gas_remaining so GAS opcode returns <= TX_MAX_GAS_LIMIT_AMSTERDAM | ||
| // Pre-consume reservoir from gas_remaining so GAS opcode returns gas_left only. | ||
| let reservoir_i64 = | ||
| i64::try_from(reservoir).map_err(|_| InternalError::Overflow)?; | ||
| self.current_call_frame.gas_remaining = self | ||
|
|
@@ -568,14 +591,17 @@ impl<'a> VM<'a> { | |
| }; | ||
|
|
||
| if fork >= Fork::Amsterdam { | ||
| // EIP-8037: per-auth regular cost is PER_AUTH_BASE_COST, state is STATE_BYTES_PER_AUTH_TOTAL * cost_per_state_byte | ||
| // EIP-8037 (ethereum/EIPs#11573): per-auth regular cost is PER_AUTH_BASE_COST; | ||
| // state cost assumes account creation, i.e. | ||
| // (STATE_BYTES_PER_NEW_ACCOUNT + STATE_BYTES_PER_AUTH_BASE) * cost_per_state_byte. | ||
| let regular_auth_cost = PER_AUTH_BASE_COST | ||
| .checked_mul(amount_of_auth_tuples) | ||
| .ok_or(InternalError::Overflow)?; | ||
| regular_gas = regular_gas.checked_add(regular_auth_cost).ok_or(OutOfGas)?; | ||
| #[expect(clippy::arithmetic_side_effects, reason = "bounded constants")] | ||
| let auth_total_state_gas = | ||
| gas_cost::STATE_BYTES_PER_AUTH_TOTAL * self.cost_per_state_byte; | ||
| let auth_total_state_gas = (gas_cost::STATE_BYTES_PER_NEW_ACCOUNT | ||
| + gas_cost::STATE_BYTES_PER_AUTH_BASE) | ||
| * self.cost_per_state_byte; | ||
| let state_auth_cost = auth_total_state_gas | ||
| .checked_mul(amount_of_auth_tuples) | ||
| .ok_or(InternalError::Overflow)?; | ||
|
|
@@ -689,14 +715,19 @@ pub fn intrinsic_gas_dimensions( | |
|
|
||
| let (state_gas_new_account, state_gas_auth_total) = if fork >= Fork::Amsterdam { | ||
| let cpsb = cost_per_state_byte(block_gas_limit); | ||
| ( | ||
| STATE_BYTES_PER_NEW_ACCOUNT | ||
| .checked_mul(cpsb) | ||
| .ok_or(InternalError::Overflow)?, | ||
| STATE_BYTES_PER_AUTH_TOTAL | ||
| .checked_mul(cpsb) | ||
| .ok_or(InternalError::Overflow)?, | ||
| ) | ||
| let state_gas_new_account = STATE_BYTES_PER_NEW_ACCOUNT | ||
| .checked_mul(cpsb) | ||
| .ok_or(InternalError::Overflow)?; | ||
| // EIP-8037 (ethereum/EIPs#11573): a fresh authorization charges | ||
| // STATE_BYTES_PER_NEW_ACCOUNT + STATE_BYTES_PER_AUTH_BASE bytes; the | ||
| // per-byte cost is the same constant CPSB. | ||
| let auth_total_bytes = STATE_BYTES_PER_NEW_ACCOUNT | ||
| .checked_add(STATE_BYTES_PER_AUTH_BASE) | ||
| .ok_or(InternalError::Overflow)?; | ||
| let state_gas_auth_total = auth_total_bytes | ||
| .checked_mul(cpsb) | ||
| .ok_or(InternalError::Overflow)?; | ||
| (state_gas_new_account, state_gas_auth_total) | ||
| } else { | ||
| (0, 0) | ||
| }; | ||
|
|
||
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.
SYS_CALL_STATE_GAS_RESERVOIRinlines1174directly rather than deriving it throughcost_per_state_byte. This means a future change to the CPSB value ingas_cost::cost_per_state_bytewill silently leaveSYS_CALL_STATE_GAS_RESERVOIR(and thereforeSYS_CALL_GAS_LIMIT_AMSTERDAM) stale. The comment already flags this risk, but it may be worth at minimum adebug_assert_eq!(SYS_CALL_STATE_GAS_RESERVOIR, gas_cost::STATE_BYTES_PER_STORAGE_SET * cost_per_state_byte(0) * SYSTEM_MAX_SSTORES_PER_CALL)in a test to make the divergence loud.Prompt To Fix With AI