Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

op-supervisor,op-node: introduce follow/managed interop mode #13285

Merged
merged 5 commits into from
Dec 10, 2024

Conversation

protolambda
Copy link
Contributor

@protolambda protolambda commented Dec 6, 2024

Description

This PR changes the op-node and op-supervisor to support two ways of interacting with each other:

  • follow mode: op-node follows a read-only supervisor RPC endpoint (ideally we add proofs here later, to make this an untrusted thing)
  • managed mode: op-node is actively managed by an op-supervisor, and serves an RPC server that can handle sync requests and instructions of the op-supervisor.

Changes:

  • implement Websocket endpoint option for RPC server op-service util
  • make obtain-JWT secret util, and de-duplicate usage, so op-node engine-API can use the same
  • change interop CLI flags of op-node to have the options for these two modes
  • change interop CLI flags of op-supervisor to connect to op-nodes
  • change op-supervisor to have 1 kind of L2 node; a "sync source", aka a managed node used for syncing. This is where it will fetch receipts through, and instruct forkchoice / derivation work
  • op-supervisor sync-source constructs:
    • a collection of endpoints, that can share a JWT secret
    • an endpoint setup, that can create a sync-source. With RPC setup as first implementation. More test-specific setups might follow.
    • an RPC based sync-source, to interact over json RPC client
  • op-node generalization of interop: it's now a "sub system", so we can either instantiate it in follow-mode, or managed-mode, and simply attach it to the rest of op-node, while encapsulating the difference.
  • fix op-supervisor Add-RPC functionality, so you can add an op-node interop endpoint as sync-source.
    • addL2RPC now has a JWT secret arg, to connect to an authenticated RPC
  • EthClient provides blockref fetching methods now, to not have to rely on the L1BlockRef methods in a L2 context.
    • L1Client uses these blockref methods
    • MockEthClient supports mocking of these methods

Tests

All existing op-e2e tests pass, using the new RPC and configuration functionality. Some workarounds were required, since the full standard/managed mode has not been implemented yet.

Additional context

Metadata

Fix #13182

Copy link
Contributor

@axelKingsley axelKingsley left a comment

Choose a reason for hiding this comment

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

Still some todos, but thank you for putting this together, it's looking good

op-node/flags/flags.go Outdated Show resolved Hide resolved
op-node/rollup/interop/config.go Outdated Show resolved Hide resolved
op-node/rollup/interop/follow.go Outdated Show resolved Hide resolved
op-service/rpc/jwt.go Show resolved Hide resolved
op-supervisor/supervisor/service.go Show resolved Hide resolved
@protolambda protolambda added this to the Interop: Stable Devnet milestone Dec 9, 2024
Copy link

codecov bot commented Dec 9, 2024

Codecov Report

Attention: Patch coverage is 14.62830% with 356 lines in your changes missing coverage. Please review.

Project coverage is 42.82%. Comparing base (1866540) to head (8e9e487).
Report is 5 commits behind head on develop.

Files with missing lines Patch % Lines
op-node/rollup/interop/managed.go 0.00% 62 Missing ⚠️
op-node/rollup/interop/config.go 0.00% 44 Missing ⚠️
op-service/sources/eth_client.go 3.12% 31 Missing ⚠️
op-node/rollup/interop/tmpapi.go 0.00% 30 Missing ⚠️
op-service/rpc/jwt.go 0.00% 27 Missing ⚠️
...upervisor/supervisor/backend/syncsrc/collection.go 30.76% 26 Missing and 1 partial ⚠️
op-supervisor/supervisor/backend/backend.go 11.11% 22 Missing and 2 partials ⚠️
op-node/service.go 0.00% 16 Missing ⚠️
op-node/node/node.go 0.00% 15 Missing ⚠️
op-service/testutils/mock_eth_client.go 0.00% 15 Missing ⚠️
... and 12 more
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #13285      +/-   ##
===========================================
- Coverage    47.39%   42.82%   -4.57%     
===========================================
  Files          928      767     -161     
  Lines        78145    68662    -9483     
  Branches       849        0     -849     
===========================================
- Hits         37036    29406    -7630     
+ Misses       38392    36756    -1636     
+ Partials      2717     2500     -217     
Flag Coverage Δ
cannon-go-tests-32 ?
cannon-go-tests-64 ?
contracts-bedrock-tests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
op-node/flags/flags.go 83.33% <ø> (ø)
op-node/node/client.go 17.58% <ø> (+1.89%) ⬆️
op-supervisor/config/config.go 100.00% <100.00%> (ø)
op-supervisor/supervisor/frontend/frontend.go 100.00% <100.00%> (ø)
op-supervisor/supervisor/service.go 54.86% <ø> (+2.88%) ⬆️
op-node/rollup/interop/interop.go 55.97% <0.00%> (ø)
op-service/sources/l2_client.go 0.00% <0.00%> (ø)
op-supervisor/supervisor/backend/mock.go 0.00% <0.00%> (ø)
op-service/sources/supervisor_client.go 0.00% <0.00%> (ø)
...r/supervisor/backend/processors/chain_processor.go 0.00% <0.00%> (ø)
... and 17 more

... and 177 files with indirect coverage changes

@protolambda protolambda marked this pull request as ready for review December 10, 2024 20:31
@protolambda protolambda requested review from a team as code owners December 10, 2024 20:31
@protolambda protolambda requested review from axelKingsley and ajsutton and removed request for ajsutton December 10, 2024 20:31
Copy link
Contributor

@axelKingsley axelKingsley left a comment

Choose a reason for hiding this comment

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

  • Had reviewed this last week synchronously
  • I am building new code off of this branch so I can confirm it works and I am motivated to merge it
  • I did not look too closely at the Temporary server, as that exists almost entirely to provide a smooth experience in testing as we build out features. So long as it's functional (which by test results it is) I am fine with it.

LGTM, thanks!

@axelKingsley axelKingsley added this pull request to the merge queue Dec 10, 2024
Merged via the queue into develop with commit 6cfe76f Dec 10, 2024
42 checks passed
@axelKingsley axelKingsley deleted the interop-rpc branch December 10, 2024 21:22
sigma pushed a commit that referenced this pull request Dec 19, 2024
* op-supervisor,op-node: introduce follow/managed interop mode v2

* interop: test workaround, serve supervisor while using old deriver

* interop: fix API setup/usage, fix action test setup

* op-node: fix interop server endpoint getter

* interop: minor fixes, update TODO references
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

interop: Bidirectional RPC For Owned Nodes
2 participants