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

Move SUBNETS_PER_NODE from Constants to Configuration #3365

Closed
wants to merge 1 commit into from

Conversation

hwwhww
Copy link
Contributor

@hwwhww hwwhww commented May 16, 2023

Per #3312 (comment),

This PR moves SUBNETS_PER_NODE from "Constants" to "Configuration".

/cc @AgeManning

@AgeManning
Copy link
Contributor

Yeah. I thought it might be nice to have this flexibility so we can change it between networks which have different node-counts.

It might make it easier for testnets with very-low node counts to function by increasing this parameter.

@hwwhww hwwhww requested a review from djrtwo May 17, 2023 13:58

| Name | Value | Unit | Duration |
| - | - | :-: | :-: |
| `SUBNETS_PER_NODE` | `2` | The number of long-lived subnets a beacon node should be subscribed to. |
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure this should go into validator.md. As it pertains to all nodes rather than just validator nodes

maybe in the p2p spec? what do you think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. Hmm, I can move the whole "## Phase 0 attestation subnet stability" section to p2p spec.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ended up opening #3375 for it.

@ppopth
Copy link
Member

ppopth commented May 22, 2023

Should this be closed to avoid confusion?

@hwwhww
Copy link
Contributor Author

hwwhww commented May 23, 2023

replaced by #3375

@hwwhww hwwhww closed this May 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants