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

fix(derive): Pipeline#129

Merged
refcell merged 3 commits intorefcell/pipeline-builderfrom
refcell/ugly-refactor
Apr 25, 2024
Merged

fix(derive): Pipeline#129
refcell merged 3 commits intorefcell/pipeline-builderfrom
refcell/ugly-refactor

Conversation

@refcell
Copy link
Contributor

@refcell refcell commented Apr 22, 2024

Description

This is an ugly refactor of the derivation pipeline builder using a simple, concrete online stack of the stages.

@refcell refcell requested a review from clabby April 22, 2024 18:35
@refcell refcell self-assigned this Apr 22, 2024
@refcell
Copy link
Contributor Author

refcell commented Apr 22, 2024

Struggled with iteration over previous stages due to conflicting nested associated trait types. So I've modified each stage's reset to first call the previous stage's reset and bubble up any errors there.

This follows the reference implementation but optimistically resets all stages instead of iterating over each and resetting in order. See: https://github.com/ethereum-optimism/optimism/blob/develop/op-node/rollup/derive/pipeline.go#L93-L96

If we need to implement iterative reset, we can have the ResettableStage trait's reset method return an enum that has an already reset variant that'd allow the recursive reset to dispatch on the results at each stage level.

@refcell
Copy link
Contributor Author

refcell commented Apr 22, 2024

Additionally, the current pipeline gives ownership of each stage to the successive stage. This means the top-level pipeline object or "stage stack" wrapper does not have a handle to the concrete L1Traversal stage. But the pipeline needs to call the advance_l1_block method when a StageError::Eof is encountered. The current naive solution just adds an atomic reference advance flag to the L1 traversal stage that the stack can set when an eof error is received when fetching the next attributes.

@refcell
Copy link
Contributor Author

refcell commented Apr 22, 2024

I don't really like either of these introduced changes and would like to think through a simpler solution that also doesn't require a large refactor of the stage nesting architecture of the current pipeline.

@refcell refcell changed the title [RFC]: Ugly Pipeline Refactor fix(derive): Pipeline Apr 25, 2024
@refcell
Copy link
Contributor Author

refcell commented Apr 25, 2024

Merged pr handled the above questions 😄

@refcell refcell marked this pull request as ready for review April 25, 2024 14:56
@refcell refcell merged commit 31272dc into refcell/pipeline-builder Apr 25, 2024
@clabby clabby deleted the refcell/ugly-refactor branch July 2, 2024 03:39
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.

1 participant