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

Interop: implement ManagedMode in op-node #13336

Closed
protolambda opened this issue Dec 10, 2024 · 10 comments · Fixed by #13406
Closed

Interop: implement ManagedMode in op-node #13336

protolambda opened this issue Dec 10, 2024 · 10 comments · Fixed by #13406
Assignees
Labels
A-op-node Area: op-node

Comments

@protolambda
Copy link
Contributor

Managed mode allows the op-supervisor to use the op-node to sync new blocks, while having the driver control over which L2 block and from what L1 data the node will derive.

op-node/rollup/interop/managed.go:

  • start/stop RPC server, with websocket-enabled, and JWT enabled.
  • implement missing API methods of ManagedMode
  • make it attach to the rest of the op-node as a deriver/emitter
  • implement API methods:
    • SubscribeUnsafeBlocks
    • UpdateCrossUnsafe
    • UpdateCrossSafe
    • UpdateFinalized
    • AnchorPoint
    • Reset
    • TryDeriveNext
    • FetchReceipts
    • BlockRefByNumber
    • ChainID
@axelKingsley
Copy link
Contributor

This PR takes the signal to derive from a L1 all the way from the Supervisor to the Sync Nodes:

#13344

From this PR it should be possible to build the derivation path on the node.

@protolambda protolambda moved this to In progress in Interoperability Dec 12, 2024
@axelKingsley
Copy link
Contributor

L1 Traversal Managed Mode WIP:
#13392

@protolambda
Copy link
Contributor Author

TODOs

op-supervisor:

  • Merge interop-managed-sync
  • Error handling when local-safe DB update
    • Includes sending the right reset to node
  • Exhaust-L1 updates, pull from L1 accessor, reply to sync node
    • backend needs to expose L1BlockRefByNumber so that sync-controller can use it through the backend interface.
  • Fix UpdateFinalizedL1 deadlock: db.finalizedL1 is locked while trying to read it in NotifyL2Finalized

op-node:

  • L1Traversal AdvanceL1Block to use the data from supervisor
  • op-node ManagedMode unit tests

Low-hanging fruit:

  • docs on new types
    • DerivedPair
    • L1Accessor
  • AddSubscriptions overwriting subscriptions seems bad, can we fix?
  • fix import lint in op-node/node
  • check sufficient channel capacity for each subscription, prevent blocking on slow consumers
  • Ensure we stop legacy op-node finalizer on interop fork
  • Ensure we remove update API methods from supervisor frontend

Later polish:

  • L1Accessor should not use PollBlockChanges, but use eth-subscribe, with the polling as fallback, like op-node does.
  • L1Accessor needs to lock on SetTipHeight
  • UpdateFinalizedL1 typo
  • NotifyL2Finalized log if finality subscription of chain not found

@axelKingsley
Copy link
Contributor

Random thing I remembered: We are newly recording oldL2:newL1 in the database when a new L1 is discovered, to enforce the "only one block height increments" invariant. Now that the L1 Processor doesn't exist, this isn't happening. It'll have to happen from the L1 Exhaustion signal now.

@protolambda
Copy link
Contributor Author

Axel:

  • fix onDerivationUpdate
  • maybe use same helper func to fix onUnsafeBlock
  • deadlock
  • low hanging fruit

Proto:

  • fix onExhaustL1Event
  • fix op-node side of L1 traversal with events

@axelKingsley
Copy link
Contributor

@axelKingsley
Copy link
Contributor

First half of the low hanging fruit list: #13507

@axelKingsley
Copy link
Contributor

Regarding: "Ensure we remove update API methods from supervisor frontend"

The UpdateFrontend is already gone it would appear. ✅

@protolambda
Copy link
Contributor Author

Updated TODOs:
Remaining low-hanging fruit
- AddSubscriptions overwriting subscriptions seems bad, can we fix?

  • Improve op-supervisor testing
    • Controller test doesn’t cover everything yet.
    • Model the depencies of sync controller better
  • op-node ManagedMode unit tests
    • Like other deriver tests, just with mock emitter, assert in/output events
  • Fix action-tests
    • Make the background routines of the sync-controller optional. Disable in action tests.
    • Add some method to drain all buffered events, basically run what the background routine would do, synchronously.
  • Fix reset loop: see todo about tracking the last known block of the ManagedNode, and then reset to one prior, until it succeeds.

@axelKingsley
Copy link
Contributor

  • AddSubscriptions overwriting subscriptions seems bad, can we fix?

I believe this was fixed in 38c0bb5

@github-project-automation github-project-automation bot moved this from In progress to Done in Interoperability Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-op-node Area: op-node
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants