Conversation
🟡 Heimdall Review Status
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| pub index: u64, | ||
| pub base: Option<ExecutionPayloadBaseV1>, | ||
| pub diff: ExecutionPayloadFlashblockDeltaV1, | ||
| pub metadata: Value, // Contains "flashblock_access_list" |
There was a problem hiding this comment.
The spec refers to the metadata field as flashblock_access_list here (and in the surrounding prose at line 48), but the actual FlashblocksMetadata struct in crates/builder/core/src/flashblocks/payload.rs:880 serializes it as access_list. The V1 exec-engine spec page also uses access_list in its JSON example.
This naming mismatch between the spec and the implementation (and the other spec page) could cause confusion for third-party implementors. Should the spec use access_list to match the code and exec-engine.md?
|
|
||
| - `0`: System transaction (L1 Attributes) | ||
| - `1 to n`: Regular L2 transactions (including L1→L2 user deposits) | ||
| - `n + 1`: Post-execution system contracts (if any) |
There was a problem hiding this comment.
This "Key Differences" summary says transaction indices are 1 to n, but the detailed "BlockAccessIndex Assignment" section at line 161 says transactions use min_tx_index … max_tx_index - 1 (the overall block index). The first flashblock's L1 attributes tx uses index 0, so user transactions start at index 1 in that flashblock. But for subsequent flashblocks, user transactions start at min_tx_index (e.g., 5, 10, etc.), not 1.
This summary should clarify that 1 to n describes the full-block view, or reference the flashblock-specific indexing to avoid confusing implementors who read only this section.
| StorageKey = bytes # 32-byte storage slot key | ||
| StorageValue = bytes # 32-byte storage value | ||
| CodeData = bytes # Variable-length contract bytecode | ||
| BlockAccessIndex = uint16 # Block access index (0 for pre-execution, 1..n for transactions, n+1 for post-execution) |
There was a problem hiding this comment.
BlockAccessIndex is typed as uint16 (max 65535), inherited from BAL. However, the spec also says transaction indices use the overall block transaction index (line 164), and MAX_TXS = 30_000. With the L1 attributes tx at index 0 and a post-execution slot at n+1, the max index would be 30001, which fits in uint16.
However, given that flashblocks use overall block indices, it may be worth explicitly noting the uint16 upper bound (65535) as a constraint on MAX_TXS, since an implementor might otherwise assume these are independent constants.
| `BlockAccessIndex` values **MUST** be assigned as follows: | ||
|
|
||
| - `0` for **pre-execution** system contract calls and **L1 attributes transaction** (only in the first flashblock where `min_tx_index = 0`) | ||
| - `min_tx_index … max_tx_index - 1` for transactions in this flashblock (including L1→L2 user deposits and regular L2 transactions) | ||
| - `n + 1` for **post-execution** system contract calls (if any, only in the final flashblock) |
There was a problem hiding this comment.
The L1 attributes transaction is described both as using block_access_index = 0 (pre-execution) and as being "at index 0" (deposited tx). It seems like block_access_index 0 serves double duty here — it covers both the pre-execution system contract calls and the L1 attributes transaction execution.
A clarification question: if the first flashblock has min_tx_index = 0, does the L1 attributes deposited tx at position 0 in diff.transactions also use block_access_index = 0? In that case, the pre-execution system changes and the L1 attributes tx changes are merged under the same index. Is this intentional? BAL uses index 0 exclusively for pre-execution, and actual transactions start at index 1. The concrete example at line 305 shows tx_index = min_tx_index + i which would give the L1 attributes tx (at i=0, min_tx_index=0) index 0, confirming the merge — but this deviates from BAL's separation.
| pub struct FlashblockAccessList { | ||
| pub min_tx_index: u64, // Inclusive starting transaction index in the overall block | ||
| pub max_tx_index: u64, // Exclusive ending transaction index in the overall block | ||
| pub account_changes: Vec<AccountChanges>, // List of all account changes | ||
| pub fal_hash: B256, // Keccak-256 hash of RLP-encoded account_changes | ||
| } |
There was a problem hiding this comment.
Note: the actual Rust struct field ordering is account_changes, min_tx_index, max_tx_index, fal_hash (see crates/alloy/access-lists/src/types.rs). Since this struct derives RlpEncodable, the RLP encoding follows field declaration order — so the RLP encoding puts account_changes first, then min_tx_index, then max_tx_index, then fal_hash.
The spec struct shown here has the same ordering, which is good. But the spec struct omits RlpEncodable/RlpDecodable derives — consider mentioning RLP field ordering matters for deterministic encoding, since implementors in other languages will need to match this exact order.
| pub struct FlashblockAccessList { | ||
| pub min_tx_index: u64, // Inclusive starting transaction index in the overall block | ||
| pub max_tx_index: u64, // Exclusive ending transaction index in the overall block | ||
| pub account_changes: Vec<AccountChanges>, // List of all account changes | ||
| pub fal_hash: B256, // Keccak-256 hash of RLP-encoded account_changes | ||
| } |
There was a problem hiding this comment.
The Rust snippet here shows field order min_tx_index, max_tx_index, account_changes, fal_hash, but the actual implementation in crates/alloy/access-lists/src/types.rs declares it as account_changes, min_tx_index, max_tx_index, fal_hash.
Since the struct derives RlpEncodable/RlpDecodable, field declaration order determines the RLP encoding. This mismatch means an implementor following this spec would produce different RLP encodings than the code, breaking fal_hash validation.
Either the spec snippet should match the code's field order, or (if the spec ordering is the intended one) the code needs to be updated. Either way, this should be resolved before the spec is finalized.
| pub struct FlashblockAccessList { | |
| pub min_tx_index: u64, // Inclusive starting transaction index in the overall block | |
| pub max_tx_index: u64, // Exclusive ending transaction index in the overall block | |
| pub account_changes: Vec<AccountChanges>, // List of all account changes | |
| pub fal_hash: B256, // Keccak-256 hash of RLP-encoded account_changes | |
| } | |
| pub struct FlashblockAccessList { | |
| pub account_changes: Vec<AccountChanges>, // List of all account changes | |
| pub min_tx_index: u64, // Inclusive starting transaction index in the overall block | |
| pub max_tx_index: u64, // Exclusive ending transaction index in the overall block | |
| pub fal_hash: B256, // Keccak-256 hash of RLP-encoded account_changes | |
| } |
| # Pre-execution: L1 attributes transaction (block_access_index = 0) | ||
| # Only include for first flashblock (min_tx_index == 0) | ||
| if min_tx_index == 0: | ||
| track_l1_attributes_tx(flashblock, accesses, block_access_index=0) | ||
| track_system_contracts_pre(flashblock, accesses, block_access_index=0) | ||
|
|
||
| # Execute transactions (block_access_index = min_tx_index..max_tx_index) | ||
| # This includes both L1→L2 deposits and regular L2 transactions | ||
| for i, tx in enumerate(flashblock.diff.transactions): | ||
| tx_index = min_tx_index + i | ||
| execute_transaction(tx) | ||
| track_state_changes(tx, accesses, block_access_index=tx_index) | ||
| track_fee_vault_changes(tx, accesses, block_access_index=tx_index) |
There was a problem hiding this comment.
The pseudocode has a double-counting issue for the first flashblock. When min_tx_index == 0:
- Lines 300-301 call
track_l1_attributes_tx()andtrack_system_contracts_pre()withblock_access_index=0 - Then the loop at line 305 iterates over
flashblock.diff.transactions, and the first iteration givestx_index = 0 + 0 = 0
If the L1 attributes transaction is included in diff.transactions (which it typically is as the deposited tx at position 0), its state changes will be recorded twice under block_access_index=0: once by track_l1_attributes_tx() and once by the general track_state_changes() call.
The pseudocode should either skip the L1 attributes tx in the loop (e.g., start enumeration at index 1 for the first flashblock), or remove the separate track_l1_attributes_tx() call and let the general loop handle it. As written, an implementor following this pseudocode literally would produce incorrect FALs.
Spec Review SummaryThis PR adds a well-structured FAL (Flashblock-Level Access Lists) specification adapting EIP-7928 for flashblocks, along with updates to the V1 exec-engine spec and sidebar navigation. FindingsStruct field ordering mismatch (correctness): The Rust snippet in the spec defines Pseudocode double-counting (correctness): The validation pseudocode in Non-blocking observationsThe exec-engine.md and overview.md changes are clean — the section rename from "Remove Account Balances & Receipts" to "Flashblocks Metadata Changes" accurately reflects the expanded scope, and the JSON example now shows the FAL structure. |
2 participants
Update V1 specs to specify that access lists are included in flashblock metadata Base V1.