Skip to content

Conversation

avilagaston9
Copy link
Contributor

Motivation

The join() of the verifier task was accidentally removed in #3635.

Description

This PR is a quick fix that restores the removed join(). The verifier task is being replaced by spawned in #3761.

Closes None

@github-actions github-actions bot added the L2 Rollup client label Jul 22, 2025
@avilagaston9 avilagaston9 marked this pull request as ready for review July 22, 2025 19:21
@Copilot Copilot AI review requested due to automatic review settings July 22, 2025 19:21
@avilagaston9 avilagaston9 requested a review from a team as a code owner July 22, 2025 19:21
@avilagaston9 avilagaston9 moved this to In Review in ethrex_l2 Jul 22, 2025
Copy link
Contributor

@Copilot 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 restores a missing join() operation for the verifier task that was accidentally removed in a previous change (#3635). The fix ensures proper task lifecycle management in the L2 sequencer by waiting for spawned tasks to complete and handling their results appropriately.

  • Adds back the task_set.join_next().await call to wait for task completion
  • Implements proper error handling for task results with logging for both task errors and JoinSet errors
  • Adds a fallback loop to keep the sequencer running when no tasks are spawned

Copy link

github-actions bot commented Jul 22, 2025

Lines of code report

Total lines added: 11
Total lines removed: 0
Total lines changed: 11

Detailed view
+-----------------------------------+-------+------+
| File                              | Lines | Diff |
+-----------------------------------+-------+------+
| ethrex/crates/l2/sequencer/mod.rs | 174   | +11  |
+-----------------------------------+-------+------+

Comment on lines 177 to 195
if let Some(res) = task_set.join_next().await {
// If a task finishes, the whole sequencer should stop
match res {
Ok(Ok(_)) => {}
Ok(Err(err)) => {
error!("Error starting Sequencer: {err}");
}
Err(err) => {
error!("JoinSet error: {err}");
}
};
task_set.abort_all();
} else {
// If no tasks were spawned, we let the sequencer run until it is cancelled
loop {
tokio::time::sleep(tokio::time::Duration::from_secs(1)).await;
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

We probably can omit the else, it is not needed anymore, also since there is only one task, the abort all does nothing

Copy link
Contributor

Choose a reason for hiding this comment

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

If we are going to make this change, then we could just spawn it directly instead of using the join set

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 cff4abd!

@@ -174,5 +173,17 @@ pub async fn start_l2(
.await?;
}

if let Some(handle) = verifier_handle {
Copy link
Contributor

Choose a reason for hiding this comment

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

Use let else to remove one indentation.

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 61b429e!

@avilagaston9 avilagaston9 added this pull request to the merge queue Jul 23, 2025
Merged via the queue into main with commit 212d72a Jul 23, 2025
23 checks passed
@avilagaston9 avilagaston9 deleted the fix/l2/await_verifier branch July 23, 2025 21:54
@github-project-automation github-project-automation bot moved this from In Review to Done in ethrex_l2 Jul 23, 2025
pedrobergamini pushed a commit to pedrobergamini/ethrex that referenced this pull request Aug 24, 2025
**Motivation**

The `join()` of the verifier task was accidentally removed in lambdaclass#3635.

**Description**

This PR is a quick fix that restores the removed `join()`. The verifier
task is being replaced by spawned in lambdaclass#3761.

Closes None
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