Skip to content
This repository was archived by the owner on Jan 16, 2026. It is now read-only.

Comments

feat(docs): Custom Derivation Pipeline Example#2702

Merged
refcell merged 3 commits intomainfrom
rf/custom-derivation-example
Aug 13, 2025
Merged

feat(docs): Custom Derivation Pipeline Example#2702
refcell merged 3 commits intomainfrom
rf/custom-derivation-example

Conversation

@refcell
Copy link
Contributor

@refcell refcell commented Aug 13, 2025

Description

Adds a custom derivation pipeline example to the docs.

Progress on #2581

@refcell refcell self-assigned this Aug 13, 2025
@refcell
Copy link
Contributor Author

refcell commented Aug 13, 2025

📚 $\text{Stack Overview}$

Pulls submitted in this stack:

This comment was automatically generated by st.

@claude
Copy link
Contributor

claude bot commented Aug 13, 2025

Claude finished @refcell's task —— View job


PR Review: Custom Derivation Pipeline Example

Review Tasks:

  • Read and analyze the new custom derivation pipeline documentation
  • Review modifications to intro.mdx and sidebar.ts
  • Assess documentation quality, accuracy, and best practices
  • Check for potential issues or improvements
  • Provide comprehensive feedback

Overall Assessment

This PR adds well-structured documentation for creating custom derivation pipeline stages. The example is comprehensive and demonstrates proper Rust patterns for trait implementation and pipeline integration.

Strengths

  1. Accurate API References: The documentation correctly references the actual traits from kona-derive:

    • NextAttributes trait matches crates/protocol/derive/src/traits/attributes.rs:25-31
    • SignalReceiver trait matches crates/protocol/derive/src/traits/stages.rs:11-14
    • OriginProvider trait matches crates/protocol/derive/src/traits/stages.rs:17-20
    • OriginAdvancer trait matches crates/protocol/derive/src/traits/stages.rs:24-28
  2. Practical Example: The LoggingStage wrapper demonstrates a real-world use case for observability and monitoring.

  3. Complete Implementation: Shows all required trait implementations with proper async/await patterns and error propagation.

  4. Testing Guidance: Includes test example using the actual TestNextAttributes from test utilities.

Issues and Improvements

Critical Issues

  1. Incorrect Import Path (custom-derivation-pipeline.mdx:172):

    use alloc::sync::Arc; // ❌ Should be std::sync::Arc

    The alloc crate doesn't provide sync::Arc - this should be std::sync::Arc.

  2. Missing Import (custom-derivation-pipeline.mdx:97):
    The code uses info! macro but doesn't import it. Should add:

    use tracing::info;

Documentation Improvements

  1. Inconsistent Signal Handling (custom-derivation-pipeline.mdx:126-130):

    if let Signal::Reset(reset) = &signal {
        self.last_origin = Some(reset.l1_origin);
        self.attributes_count = 0; // Reset counter
    }

    Based on the actual Signal enum, Reset signals contain a ResetSignal struct with l1_origin, but this field access should be validated against the actual struct definition.

  2. Missing Error Context: The logging statements could benefit from more context about potential error conditions and edge cases.

Code Quality

  1. Proper Trait Bounds: The generic constraints are correctly implemented with Send + Sync bounds for async traits.

  2. Resource Management: The example properly delegates to inner stages and maintains state consistently.

Minor Issues

  1. Navigation: The sidebar and intro updates are correctly implemented for proper documentation navigation.

  2. Code Style: The Rust code follows proper formatting and naming conventions.

Security Considerations

  • No security concerns identified - this is documentation for extending an existing pipeline
  • The example doesn't introduce any unsafe operations or potential vulnerabilities

Recommendations

  1. Fix Import Issues: Address the alloc::sync::Arc and missing tracing::info imports
  2. Validate Signal Handling: Verify the reset.l1_origin field access against actual ResetSignal struct
  3. Add Error Examples: Consider showing how custom stages should handle and propagate pipeline errors
  4. Performance Notes: Maybe add a brief note about the performance implications of wrapping stages

Test Coverage

The documentation includes appropriate test examples using the existing test utilities, which is excellent for demonstrating proper testing practices.


Summary: This is high-quality documentation with accurate API references and practical examples. The main issues are minor import problems that should be fixed before merging. The content effectively demonstrates how to extend Kona's derivation pipeline with custom logic.

@codecov
Copy link

codecov bot commented Aug 13, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.0%. Comparing base (c85425f) to head (0924724).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Base automatically changed from rf/p2p-scoring-example to main August 13, 2025 15:01
Copilot AI review requested due to automatic review settings August 13, 2025 15:31
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds comprehensive documentation for creating custom derivation pipeline stages in Kona, enabling developers to extend the pipeline with monitoring, validation, or transformation logic.

  • Adds a detailed example showing how to wrap the AttributesQueue stage with custom logging functionality
  • Updates navigation to include the new custom derivation pipeline example
  • Provides complete implementation including testing and integration examples

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
docs/sidebar.ts Adds navigation entry for the new custom derivation pipeline example
docs/docs/pages/sdk/examples/intro.mdx Updates the examples index to include the new custom pipeline documentation
docs/docs/pages/sdk/examples/custom-derivation-pipeline.mdx Creates comprehensive documentation with code examples for custom derivation pipeline stages

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

- [Create a new L1BlockInfoTx Hardfork Variant](/sdk/examples/new-l1-block-info-tx-hardfork)
- [Transform Frames to a Batch](/sdk/examples/frames-to-batch)
- [Transform a Batch to Frames](/sdk/examples/batch-to-frames)
- [Create a new L1BlockInfoTx Hardfork Variant](/sdk/examples/new-l1-block-info-tx-hardfork)
Copy link

Copilot AI Aug 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The L1BlockInfoTx hardfork variant link appears twice in the list (lines 6 and 8). Remove the duplicate entry on line 6 to maintain a clean, non-redundant documentation structure.

Suggested change
- [Create a new L1BlockInfoTx Hardfork Variant](/sdk/examples/new-l1-block-info-tx-hardfork)

Copilot uses AI. Check for mistakes.
self.attributes_count += 1;
let duration = start.elapsed();

info!(
Copy link

Copilot AI Aug 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code example uses the info! macro without importing or explaining the logging framework. Add an import statement like use log::info; or use tracing::info; to make the example complete and executable.

Copilot uses AI. Check for mistakes.
S: SignalReceiver + Send + Sync,
{
async fn signal(&mut self, signal: Signal) -> PipelineResult<()> {
info!(target: "pipeline::logging", ?signal, "Received signal");
Copy link

Copilot AI Aug 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to the previous logging issue, the info! macro usage needs proper imports to be a complete working example.

Copilot uses AI. Check for mistakes.
@refcell refcell merged commit a688680 into main Aug 13, 2025
22 of 29 checks passed
@refcell refcell deleted the rf/custom-derivation-example branch August 13, 2025 17:26
theochap pushed a commit to ethereum-optimism/optimism that referenced this pull request Dec 10, 2025
### Description

Adds a custom derivation pipeline example to the docs.

Progress on op-rs/kona#2581
theochap pushed a commit to ethereum-optimism/optimism that referenced this pull request Jan 14, 2026
### Description

Adds a custom derivation pipeline example to the docs.

Progress on #2581
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants