feat(l1): skip receipt validation during full sync batch execution

[azteca1998]

Summary

• Adds skip_receipt_validation parameter to execute_block_from_state()
• Skips validate_receipts_root() only in the add_blocks_in_batch() path (full sync)
• All other execution paths (CL blocks, pipeline) still validate receipts
• Since we download from trusted peers with a verified header chain, and the state root validation at end of each batch ensures correctness, receipt re-validation is redundant during full sync

Closes #6482

Test plan

• Run full sync on hoodi with FULL_SYNC_BLOCK_LIMIT=50000 and verify it completes without errors
• Compare sync time with and without this change to measure improvement
• Verify normal CL block processing still validates receipts

[github-actions]

🤖 Kimi Code Review

Review of PR #6489 - Receipt validation bypass

Security & Correctness

1. Consensus-critical bypass (Line 1141, 2214)
   Skipping receipt validation bypasses a consensus check (EIP-658). At line 2214, passing true permanently disables receipt validation for that code path. Ensure this is only used during block production (where receipts are being generated, not verified) and never during block import/sync.
   Action required: Add a // SAFETY: comment at line 2214 explaining why skipping validation is correct in this specific context (e.g., "executing pending block, receipts root not yet finalized").

2. API Safety - Boolean Blindness (Line 1141)
   Using bool for skip_receipt_validation creates a footgun. Reading execute_block_from_state(..., true) at line 2214 gives no indication what true means.
   Suggestion: Replace with an enum:

   enum ReceiptValidation {
       Validate,
       Skip, // Only safe during block production
   }

Code Quality

3. Documentation (Line 1141)
   The new parameter lacks documentation. Add rustdoc explaining:

   • When it's safe to skip (block production only)
   • The consensus risk of skipping during validation

4. Partial Validation Logic (Lines 1147-1149)
   You skip validate_receipts_root but still validate requests_hash and gas_used. Verify that validate_receipts_root doesn't perform side effects (like bloom filter validation) that other parts of the system depend on being executed.

Verification Needed

5. Call Site Audit
   The function signature change is breaking. Confirm that:
   • All other call sites of execute_block_from_state are updated (the diff only shows one)
   • No code paths that should validate receipts accidentally pass true due to default arguments or refactoring

If line 2214 is indeed in the block production pipeline, the approach is valid but dangerous. Consider splitting into two methods (execute_block_for_validation vs execute_block_for_proposal) to make the safety invariant unrepresentable at the type level.

---

Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt

[github-actions]

🤖 Codex Code Review

Findings

1. High: This change weakens consensus validation during batch import by allowing receipts_root mismatches to pass. The new flag skips validate_receipts_root in crates/blockchain/blockchain.rs:1141 and batch execution now calls it with true at crates/blockchain/blockchain.rs:2217. In the full-sync path, most blocks go through this batch importer (false => blockchain.add_blocks_in_batch(...)) at crates/networking/p2p/sync/full.rs:156 and crates/networking/p2p/sync/full.rs:300. There is no compensating per-block receipt-root check later; batch mode only validates the final state_root before commit at crates/blockchain/blockchain.rs:2261. That means a peer can feed a block whose execution is otherwise valid but whose header has an incorrect receipts_root, and this code will still import it into the canonical chain. I would keep receipt-root validation on for batch import, or add an explicit equivalent validation before storing the batch.

No other findings in this small diff.

---

Automated review by OpenAI Codex · gpt-5.4 · custom prompt

[github-actions]

🤖 Claude Code Review

Now I have enough context for a thorough review.

---

PR Review: feat(blockchain): skip receipt validation during full sync batch execution

PR: #6489
Author: azteca1998
Diff size: +5 / -2 lines

---

Overview

This PR adds a skip_receipt_validation: bool parameter to the private execute_block_from_state() function and passes true only from add_blocks_in_batch() (the full sync path). All other execution paths (CL block processing, pipeline) are unaffected.

The rationale: during full sync, block headers are already verified through the header chain, making receipts_root in the header trusted. Since receipts are produced by local re-execution (not downloaded), the more comprehensive per-batch validate_state_root() is argued to be sufficient.

---

Correctness Analysis

State root does not fully commit to receipt data.

This is the core security concern. validate_state_root verifies that account state changes (balances, nonces, storage) are correct. But receipts carry:

• Transaction success/failure status
• Cumulative gas used per transaction
• Log events (topics and data)

Logs in particular are not reflected in the state trie. A bug in log generation (e.g., incorrect topic hashing, wrong event ABI encoding) would produce a wrong receipts_root detectable by validate_receipts_root, but would go undetected against the state root alone. This removes a defense-in-depth check against EVM implementation bugs in the log-emitting path.

This concern is scoped to full sync only, and the author's trust model (verified header chain from peers) is reasonable. But the reviewer should be aware of this trade-off.

---

API Design (Significant Concern)

Using a bare bool for skip_receipt_validation is a code smell and a readiness risk. At the call site:

// crates/blockchain/blockchain.rs:2217
.execute_block_from_state(&parent_header, block, &chain_config, &mut vm, true)

The true argument is completely opaque. A future caller could easily pass the wrong value. The idiomatic Rust approach is an enum:

enum ReceiptValidation {
    Validate,
    Skip,
}

Which makes the call site self-documenting:

.execute_block_from_state(&parent_header, block, &chain_config, &mut vm, ReceiptValidation::Skip)

This is especially important because execute_block_from_state is the only path that can skip validation — if a second caller is added in the future, it's easy to get the bool wrong.

---

Scope of Validation Still Running

Confirmed that the following validations still execute unconditionally in the batch path, which is good:

• validate_block_pre_execution() — runs per block (line 1144)
• validate_gas_used() — runs per block (line 1147)
• validate_requests_hash() — runs per block (line 1151)
• validate_state_root() — runs once per batch on the final state (line 2262)

Receipts are still accumulated and stored correctly (all_receipts.push, line 2231). Since they come from local execution, the stored receipts will be consistent with what was actually executed regardless of whether the root was checked.

---

Function Visibility

execute_block_from_state is a private fn (no pub), which limits blast radius. The dangerous true argument can't be passed from outside the module. This is good.

---

Missing Tests

The PR has no automated test additions — only a manual test plan. Given the change touches a validation gate in a security-sensitive execution path, at minimum a unit test asserting that:

• skip_receipt_validation = false (the default for all non-batch paths) still catches a receipt mismatch
• skip_receipt_validation = true does not reject a block with a corrupted receipts_root

...would increase confidence and serve as a regression guard.

---

Minor Points

• The commit that closed Reduce Receipt Validation Overhead in full sync #6482 is already merged (72c6a23) based on git log. This PR appears to be the formalization/refinement of that. Confirm this PR doesn't duplicate the landed change.
• The PR is in DRAFT — appropriate given the open test plan items.
• No doc comment was added to the function explaining when/why skipping is safe. A one-line // Safety: comment at the call site (line 2217) would help future readers understand the trust model.

---

Summary

Area	Assessment
Correctness	Mostly sound; receipts_root from header is trusted. Logs not covered by state root validation — minor gap.
Security	Acceptable for full sync with verified headers; removes one validation layer.
API design	bool flag is a code smell — prefer an enum.
Test coverage	No automated tests added.
Blast radius	Limited by private function visibility.

Recommendation: Address the bool → enum refactor before merging; it's a small change that significantly improves readability and safety. A doc comment at the call site explaining the trust model would also be valuable. Consider whether a test covering the skip=false path catching mismatches is feasible to add.

---

Automated review by Claude (Anthropic) · sonnet · custom prompt

[github-actions]

Lines of code report

Total lines added: 3
Total lines removed: 0
Total lines changed: 3

Detailed view

+----------------------------------------+-------+------+
| File                                   | Lines | Diff |
+----------------------------------------+-------+------+
| ethrex/crates/blockchain/blockchain.rs | 2490  | +3   |
+----------------------------------------+-------+------+