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

Comments

cleanup(node/actor): Consolidate DerivationActor inbound channels#3229

Merged
op-will merged 2 commits intomainfrom
op-will/consolidateDerivationActorInbound
Jan 13, 2026
Merged

cleanup(node/actor): Consolidate DerivationActor inbound channels#3229
op-will merged 2 commits intomainfrom
op-will/consolidateDerivationActorInbound

Conversation

@op-will
Copy link
Collaborator

@op-will op-will commented Jan 5, 2026 •
edited
Loading

See: #3223

This also reworks aspects of EngineActor. See referenced issues.

closes #3223, #3213, #3215, #3216, #3219, and #3072

@op-will op-will mentioned this pull request Jan 5, 2026
Merged
@codecov
Copy link

codecov bot commented Jan 5, 2026 •
edited
Loading

Codecov Report

❌ Patch coverage is 0% with 427 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.0%. Comparing base (f7f5338) to head (7980116).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...vice/src/actors/engine/engine_request_processor.rs 0.0% 161 Missing ⚠️
crates/node/service/src/service/node.rs 0.0% 82 Missing ⚠️
crates/node/service/src/actors/derivation/actor.rs 0.0% 69 Missing ⚠️
...service/src/actors/engine/rpc_request_processor.rs 0.0% 48 Missing ⚠️
crates/node/service/src/actors/engine/config.rs 0.0% 16 Missing ⚠️
crates/node/service/src/actors/engine/client.rs 0.0% 15 Missing ⚠️
crates/node/service/src/actors/l1_watcher/actor.rs 0.0% 10 Missing ⚠️
...rates/node/service/src/actors/l1_watcher/client.rs 0.0% 10 Missing ⚠️
...node/service/src/actors/sequencer/engine_client.rs 0.0% 7 Missing ⚠️
...ode/service/src/actors/derivation/engine_client.rs 0.0% 5 Missing ⚠️
... and 1 more

❗ There is a different number of reports uploaded between BASE (f7f5338) and HEAD (7980116). Click for more details.

HEAD has 18 uploads less than BASE
Flag BASE (f7f5338) HEAD (7980116)
proof 7 0
e2e 10 0
unit 2 1

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

@op-will op-will mentioned this pull request Jan 6, 2026
Open
@op-will op-will changed the title WIP: Consolidate DerivationActor inbound channels WIP: cleanup(node/actor): Consolidate DerivationActor inbound channels Jan 6, 2026
einar-oplabs
einar-oplabs reviewed Jan 6, 2026
View reviewed changes
@op-will op-will mentioned this pull request Jan 7, 2026
Merged
@op-will op-will force-pushed the op-will/consolidateEngineActorInboundChannels branch from cabd1ab to b13b3b0 Compare January 8, 2026 21:09
Base automatically changed from op-will/consolidateEngineActorInboundChannels to main January 8, 2026 22:02
@op-will op-will force-pushed the op-will/consolidateDerivationActorInbound branch from 2eb2f23 to 7434163 Compare January 9, 2026 20:07
@op-will op-will changed the title WIP: cleanup(node/actor): Consolidate DerivationActor inbound channels cleanup(node/actor): Consolidate DerivationActor inbound channels Jan 9, 2026
@op-will op-will force-pushed the op-will/consolidateDerivationActorInbound branch from 560eefa to 053cc5f Compare January 9, 2026 23:01
@op-will op-will marked this pull request as ready for review January 9, 2026 23:01
@op-will op-will mentioned this pull request Jan 12, 2026
Merged
@op-will op-will force-pushed the op-will/consolidateDerivationActorInbound branch 2 times, most recently from b3062d0 to b1b1f7d Compare January 12, 2026 17:46
@op-will op-will changed the base branch from main to pcw109550/move-l2-finalization-derivationActor January 12, 2026 17:47
@op-will op-will force-pushed the op-will/consolidateDerivationActorInbound branch from b1b1f7d to 7c564d5 Compare January 12, 2026 17:55
theochap
theochap reviewed Jan 12, 2026
View reviewed changes
crates/node/service/src/actors/derivation/actor.rs
pipeline: PipelineSignalReceiver,

/// The engine's L2 safe head, according to updates from the Engine.
engine_l2_safe_head: L2BlockInfo,
Copy link
Member

@theochap theochap Jan 12, 2026

Choose a reason for hiding this comment

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

Why isn't that a channel? It looks like this should be a watch::Receiver?

Copy link
Collaborator Author

@op-will op-will Jan 12, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

There's little value in making this a watch::Receiver, as the process (at a very high level) is:

  1. DerivationActor sends EngineActor the safe head attributes
  2. DerivationActor halts derivation until it hears that EngineActor has updated the safe head
  3. Repeat

So there should never be safe head updates from the engine that the DerivationActor didn't send it (and is waiting on for an ack). We don't need to worry about frequent updates or stale data.

It also creates the explicit dependency between Engine and DerivationActor. The only [current] reason EngineActor exposes this info is for DerivationActor to read it.

theochap
theochap reviewed Jan 12, 2026
View reviewed changes
theochap
theochap reviewed Jan 12, 2026
View reviewed changes
@pcw109550 pcw109550 mentioned this pull request Jan 12, 2026
Merged
3 tasks
Base automatically changed from pcw109550/move-l2-finalization-derivationActor to main January 12, 2026 20:56
theochap
theochap reviewed Jan 12, 2026
View reviewed changes
theochap
theochap reviewed Jan 12, 2026
View reviewed changes
theochap
theochap reviewed Jan 12, 2026
View reviewed changes
theochap
theochap reviewed Jan 12, 2026
View reviewed changes
theochap
theochap reviewed Jan 12, 2026
View reviewed changes
@op-will op-will force-pushed the op-will/consolidateDerivationActorInbound branch from 7c564d5 to 1b933ec Compare January 12, 2026 21:17
theochap
theochap approved these changes Jan 12, 2026
View reviewed changes
Copy link
Member

@theochap theochap left a comment

Choose a reason for hiding this comment

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

Overall looks really good. A few nits here and there. Please make sure to run all the CI pipeline (including action-tests and acceptance tests that are skipped in CI) before merging that PR. This is a pretty big refactor so there may be some areas that may have been broken

crates/node/service/src/service/node.rs
engine_processor,
engine_rpc_processor,
))
}
Copy link
Member

@theochap theochap Jan 12, 2026

Choose a reason for hiding this comment

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

The logic to build the engine here is a bit complicated and I am slightly worried that we're pushing so much explicit behavior at the rollup-node-service level. It may be fine to keep it that way for this PR, but we should think of a way to fit all the actors under similar patterns so that we can make the RollupNode more generic/easier to modify. This doesn't need to be done here and it will probably require changing the other actors but I'd like us to keep this goal in mind for future changes

Copy link
Collaborator Author

@op-will op-will Jan 12, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Hopefully we can remove StartData from RpcActor, which should allow us to remove StartData from NodeActor, and then this function can just take a Vec of NodeActor.

That will push this configuration up into the NodeCommand, but

  1. To me, it makes more sense for it to be there since the intention is for this to be a lib
  2. We can still have all of the construction helpers in this lib so they can be used by our CLI as well as anyone else using the lib (if they choose)

@op-will op-will force-pushed the op-will/consolidateDerivationActorInbound branch from 0769d7c to 9127b94 Compare January 12, 2026 23:10
@op-will
Consolidating DerivationActor inbound channels
056ff82 
- Creates enum of supported inboud requests handled via a single channel
- Creates client traits for all references and adds code to use them

Also:
- Reworks EngineActor to inject dependencies and remove NodeActor::StartData
- Creates processor task traits and makes EngineActor generic over them
@op-will
Addressing review feedback
7980116
@op-will op-will force-pushed the op-will/consolidateDerivationActorInbound branch from 9127b94 to 7980116 Compare January 13, 2026 05:17
@op-will op-will mentioned this pull request Jan 13, 2026
Closed
@op-will
Copy link
Collaborator Author

op-will commented Jan 13, 2026 •
edited
Loading

Overall looks really good. A few nits here and there. Please make sure to run all the CI pipeline (including action-tests and acceptance tests that are skipped in CI) before merging that PR. This is a pretty big refactor so there may be some areas that may have been broken

Confirming that the following tests have passed:

  • just test-e2e-sysgo node node/common large-kona-sequencer
  • just test-e2e-sysgo node node/reorgs large-kona-sequencer
  • just test-e2e-sysgo node node/restart large-kona-sequencer
  • just acceptance-tests kona-node op-reth

cc: @theochap

@op-will op-will enabled auto-merge January 13, 2026 05:27
@op-will op-will disabled auto-merge January 13, 2026 05:30
@op-will op-will enabled auto-merge January 13, 2026 05:36
@op-will op-will disabled auto-merge January 13, 2026 05:36
@op-will op-will added this pull request to the merge queue Jan 13, 2026
Merged via the queue into main with commit 5a4457f Jan 13, 2026
27 of 32 checks passed
@op-will op-will deleted the op-will/consolidateDerivationActorInbound branch January 13, 2026 06:06
@theochap theochap mentioned this pull request Jan 14, 2026
Merged
github-merge-queue bot pushed a commit that referenced this pull request Jan 14, 2026
@pcw109550
feat(node): Kona Light CL (#3245)
6a3e71c 
~Base branch: #3242

~May conflict with #3229,
#3253

Implement the kona light CL([Design
doc](https://github.com/ethereum-optimism/design-docs/blob/main/protocol/kona-node-light-cl.md)):
- [DerivationActor - Target
Determination](https://github.com/ethereum-optimism/design-docs/blob/main/protocol/kona-node-light-cl.md#derivationactor---target-determination)
- [EngineActor - Fork Choice
Update](https://github.com/ethereum-optimism/design-docs/blob/main/protocol/kona-node-light-cl.md#engineactor---fork-choice-update)

```mermaid
flowchart TB

subgraph A ["Normal Mode (Derivation)"]
  direction TB

  subgraph A0 ["Rollup Node Service"]
    direction TB
    A_Derivation["DerivationActor<br/>(L1->L2 derivation)"]
    A_Engine["EngineActor"]
    A_UnsafeSrc["Unsafe Source<br/>(P2P gossip / Sequencer)"]
  end

  A_L1[(L1 RPC)]
  A_EL[(Execution Layer)]

  A_L1 -->|L1 info| A_Derivation
  A_UnsafeSrc -->|unsafe| A_Engine
  A_Derivation -->|"safe(attr)/finalized"| A_Engine
  A_Engine -->|engine API| A_EL
end

subgraph B ["Light CL Mode"]
  direction TB

  subgraph B0 ["Rollup Node Service"]
    direction TB
    B_DerivationX[["DerivationActor<br/>(NEW: Poll external syncStatus)"]]
    B_Engine["EngineActor"]
    B_UnsafeSrc["Unsafe Source<br/>(P2P gossip / Sequencer)"]
  end

  B_L1[(L1 RPC)]
  B_Ext[(External CL RPC<br/>optimism_syncStatus)]
  B_EL[(Execution Layer)]

  %% Connections
  B_Ext -->|safe/finalized/currentL1| B_DerivationX
  B_L1 -->|canonical L1 check| B_DerivationX
  B_DerivationX -->|"safe(blockInfo)/finalized (validated)"| B_Engine
  B_UnsafeSrc -->|unsafe| B_Engine

  %% Visual indicator for disabled actor
  B_Engine -->|engine API| B_EL
end
```

### Testing

#### Acceptance Tests

Running guidelines detailed at #3199:

- [x] `TestFollowL2_Safe_Finalized_CurrentL1`
- [x] `TestFollowL2_WithoutCLP2P`
- [ ] `TestFollowL2_ReorgRecovery` (blocked by [kona: Check L2 reorg due
to L1
reorg](ethereum-optimism/optimism#18676))

Injecting CurrentL1 is blocked by [kona: Revise SyncStatus CurrentL1
Selection](ethereum-optimism/optimism#18673)

#### Local Sync Tests

Validated with syncing op-sepolia between kona-node light CL <> sync
tester, successfully finishing the initial EL sync and progress every
safety levels reaching each tip.

#### Devnet Tests

Commit
0b36fdd
is baked to
`us-docker.pkg.dev/oplabs-tools-artifacts/dev-images/kona-node:0b36fdd-light-cl`
and deployed at `changwan-0` devnet:
- As a verifier: `changwan-0-kona-geth-f-rpc-3`
[[grafana]](https://optimistic.grafana.net/d/nUSlc3d4k/bedrock-networks?orgId=1&refresh=30s&from=now-1h&to=now&timezone=browser&var-network=changwan-0&var-node=$__all&var-layer=$__all&var-safety=l2_finalized&var-cluster=$__all&var-konaNodes=changwan-0-kona-geth-f-rpc-3)
- As a sequencer: `changwan-0-kona-geth-f-sequencer-3`
[[grafana]](https://optimistic.grafana.net/d/nUSlc3d4k/bedrock-networks?orgId=1&refresh=30s&from=now-1h&to=now&timezone=browser&var-network=changwan-0&var-node=$__all&var-layer=$__all&var-safety=l2_finalized&var-cluster=$__all&var-konaNodes=changwan-0-kona-geth-f-sequencer-3)
    - As a standby | leader

Noticed all {unsafe, safe, finalized} head progression as a kona node
light CL.
theochap pushed a commit to ethereum-optimism/optimism that referenced this pull request Jan 15, 2026
@op-will
cleanup(node/actor): Consolidate DerivationActor inbound channels (op…
49881c3 
…-rs/kona#3229)

See: #3223 

This also reworks aspects of `EngineActor`. See referenced issues.

closes #3223, #3213, #3215, #3216, #3219, and #3072
theochap pushed a commit to ethereum-optimism/optimism that referenced this pull request Jan 15, 2026
@pcw109550
feat(node): Kona Light CL (op-rs/kona#3245)
088fac6 
~Base branch: op-rs/kona#3242

~May conflict with op-rs/kona#3229,
op-rs/kona#3253

Implement the kona light CL([Design
doc](https://github.com/ethereum-optimism/design-docs/blob/main/protocol/kona-node-light-cl.md)):
- [DerivationActor - Target
Determination](https://github.com/ethereum-optimism/design-docs/blob/main/protocol/kona-node-light-cl.md#derivationactor---target-determination)
- [EngineActor - Fork Choice
Update](https://github.com/ethereum-optimism/design-docs/blob/main/protocol/kona-node-light-cl.md#engineactor---fork-choice-update)

```mermaid
flowchart TB

subgraph A ["Normal Mode (Derivation)"]
  direction TB

  subgraph A0 ["Rollup Node Service"]
    direction TB
    A_Derivation["DerivationActor<br/>(L1->L2 derivation)"]
    A_Engine["EngineActor"]
    A_UnsafeSrc["Unsafe Source<br/>(P2P gossip / Sequencer)"]
  end

  A_L1[(L1 RPC)]
  A_EL[(Execution Layer)]

  A_L1 -->|L1 info| A_Derivation
  A_UnsafeSrc -->|unsafe| A_Engine
  A_Derivation -->|"safe(attr)/finalized"| A_Engine
  A_Engine -->|engine API| A_EL
end

subgraph B ["Light CL Mode"]
  direction TB

  subgraph B0 ["Rollup Node Service"]
    direction TB
    B_DerivationX[["DerivationActor<br/>(NEW: Poll external syncStatus)"]]
    B_Engine["EngineActor"]
    B_UnsafeSrc["Unsafe Source<br/>(P2P gossip / Sequencer)"]
  end

  B_L1[(L1 RPC)]
  B_Ext[(External CL RPC<br/>optimism_syncStatus)]
  B_EL[(Execution Layer)]

  %% Connections
  B_Ext -->|safe/finalized/currentL1| B_DerivationX
  B_L1 -->|canonical L1 check| B_DerivationX
  B_DerivationX -->|"safe(blockInfo)/finalized (validated)"| B_Engine
  B_UnsafeSrc -->|unsafe| B_Engine

  %% Visual indicator for disabled actor
  B_Engine -->|engine API| B_EL
end
```

### Testing

#### Acceptance Tests

Running guidelines detailed at op-rs/kona#3199:

- [x] `TestFollowL2_Safe_Finalized_CurrentL1`
- [x] `TestFollowL2_WithoutCLP2P`
- [ ] `TestFollowL2_ReorgRecovery` (blocked by [kona: Check L2 reorg due
to L1
reorg](#18676))

Injecting CurrentL1 is blocked by [kona: Revise SyncStatus CurrentL1
Selection](#18673)

#### Local Sync Tests

Validated with syncing op-sepolia between kona-node light CL <> sync
tester, successfully finishing the initial EL sync and progress every
safety levels reaching each tip.

#### Devnet Tests

Commit
op-rs/kona@0b36fdd
is baked to
`us-docker.pkg.dev/oplabs-tools-artifacts/dev-images/kona-node:0b36fdd-light-cl`
and deployed at `changwan-0` devnet:
- As a verifier: `changwan-0-kona-geth-f-rpc-3`
[[grafana]](https://optimistic.grafana.net/d/nUSlc3d4k/bedrock-networks?orgId=1&refresh=30s&from=now-1h&to=now&timezone=browser&var-network=changwan-0&var-node=$__all&var-layer=$__all&var-safety=l2_finalized&var-cluster=$__all&var-konaNodes=changwan-0-kona-geth-f-rpc-3)
- As a sequencer: `changwan-0-kona-geth-f-sequencer-3`
[[grafana]](https://optimistic.grafana.net/d/nUSlc3d4k/bedrock-networks?orgId=1&refresh=30s&from=now-1h&to=now&timezone=browser&var-network=changwan-0&var-node=$__all&var-layer=$__all&var-safety=l2_finalized&var-cluster=$__all&var-konaNodes=changwan-0-kona-geth-f-sequencer-3)
    - As a standby | leader

Noticed all {unsafe, safe, finalized} head progression as a kona node
light CL.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@BioMark3r BioMark3r Awaiting requested review from BioMark3r

@emhane emhane Awaiting requested review from emhane

2 more reviewers

@einar-oplabs einar-oplabs einar-oplabs left review comments

@theochap theochap theochap approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

cleanup(node/actor): Consolidate DerivationActor inbound channels into single request channel

3 participants

@op-will @theochap @einar-oplabs