Skip to content

fix: Separate flashblocks payloads handling and full built payloads in handler #354

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.

Sign up for GitHub

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

Merged
avalonche merged 8 commits into from
Feb 9, 2026
Merged

fix: Separate flashblocks payloads handling and full built payloads in handler #354

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b735266
Fix payload handler for p2p flashblocks and full built payloads
sieniven Jan 15, 2026
38204c5
Update
sieniven Jan 15, 2026
4b58264
Fix
sieniven Jan 16, 2026
f3dbba2
Merge branch 'main' of github.com:flashbots/op-rbuilder into niven/se…
sieniven Jan 19, 2026
91673ef
Merge branch 'main' of github.com:flashbots/op-rbuilder into niven/se…
sieniven Jan 26, 2026
24ab125
Revert default behaviour
sieniven Jan 28, 2026
fa0fb5b
Revert "Revert default behaviour"
sieniven Feb 4, 2026
dca7b92
Merge branch 'main' of github.com:flashbots/op-rbuilder into niven/se…
sieniven Feb 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 26 additions & 7 deletions crates/op-rbuilder/src/builders/flashblocks/payload.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,12 @@ pub(super) struct OpPayloadBuilder<Pool, Client, BuilderTx> {
pub pool: Pool,
/// Node client
pub client: Client,
/// Sender for sending built payloads to [`PayloadHandler`],
/// which broadcasts outgoing payloads via p2p.
pub payload_tx: mpsc::Sender<OpBuiltPayload>,
/// Sender for sending built flashblock payloads to [`PayloadHandler`],
/// which broadcasts outgoing flashblock payloads via p2p.
pub built_fb_payload_tx: mpsc::Sender<OpBuiltPayload>,
/// Sender for sending built full block payloads to [`PayloadHandler`],
/// which updates the engine tree state.
pub built_payload_tx: mpsc::Sender<OpBuiltPayload>,
/// WebSocket publisher for broadcasting flashblocks
/// to all connected subscribers.
pub ws_pub: Arc<WebSocketPublisher>,
Expand All @@ -203,7 +206,8 @@ impl<Pool, Client, BuilderTx> OpPayloadBuilder<Pool, Client, BuilderTx> {
client: Client,
config: BuilderConfig<FlashblocksConfig>,
builder_tx: BuilderTx,
payload_tx: mpsc::Sender<OpBuiltPayload>,
built_fb_payload_tx: mpsc::Sender<OpBuiltPayload>,
built_payload_tx: mpsc::Sender<OpBuiltPayload>,
ws_pub: Arc<WebSocketPublisher>,
metrics: Arc<OpRBuilderMetrics>,
task_metrics: Arc<FlashblocksTaskMetrics>,
Expand All @@ -213,7 +217,8 @@ impl<Pool, Client, BuilderTx> OpPayloadBuilder<Pool, Client, BuilderTx> {
evm_config,
pool,
client,
payload_tx,
built_fb_payload_tx,
built_payload_tx,
ws_pub,
config,
metrics,
Expand Down Expand Up @@ -408,9 +413,16 @@ where
!disable_state_root || ctx.attributes().no_tx_pool, // need to calculate state root for CL sync
)?;

self.payload_tx
self.built_fb_payload_tx
.try_send(payload.clone())
.map_err(PayloadBuilderError::other)?;
if let Err(e) = self.built_payload_tx.try_send(payload.clone()) {
warn!(
target: "payload_builder",
error = %e,
"Failed to send updated payload"
);
}
Comment on lines +416 to +425
Copy link
Contributor

@noot noot Feb 3, 2026

Choose a reason for hiding this comment

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

i'm a bit confused on what the point of the two separate channels is, in both spots where we send on the channels we send the exact same thing on both channels. then inside the payload handler, it's going to call the same code on the payload twice, since it receives the same thing on both branches. am i missing something here? did you mean to only send on built_payload_tx for the final flashblock?

Copy link
Contributor Author

@sieniven sieniven Feb 3, 2026

Choose a reason for hiding this comment

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

make sense, let me revert the fixes from the above comments to the original version.

Copy link
Contributor Author

@sieniven sieniven Feb 4, 2026

Choose a reason for hiding this comment

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

@noot resolved in fa0fb5b

Copy link
Contributor

@noot noot Feb 5, 2026

Choose a reason for hiding this comment

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

thanks! i'm still not sure i understand the change, the logic is the same as previously, as the same data is being sent on both channels?

Copy link
Contributor Author

@sieniven sieniven Feb 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

@noot this PR was meant to be a refactor and was originally inside the async SR calculation PR here which resolves the logic issue on the flashblocks builder when no SR calculation flag is set. From the comments by @SozinM here - #334 (comment), this PR separates it out from that fixes for better clarity.

The reason for the separation is because there is no reason to commit execution payloads with missing state roots since it will result in 100% cache misses on the subsequent payload commits on engine_newPayload. This will cause the locally built payloads to still go through payload re-validation regardless, which takes up precious sequencing time.

Copy link
Contributor

@noot noot Feb 6, 2026

Choose a reason for hiding this comment

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

i see, with the context of the other PR then this makes sense. thanks for clarifying!

best_payload.set(payload);

info!(
Expand Down Expand Up @@ -832,9 +844,16 @@ where
.ws_pub
.publish(&fb_payload)
.wrap_err("failed to publish flashblock via websocket")?;
self.payload_tx
self.built_fb_payload_tx
.try_send(new_payload.clone())
.wrap_err("failed to send built payload to handler")?;
if let Err(e) = self.built_payload_tx.try_send(new_payload.clone()) {
warn!(
target: "payload_builder",
error = %e,
"Failed to send updated payload"
);
}
best_payload.set(new_payload);

// Record flashblock build duration
Expand Down
24 changes: 16 additions & 8 deletions crates/op-rbuilder/src/builders/flashblocks/payload_handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,10 @@ use tracing::warn;
/// In the case of a payload built by this node, it is broadcast to peers and an event is sent to the payload builder.
/// In the case of a payload received from a peer, it is executed and if successful, an event is sent to the payload builder.
pub(crate) struct PayloadHandler<Client, Tasks> {
// receives new payloads built by this builder.
built_rx: mpsc::Receiver<OpBuiltPayload>,
// receives new flashblock payloads built by this builder.
built_fb_payload_rx: mpsc::Receiver<OpBuiltPayload>,
// receives new full block payloads built by this builder.
built_payload_rx: mpsc::Receiver<OpBuiltPayload>,
// receives incoming p2p messages from peers.
p2p_rx: mpsc::Receiver<Message>,
// outgoing p2p channel to broadcast new payloads to peers.
Expand All @@ -58,7 +60,8 @@ where
{
#[allow(clippy::too_many_arguments)]
pub(crate) fn new(
built_rx: mpsc::Receiver<OpBuiltPayload>,
built_fb_payload_rx: mpsc::Receiver<OpBuiltPayload>,
built_payload_rx: mpsc::Receiver<OpBuiltPayload>,
p2p_rx: mpsc::Receiver<Message>,
p2p_tx: mpsc::Sender<Message>,
payload_events_handle: tokio::sync::broadcast::Sender<Events<OpEngineTypes>>,
Expand All @@ -68,7 +71,8 @@ where
cancel: tokio_util::sync::CancellationToken,
) -> Self {
Self {
built_rx,
built_fb_payload_rx,
built_payload_rx,
p2p_rx,
p2p_tx,
payload_events_handle,
Expand All @@ -81,7 +85,8 @@ where

pub(crate) async fn run(self) {
let Self {
mut built_rx,
mut built_fb_payload_rx,
mut built_payload_rx,
mut p2p_rx,
p2p_tx,
payload_events_handle,
Expand All @@ -95,12 +100,15 @@ where

loop {
tokio::select! {
Some(payload) = built_rx.recv() => {
Some(payload) = built_fb_payload_rx.recv() => {
// ignore error here; if p2p was disabled, the channel will be closed.
let _ = p2p_tx.send(payload.into()).await;
}
Some(payload) = built_payload_rx.recv() => {
// Update engine tree state with locally built block payloads
if let Err(e) = payload_events_handle.send(Events::BuiltPayload(payload.clone())) {
warn!(target: "payload_builder", e = ?e, "failed to send BuiltPayload event");
}
// ignore error here; if p2p was disabled, the channel will be closed.
let _ = p2p_tx.send(payload.into()).await;
}
Some(message) = p2p_rx.recv() => {
match message {
Expand Down
5 changes: 5 additions & 0 deletions crates/op-rbuilder/src/builders/flashblocks/service.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ impl FlashblocksServiceBuilder {

let metrics = Arc::new(OpRBuilderMetrics::default());
let task_metrics = Arc::new(FlashblocksTaskMetrics::new());

// Channels for built flashblock payloads
let (built_fb_payload_tx, built_fb_payload_rx) = tokio::sync::mpsc::channel(16);
let (built_payload_tx, built_payload_rx) = tokio::sync::mpsc::channel(16);

let ws_pub: Arc<WebSocketPublisher> = WebSocketPublisher::new(
Expand All @@ -124,6 +127,7 @@ impl FlashblocksServiceBuilder {
ctx.provider().clone(),
self.0.clone(),
builder_tx,
built_fb_payload_tx,
built_payload_tx,
ws_pub.clone(),
metrics.clone(),
Expand Down Expand Up @@ -152,6 +156,7 @@ impl FlashblocksServiceBuilder {
.wrap_err("failed to create flashblocks payload builder context")?;

let payload_handler = PayloadHandler::new(
built_fb_payload_rx,
built_payload_rx,
incoming_message_rx,
outgoing_message_tx,
Expand Down
Loading