fix: Separate flashblocks payloads handling and full built payloads in handler #354
Conversation
sieniven
requested review from
0x416e746f6e,
SozinM,
avalonche and
karim-agha
as code owners
…parate-full-payload-handler
sieniven
force-pushed
the
niven/separate-full-payload-handler
branch
from
3d22728 to
f3dbba2
Compare
…parate-full-payload-handler
|
@SozinM @akundaz @avalonche hey guys, any updates regarding this PR? |
sieniven
force-pushed
the
niven/separate-full-payload-handler
branch
from
3d71fd3 to
24ab125
Compare
|
@avalonche @SozinM hey! any updates wrt this PR? |
| 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" | ||
| ); | ||
| } |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
make sense, let me revert the fixes from the above comments to the original version.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
i see, with the context of the other PR then this makes sense. thanks for clarifying!
This reverts commit 24ab125.
…parate-full-payload-handler
…n handler (flashbots#354) (#82) * Fix payload handler for p2p flashblocks and full built payloads * Update * Fix * Revert default behaviour * Revert "Revert default behaviour" This reverts commit 24ab125.
3 participants
Summary
This PR separates out original fixes from PR #334. The original handler handles both flashblock payloads vs full built payloads with the same transmitter. This PR separates the handling logic on the 2 different triggers:
Note that since the subsequent PR #334 will be based on this fixes, there is no optimization done here and it maintains the same logic - full built payloads are sent after every flashblock execution. The subsequent SR optimization PR will only send the full built payloads (with SR calculated) on async payload resolution.
✅ I have completed the following steps:
make lintmake test