Skip to content

feat(rpc): introduce auth api taikoAuth_lastCertainL1OriginByBatchID#136

Merged
YoGhurt111 merged 3 commits intomainfrom
feature/last_certain_l1_origin_by_batch_id
Mar 31, 2026
Merged

feat(rpc): introduce auth api taikoAuth_lastCertainL1OriginByBatchID#136
YoGhurt111 merged 3 commits intomainfrom
feature/last_certain_l1_origin_by_batch_id

Conversation

@YoGhurt111
Copy link
Copy Markdown
Member

No description provided.

@YoGhurt111 YoGhurt111 self-assigned this Mar 30, 2026
@YoGhurt111 YoGhurt111 enabled auto-merge (squash) March 30, 2026 02:23
@claude
Copy link
Copy Markdown

claude bot commented Mar 30, 2026

Claude finished @YoGhurt111's task —— View job


Code Review Complete ✓

I've reviewed this PR implementing the new taikoAuth_lastCertainL1OriginByBatchID API. Here's my feedback:


Summary

This PR adds a new auth API endpoint lastCertainL1OriginByBatchID that retrieves the cached L1 origin for a given batch ID without falling back to chain scanning. This complements the existing lastL1OriginByBatchID endpoint which does perform fallback scanning.

Overall assessment: The implementation is solid with good test coverage. The code is clean, well-tested, and follows the existing patterns in the codebase.


Positive Observations

  1. Consistent API design - The new endpoint follows the same pattern as the recently added lastCertainBlockIDByBatchID endpoint (lines 83-87, 228-237 in mod.rs)

  2. Comprehensive test coverage - Two new tests specifically cover the new endpoint:

    • returns_none_for_last_certain_l1_origin_without_mapping (lines 457-471 in tests.rs)
    • returns_last_certain_l1_origin_from_mapping (lines 475-509 in tests.rs)
  3. Correct implementation - The implementation properly:

    • Returns None when no cached mapping exists (early return)
    • Only queries the L1 origin if the batch mapping exists
    • Reuses the existing read_l1_origin_by_block_id helper
  4. Clear separation of concerns - The "certain" (cached-only) vs regular (with fallback) distinction is well-maintained


Code Quality Issues

Minor: Redundant early return check

crates/rpc/src/eth/auth/mod.rs:232-234

The early return for None could be simplified using ? operator:

// Current
let Some(block_id) = self.read_cached_last_block_number_by_batch_id(batch_id)? else {
    return Ok(None);
};

// Cleaner
let block_id = match self.read_cached_last_block_number_by_batch_id(batch_id)? {
    Some(id) => id,
    None => return Ok(None),
};

// Or even simpler with combinators
return self.read_cached_last_block_number_by_batch_id(batch_id)?
    .map(|block_id| self.read_l1_origin_by_block_id(block_id))
    .transpose();

The last option is most idiomatic Rust and avoids intermediate variables entirely.


Test Coverage

Strong points:

  • Both success and failure cases are tested
  • Integration with the full database stack is covered
  • Verification that no fallback scanning occurs (lines 512-548 in tests.rs)

Complete coverage: No additional test cases needed.


Performance Considerations

Efficient - This endpoint explicitly avoids expensive chain scanning by design, making it suitable for high-frequency queries.

Database access - Single read-only transaction, no iteration.


Security Concerns

No security issues identified

  • Read-only operation
  • No user-controlled database writes
  • Proper error handling (no information leakage)
  • Database errors mapped through internal_eth_error

Documentation

Missing: The PR description is empty. Consider adding:

  • Why this endpoint is needed (use case)
  • How it differs from lastL1OriginByBatchID
  • When clients should use "certain" vs regular endpoints

Recommendations

  1. Low priority: Consider using the combinator-based approach for the implementation (see code quality section)
  2. PR description: Add context about why this endpoint is needed
  3. Before merge: Run just clippy and just test to ensure no regressions

Final Verdict

Approve with suggestions

The implementation is correct, well-tested, and ready to merge. The suggested improvements are minor style optimizations that would improve code clarity but aren't blocking issues.

--- | Branch: feature/last_certain_l1_origin_by_batch_id

@YoGhurt111 YoGhurt111 changed the title feat(taiko): introduce auth api taikoAuth_lastCertainL1OriginByBatchID feat(rpc): introduce auth api taikoAuth_lastCertainL1OriginByBatchID Mar 30, 2026
@YoGhurt111 YoGhurt111 merged commit 1a60419 into main Mar 31, 2026
5 checks passed
@YoGhurt111 YoGhurt111 deleted the feature/last_certain_l1_origin_by_batch_id branch March 31, 2026 00:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants