Skip to content

supervisor: L1 Processor#13206

Merged
protolambda merged 5 commits intodevelopfrom
supervisor-l1-processor
Dec 9, 2024
Merged

supervisor: L1 Processor#13206
protolambda merged 5 commits intodevelopfrom
supervisor-l1-processor

Conversation

@axelKingsley
Copy link
Copy Markdown
Contributor

@axelKingsley axelKingsley commented Dec 3, 2024

What

Introduces L1 Processor for Supervisor

Why

We would like for the Supervisor to control the canonical L1 source for "Owned Nodes" in order to have better consistency and control over the data in the system.

How

Introduces the following:

  • New Required Flag and Config L1RPC
  • Integration of L1RPC throughout, including in supersystem
  • New Component L1Processor
  • L1 Processor fully integrated to the SupervisorBackend to Start and Stop with the rest of the system
  • New ChainsDB functionality to support the L1 Processor

L1 Processor

The L1 processor spawns a worker routine which uses an L1 RPC Client to periodically check for new L1 blocks. If a new block is discovered, the ChainsDB is asked to Record it

New ChainsDB Functions

LastCommonL1

For each chain being tracked, the latest derivation data is checked, and the lowest L1 block height amongst them is returned. This is used to initialize the processor, so that it can start up at the first L1 block which isn't fully recorded. If an error is encountered, the processor defaults to starting at 0. This works well when the database is uninitialized, as an ErrFuture. The L1 blocks should never diverge once the Supervisor is in charge of the L1 data, so only tracking heights is sufficient.

RecordNewL1

RecordNewL1 fetches the lastest derived for each chain, and extends each database with NewL1:ExistingL2. This happens so that the database records the new L1 row before any new L2 rows appear, making the FromDA database only ever increment one element at a time.

Testing

  • Basic Unit Tests for L1 Processor
  • Ran the Interop Block Building E2E test with an additional 30s of waiting and observed correct operation in the logs:
DEBUG[12-03|17:09:40.756] Checking for new L1 block                role=supervisor service=l1-processor current=0
WARN [12-03|17:09:40.757] Failed to process L1                     role=supervisor service=l1-processor err="failed to fetch header by num 1: not found"
DEBUG[12-03|17:09:46.756] Checking for new L1 block                role=supervisor service=l1-processor current=0
DEBUG[12-03|17:09:46.758] Processing new L1 block                  role=supervisor service=l1-processor block=73c127..379a9e:1
DEBUG[12-03|17:09:52.756] Checking for new L1 block                role=supervisor service=l1-processor current=1
DEBUG[12-03|17:09:52.757] Processing new L1 block                  role=supervisor service=l1-processor block=6edeb2..f8d0fb:2
DEBUG[12-03|17:09:58.757] Checking for new L1 block                role=supervisor service=l1-processor current=2
DEBUG[12-03|17:09:58.758] Processing new L1 block                  role=supervisor service=l1-processor block=af47a7..35d078:3
DEBUG[12-03|17:10:04.756] Checking for new L1 block                role=supervisor service=l1-processor current=3
DEBUG[12-03|17:10:04.757] Processing new L1 block                  role=supervisor service=l1-processor block=eaeae3..48e4fa:4
DEBUG[12-03|17:10:10.756] Checking for new L1 block                role=supervisor service=l1-processor current=4
DEBUG[12-03|17:10:10.758] Processing new L1 block                  role=supervisor service=l1-processor block=758d60..24dbc2:5
DEBUG[12-03|17:10:16.756] Checking for new L1 block                role=supervisor service=l1-processor current=5
DEBUG[12-03|17:10:16.757] Processing new L1 block                  role=supervisor service=l1-processor block=db8d92..455792:6
DEBUG[12-03|17:10:22.756] Checking for new L1 block                role=supervisor service=l1-processor current=6
DEBUG[12-03|17:10:22.756] Processing new L1 block                  role=supervisor service=l1-processor block=66cdf3..58e9db:7

Up Next

Still to come is using the detected L1 block to drive derivation of the Owned Nodes. For that we need a pathway to call down to those Owned Nodes. I may start building a PR to put OP Nodes into that mode of operation to accept the calls, but I am out tomorrow.

We can also add Finalization updating in a future PR, the existing worker can manage the last known finalization and feed it into the database when new values are noticed. I'd like to keep the scope of this initial PR tight so we can start integrating pieces (like Bidirectional RPC)

@axelKingsley axelKingsley force-pushed the supervisor-l1-processor branch from a9458ac to 7947777 Compare December 4, 2024 00:27
@axelKingsley axelKingsley marked this pull request as ready for review December 4, 2024 00:32
@axelKingsley axelKingsley requested review from a team as code owners December 4, 2024 00:32
@codecov
Copy link
Copy Markdown

codecov bot commented Dec 4, 2024

Codecov Report

Attention: Patch coverage is 33.33333% with 104 lines in your changes missing coverage. Please review.

Project coverage is 42.65%. Comparing base (010c8a9) to head (3809c6c).
Report is 81 commits behind head on develop.

Files with missing lines Patch % Lines
...isor/supervisor/backend/processors/l1_processor.go 54.68% 24 Missing and 5 partials ⚠️
op-supervisor/supervisor/backend/db/update.go 0.00% 28 Missing ⚠️
op-supervisor/supervisor/backend/backend.go 36.58% 24 Missing and 2 partials ⚠️
op-supervisor/supervisor/backend/db/query.go 0.00% 20 Missing ⚠️
op-supervisor/flags/flags.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #13206      +/-   ##
===========================================
- Coverage    44.38%   42.65%   -1.73%     
===========================================
  Files          807      752      -55     
  Lines        72694    68183    -4511     
===========================================
- Hits         32265    29084    -3181     
+ Misses       37805    36652    -1153     
+ Partials      2624     2447     -177     
Flag Coverage Δ
cannon-go-tests-32 ?
cannon-go-tests-64 ?

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

Files with missing lines Coverage Δ
op-supervisor/config/config.go 100.00% <100.00%> (ø)
op-supervisor/flags/flags.go 34.48% <0.00%> (-1.24%) ⬇️
op-supervisor/supervisor/backend/db/query.go 0.00% <0.00%> (ø)
op-supervisor/supervisor/backend/backend.go 35.79% <36.58%> (-0.11%) ⬇️
op-supervisor/supervisor/backend/db/update.go 0.00% <0.00%> (ø)
...isor/supervisor/backend/processors/l1_processor.go 54.68% <54.68%> (ø)

... and 66 files with indirect coverage changes

@axelKingsley axelKingsley force-pushed the supervisor-l1-processor branch from 7947777 to b978583 Compare December 4, 2024 00:43
@axelKingsley axelKingsley requested review from a user and protolambda December 4, 2024 00:46
@axelKingsley axelKingsley force-pushed the supervisor-l1-processor branch from b978583 to 0ec4830 Compare December 4, 2024 02:26
Copy link
Copy Markdown
Contributor

@protolambda protolambda left a comment

Choose a reason for hiding this comment

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

Generally LGTM, but a little worried that LastCommonL1 and RecordNewL1 could fall out of sync. But there are consistency checks, so it should still be safe.

@axelKingsley
Copy link
Copy Markdown
Contributor Author

Discussed the potential for de-sync @protolambda had mentioned offline -- while there is a slight lock gap which would allow this,

  • There are no other entities who would change this value (it's during initialization)
  • There are consistency checks in place in the database which would guard if this did somehow happen
  • If the database advanced, the L1 Processor would avoid writing anyway, as it already defends against uneven block heights across chains
  • The alternative would mean the database-write lock would have to be held during the L1 lookup, which is not what we would want

So even though the gap does indeed exist, we think it is safe and acceptable.

@protolambda protolambda added this pull request to the merge queue Dec 9, 2024
Merged via the queue into develop with commit b613161 Dec 9, 2024
@protolambda protolambda deleted the supervisor-l1-processor branch December 9, 2024 15:26
sigma pushed a commit that referenced this pull request Dec 19, 2024
* supervisor: L1 Processor

* comments ; test fixes

* Make L1 source separate from RPC Addr

* fix test

* Add atomic bool for singleton processor routine
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.

2 participants