feat(protocol): end to end lookahead slashing with EIP-4788 and EIP-7917

[AnshuJalan]

This PR completes the migration of lookahead slashing over to the new design:

• PreconfSlasherL1 is the sole slasher responsible for all kinds of slashing against the URC collateral (feat(protocol): new base for permissionless preconf contracts #21337)
• Full integration of EIP-4788 and EIP-7917 (taken from the now closed PR: feat(protocol): simulate beacon chain proof verification and improve eip4788 library #20549)
• Removes old BLS contracts that are no longer needed since the URC handles BLS.

[claude]

Claude finished @AnshuJalan's task in 3m 32s —— View job

---

PR Review: End-to-End Lookahead Slashing ✅

I've completed a comprehensive review of this PR. Here are my findings:

---

Summary

This PR successfully migrates lookahead slashing to a cleaner design with PreconfSlasherL1 as the sole slasher, integrates EIP-4788 and EIP-7917 for beacon chain proofs, and removes deprecated BLS contracts. The changes are well-structured with a net reduction of ~400 lines while adding critical functionality.

---

Critical Issues

1. Missing NatSpec Documentation

Location: PreconfSlasherL1.sol:69-98, LibEIP4788.sol:68-185

Several functions lack proper NatSpec documentation:

• PreconfSlasherL1.slash() (lines 69-98) - missing @param and @return docs
• LibEIP4788.verifyBeaconProofs() (lines 68-185) - missing complete documentation
• LibEIP4788._verifyProof() (lines 187-227) - missing @dev tag

Per CLAUDE.md Solidity NatSpec requirements, all public/external functions need full documentation.

2. View Function Called Internally Should Be Pure

Location: PreconfSlasherL1.sol:447-452

function getSlashingAmounts() public pure returns (IPreconfSlasher.SlashingAmounts memory) {

The function getSlashingAmounts() is marked as pure but returns hardcoded values with a comment "Note: These amounts will change". If these are meant to be configurable, consider storing them in state or making this clear in documentation that they're intentionally hardcoded placeholders.

3. Magic Numbers in Beacon State Indices

Location: LibEIP4788.sol:125, 170, 181

11 // validators index in beacon state
37 // proposer_lookahead index in beacon state  
3 // state_root index in beacon block

While comments are present, these magic numbers should be extracted as named constants for clarity and maintainability:

uint256 private constant VALIDATORS_INDEX_IN_BEACON_STATE = 11;
uint256 private constant PROPOSER_LOOKAHEAD_INDEX_IN_BEACON_STATE = 37;
uint256 private constant STATE_ROOT_INDEX_IN_BEACON_BLOCK = 3;

---

Security Concerns

4. Assembly Block Without Safety Documentation

Location: PreconfSlasherL1.sol:458-479

The _decodeEvidenceTuple function uses inline assembly for calldata decoding. While the logic appears sound, it lacks:

• Safety documentation explaining why assembly is necessary
• Bounds checking comments
• Overflow/underflow analysis

Add safety documentation →

5. Timestamp-Based Validation Logic

Location: PreconfSlasherL1.sol:419-424

if (
    LibPreconfUtils.getBeaconBlockRootAt(preconfirmation.submissionWindowEnd)
        != bytes32(0)
) {
    return IPreconfSlasher.PreconfirmationFault.Safety;
}

The classification of liveness faults as safety faults based on beacon block root availability could be manipulated if EIP-4788 data is unavailable due to timing. Consider adding explicit documentation about the timing assumptions and potential edge cases.

6. Unchecked Arithmetic in Lookahead Validation

Location: LookaheadStore.sol:559-584

The _updateLookahead function uses unchecked blocks for timestamp validation. While the logic appears correct, the unchecked arithmetic on timestamps could theoretically overflow in year 2106+ scenarios:

unchecked {
    uint256 prevSlotTimestamp = _nextEpochTimestamp - LibPreconfConstants.SECONDS_IN_SLOT;
    // ...
}

Consider adding explicit overflow checks or documenting the assumptions about timestamp ranges.

---

Code Quality Issues

7. Inefficient Memory Copying

Location: PreconfSlasherL1.sol:228-231

IPreconfSlasher.EvidenceLookahead calldata evidenceLookahead;
assembly {
    evidenceLookahead := add(_evidenceLookaheadBytes.offset, 0x20)
}

This pattern is repeated multiple times (lines 228-231, 282-285, 332-335, 375-378). Consider extracting this into a helper function to reduce code duplication and improve maintainability.

8. Complex Nested Conditionals

Location: PreconfSlasherL1.sol:163-214

The lookahead validation logic has deeply nested conditionals that are difficult to follow. Consider breaking this into smaller, well-named helper functions for each fault type:

// Instead of one large function, create:
_validateInactiveOperatorFault(...)
_validateInvalidValidatorLeafIndexFault(...)
_validateInvalidOperatorFault(...)
_validateMissingOperatorFault(...)

9. Inconsistent Error Naming

Location: PreconfSlasherL1.sol:485-502

Error names are inconsistent:

• BeaconBlockRootMismatch() vs BeaconBlockRootNotAvailable()
• OperatorIsActive() vs OperatorNotActive()

Consider using consistent positive/negative pairs or always using Invalid* prefix for error states.

---

Performance Observations

10. Gas-Intensive Merkle Proof Verification

Location: PreconfSlasherL1.sol:357-361

bytes32[] memory leaves = MerkleTree.hashToLeaves(
    evidenceInvalidOperator.operatorRegistrations, operatorData.owner
);
bytes32 registrationRoot = MerkleTree.generateTree(leaves);

This reconstructs the entire merkle tree for every slashing attempt. While necessary for validation, document the expected gas costs and consider if there are optimization opportunities.

Based on the test file, proof verification costs ~300k gas. Document this clearly for users.

11. Multiple External Calls in View Functions

Location: LookaheadStore.sol:206-241

The isOperatorActive function makes 4 external calls to the URC registry. While marked as view, this could be expensive when called in transaction context. Consider caching patterns or batching if called frequently.

---

Test Coverage Assessment

12. Excellent Test Coverage for LibEIP4788

Strength: The new LibEIP4788.t.sol test file is comprehensive with:

• 1 success test with gas benchmarking
• 12 revert tests covering all error paths
• Real devnet data (Fusaka slot 688808)

This is exemplary test coverage.

13. Missing Integration Tests

Gap: No integration tests found for the complete slashing flow through PreconfSlasherL1. Consider adding tests that:

• Test the full lookahead slashing path from URC → PreconfSlasherL1
• Test the preconf slashing path from bridge → PreconfSlasherL1 → URC
• Test edge cases where operators become inactive mid-epoch

---

Documentation Issues

14. Insufficient Comment on Lookahead Buffer Size

Location: LookaheadStore.sol:38

uint256 public constant LOOKAHEAD_BUFFER_SIZE = 503;

Why 503? This appears to be a prime number for better hash distribution, but this should be documented. Also document the implications of this size limit on epoch storage.

15. Missing Migration Guide

Impact: This PR removes significant functionality (LibBLS12381, LibBLSSignature, etc.). No migration guide or explanation of how existing code should adapt. Consider adding a MIGRATION.md or inline comments explaining the replacement.

---

Positive Observations

✅ Clean Architecture: The unified slasher design is cleaner than having separate L2 slasher interfaces
✅ Code Reduction: Net -400 lines while adding functionality shows good refactoring
✅ Modern BLS: Using Solady's BLS library instead of custom implementation
✅ Comprehensive Proof Validation: The beacon proof verification is thorough and well-structured
✅ Simple Helpers: LibBLSG1.equals() is clean and reusable
✅ Good Error Handling: Custom errors throughout for better gas efficiency

---

Recommendations

1. Add NatSpec documentation to all public/external functions (critical)
2. Extract magic numbers to named constants
3. Add safety comments to assembly blocks
4. Document gas costs for expensive operations
5. Add integration tests for complete slashing flows
6. Create helper functions to reduce complexity in PreconfSlasherL1
7. Document timestamp assumptions and overflow safety
8. Add migration notes for removed contracts

---

Verdict

This is a solid architectural improvement with good test coverage for the new beacon proof functionality. The main areas for improvement are documentation, magic number extraction, and integration test coverage. The security-critical beacon proof validation logic is well-tested and appears sound.

The PR can be merged after addressing the critical documentation issues and considering the security notes about timestamp validation and assembly safety.

---

Three additional inline comments have been posted with specific code suggestions.

---

[claude]

Magic Numbers Should Be Named Constants

In LibEIP4788.sol, the beacon state indices should be extracted as named constants:

// Current (lines 125, 170, 181):
11 // validators index in beacon state
37 // proposer_lookahead index in beacon state
3 // state_root index in beacon block

// Suggested:
uint256 private constant VALIDATORS_INDEX_IN_BEACON_STATE = 11;
uint256 private constant PROPOSER_LOOKAHEAD_INDEX_IN_BEACON_STATE = 37;
uint256 private constant STATE_ROOT_INDEX_IN_BEACON_BLOCK = 3;

This improves readability and makes it easier to update if the beacon chain spec changes.

[claude]

Assembly Block Needs Safety Documentation

The _decodeEvidenceTuple function in PreconfSlasherL1.sol:458-479 uses inline assembly without safety documentation. Please add comments explaining:

1. Why assembly is necessary (gas optimization for calldata slicing)
2. Bounds checking guarantees
3. Overflow/underflow safety properties

Example:

/// @dev Uses assembly for efficient calldata slicing without copying
/// @dev Safety: calldataload bounds are guaranteed by Solidity calldata validation
/// @dev No overflow risk as we're reading fixed 32-byte words
function _decodeEvidenceTuple(...) { /* ... */ }

[claude]

Missing NatSpec Documentation

Several functions are missing required NatSpec documentation per CLAUDE.md:

1. PreconfSlasherL1.slash() (lines 69-98) - needs @param and @return tags
2. LibEIP4788.verifyBeaconProofs() (lines 68-185) - needs complete NatSpec
3. LibEIP4788._verifyProof() (lines 187-227) - needs @dev tag

Example for slash():

/// @inheritdoc ISlasher
/// @param _commitment The commitment being challenged
/// @param _evidence Evidence bytes: [SlashingPath][fault-specific evidence]
/// @param _challenger Address initiating the slash
/// @return slashAmount_ Amount to slash from the operator
function slash(...) external view returns (uint256 slashAmount_) { /* ... */ }

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 478c7a018c

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".