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

[Merged by Bors] - validator client: start http api before genesis #4714

Closed
wants to merge 2 commits into from

Conversation

jxs
Copy link
Member

@jxs jxs commented Sep 8, 2023

Issue Addressed

On a new network a user might require importing validators before waiting until genesis has occurred.

Proposed Changes

Starts the validator client http api before waiting for genesis

Additional Info

cc @antondlr

@jxs jxs changed the base branch from stable to unstable September 8, 2023 16:53
@jxs jxs force-pushed the start-http-before-genesis branch from f154366 to aebe3dd Compare September 8, 2023 17:31
@@ -611,6 +609,9 @@ impl<T: EthSpec> ProductionValidatorClient<T> {
);
}

// Wait until genesis has occurred.
wait_for_genesis(&self.beacon_nodes, self.genesis_time, &self.context).await?;
Copy link
Member

Choose a reason for hiding this comment

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

We also start all the other services like the block_service/etc. I haven't had time to run this PR, but if someone (@antondlr) can confirm that these services run without errors prior to genesis I think we can merge.

Copy link
Member

@antondlr antondlr Sep 11, 2023

Choose a reason for hiding this comment

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

just pushed this to all the holly VCs; looking good

Copy link
Member

Choose a reason for hiding this comment

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

I see a CRIT Failed to poll sync committee duties

@michaelsproul michaelsproul added val-client Relates to the validator client binary ready-for-review The code is ready for review v4.5.0 ETA Q4 2023 waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Sep 11, 2023
@michaelsproul
Copy link
Member

@jxs If you have time would you mind adding some logic here to have the VC ignore the BN's el_offline status prior to genesis too?

let el_offline = resp.data.el_offline.unwrap_or(false);

Alternatively, we could change the status to false in the BN, but that might be a more invasive change.

@jxs
Copy link
Member Author

jxs commented Sep 12, 2023

@jxs If you have time would you mind adding some logic here to have the VC ignore the BN's el_offline status prior to genesis too?

let el_offline = resp.data.el_offline.unwrap_or(false);

Alternatively, we could change the status to false in the BN, but that might be a more invasive change.

Hi Michael, I put the wait_for_genesis call after the http api but before the start of the services as it was before, if you prefer I can instead as you suggest.

Thanks

@michaelsproul
Copy link
Member

@jxs That sounds good, but I think the el_offline issue is separate. If you look at the logs of the VCs on Holesky they're logging constant warnings about the BN not being synced due to el_offline=true. I figured we could roll a fix into this PR so that we have a version with cleaner logs that people can run if they're interested

@michaelsproul
Copy link
Member

I implemented a quick fix for the el_offline issue here: #4730.

I'll test our PRs together and then we can merge them (assuming CI recovers from the mess it's in).

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Sep 13, 2023
@jxs
Copy link
Member Author

jxs commented Sep 13, 2023

Thanks Michael, rerun the CI and it's now green 🎉

@michaelsproul
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Sep 15, 2023
## Issue Addressed

On a new network a user might require importing validators before waiting until genesis has occurred.

## Proposed Changes

Starts the validator client http api before waiting for genesis 

## Additional Info

cc @antondlr
@bors
Copy link

bors bot commented Sep 15, 2023

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Sep 15, 2023
## Issue Addressed

On a new network a user might require importing validators before waiting until genesis has occurred.

## Proposed Changes

Starts the validator client http api before waiting for genesis 

## Additional Info

cc @antondlr
@bors
Copy link

bors bot commented Sep 15, 2023

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Sep 15, 2023
## Issue Addressed

On a new network a user might require importing validators before waiting until genesis has occurred.

## Proposed Changes

Starts the validator client http api before waiting for genesis 

## Additional Info

cc @antondlr
@bors
Copy link

bors bot commented Sep 15, 2023

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Sep 15, 2023
## Issue Addressed

On a new network a user might require importing validators before waiting until genesis has occurred.

## Proposed Changes

Starts the validator client http api before waiting for genesis 

## Additional Info

cc @antondlr
@bors
Copy link

bors bot commented Sep 15, 2023

@bors bors bot changed the title validator client: start http api before genesis [Merged by Bors] - validator client: start http api before genesis Sep 15, 2023
@bors bors bot closed this Sep 15, 2023
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
## Issue Addressed

On a new network a user might require importing validators before waiting until genesis has occurred.

## Proposed Changes

Starts the validator client http api before waiting for genesis 

## Additional Info

cc @antondlr
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
## Issue Addressed

On a new network a user might require importing validators before waiting until genesis has occurred.

## Proposed Changes

Starts the validator client http api before waiting for genesis 

## Additional Info

cc @antondlr
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge This PR is ready to merge. v4.5.0 ETA Q4 2023 val-client Relates to the validator client binary
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants