Add networking configs to config files #3375

hwwhww · 2023-05-19T14:55:49Z

A more comprehensive solution to address #3365.

To enable clients to have more flexibility in networking setups, this PR adds many network configurations:

Move most networking custom types, constants, preset, and configurations from validator.md to p2p-networking.md
Add p2p-networking.md spec to pyspec scope
Update config and preset files
Move "Phase 0 attestation subnet stability" section from validator.md to p2p-networking.md as @djrtwo suggested (Move SUBNETS_PER_NODE from Constants to Configuration #3365 (comment))

However, a drawback of this PR is the circular dependency it creates: validator.md uses types & configs defined in p2p-networking.md, while p2p-networking.md in turn utilizes functions defined in validator.md.

I think it's better to make only validator.md depends on p2p-networking.md. Maybe in the following PR so we can keep this PR small and merge soon.

/cc @djrtwo @AgeManning

AgeManning · 2023-05-22T06:31:36Z

configs/minimal.yaml

+ATTESTATION_SUBNET_COUNT: 64
+ATTESTATION_SUBNET_EXTRA_BITS: 0
+# ceillog2(ATTESTATION_SUBNET_COUNT) + ATTESTATION_SUBNET_EXTRA_BITS
+ATTESTATION_SUBNET_PREFIX_BITS: 6


As you've commented, this is fixed by the other two parameters. We could probably leave this one out, but if it's here for consistency, fine by me :)

Yeah, I tried to replace it with a compute_attestation_prefix_bits() helper. However, it seemed a bit unusual to me, so I changed it to determine ATTESTATION_SUBNET_PREFIX_BITS here and verify the correctness in test_config_invariants.py. 😅

AgeManning · 2023-05-22T06:31:59Z

Yeah nice. This seems good to me.

djrtwo

looks good! one quick question

djrtwo · 2023-05-22T14:16:58Z

presets/minimal/phase0.yaml

+# Networking
+# ---------------------------------------------------------------
+# 2**8 (= 256)
+EPOCHS_PER_SUBNET_SUBSCRIPTION: 256


Why is this one a preset?

I guess that if this one is too low, the churn rate will be too high. So, no network probably wants this number to be too low.

That is, around 1/EPOCHS_PER_SUBNET_SUBSCRIPTION of the nodes will change the subnets in each epoch.

However, I just guess.

Because our previous EPOCHS_PER_* settings are all in presets. e.g, EPOCHS_PER_ETH1_VOTING_PERIOD, EPOCHS_PER_SLASHINGS_VECTOR, EPOCHS_PER_SYNC_COMMITTEE_PERIOD. It seems less likely we need to tune it for testing the networking configurations.

I can move it to configs if you prefer.

ah but the other EPOCHS_PER_ are things that impact the shape of merkle trees/state transitions.

This seems a bit more like something you could mess with on a network/testnet without affecting compile time build in a negative way

djrtwo

I moved EPOCHS_PER_SUBNET_SUBSCRIPTION from present to config.

looks good!

rolfyone · 2023-05-31T02:47:29Z

I realised there's GOSSIP_MAX_SIZE_BELLATRIX that should really be able to be in config now after this change... also MAX_CHUNK_SIZE_BELLATRIX..

Now that `MAX_CHUNK_SIZE` and `GOSSIP_MAX_SIZE` are in configuration, we no longer need separate constants to represent them in the spec when they change in Bellatrix. I've changed the usage, and put the values into the presets, but I'm not sure if I've updated the descriptions in the best way... This is following on from the work in ethereum#3375 where a number of constants got moved into configuration, so we no longer need these constants to be separately represented, they can simply be updated in presets.

* Moved configuration into network preset instead of constants. Now that `MAX_CHUNK_SIZE` and `GOSSIP_MAX_SIZE` are in configuration, we no longer need separate constants to represent them in the spec when they change in Bellatrix. I've changed the usage, and put the values into the presets, but I'm not sure if I've updated the descriptions in the best way... This is following on from the work in #3375 where a number of constants got moved into configuration, so we no longer need these constants to be separately represented, they can simply be updated in presets. * Update presets/minimal/bellatrix.yaml Co-authored-by: Hsiao-Wei Wang <hsiaowei.eth@gmail.com> * Update presets/mainnet/bellatrix.yaml Co-authored-by: Hsiao-Wei Wang <hsiaowei.eth@gmail.com> * Moved preset items into the correct section and updated TOC. It looked like the items listed in configuration about the max size and chunk size were no longer needed since we're updating preset values now and the preset changes seem to only be listed in the changes at the top. * review feedback * hopefully correct this time! Moved the 2 fields from configs into presets completely as suggested. * WIP - changing back to being in config and updating the phase 0 value... I think this should be close but want to see what's outstanding. * fix intellij's formatting of table. * more fixes --------- Co-authored-by: Hsiao-Wei Wang <hsiaowei.eth@gmail.com>

…um#3394) * Moved configuration into network preset instead of constants. Now that `MAX_CHUNK_SIZE` and `GOSSIP_MAX_SIZE` are in configuration, we no longer need separate constants to represent them in the spec when they change in Bellatrix. I've changed the usage, and put the values into the presets, but I'm not sure if I've updated the descriptions in the best way... This is following on from the work in ethereum#3375 where a number of constants got moved into configuration, so we no longer need these constants to be separately represented, they can simply be updated in presets. * Update presets/minimal/bellatrix.yaml Co-authored-by: Hsiao-Wei Wang <hsiaowei.eth@gmail.com> * Update presets/mainnet/bellatrix.yaml Co-authored-by: Hsiao-Wei Wang <hsiaowei.eth@gmail.com> * Moved preset items into the correct section and updated TOC. It looked like the items listed in configuration about the max size and chunk size were no longer needed since we're updating preset values now and the preset changes seem to only be listed in the changes at the top. * review feedback * hopefully correct this time! Moved the 2 fields from configs into presets completely as suggested. * WIP - changing back to being in config and updating the phase 0 value... I think this should be close but want to see what's outstanding. * fix intellij's formatting of table. * more fixes --------- Co-authored-by: Hsiao-Wei Wang <hsiaowei.eth@gmail.com>

Add networking configs to config files

f87e143

hwwhww added scope:v-guide Validator guide scope:networking labels May 19, 2023

hwwhww mentioned this pull request May 19, 2023

Move SUBNETS_PER_NODE from Constants to Configuration #3365

Closed

AgeManning reviewed May 22, 2023

View reviewed changes

djrtwo reviewed May 22, 2023

View reviewed changes

djrtwo mentioned this pull request May 23, 2023

v1.4.0-alpha.0 release #3380

Merged

8 tasks

djrtwo force-pushed the networking-configs branch from 65cff8c to a49803e Compare May 23, 2023 19:54

move epoch subscription length to config

68ce45b

djrtwo force-pushed the networking-configs branch from a49803e to 68ce45b Compare May 23, 2023 19:56

djrtwo approved these changes May 23, 2023

View reviewed changes

hwwhww merged commit 738b981 into dev May 24, 2023

hwwhww deleted the networking-configs branch May 24, 2023 02:38

lucassaldanha mentioned this pull request May 25, 2023

Move p2p networking setting from Constants to SpecConfig Consensys/teku#7179

Closed

rolfyone mentioned this pull request May 31, 2023

Moved configuration into network preset instead of constants. #3394

Merged

nisdas mentioned this pull request May 31, 2023

Update Config To Include Networking Config prysmaticlabs/prysm#12478

Closed

AgeManning mentioned this pull request Jun 14, 2023

Shift networking configuration into configs sigp/lighthouse#4401

Closed

StefanBratanov mentioned this pull request Jul 28, 2023

Client Diversity: Teku VC <> Lighthouse BN does not work Consensys/teku#7397

Closed

ybstaked mentioned this pull request Jul 28, 2023

Client Diversity: Teku VC <> Lighthouse BN does not work sigp/lighthouse#4550

Closed

tersec mentioned this pull request Aug 11, 2023

Update constants/presets for consistency with consensus-specs v1.4.0 status-im/nimbus-eth2#5284

Merged

michaelsproul mentioned this pull request Sep 7, 2023

Compatibility with Teku VC for voluntary-exit sigp/lighthouse#4711

Closed

james-prysm mentioned this pull request Sep 27, 2023

Deneb - web3signer prysmaticlabs/prysm#12767

Merged

2 tasks

siladu mentioned this pull request Sep 28, 2023

Custom network configs break unless GOSSIP_MAX_SIZE and others are defined Consensys/teku#7555

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add networking configs to config files #3375

Add networking configs to config files #3375

hwwhww commented May 19, 2023

AgeManning May 22, 2023

hwwhww May 23, 2023

AgeManning commented May 22, 2023

djrtwo left a comment

djrtwo May 22, 2023

ppopth May 22, 2023

hwwhww May 23, 2023 •

edited

Loading

djrtwo May 23, 2023

djrtwo left a comment

rolfyone commented May 31, 2023

Add networking configs to config files #3375

Add networking configs to config files #3375

Conversation

hwwhww commented May 19, 2023

AgeManning May 22, 2023

Choose a reason for hiding this comment

hwwhww May 23, 2023

Choose a reason for hiding this comment

AgeManning commented May 22, 2023

djrtwo left a comment

Choose a reason for hiding this comment

djrtwo May 22, 2023

Choose a reason for hiding this comment

ppopth May 22, 2023

Choose a reason for hiding this comment

hwwhww May 23, 2023 • edited Loading

Choose a reason for hiding this comment

djrtwo May 23, 2023

Choose a reason for hiding this comment

djrtwo left a comment

Choose a reason for hiding this comment

rolfyone commented May 31, 2023

hwwhww May 23, 2023 •

edited

Loading