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

op-node: handle crit error events, lift event-system out of driver #11932

Merged
merged 1 commit into from
Sep 16, 2024

Conversation

protolambda
Copy link
Contributor

Description

The OnEvent handler of Driver wasn't properly hooked up, causing it to miss CriticalErrorEvent.

Additionally, I disliked how the critical-error still allowed continued event processing, while being queued up. I changed the event-system to enshrine the idea of the CriticalErrorEvent, such that no further events will be processed, except the critical error event itself.

The event-system is now initialized in the OpNode, rather than in the Driver.
This gets us one step closer to removing the Driver event-loop:
once the OpNode emits events for the arrived subscription data (L1 head updates and such), gossip data (L2 unsafe blocks from P2P), and once we make the driver schedule the sequencer actions separately, then we don't really need the monolithic event-loop anymore.

And by hosting the event-system in OpNode, we can potentially also attach an external API to it at some point (for event-streaming).

Tests

Added a test that the event-system handles CriticalErrorEvent.

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.

@protolambda protolambda added this pull request to the merge queue Sep 16, 2024
Merged via the queue into develop with commit 21375b4 Sep 16, 2024
61 checks passed
@protolambda protolambda deleted the fix-crit-event-handling branch September 16, 2024 23:18
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