Skip to content

Conversation

@tomip01
Copy link
Contributor

@tomip01 tomip01 commented Nov 6, 2025

Motivation

The L2 node is not stopping after pressing ctrl+c. This is because the abort of the l2_sequencer does not stop block task such as block producer and l1 committer

Description

  • Add new InMessage to abort l1 committer and block producer tasks
  • Change start_l2 to return handles of the blocking tasks

@tomip01 tomip01 changed the title Fix l2 exit fix(l2): shutdown node with ctrl+c Nov 6, 2025
@github-actions github-actions bot added the L2 Rollup client label Nov 6, 2025
@github-actions
Copy link

github-actions bot commented Nov 6, 2025

Lines of code report

Total lines added: 67
Total lines removed: 0
Total lines changed: 67

Detailed view
+----------------------------------------------+-------+------+
| File                                         | Lines | Diff |
+----------------------------------------------+-------+------+
| ethrex/cmd/ethrex/l2/initializers.rs         | 333   | +27  |
+----------------------------------------------+-------+------+
| ethrex/crates/l2/sequencer/block_producer.rs | 261   | +8   |
+----------------------------------------------+-------+------+
| ethrex/crates/l2/sequencer/l1_committer.rs   | 1075  | +12  |
+----------------------------------------------+-------+------+
| ethrex/crates/l2/sequencer/mod.rs            | 246   | +20  |
+----------------------------------------------+-------+------+

@tomip01 tomip01 marked this pull request as ready for review November 6, 2025 12:32
@tomip01 tomip01 requested a review from a team as a code owner November 6, 2025 12:32
Copilot AI review requested due to automatic review settings November 6, 2025 12:32
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the sequencer startup to return handles for the L1 committer and block producer, along with a driver future, enabling proper shutdown coordination. It adds Abort message handling to both the L1Committer and BlockProducer GenServers and updates the initialization logic to send abort messages during shutdown.

  • Modifies start_l2 to return handles and a driver future instead of running to completion
  • Adds Abort message variants to L1Committer and BlockProducer for graceful shutdown
  • Updates shutdown logic to send abort messages to GenServer handles before cancellation

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
crates/l2/sequencer/mod.rs Changes return type to include GenServer handles and driver future; wraps sequencer logic in pinned async block
crates/l2/sequencer/l1_committer.rs Adds Abort message handling with CastResponse::Stop and refactors handle_cast to use pattern matching
crates/l2/sequencer/block_producer.rs Adds Abort message handling with CastResponse::Stop and refactors handle_cast to use pattern matching
cmd/ethrex/l2/initializers.rs Updates initialization to receive handles and driver, sends abort messages in both shutdown branches

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@tomip01 tomip01 self-assigned this Nov 6, 2025
Comment on lines 307 to 320
if let Some(mut handle) = committer_handle.clone() {
handle
.cast(l1_committer::InMessage::Abort)
.await
.inspect_err(|err| warn!("Failed to send committer abort: {err:?}"))
.ok();
}
if let Some(mut handle) = block_producer_handle.clone() {
handle
.cast(block_producer::InMessage::Abort)
.await
.inspect_err(|err| warn!("Failed to send block producer abort: {err:?}"))
.ok();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we abstract this functionality into a shutdown function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

self.last_committed_batch = current_last_committed_batch;
self.last_committed_batch_timestamp = current_time;
match message {
InMessage::Commit => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can move the handling of this message to its own function for clarity.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done cc0fd39

@ilitteri ilitteri added this pull request to the merge queue Nov 10, 2025
@ilitteri ilitteri removed this pull request from the merge queue due to a manual request Nov 10, 2025
@ilitteri ilitteri enabled auto-merge November 10, 2025 20:39
@ilitteri ilitteri added this pull request to the merge queue Nov 10, 2025
Merged via the queue into main with commit fe53dcb Nov 10, 2025
43 checks passed
@ilitteri ilitteri deleted the fix_l2_exit branch November 10, 2025 23:03
xqft pushed a commit that referenced this pull request Nov 11, 2025
**Motivation**

The L2 node is not stopping after pressing `ctrl+c`. This is because the
abort of the `l2_sequencer` does not stop block task such as block
producer and l1 committer

**Description**

- Add new `InMessage` to abort l1 committer and block producer tasks
- Change `start_l2` to return handles of the blocking tasks

---------

Co-authored-by: Ivan Litteri <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L2 Rollup client

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants