Skip to content

Conversation

@MegaRedHand
Copy link
Collaborator

Motivation

Our --dev block production had a bug where we waited an interval before sending an FCU and called getPayload immediately after, causing inclusion latency to be 1.5x the interval.

Description

This PR moves the sleep between the FCU and getPayload calls, fixing the issue. It also switches to tokio::time::interval for more precision.

Copilot AI review requested due to automatic review settings November 12, 2025 22:57
@MegaRedHand MegaRedHand requested a review from a team as a code owner November 12, 2025 22:57
@github-actions github-actions bot added the L1 Ethereum client label Nov 12, 2025
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 attempts to fix a timing issue in dev-mode block production by moving the sleep interval from after block production to between the FCU (forkchoiceUpdated) and getPayload calls. The change switches from tokio::time::sleep to tokio::time::interval for more precise timing control.

Key Changes:

  • Added Duration import and created a tokio::time::interval ticker
  • Moved the timing delay from the end of the loop to between FCU and getPayload calls
  • Removed the tokio::time::sleep at the end of the loop

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

@github-actions
Copy link

github-actions bot commented Nov 12, 2025

Lines of code report

Total lines added: 0
Total lines removed: 1
Total lines changed: 1

Detailed view
+------------------------------------------------+-------+------+
| File                                           | Lines | Diff |
+------------------------------------------------+-------+------+
| ethrex/crates/blockchain/dev/block_producer.rs | 116   | -1   |
+------------------------------------------------+-------+------+

@MegaRedHand MegaRedHand moved this to In Review in ethrex_l1 Nov 12, 2025
@SDartayet SDartayet added this pull request to the merge queue Nov 13, 2025
Merged via the queue into main with commit 43e64eb Nov 13, 2025
45 of 48 checks passed
@SDartayet SDartayet deleted the fix-dev-block-production branch November 13, 2025 20:22
@github-project-automation github-project-automation bot moved this from In Review to Done in ethrex_l1 Nov 13, 2025
github-merge-queue bot pushed a commit that referenced this pull request Nov 13, 2025
**Description**

This commit reverts the revert of #5205 done in #5257. 
As of #5310, the dev mode block producer now waits for the configured
amount of time after initiating the payload build process and before
retrieving the built payload, rather than after payload validation. This
solves the issue we were having after merging #5205 where empty payloads
were retrieved because the `engine_getPayload` call was immediately done
after the `engine_forkChoiceUpdate`.
lakshya-sky pushed a commit to lakshya-sky/ethrex that referenced this pull request Nov 17, 2025
…ducer (lambdaclass#5310)

**Motivation**

Our `--dev` block production had a bug where we waited an interval
before sending an FCU and called getPayload immediately after, causing
inclusion latency to be 1.5x the interval.

**Description**

This PR moves the sleep between the FCU and getPayload calls, fixing the
issue. It also switches to `tokio::time::interval` for more precision.
lakshya-sky pushed a commit to lakshya-sky/ethrex that referenced this pull request Nov 17, 2025
**Description**

This commit reverts the revert of lambdaclass#5205 done in lambdaclass#5257. 
As of lambdaclass#5310, the dev mode block producer now waits for the configured
amount of time after initiating the payload build process and before
retrieving the built payload, rather than after payload validation. This
solves the issue we were having after merging lambdaclass#5205 where empty payloads
were retrieved because the `engine_getPayload` call was immediately done
after the `engine_forkChoiceUpdate`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L1 Ethereum client

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants