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

count is not considered when doing the check for actual_num_validators #111

Closed
barnabasbusa opened this issue Jul 31, 2023 · 7 comments · Fixed by #112
Closed

count is not considered when doing the check for actual_num_validators #111

barnabasbusa opened this issue Jul 31, 2023 · 7 comments · Fixed by #112
Assignees

Comments

@barnabasbusa
Copy link
Contributor

{
  "participants": [
    {
      "el_client_type": "geth",
      "el_client_image": "ethpandaops/geth:master",
      "cl_client_type": "teku",
      "cl_client_image": "consensys/teku:develop",
      "el_max_mem": 1000,
      "count": 10
    },
    {
      "el_client_type": "geth",
      "el_client_image": "ethpandaops/geth:master",
      "cl_client_type": "lighthouse",
      "cl_client_image": "sigp/lighthouse:latest",
      "el_max_mem": 1000,
      "count": 10
    },
    {
      "el_client_type": "geth",
      "el_client_image": "ethpandaops/geth:master",
      "cl_client_type": "lodestar",
      "cl_client_image": "chainsafe/lodestar:latest",
      "el_max_mem": 1000,
      "count": 10
    },
    {
      "el_client_type": "geth",
      "el_client_image": "ethpandaops/geth:master",
      "cl_client_type": "prysm",
      "cl_client_image": "prysmaticlabs/prysm-beacon-chain:latest,prysmaticlabs/prysm-validator:latest",
      "el_max_mem": 1000,
      "count": 10
    },
    {
      "el_client_type": "geth",
      "el_client_image": "ethpandaops/geth:master",
      "cl_client_type": "nimbus",
      "cl_client_image": "statusim/nimbus-eth2:amd64-latest",
      "el_max_mem": 1000,
      "count": 10
    }
  ],
  "network_params": {
    "network_id": "3151908",
    "deposit_contract_address": "0x4242424242424242424242424242424242424242",
    "seconds_per_slot": 12,
    "slots_per_epoch": 32,
    "genesis_delay": 1800,
    "capella_fork_epoch": 2,
    "deneb_fork_epoch": 1000,
    "num_validator_keys_per_node": 10,
    "preregistered_validator_keys_mnemonic": "giant issue aisle success illegal bike spike question tent bar rely arctic volcano long crawl hungry vocal artwork sniff fantasy very lucky have athlete"
  },
  "launch_additional_services": true,
  "wait_for_finalization": false,
  "wait_for_verifications": false,
  "verifications_epoch_limit": 5,
  "global_client_log_level": "info"
}
Evaluation error: fail: required_num_validtors - 64 is greater than actual_num_validators - 50
	at [github.com/kurtosis-tech/eth2-package/main.star:27:77]: run
	at [github.com/kurtosis-tech/eth2-package/src/package_io/parse_input.star:106:7]: parse_input
	at [0:0]: fail

Error encountered running Starlark code.```
@adschwartz
Copy link
Contributor

I was able to reproduce this error, continuing to look into it.

@barnabasbusa
Copy link
Contributor Author

Its probably this line that needs adjusting, to use the count also: https://github.com/kurtosis-tech/eth2-package/blob/main/src/package_io/parse_input.star#L104

@adschwartz
Copy link
Contributor

@barnabasbusa I'm not sure what you mean about it shoud use the count as well? As I see it the immediate issue looks to be that slots_per_epoch is set to 32. Would it make sense to lower it? When I do it seems to run. But not sure what the best value is.

@barnabasbusa
Copy link
Contributor Author

barnabasbusa commented Jul 31, 2023

this has nothing to do with slots_per_epoch. The count is a new participant parameter that can be configured.
Its basically requests that many sets of client pairs of a certain kind.
For example, it will create

teku-geth-01
teku-geth-02
...
teku-geth-10
lighthouse-geth-01
...

...
But instead it assumes that there is one occurrence of each, and thus, actual_num_validators is 5x10 = 50. (which is no longer the correct way to calculate how many validators there are actually)

@adschwartz
Copy link
Contributor

adschwartz commented Jul 31, 2023

@barnabasbusa Got it! What would be the correct way?

Would it be summing up the counts in the array? If so, I can make that change.

@barnabasbusa
Copy link
Contributor Author

sum of all counts (default to 1 if count is not present) * num_validator_keys_per_node

@adschwartz
Copy link
Contributor

@barnabasbusa made a PR. I was unable to test it with count=10 for each participant as my laptop just doesn't have enough memory. But ran it with count=2 and it seems to be working. Let me know if you have any issues!

@adschwartz adschwartz self-assigned this Jul 31, 2023
adschwartz added a commit that referenced this issue Aug 2, 2023
This PR changes the way we count the actual number of validators by
summing `count`s for each participant and multiplying by the number of
validator keys per node.

Resolves #111

Changelog picked up from commits here:

feat: counting by summing each participant
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 a pull request may close this issue.

2 participants