[Merged by Bors] - Remove builder redundancy #3294

realbigsean · 2022-06-29T18:10:13Z

Issue Addressed

This PR is a subset of the changes in #3134. Unstable will still not function correctly with the new builder spec once this is merged, #3134 should be used on testnets

Proposed Changes

Removes redundancy in "builders" (servers implementing the builder spec)
Renames payload-builder flag to builder
Moves from old builder RPC API to new HTTP API, but does not implement the validator registration API (implemented in [Merged by Bors] - Register validator api #3194)

…move-builder-redundancy � Conflicts: � Cargo.lock � beacon_node/execution_layer/src/engine_api.rs � beacon_node/execution_layer/src/engine_api/http.rs � beacon_node/execution_layer/src/lib.rs � beacon_node/src/cli.rs � beacon_node/src/config.rs

…ent, fix compile errors

paulhauner

Looking good! Only a few minor requests 🙏

paulhauner · 2022-06-29T23:29:48Z

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -3218,6 +3218,14 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
        let slot = state.slot();
        let proposer_index = state.get_beacon_proposer_index(state.slot(), &self.spec)? as u64;

+        let pubkey_opt = match self.validator_pubkey_bytes(proposer_index as usize) {


Since you have a BeaconState in scope here, I'd suggest doing state.validators().get(proposer_index as usize).map(|v| v.pubkey) rather than hitting the pubkey cache.

There can be some lock contention on the validator pubkey cache at times. There's also something very definite about saying "this validator doesn't exist in the state of the block they're proposing". You could even return an error in that case, I don't see how any block could be valid if the proposer index isn't in the validator registry. I would only do that if there's a benefit in dropping the Option around the pubkey, otherwise we might as well just leave it a warning and have one less way that block proposal can fail.

Ok used this state to get the pubkey here: ac15207

I've left the Option because dropping it doesn't gain us anything, but that's a good point, if the validator isn't in the state I too have no idea how it could propose. Seemed more like a possibility when I was using the cache 😂

paulhauner · 2022-06-29T23:54:28Z

beacon_node/execution_layer/src/lib.rs

+        prev_randao: Hash256,
+        finalized_block_hash: ExecutionBlockHash,
+        suggested_fee_recipient: Address,
+        f: fn(&ExecutionLayer<T>, &ExecutionPayload<T>) -> Option<ExecutionPayload<T>>,


From what I can see this function is always a noop. Is it here for future use?

Yes! In the fallback functionality in #3134, this populates a cache

So it'll either noop when getting a blinded block from a relay (no way to cache the payload) or it will cache when falling back to a local EE (have to cache the fully payload and return a blinded payload becaues the VC expects a blinded payload from this endpoint)

realbigsean · 2022-06-30T00:59:51Z

Ok, I've addressed comment!

paulhauner

LGTM! 🚀

Feel free to bors at will.

realbigsean · 2022-06-30T23:12:47Z

bors r+

## Issue Addressed This PR is a subset of the changes in #3134. Unstable will still not function correctly with the new builder spec once this is merged, #3134 should be used on testnets ## Proposed Changes - Removes redundancy in "builders" (servers implementing the builder spec) - Renames `payload-builder` flag to `builder` - Moves from old builder RPC API to new HTTP API, but does not implement the validator registration API (implemented in #3194) Co-authored-by: sean <seananderson33@gmail.com> Co-authored-by: realbigsean <sean@sigmaprime.io>

bors · 2022-07-01T03:44:31Z

Pull request successfully merged into unstable.

Build succeeded:

## Issue Addressed Part of #3118, continuation of #3257 ## Proposed Changes - the [ `first_success_without_retry` ](https://github.com/sigp/lighthouse/blob/9c429d0764ed91cf56efb8a47a35a556b54a86a4/beacon_node/execution_layer/src/engines.rs#L348-L351) function returns a single error. - the [`first_success`](https://github.com/sigp/lighthouse/blob/9c429d0764ed91cf56efb8a47a35a556b54a86a4/beacon_node/execution_layer/src/engines.rs#L324) function returns a single error. - [ `EngineErrors` ](https://github.com/sigp/lighthouse/blob/9c429d0764ed91cf56efb8a47a35a556b54a86a4/beacon_node/execution_layer/src/lib.rs#L69) carries a single error. - [`EngineError`](https://github.com/sigp/lighthouse/blob/9c429d0764ed91cf56efb8a47a35a556b54a86a4/beacon_node/execution_layer/src/engines.rs#L173-L177) now does not need to carry an Id - [`process_multiple_payload_statuses`](https://github.com/sigp/lighthouse/blob/9c429d0764ed91cf56efb8a47a35a556b54a86a4/beacon_node/execution_layer/src/payload_status.rs#L46-L50) now doesn't need to receive an iterator of statuses and weight in different errors ## Additional Info This is built on top of #3294

realbigsean and others added 6 commits June 22, 2022 22:42

apply diva's patch

4becc36

undo version decrement

558adf5

merge unstable, add builder client, add types required by builder cli…

e944326

…ent, fix compile errors

fix cli tests

2854bea

add generic to el in invalid payload rig

a94c0b9

realbigsean added ready-for-review The code is ready for review bellatrix Required to support the Bellatrix Upgrade labels Jun 29, 2022

realbigsean mentioned this pull request Jun 29, 2022

Builder API Tracking #3091

Closed

14 tasks

paulhauner requested changes Jun 29, 2022

View reviewed changes

paulhauner added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Jun 29, 2022

pr feedback

ac15207

realbigsean added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Jun 30, 2022

paulhauner approved these changes Jun 30, 2022

View reviewed changes

paulhauner added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Jun 30, 2022

bors bot changed the title ~~Remove builder redundancy~~ [Merged by Bors] - Remove builder redundancy Jul 1, 2022

bors bot closed this Jul 1, 2022

divagant-martian mentioned this pull request Jul 1, 2022

[Merged by Bors] - Simplify error handling after engines fallback removal #3283

Closed

come-maiz mentioned this pull request Sep 8, 2022

Support mev-boost in lighthouse flashbots/mev-boost#160

Closed

realbigsean deleted the remove-builder-redundancy branch November 21, 2023 16:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - Remove builder redundancy #3294

[Merged by Bors] - Remove builder redundancy #3294

realbigsean commented Jun 29, 2022

paulhauner left a comment

paulhauner Jun 29, 2022

realbigsean Jun 30, 2022

paulhauner Jun 29, 2022

realbigsean Jun 30, 2022 •

edited

Loading

realbigsean Jun 30, 2022

realbigsean commented Jun 30, 2022

paulhauner left a comment

realbigsean commented Jun 30, 2022

bors bot commented Jul 1, 2022

[Merged by Bors] - Remove builder redundancy #3294

[Merged by Bors] - Remove builder redundancy #3294

Conversation

realbigsean commented Jun 29, 2022

Issue Addressed

Proposed Changes

paulhauner left a comment

Choose a reason for hiding this comment

paulhauner Jun 29, 2022

Choose a reason for hiding this comment

realbigsean Jun 30, 2022

Choose a reason for hiding this comment

paulhauner Jun 29, 2022

Choose a reason for hiding this comment

realbigsean Jun 30, 2022 • edited Loading

Choose a reason for hiding this comment

realbigsean Jun 30, 2022

Choose a reason for hiding this comment

realbigsean commented Jun 30, 2022

paulhauner left a comment

Choose a reason for hiding this comment

realbigsean commented Jun 30, 2022

bors bot commented Jul 1, 2022

realbigsean Jun 30, 2022 •

edited

Loading