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

Refactor mock builder #6735

Merged
merged 15 commits into from
Jan 21, 2025
Merged

Conversation

pawanjay176
Copy link
Member

@pawanjay176 pawanjay176 commented Dec 19, 2024

Issue Addressed

N/A

Proposed Changes

Adds electra support to the builder flow.
Refactor the mock builder to extract all builder functions into separate functions.
Also adds a prepare_execution_layer method that prepares the execution layer for payload creation for each slot.

The motivation is to provide a backend for a simple builder service that follows the builder api and produces execution payloads from the local mempool of the connected EL. This allows us to have a faster turnaround for testing changes to the builder api in kurtosis testnets and new fork devnets.
See https://github.com/pawanjay176/rustic-builder/

Additional info

The default parameters to the MockBuilder::new_for_testing produces a MockBuilder with same functionality as before, so the tests in lighthouse should work as before

@pawanjay176 pawanjay176 force-pushed the refactor-mock-builder branch from 80f1d42 to 91c1cf9 Compare January 2, 2025 23:23
@pawanjay176 pawanjay176 force-pushed the refactor-mock-builder branch from c185184 to 8c93b11 Compare January 3, 2025 22:20
@pawanjay176 pawanjay176 added ready-for-review The code is ready for review electra-alpha10 Electra release for devnet 5 and removed do-not-merge labels Jan 16, 2025
realbigsean
realbigsean previously approved these changes Jan 16, 2025
Copy link
Member

@realbigsean realbigsean left a comment

Choose a reason for hiding this comment

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

Excellent work

So you know I have axum implemented for this api here, we can add this later though

https://github.com/sigp/ethereum_apis/blob/main/builder-server/src/server.rs

edit: nevermind you're already using it 😂

@realbigsean realbigsean dismissed their stale review January 16, 2025 20:57

looks like pawan just found a bug

@pawanjay176 pawanjay176 force-pushed the refactor-mock-builder branch from 5e22c79 to 1f7b4a3 Compare January 17, 2025 20:36
@pawanjay176 pawanjay176 force-pushed the refactor-mock-builder branch from b58ecaa to 428dff4 Compare January 21, 2025 00:56
@pawanjay176 pawanjay176 force-pushed the refactor-mock-builder branch from 428dff4 to 36960d0 Compare January 21, 2025 01:18
@pawanjay176
Copy link
Member Author

The bug Sean was referring to is that in scenarios where validators are signalling a gas limit increase/decrease with the validator regisration message, the mock builder is not equipped to increase/decrease the gas limit of the payloads it builds.
The gas limit is set in the connected EL at startup and runtime changes to it are not possible.

After spending some time trying to allow for this feature, I have come to the conclusion that its very hard to do and probably not worth the effort. The mock builder is meant to work as a backend in controlled testing scenarios of the builder flow. Its pretty easy to configure kurtosis to set the gas limit for the builder EL to be the same as all the CLs in the test environment so its easier to fix it in the testing config.

@realbigsean
Copy link
Member

@Mergifyio queue

Copy link

mergify bot commented Jan 21, 2025

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at c33307d

@mergify mergify bot merged commit c33307d into sigp:unstable Jan 21, 2025
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
electra-alpha10 Electra release for devnet 5 ready-for-review The code is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants