Skip to content

op-supervisor: cleanup, refactor, local-safe info from op-node#12427

Merged
axelKingsley merged 4 commits intodevelopfrom
interop-cleanup
Oct 15, 2024
Merged

op-supervisor: cleanup, refactor, local-safe info from op-node#12427
axelKingsley merged 4 commits intodevelopfrom
interop-cleanup

Conversation

@protolambda
Copy link
Contributor

@protolambda protolambda commented Oct 11, 2024

Description

This is a cleanup PR of the op-supervisor:

  • backend/db/entrydb:
    • separate the event-log-entry specifics from the basic entrydb logic
    • generalize the error types, to be reused across more DBs / functions
  • backend/db/heads: remove. Cross-safety was not properly updated, and we'll need to do more work to sync L1
  • backend/db/logs:
    • remove entry-index from public API.
    • entries.go to host the encoding/decoding logic that was moved out of entrydb
    • expose the timestamp information of block seals
  • backend/db, ChainsDB:
    • split out queries into query.go
    • split out updates into update.go
    • RW-mutex to prevent concurrent chain set changes while querying the DBs.
    • remove db_test.go that was commented / broken.
    • and mock up queries for derived-from / finality against DB interfaces. Map entries are not initialized for now (queries will return unknown chain), but it completes the API flow to query safety data.
  • backend/processors:
    • Move chain_processor.go, log_processor.go, client.go into this. For fetching and processing of unsafe blocks and their events.
  • backend/safety: remove in-memory safety-index draft. It was broken in a few ways, and we need to address the design before putting something into the supervisor.
  • backend/source:
    • see processors package for the things that stay.
    • dropped head_monitor.go, chain.go, head_processor.go: we are getting local-safe data from the op-node, not from geth
  • backend/backend.go:
    • RW-mutex to prevent concurrent map writes during queries etc. (when adding/removing chain processors)
    • Directly maintain a set of chain-processors for the unsafe-block/event updates, no need to wrap around a chain-monitor anymore, as we're getting signals from op-node, not from geth.
  • backend/mock.go: updated the mock to fit latest backend interface. It's returning zeroed values, and should be further updated for what we really need in the e2e / testing related PRs.
  • frontend:
    • Add derived-from and unsafe/safe/finalized queries, to fit the op-node needs for reading data
    • Add error-return values to the update methods, to handle when the supervisor is unable to process the updates from op-node.
  • types:
    • Add BlockSeal: like eth.BlockID but with timestamp. Or like eth.BlockRef but without parent-hash. Having just the seal, without parent-hash, is useful as return type in many places where we don't have the parent-hash handy.
  • service.go: register the update RPC frontend.

Tests

  • Updated event-logs DB tests to handle minor changes in functionality.
  • Removed the commented ChainsDB tests. Work in progress, considering options on what / how to reinstate proper testing there.

Additional context

This takes a few changes from #12213

Metadata

Fix #12358

@protolambda protolambda added this to the Interop: Devnet 1 milestone Oct 11, 2024
@protolambda protolambda requested review from a team as code owners October 11, 2024 15:11
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.

Taking notes as I go:

  • Modifications to E2E test from #12213
  • Corrections to supervisor_client from #12213
  • Addition of a RW lock to explicitly prevent concurrent RW
  • Replacing chainMonitors with chainProcessors from #12213
  • Rename db to chainsDB
  • Insert Lock and defer.Unlock to new functions
  • New backend functions UnsafeView, SafeView, Finalized, Update[...], Derived From.
  • ChainsDB now expanded to map each L2 to a "Derived From Storage"
  • ChainsDB now also has a RW lock
  • entry_db now uses a type-generic T to operate on EntryTypes
  • EntryTypes are uint64s which can yield to string
  • entry.go now houses all the application specific entry types
  • query.go and update.go now houses the ChainsDB functions

Removal of unused:

  • Backend methods (old API)
  • Frontend methods (old API)
  • db/heads

@semgrep-app
Copy link
Contributor

semgrep-app bot commented Oct 15, 2024

Semgrep found 3 sol-style-require-reason findings:

require() must include a reason string

Ignore this finding from sol-style-require-reason.

@axelKingsley axelKingsley disabled auto-merge October 15, 2024 22:41
@axelKingsley axelKingsley added this pull request to the merge queue Oct 15, 2024
Merged via the queue into develop with commit 745b251 Oct 15, 2024
@axelKingsley axelKingsley deleted the interop-cleanup branch October 15, 2024 22:53
samlaf pushed a commit to samlaf/optimism that referenced this pull request Nov 10, 2024
…eum-optimism#12427)

* op-supervisor: cleanup, refactor to take local-safe info from op-node

* Refactor ChainProcessor Worker

* remove unneeded err check

* semgrep

---------

Co-authored-by: axelKingsley <axel.kingsley@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Interop: connect op-supervisor RPC frontend functions to new RPC backend

2 participants