fix(l1): reject L2-only transaction types (0x7d/0x7e) in L1 blocks#6752
fix(l1): reject L2-only transaction types (0x7d/0x7e) in L1 blocks#6752ElFantasma wants to merge 2 commits into
Conversation
|
🤖 Kimi Code ReviewThis PR correctly addresses a critical consensus vulnerability where L2-only transaction types (0x7d, 0x7e) could be forged into L1 blocks. The implementation is security-conscious and uses Rust's type system effectively. Strengths
Suggestions1. Verify validation uniqueness (Line 441, 519, 1335)The check appears in three methods. If 2. Strengthen test assertions (Line 3478+)The test verifies rejection via // Around line 3618
let err = result.expect_err("L1 must reject type-0x7e transaction");
assert!(matches!(
err,
ChainError::InvalidBlock(InvalidBlockError::UnsupportedTransactionType(0x7e))
), "Expected UnsupportedTransactionType(0x7e), got: {err:?}");3. Add coverage for FeeToken type (0x7d)The test only covers 4. Nit: Error formatting consistencyIn Security Verification
The implementation is ready to merge after addressing Item 2 (test specificity) to prevent regression test brittleness. Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt |
🤖 Claude Code ReviewNow I have all the context I need for a thorough review. Review: fix(l1): reject L2-only transaction types (0x7d/0x7e) in L1 blocksSummaryThis is a targeted security fix that correctly guards all three pre-execution entry points on the L1 path. The overall approach is sound and the exhaustive match pattern is a good compile-time safeguard. A few minor issues noted below. CorrectnessThree-point coverage — Check ordering — The validation fires immediately after Exhaustive match in Minor IssuesDouble call to if tx.tx_type().is_l2_only() {
return Err(ChainError::InvalidBlock(
InvalidBlockError::UnsupportedTransactionType(tx.tx_type() as u8),
));
}
let ty = tx.tx_type();
if ty.is_l2_only() {
return Err(ChainError::InvalidBlock(
InvalidBlockError::UnsupportedTransactionType(u8::from(ty)),
));
}This also brings the conversion in line with the existing Test CoverageThe test The test also constructs a fully valid No Issues Found With
SummaryThis is a correct and well-scoped fix. The two actionable items are:
Automated review by Claude (Anthropic) · sonnet · custom prompt |
Lines of code reportTotal lines added: Detailed view |
🤖 Codex Code ReviewFindings:
Aside from those gaps, the main import-path change and the exhaustive Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
Greptile SummaryThis PR closes a consensus-divergence and sender-forgery vulnerability by rejecting L2-only transaction types (
Confidence Score: 4/5The change is a targeted security fix that correctly gates on BlockchainType::L1 and is applied at every block execution entry point; L2 paths are untouched. All three execution entry points are covered, the No files require special attention beyond the double
|
| Filename | Overview |
|---|---|
| crates/common/types/transaction.rs | Adds is_l2_only() to TxType with an exhaustive match (no wildcard arm), ensuring any future TxType variant must be explicitly classified or the code won't compile. |
| crates/common/errors.rs | Adds InvalidBlockError::UnsupportedTransactionType(u8) with a hex-formatted error message; straightforward and correct. |
| crates/blockchain/blockchain.rs | Adds validate_l1_transaction_types() called at all three execute_block* sites; regression test correctly constructs a block whose state root is valid pre-fix to prove the vulnerability. Minor: tx_type() is called twice inside the validation loop. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Block received] --> B[find_parent_header]
B --> C[validate_block_pre_execution]
C --> D{validate_l1_transaction_types}
D -- BlockchainType::L2 --> E[skip check, return Ok]
D -- BlockchainType::L1 --> F{any tx where is_l2_only?}
F -- No --> G[execute_block / execute_block_pipeline / execute_block_from_state]
F -- Yes 0x7d or 0x7e --> H[Err: UnsupportedTransactionType]
G --> I[validate_gas_used + state_root checks]
I --> J[store_block]
style H fill:#f55,color:#fff
style D fill:#ff9,color:#000
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
crates/blockchain/blockchain.rs:415-421
`tx.tx_type()` is called twice in the hot path — once for the predicate and once to produce the error payload. Since `TxType` is `Copy` this is functionally safe, but caching the value in a `let` binding makes the intent clearer and avoids the redundant call.
```suggestion
for tx in &block.body.transactions {
let tx_type = tx.tx_type();
if tx_type.is_l2_only() {
return Err(ChainError::InvalidBlock(
InvalidBlockError::UnsupportedTransactionType(tx_type as u8),
));
}
}
```
Reviews (1): Last reviewed commit: "fix(l1): reject L2-only transaction type..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| for tx in &block.body.transactions { | ||
| if tx.tx_type().is_l2_only() { | ||
| return Err(ChainError::InvalidBlock( | ||
| InvalidBlockError::UnsupportedTransactionType(tx.tx_type() as u8), | ||
| )); | ||
| } | ||
| } |
There was a problem hiding this comment.
tx.tx_type() is called twice in the hot path — once for the predicate and once to produce the error payload. Since TxType is Copy this is functionally safe, but caching the value in a let binding makes the intent clearer and avoids the redundant call.
| for tx in &block.body.transactions { | |
| if tx.tx_type().is_l2_only() { | |
| return Err(ChainError::InvalidBlock( | |
| InvalidBlockError::UnsupportedTransactionType(tx.tx_type() as u8), | |
| )); | |
| } | |
| } | |
| for tx in &block.body.transactions { | |
| let tx_type = tx.tx_type(); | |
| if tx_type.is_l2_only() { | |
| return Err(ChainError::InvalidBlock( | |
| InvalidBlockError::UnsupportedTransactionType(tx_type as u8), | |
| )); | |
| } | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/blockchain/blockchain.rs
Line: 415-421
Comment:
`tx.tx_type()` is called twice in the hot path — once for the predicate and once to produce the error payload. Since `TxType` is `Copy` this is functionally safe, but caching the value in a `let` binding makes the intent clearer and avoids the redundant call.
```suggestion
for tx in &block.body.transactions {
let tx_type = tx.tx_type();
if tx_type.is_l2_only() {
return Err(ChainError::InvalidBlock(
InvalidBlockError::UnsupportedTransactionType(tx_type as u8),
));
}
}
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Summary
ethrex uses one shared
Transactiondecoder for L1 and L2. It accepts the L2-only transaction types0x7d(FeeToken) and0x7e(PrivilegedL2Transaction) with no L1/L2 gate, and a privileged transaction's sender is its unsigned, caller-chosen embeddedfrom(no signature recovery). Nothing on the L1 import path (validate_block_pre_execution,validate_block_body, the default LEVM hook) rejects these types, andengine_newPayloaddecodes through the same path.As a result, an L1 block containing a type-
0x7etransaction is decoded, accepted, and executed — using the attacker-chosenfromas the sender. geth/reth and the execution spec reject these types outright, so ethrex would both diverge consensus (accept a block the rest of the network rejects) and let a block forge a sender on L1.Fix
Reject L2-only transaction types on the L1 import path:
TxType::is_l2_only()(incrates/common/types/transaction.rs) classifies each transaction type. The match is exhaustive (no wildcard), so adding a newTxTypevariant won't compile until it is explicitly classified — an L2-only type can never be silently accepted on L1 by omission.Blockchain::validate_l1_transaction_types, gated onBlockchainType::L1, rejects any block transaction whose type is L2-only. It is called right aftervalidate_block_pre_executionat all three pre-execution sites (execute_block,execute_block_pipeline,execute_block_from_state), so every L1 import/execute path is covered.InvalidBlockError::UnsupportedTransactionType(u8).L2 is unaffected (the check early-returns when not
BlockchainType::L1); the zkVM prover and L2 sequencer paths are untouched.Test
Adds
l1_rejects_privileged_l2_transaction_block, which builds an L1 block containing a type-0x7etransaction (post-state consistent so it reaches type acceptance) and assertsadd_blockrejects it. Confirmed red on the pre-fix code (add_blockreturnedOk(())) and green after the fix.Scope
This closes the consensus / block-import vector. The mempool/RPC ingress (
eth_sendRawTransactionadmitting these types on L1) is tracked separately under thefee-token-l1-txfinding.