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

Remove Execution Client Fallback and Make Providing an Execution Client Required #10921

Merged
merged 33 commits into from
Aug 16, 2022

Conversation

terencechain
Copy link
Member

@terencechain terencechain commented Jun 22, 2022

Fixes #10588.

This PR removes all things related to execution client fallbacks, which will not be supported starting in v3. This also makes providing a non-empty execution client endpoint a requirement to run a node. Prysm nodes have a default value of localhost:8545 at this time

@terencechain terencechain marked this pull request as ready for review July 25, 2022 17:46
@terencechain terencechain requested a review from a team as a code owner July 25, 2022 17:46
@terencechain terencechain marked this pull request as draft July 26, 2022 18:46
@rauljordan rauljordan marked this pull request as ready for review August 11, 2022 18:57
@rauljordan rauljordan changed the title Remove eth1 fallback Remove Execution Client Fallback and Make Providing an Execution Client Required Aug 11, 2022
@rauljordan rauljordan added the Ready For Review A pull request ready for code review label Aug 11, 2022
@@ -105,46 +104,16 @@ func (s *Service) retryExecutionClientConnection(ctx context.Context, err error)
// is ready to serve we connect to it again. This method is only
// relevant if we are on our backup endpoint.
func (s *Service) checkDefaultEndpoint(ctx context.Context) {
Copy link
Member

Choose a reason for hiding this comment

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

We only have 1 endpoint now, this will basically cause prysm to keep on creating new connections to our execution node. This method should be removed since it serves no purpose with 0 fallback endpoints.

nisdas
nisdas previously approved these changes Aug 16, 2022
Copy link
Member

@nisdas nisdas left a comment

Choose a reason for hiding this comment

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

Lgtm

nisdas
nisdas previously approved these changes Aug 16, 2022
Copy link
Contributor

@rkapka rkapka left a comment

Choose a reason for hiding this comment

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

re-approving

@prylabs-bulldozer prylabs-bulldozer bot merged commit 2728b83 into develop Aug 16, 2022
@delete-merged-branch delete-merged-branch bot deleted the rm-eth1-fallback branch August 16, 2022 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready For Review A pull request ready for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Don't support redundancy for EE
4 participants