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

feat(sequencer): Origin Selector asynchronously prefetches the next origin from events #12134

Conversation

BrianBland
Copy link
Contributor

Description
Updates the Sequencer's L1 origin selector to locally cache the currentOrigin and nextOrigin attributes and also optimistically prefetch the nextOrigin whenever an ForkchoiceUpdateEvent event is received. This is expected to remove synchronous origin selection from the critical path of sequencer block production, although the impact of this is reduced when using a single-threaded event loop. (Addressed by #11100)

In many cases, the existing L1 origin is "good enough" for building a new block from the sequencer's perspective, and it's better to fetch the next L1 origin in the background instead of spending precious time that could otherwise be used for building the next block. Whenever the nextOrigin is already available and valid, this is returned instead.

Under specific circumstances, such as when a new L1 origin has not been found for a long period of time, the sequencer will actually need to review the nextOrigin and determine if this is valid (behind the next L2 block time). In these cases, the FindL1Origin call will fetch the next possible origin synchronously, and compare these potential origins immediately.

Note: This is a simplified version of #12073, which uses no goroutines and fully embraces the async event model.

Tests

Added several new origin selector tests to ensure that this new prefetching behavior is safe under all conditions.

Metadata

Partially addresses #11960. In order to fully resolve this ticket, we may need to merge the origin selector and attributes builder, so that an origin is only selected if and only if it also has the relevant attributes already built or has all of the data needed to build attributes (L1 receipts).

@BrianBland BrianBland requested review from a team as code owners September 25, 2024 21:07
Copy link

codecov bot commented Sep 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.97%. Comparing base (d141b53) to head (bec9b29).
Report is 36 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop   #12134   +/-   ##
========================================
  Coverage    74.97%   74.97%           
========================================
  Files           49       49           
  Lines         3656     3656           
========================================
  Hits          2741     2741           
  Misses         743      743           
  Partials       172      172           
Flag Coverage Δ
cannon-go-tests 74.97% <ø> (ø)

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

@BrianBland BrianBland force-pushed the origin-selector-async-prefecthing branch from fcd895e to 87f0317 Compare September 27, 2024 00:34
@BrianBland BrianBland force-pushed the origin-selector-async-prefecthing branch from 87f0317 to bec9b29 Compare September 27, 2024 19:46
Copy link
Contributor

@ajsutton ajsutton left a comment

Choose a reason for hiding this comment

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

LGTM. Sorry for the delay in reviewing.

@ajsutton ajsutton added this pull request to the merge queue Oct 2, 2024
Merged via the queue into ethereum-optimism:develop with commit 445a3d4 Oct 2, 2024
60 checks passed
samlaf pushed a commit to samlaf/optimism that referenced this pull request Nov 10, 2024
…rigin from events (ethereum-optimism#12134)

* Sequencer: Origin Selector optimistically prefetches the next origin in background

* L1OriginSelector erases cached state on reset

* L1OriginSelector attempts to fetch on ForkchoiceUpdateEvent

* Move to a fully event-driven model, no extra goroutines

* Add missing test comment

* Minor cleanup, more tests

* Tune the context timeouts
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