Remove redunancy for execution engines#3160
Closed
paulhauner wants to merge 6 commits into
Closed
Conversation
paulhauner
added
work-in-progress
paulhauner
force-pushed
the
no-redundant-ees
branch
from
091a394 to
f990f32
Compare
Member
Author
|
I'm going to close this due to merge conflicts and lack of time. |
bors Bot
pushed a commit
that referenced
this pull request
Jun 22, 2022
…ate (#3257) ## Issue Addressed Part of #3160 ## Proposed Changes Use only the first url given in the execution engine, if more than one is provided log it. This change only moves having multiple engines to one. The amount of code cleanup that can and should be done forward is not small and would interfere with ongoing PRs. I'm keeping the changes intentionally very very minimal. ## Additional Info Future works: - In [ `EngineError` ](https://github.com/sigp/lighthouse/blob/9c429d0764ed91cf56efb8a47a35a556b54a86a4/beacon_node/execution_layer/src/engines.rs#L173-L177) the id is not needed since it now has no meaning. - the [ `first_success_without_retry` ](https://github.com/sigp/lighthouse/blob/9c429d0764ed91cf56efb8a47a35a556b54a86a4/beacon_node/execution_layer/src/engines.rs#L348-L351) function can return a single error. - the [`first_success`](https://github.com/sigp/lighthouse/blob/9c429d0764ed91cf56efb8a47a35a556b54a86a4/beacon_node/execution_layer/src/engines.rs#L324) function can return a single error. - After the redundancy is removed for the builders, we can probably make the [ `EngineErrors` ](https://github.com/sigp/lighthouse/blob/9c429d0764ed91cf56efb8a47a35a556b54a86a4/beacon_node/execution_layer/src/lib.rs#L69) carry a single error. - Merge the [`Engines`](https://github.com/sigp/lighthouse/blob/9c429d0764ed91cf56efb8a47a35a556b54a86a4/beacon_node/execution_layer/src/engines.rs#L161-L165) struct and [`Engine` ](https://github.com/sigp/lighthouse/blob/9c429d0764ed91cf56efb8a47a35a556b54a86a4/beacon_node/execution_layer/src/engines.rs#L62-L67) - Fix the associated configurations and cli params. Not sure if both are done in #3214 In general I think those changes can be done incrementally and in individual pull requests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue Addressed
Resolves #3118
Proposed Changes
Removes support for redundant EEs, from now on there shall be one EE to rule them all!
Changes to CLI flags
--execution-endpointsis now a deprecated alias to--execution-endpoint--jwt-secretsis now a deprecated alias to--execution-jwt--jwt-idis now a deprecated alias to--execution-jwt-id--jwt-versionis now a deprecated alias to--execution-jwt-versionBreaking Changes
Whilst the
--execution-endpointsand--jwt-secretsflags have been deprecated, they are still available as aliases to the new--execution-endpointand--execution-jwtflags.However, those flags will no longer support multiple values. Therefore, we have this matrix:
--execution-endpoints:--jwt-secrets:Additional Info
Please provide any additional information. For example, future considerations
or information useful for reviewers.