Skip to content

Conversation

@davidtaikocha
Copy link
Collaborator

Since its for post-WL preconf, not for Shasta fork.

@claude
Copy link

claude bot commented Nov 13, 2025

Claude finished @davidtaikocha's task —— View job


Code Review Complete

Tasks

  • Read changed files and analyze modifications
  • Review revert completeness and consistency
  • Check for potential issues or incomplete changes
  • Provide comprehensive feedback

Review Summary

This PR cleanly reverts the changes from #58 that updated anchorV4 based on protocol #20304. The revert is appropriate since those changes were intended for post-WL preconf, not the Shasta fork.

Code Quality: ✅ Good

Strengths:

Detailed Analysis

1. Consensus Layer (crates/consensus/src/validation.rs:29-34)

The anchorV4 function signature correctly reverts from:

// Removed (protocol #20304 version):
function anchorV4(
    (uint48, uint48, address, bytes, bytes32, (uint48, uint8, address, address)[]),
    (uint48, bytes32, bytes32, bytes32)
) external;

// Restored (pre-#20304 version):
function anchorV4(
    (uint48, address, bytes, bytes32, (uint48, uint8, address, address)[]),
    (uint48, bytes32, bytes32)
) external;

✅ Correct - removes the extra uint48 (likely submissionWindowEnd) from first tuple and one bytes32 from second tuple.

2. RPC Layer (crates/rpc/src/eth/eth.rs:162-188)

The extract_anchor_v4_proposal_id function logic reverts to simpler parsing:

  • ✅ Removes PROPOSAL_ID_WORD_OFFSET and PROPOSAL_ID_BYTE_OFFSET constants
  • ✅ Simplifies proposal ID extraction - reads directly at offset instead of skipping a word
  • ✅ This aligns with the simpler struct layout (no submissionWindowEnd to skip over)

Logic flow:

// Old (removed): skip past submissionWindowEnd word before reading proposalId
let proposal_id_start = tuple_start.checked_add(PROPOSAL_ID_BYTE_OFFSET)?;

// New (restored): read proposalId directly at offset
let proposal_id_start = SELECTOR_LEN.checked_add(offset)?;

3. Test Data (crates/rpc/src/eth/eth.rs:195-211)

✅ Test calldata correctly updated to match the pre-#20304 anchorV4 struct layout. The hex data is now much simpler (shorter), consistent with the removed field.

4. Genesis Hash (crates/chainspec/src/lib.rs:21-22)

TAIKO_DEVNET_GENESIS_HASH reverted from 0x14c6ef... to 0x3c2317... - this is expected since the genesis JSON changed.

5. Genesis JSON (crates/chainspec/src/genesis/devnet.json)

The large contract bytecode change reflects the TaikoL1 contract being reverted to the pre-#20304 version. While I cannot fully audit the bytecode, the change is:

  • ✅ Consistent with reverting the anchorV4 interface changes
  • ✅ Expected for a clean revert

Potential Issues: None Found

Completeness: All references to the protocol #20304 changes are properly reverted
Consistency: Changes are synchronized across all layers (consensus, RPC, chainspec)
Test Coverage: Test expectations updated to match reverted implementation
No Dangling References: No leftover code referencing the removed fields

Performance Considerations

Improvement: The reverted code is actually slightly more efficient - removed unnecessary offset calculations in extract_anchor_v4_proposal_id.

Security Concerns

No Issues: The revert properly handles:

  • Boundary checking with checked_add
  • Input validation before parsing
  • Proper error handling via Option returns

Recommendations

  1. Merge-ready - This is a clean revert with no issues
  2. Consider adding a comment in validation.rs explaining that anchorV4 will be updated again post-WL preconf to reduce confusion
  3. Ensure any documentation or specs that referenced protocol #20304 are also updated

Testing

The PR includes test data updates, but consider:

  • Running integration tests against devnet with the reverted genesis
  • Verifying block validation works correctly with both pre-Shasta and Shasta blocks
  • Testing the RPC method taiko_lastBlockIDByBatchID still works correctly (depends on extract_anchor_v4_proposal_id)

@davidtaikocha davidtaikocha merged commit 0e22cd5 into main Nov 13, 2025
8 checks passed
@davidtaikocha davidtaikocha deleted the revert-anchor-v4 branch November 13, 2025 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants