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

feat!: separate validator clients from CL clients #497

Merged
merged 8 commits into from
Feb 27, 2024

Conversation

eth2353
Copy link
Contributor

@eth2353 eth2353 commented Feb 20, 2024

Separates the validator clients more cleanly from the CL clients. This then allows the use of different combinations of CL/VC clients, e.g. Teku VC with Lodestar CL. The only VC that doesn't work with different beacon nodes is Prysm at the moment.

The use_separate_validator_client flag defaults to false for CL clients that can run validators in the same process as the CL (preserving the way the cl_split_mode_enabled worked before this PR).

I believe this can be quite useful to test different VC<->CL combinations for compatibility.

@eth2353 eth2353 changed the title Separate validator clients from CL clients feat: separate validator clients from CL clients Feb 20, 2024
@eth2353 eth2353 marked this pull request as draft February 20, 2024 17:01
@eth2353 eth2353 marked this pull request as ready for review February 20, 2024 17:39
@eth2353
Copy link
Contributor Author

eth2353 commented Feb 20, 2024

I am unable to add a reviewer to this PR myself.

@barnabasbusa I believe you would be the best person to review this since you introduced the cl_split_mode_enabled flag, would you mind taking a look at this? Thanks!

@barnabasbusa barnabasbusa self-requested a review February 20, 2024 18:17
@barnabasbusa
Copy link
Contributor

I'll try to review this hopefully by the end of the week.

@barnabasbusa
Copy link
Contributor

Sorry I still haven't had time to deeply look into this PR as it is touching quite a bit, I'll make sure to take a proper look before we can merge this in.
Can you please update all the tests, especially the ones that define a prysm image, as it looks like all those tests will fail.
I'll take a look if there would be a way for us to run our nightly tests on this PR.

@eth2353
Copy link
Contributor Author

eth2353 commented Feb 23, 2024

I suppose these may look like rather large changes but they really aren't that large. It's mostly refactoring/relocating existing code. But I understand you want to take a good look at this, take all the time you need.

I updated the prysm CL image definitions in the test configs, you're right, those configs with the previously used 2 image definitions would have failed.

I ran the assertoor, ephemery, mev, split-nimbus, split-teku examples locally (discovered one more thing I overlooked when a network participant has 0 keystores).

Next, I ran all CL client combinations with all valid use_separate_validator_client values. I also tried out some variations with validator_count: 0, everything worked as expected.

One more thing, I added an example config to the tests folder which shows an example of mixed CL / validator client usage. Hope that's okay, it's meant to illustrate the usecase, but feel free to remove it if you want.

@barnabasbusa barnabasbusa enabled auto-merge (squash) February 26, 2024 17:20
@barnabasbusa barnabasbusa changed the title feat: separate validator clients from CL clients feat!: separate validator clients from CL clients Feb 27, 2024
@barnabasbusa barnabasbusa merged commit 90da2c3 into ethpandaops:main Feb 27, 2024
18 checks passed
barnabasbusa pushed a commit that referenced this pull request Mar 8, 2024
🤖 I have created a release *beep* *boop*
---


##
[2.0.0](1.4.0...2.0.0)
(2024-03-08)


### ⚠ BREAKING CHANGES

* participant_network & rename participant fields.
([#508](#508))
* add node selectors features
([#491](#491))

### Features

* add keymanager to all validator processes
([#502](#502))
([836eda4](836eda4))
* add nimbus-eth1
([#496](#496))
([d599729](d599729))
* add node selectors features
([#491](#491))
([316d42f](316d42f))
* allow more detailed additional test configurations in assertoor_params
([#498](#498))
([fe2de7e](fe2de7e))
* enable api in assertoor config
([#495](#495))
([9ceae9c](9ceae9c))
* enable dencun-genesis
([#500](#500))
([beb764f](beb764f))
* make snapshot url configurable
([#507](#507))
([6fa0475](6fa0475))
* parameterize mev-boost args
([#400](#400))
([e48483a](e48483a))
* separate validator clients from CL clients
([#497](#497))
([90da2c3](90da2c3))


### Bug Fixes

* fix end index in validator ranges file
([#509](#509))
([da55be8](da55be8))
* lh vc flag logic
([#506](#506))
([bc5e725](bc5e725))
* nimbus-eth1 advertise proper extip
([#501](#501))
([1d5a779](1d5a779))
* README global node selector
([#504](#504))
([f9343a2](f9343a2))
* use the cl as the default validator image if none are defined
([#503](#503))
([181dd04](181dd04))


### Code Refactoring

* participant_network & rename participant fields.
([#508](#508))
([fab341b](fab341b))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants