Skip to content

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.

3 participants