fix: validate decoded block access lists

[rakita]

Summary

• validate EIP-7928 ordering and uniqueness while converting Alloy BAL types
• reject duplicate accounts, storage overlap, empty slot changes, and invalid change indices
• consume owned code changes without cloning bytecode

Testing

• cargo test -p evm2 --lib evm::bal --no-default-features --features std,map-foldhash,sha3-keccak
• cargo clippy -p evm2 --lib --tests --no-default-features --features std,map-foldhash,sha3-keccak -- -D warnings

ZELLIC_232

[rakita]

@decofe audit

[rakita]

cyclops audit

[tempoxyz-bot]

👁️ Cyclops Review

PR #382 hardens decoded EIP-7928 block-access-list imports. The core import validation appears sound, but two verified downstream consistency bugs remain in code reached by validated BAL data. One additional low-severity defense-in-depth note is posted inline.

🚨 [SECURITY] BAL-supplied bytecode is dropped on commit while its code hash is retained

Severity: High
File: crates/evm2/src/evm/db/cache.rs:157

AccountInfoBal::populate_account_info injects both code_hash and code from a validated BAL into an account, but CacheDB::get_account applies that BAL overlay after the only insert_contract_inner call and never registers the BAL bytecode in cache.contracts. CacheDB::commit then only caches bytecode returned by entry.changed_code(), which is false for code merely served by a BAL, and stores account.clone_no_code(). Later get_code_by_hash can cache the backing DB's empty-bytecode answer, leaving a non-empty EXTCODEHASH with empty executable code.

Recommended Fix: Register BAL-supplied bytecode when populate_bal_account adds it and/or cache any present non-empty account.code during commit before storing clone_no_code(). Consider making get_code_by_hash consult the BAL or avoid caching backing-DB misses as empty code.

🚨 [SECURITY] Destroyed storage is classified differently by the BAL commit and sink builders

Severity: Medium
File: crates/evm2/src/evm/state/storage.rs:194

StorageHandle::wipe documents resetting original alongside current, but currently only sets current to zero. A loaded slot with original value 42 becomes original = 42, current = 0: StorageBal::update_pending emits a zero write, while PendingState::visit sees the wiped-zero slot and emits a read. Both shapes pass the new importer but encode into different EIP-7928 fields, producing different RLP / block_access_list_hash values for the same destroyed slot.

Recommended Fix: Make wipe() reset both original and current for loaded slots (for example, replace the tracked value with Tracked::new(Word::ZERO)) and add a regression test that builds the overlay through wipe() and compares the commit and sink BAL builders.

Reviewer Callouts

• ⚡ Code-hash miss handling: CacheDB::get_code_by_hash (crates/evm2/src/evm/db/cache.rs:321) caches whatever the backing DB returns, and EmptyDB::get_code_by_hash returns empty bytecode for arbitrary hashes (crates/evm2/src/evm/db/mod.rs:319). Consider making missing code explicit or at least not caching a miss as hash -> empty.
• ⚡ EEST coverage for new decode errors: crates/eest/src/blockchaintest/execute.rs:1195 catches every Bal::try_from error and falls back to comparing the raw expected BAL. This appears fail-safe, but it means malformed fixture BALs do not directly exercise the new validation failures; wiring in the transaction-count-aware import would improve coverage.
• ⚡ EIP-7928 size/count limits: The import path does not enforce MAX_ACCOUNTS, MAX_SLOTS, MAX_TXS_PER_BLOCK, MAX_CODE_SIZE, or validate_gas_limit. Adding these checks to the contextual import path would better bound memory use before execution/header comparison rejects an invalid BAL.

[tempoxyz-bot]

🛡️ [DEFENSE-IN-DEPTH] Block-specific BAL index bounds are opt-in and unused

This transaction-count-aware constructor is the only new path that enforces EIP-7928's block_access_index <= transaction_count + 1 rule, but the default TryFrom<AlloyBal> / borrowed TryFrom imports do not call it and no in-tree caller uses it today. The always-on check only rejects indices above u32::MAX, so ordinary imports can accept change indices that are impossible for any valid block.

Recommended Fix:
Route block-validation consumers through these constructors wherever the transaction count is known, or enforce the tighter EIP-7928 maximum/default bound and layer the per-block transaction_count + 1 check into the validation path.