ref impl: driver agent: respond to head events and stay in sync

[protolambda]

Part of #119: staging -> main migration

This:

• EngineDriver is like a binding to a remote engine, tracking to where it's synced.
• Drive is a sync process that can be started/shutdown per remote engine, and responds to head events from L1, and syncs (happy extend-case, or worst-case sync with reorg) the L2 chain by running a driver-step on the right starting-point. There is only a single active driving process at a time per engine.

Depends on #130

Review: any team, systems team preferred.

Spec: Do we want to go-indepth, like back-off strategy and failure-cases of sync in the spec? Or maybe just the "if out of sync, run a driver-step on with L1 block X_n and L2 block parent Y_{n-1}" summary of driver routine behavior? (we already more or less have the latter)

Testing: this is the most time/event/integration heavy of all of the reference implementation. Only a single event hub, but still challenging to unit-test. It works e2e though. Any suggestions/help how to best test this?

[norswap]

Quick question: this is divergent from staging, I assume this is the canonical version?

[norswap]

Nice! I think the architecture that we conceptually have three concurrent loops for checking l1 head, l2 head and taking a sync step (with the sync loop slowing down each time it doesn't move closer to sync) is pretty neat.

Also something that could use a description. I'm thinking we probably need some markdown implementation notes.

[norswap]

Don't we want to verify that the L2 head of the execution engine is either:

• something we're already aware of
• congruent with the referenced L1 block (i.e. valid) (in the case this is a head that was synced by the execution engine that we didn't set ourselves)

[protolambda]

We only need to track what the engine says it has to determine if we can go through the happy sync path. We trust our engine to have L2 blocks with correct L1 information (since we inserted it and set it with forkchoice updates). FindSyncStart can handle any out-of-sync cases.

[norswap]

But if we're making the assumption that we are the one inserting the L2 blocks, what's the purpose of getting the execution engine to tell us about new heads (which we already know about & track).

I guess the point of this is being a stub for later, when we do have L2-level sync, at which point we will need to validate the block, right?

[protolambda]

I guess the point of this is being a stub for later, when we do have L2-level sync, at which point we will need to validate the block, right?

We don't need to validate, state-sync via engine is always towards a block-hash we already trust (i.e. we messaged the engine with a forkchoice update, finalized or head, depending on trust assumptions). And even when the engine has the wrong head, we can just reorg away if it's not canonical.

[protolambda]

Rebased on base branch

[protolambda]

Went back and forth whether or not we need refL1 in the FindSyncStart return params, but we just need a name update to nextRefL1 in the driver.

[protolambda]

Rebased on the updated impl-sync-start PR (rebased that on main)

[trianglesphere]

Testing: this is the most time/event/integration heavy of all of the reference implementation. Only a single event hub, but still challenging to unit-test. It works e2e though. Any suggestions/help how to best test this?

My recommendation for this is to make the Drive loop as small and self contained as possible. With explicit inputs, it's easy too set up a unit test. Unfortunately having timers does make this harder, but not impossible.

[protolambda]

@trianglesphere implemented your suggestion (large refactor, no functional changes, but should pay off in test-ability), can you review?

[trianglesphere]

Nice work on the refactor. I think pulling it apart like this teases out the fundamental structure and makes it a bit easier to understand.

One comment was for more comments, and the other is about what I think's a bug. Feel free to merge this once you think you've got it resolved.

[codecov-commenter]

Codecov Report

Merging #131 (94dba5a) into main (f5d7631) will decrease coverage by 6.29%.
The diff coverage is 8.84%.

@@            Coverage Diff             @@
##             main     #131      +/-   ##
==========================================
- Coverage   40.56%   34.26%   -6.30%     
==========================================
  Files           8       11       +3     
  Lines         604      750     +146     
==========================================
+ Hits          245      257      +12     
- Misses        339      468     +129     
- Partials       20       25       +5     

Impacted Files	Coverage Δ
opnode/l2/driver.go	0.00% <0.00%> (ø)
opnode/l2/driver_loop.go	0.00% <0.00%> (ø)
opnode/l2/driver_state.go	14.47% <14.47%> (ø)
opnode/l2/sync_start.go	68.57% <100.00%> (+0.45%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f5d7631...94dba5a. Read the comment docs.