Skip to content
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.

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

Process only increasing block numbers #284

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

ZanCorDX
Copy link
Contributor

📝 Summary

Instead of processing anything that comes from the CL we only process increasing block numbers.

💡 Motivation and Context

I've seen some really crazy sequences from the CL (eg: repeated block numbers, going back) and the builder is not really suited right now to build simultaneous blocks (the good one and some trash).


✅ I have completed the following steps:

  • Run make lint
  • Run make test
  • Added tests (if applicable)

Copy link

Benchmark results for 0327a7e

Report: https://flashbots-rbuilder-ci-stats.s3.us-east-2.amazonaws.com/benchmark/0327a7e-16290b6/report/index.html

Date (UTC) 2024-12-12T21:34:33+00:00
Commit 0327a7ec1db898438a29486cae9bd3b31217fc80
Base SHA 16290b62a7b1a7693b04589ce403013bee09b20b

Significant changes

None

Copy link
Contributor

@dvush dvush left a comment

Choose a reason for hiding this comment

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

I am not sure about this one, in particular rejecting "the same block number".

IIRC CL sends 2 events per slot so it can be the case that the second one is actually "correct".

Also, is this place in the code is actually a problem? The way it works is that it just sends all events to the builder and then builder only builds on the events that have parent hash that reth hash synced. So it kind of sorted out already, even thought not directly.

@ZanCorDX
Copy link
Contributor Author

I am not sure about this one, in particular rejecting "the same block number".

IIRC CL sends 2 events per slot so it can be the case that the second one is actually "correct".

Also, is this place in the code is actually a problem? The way it works is that it just sends all events to the builder and then builder only builds on the events that have parent hash that reth hash synced. So it kind of sorted out already, even thought not directly.

I think we can have any parent (it's just a historic block) but the real failure occurs on the root hash which, from what I undestand, can only be computed on the head. But we don't do root hash until we bid and that's too close to the slot start.
That added to the fact that I think the simulations are not really working for multiple parallel block building might hurt the good block.
Also, I was aiming to remove noisy logs.
That said, all of this comes from looking logs (it's not that easy to reproduce) so if you feel this is too risky I can leave it out.

@dvush
Copy link
Contributor

dvush commented Dec 13, 2024

I think its too risky but without good argument, just on the feel level.

Also, we do wait for the parent header to be correct before even starting the block building. Maybe we can make that check harder to pass for events that we can't even build on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants