Skip to content

fix(l1): add reverse check in BAL withdrawal validation #6463

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

Open
edg-l wants to merge 6 commits into
base: main
Choose a base branch
from
+290 −55
Open

fix(l1): add reverse check in BAL withdrawal validation #6463

Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d0bef64
fix(l1): add reverse check in BAL withdrawal validation
edg-l Apr 10, 2026
c67cdb0
fix(l1): add storage reverse check in BAL withdrawal validation
edg-l Apr 10, 2026
4362334
fix(l1): close storage gap and improve diagnostics in BAL withdrawal …
edg-l Apr 14, 2026
220a96c
Merge branch 'main' into fix/bal-withdrawal-reverse-check
edg-l Apr 14, 2026
28f3e58
fix(l1): defer Amsterdam block gas overflow check to post-execution (…
azteca1998 Apr 16, 2026
7c830ac
Merge branch 'main' into fix/bal-withdrawal-reverse-check
iovoid Apr 16, 2026
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
176 changes: 172 additions & 4 deletions crates/vm/backends/levm/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use ethrex_common::types::block_access_list::{
};
use ethrex_common::types::fee_config::FeeConfig;
use ethrex_common::types::{AuthorizationTuple, Code, EIP7702Transaction};
use ethrex_common::utils::u256_from_big_endian_const;
use ethrex_common::{
Address, BigEndianHash, H256, U256,
types::{
Expand Down Expand Up @@ -338,7 +339,7 @@ impl LEVM {
// post-withdrawal/request state.
#[allow(clippy::cast_possible_truncation)]
let withdrawal_idx = (block.body.transactions.len() as u16) + 1;
Self::validate_bal_withdrawal_index(db, bal, withdrawal_idx)?;
Self::validate_bal_withdrawal_index(db, bal, withdrawal_idx, &validation_index)?;

// Mark storage_reads that occurred during the withdrawal/request phase.
if !unread_storage_reads.is_empty() {
Expand Down Expand Up @@ -1441,7 +1442,7 @@ impl LEVM {

// Storage: for each slot in execution state, check it's expected
for (key_h256, &value) in &account.storage {
let slot_u256 = U256::from_big_endian(key_h256.as_bytes());
let slot_u256 = u256_from_big_endian_const(key_h256.0);
// EIP-7928 requires storage_changes sorted by slot, so use binary search.
let pos = acct
.storage_changes
Expand Down Expand Up @@ -1475,13 +1476,22 @@ impl LEVM {
/// Validates BAL entries at the withdrawal index against actual post-withdrawal state.
///
/// After `process_withdrawals` + `extract_all_requests_levm` run on the BAL-seeded
/// DB, `current_accounts_state` reflects the actual state. Any BAL claim at the
/// withdrawal index that doesn't match is either a mismatch or extraneous.
/// DB, `current_accounts_state` reflects the actual state. Validation is bidirectional:
///
/// Part A (BAL -> DB): every BAL claim at the withdrawal index must match the DB.
/// Part B (DB -> BAL): every account modified during the withdrawal/request phase
/// must have a corresponding BAL entry. Without this reverse check, a
/// malicious builder could omit a withdrawal recipient from the BAL,
/// causing the BAL-derived state root to exclude the withdrawal balance
/// change.
fn validate_bal_withdrawal_index(
db: &GeneralizedDatabase,
bal: &BlockAccessList,
withdrawal_idx: u16,
index: &BalAddressIndex,
) -> Result<(), EvmError> {
// Part A: For each BAL account with changes at the withdrawal index,
// verify the DB matches.
for acct in bal.accounts() {
let addr = acct.address;
let actual = db.current_accounts_state.get(&addr);
Expand Down Expand Up @@ -1573,6 +1583,164 @@ impl LEVM {
}
}
}

// Part B: For each account modified during the withdrawal/request phase,
// verify it has a corresponding BAL entry claiming the change.
for (addr, account) in &db.current_accounts_state {
if account.is_unmodified() {
continue;
}

let Some(&bal_acct_idx) = index.addr_to_idx.get(addr) else {
// Account modified during withdrawal/request phase but absent
// from BAL entirely. Compare with pre-state (store) to
// distinguish genuine mutations from warm-access artifacts.
let pre = db
.store
.get_account_state(*addr)
.ok()
Copy link
Copy Markdown
Contributor

@iovoid iovoid Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Silently ignores db errors

.map(|a| (a.balance, a.nonce, a.code_hash))
.unwrap_or_default();
let post = (
account.info.balance,
account.info.nonce,
account.info.code_hash,
);
if pre != post {
return Err(EvmError::Custom(format!(
"BAL validation failed for withdrawal: account {addr:?} was modified \
during withdrawal/request phase but is absent from BAL"
)));
}
// Also check storage: if any slot differs from pre-state,
// the account should have been in the BAL.
for (key_h256, &value) in &account.storage {
let pre_value = db
.store
.get_storage_value(*addr, *key_h256)
.unwrap_or_default();
if value != pre_value {
return Err(EvmError::Custom(format!(
"BAL validation failed for withdrawal: account {addr:?} storage \
slot {} changed during withdrawal/request phase but is absent \
from BAL",
u256_from_big_endian_const(key_h256.0)
)));
}
}
continue;
};

let acct = &bal.accounts()[bal_acct_idx];

// Balance: if BAL has no change at withdrawal_idx, the withdrawal
// phase must not have changed it relative to the last BAL entry.
if !has_exact_change_balance(&acct.balance_changes, withdrawal_idx) {
let seeded = acct
.balance_changes
.last()
.map(|c| c.post_balance)
.unwrap_or_else(|| {
db.store
.get_account_state(*addr)
.map(|a| a.balance)
.unwrap_or_default()
});
if account.info.balance != seeded {
return Err(EvmError::Custom(format!(
"BAL validation failed for withdrawal: account {addr:?} balance \
changed during withdrawal/request phase ({}) but BAL has no \
balance change at index {withdrawal_idx} (last_bal={seeded})",
account.info.balance
)));
}
}

// Nonce
if !has_exact_change_nonce(&acct.nonce_changes, withdrawal_idx) {
let seeded = acct
.nonce_changes
.last()
.map(|c| c.post_nonce)
.unwrap_or_else(|| {
db.store
.get_account_state(*addr)
.map(|a| a.nonce)
.unwrap_or_default()
});
if account.info.nonce != seeded {
return Err(EvmError::Custom(format!(
"BAL validation failed for withdrawal: account {addr:?} nonce \
changed during withdrawal/request phase ({}) but BAL has no \
nonce change at index {withdrawal_idx} (last_bal={seeded})",
account.info.nonce
)));
}
}

// Code
if !has_exact_change_code(&acct.code_changes, withdrawal_idx) {
let seeded_hash = acct
.code_changes
.last()
.map(|c| {
if c.new_code.is_empty() {
*EMPTY_KECCACK_HASH
Comment thread
edg-l marked this conversation as resolved.
} else {
ethrex_common::utils::keccak(&c.new_code)
}
})
.unwrap_or_else(|| {
db.store
.get_account_state(*addr)
.map(|a| a.code_hash)
.unwrap_or(*EMPTY_KECCACK_HASH)
});
if account.info.code_hash != seeded_hash {
return Err(EvmError::Custom(format!(
"BAL validation failed for withdrawal: account {addr:?} code \
changed during withdrawal/request phase but BAL has no \
code change at index {withdrawal_idx}"
)));
}
}
Comment on lines +1716 to +1743
Copy link
Copy Markdown

@greptile-apps greptile-apps bot Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Storage mutations missing from Part B

Part B checks balance, nonce, and code, but does not verify storage slot changes — unlike validate_tx_execution's Part B (lines 1442–1469) which iterates account.storage and checks every slot against its BAL entry. During the withdrawal/request phase, system calls (EIP-7002, EIP-7251) write to storage in their predeploy contracts. A block builder could omit those storage changes from the BAL; the BAL-derived state root would then exclude those slot mutations, but this check would not catch the omission.

The analogous pattern from validate_tx_execution Part B is:

// Storage: for each slot written during withdrawal/request phase,
//          verify a corresponding BAL entry exists.
for (key_h256, &value) in &account.storage {
    let slot_u256 = U256::from_big_endian(key_h256.as_bytes());
    let pos = acct
        .storage_changes
        .partition_point(|sc| sc.slot < slot_u256);
    if pos < acct.storage_changes.len() && acct.storage_changes[pos].slot == slot_u256 {
        let sc = &acct.storage_changes[pos];
        if !has_exact_change_storage(&sc.slot_changes, withdrawal_idx) {
            let seeded = sc
                .slot_changes
                .last()
                .map(|c| c.post_value)
                .unwrap_or_default();
            if value != seeded {
                return Err(EvmError::Custom(format!(
                    "BAL validation failed for withdrawal: account {addr:?} storage slot \
                     {slot_u256} changed during withdrawal/request phase ({value}) but BAL \
                     has no change at index {withdrawal_idx} (last_bal={seeded})"
                )));
            }
        }
    }
    // Slot not in BAL storage_changes: loaded from store, skip.
}
Prompt To Fix With AI 
This is a comment left during a code review.
Path: crates/vm/backends/levm/mod.rs
Line: 1664-1689

Comment:
**Storage mutations missing from Part B**

Part B checks balance, nonce, and code, but does not verify storage slot changes — unlike `validate_tx_execution`'s Part B (lines 1442–1469) which iterates `account.storage` and checks every slot against its BAL entry. During the withdrawal/request phase, system calls (EIP-7002, EIP-7251) write to storage in their predeploy contracts. A block builder could omit those storage changes from the BAL; the BAL-derived state root would then exclude those slot mutations, but this check would not catch the omission.

The analogous pattern from `validate_tx_execution` Part B is:

```rust
// Storage: for each slot written during withdrawal/request phase,
//          verify a corresponding BAL entry exists.
for (key_h256, &value) in &account.storage {
    let slot_u256 = U256::from_big_endian(key_h256.as_bytes());
    let pos = acct
        .storage_changes
        .partition_point(|sc| sc.slot < slot_u256);
    if pos < acct.storage_changes.len() && acct.storage_changes[pos].slot == slot_u256 {
        let sc = &acct.storage_changes[pos];
        if !has_exact_change_storage(&sc.slot_changes, withdrawal_idx) {
            let seeded = sc
                .slot_changes
                .last()
                .map(|c| c.post_value)
                .unwrap_or_default();
            if value != seeded {
                return Err(EvmError::Custom(format!(
                    "BAL validation failed for withdrawal: account {addr:?} storage slot \
                     {slot_u256} changed during withdrawal/request phase ({value}) but BAL \
                     has no change at index {withdrawal_idx} (last_bal={seeded})"
                )));
            }
        }
    }
    // Slot not in BAL storage_changes: loaded from store, skip.
}
```

How can I resolve this? If you propose a fix, please make it concise.


// Storage: for each slot in the withdrawal/request-phase state,
// verify the BAL has a corresponding entry or the value is unchanged.
for (key_h256, &value) in &account.storage {
let slot_u256 = u256_from_big_endian_const(key_h256.0);
let pos = acct
.storage_changes
.partition_point(|sc| sc.slot < slot_u256);
if pos < acct.storage_changes.len() && acct.storage_changes[pos].slot == slot_u256 {
Comment thread
edg-l marked this conversation as resolved.
let sc = &acct.storage_changes[pos];
if !has_exact_change_storage(&sc.slot_changes, withdrawal_idx) {
// No BAL entry at withdrawal_idx; compare against
// last BAL entry (the seeded value).
let seeded =
sc.slot_changes
.last()
.map(|c| c.post_value)
.unwrap_or_else(|| {
db.store
.get_storage_value(*addr, *key_h256)
.unwrap_or_default()
});
if value != seeded {
return Err(EvmError::Custom(format!(
"BAL validation failed for withdrawal: account {addr:?} \
storage slot {slot_u256} changed during withdrawal/request \
phase ({value}) but BAL has no change at index \
{withdrawal_idx} (last_bal={seeded})"
)));
}
}
}
// Slot not in BAL storage_changes at all: loaded from store
// during the withdrawal/request phase. Skip.
}
}

Ok(())
}

Expand Down
Loading