-
Notifications
You must be signed in to change notification settings - Fork 5
Revise devnet-3 plan #58
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
Conversation
Updated validator configuration and networking details for attestation subnets and aggregation roles.
Added new field 'is_aggregator' to validator config to indicate aggregator duty in attestation subnet. Updated example validator config and networking details.
Clarified the new field 'is_aggregator' in validator-config.yaml, specifying its default assumption if missing. Added example configuration for aggregator nodes.
Added new ENR metadata field 'is_aggregator' to the networking section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR revises the devnet-3 plan to align subnet assignment with the beacon spec by calculating subnet_id using a formula (validator_id % subnets_count) instead of manually assigning it in the validator config. It also clarifies the is_aggregator ENR record configuration.
Changes:
- Moved subnet_id from manual configuration to calculated assignment using
validator_id % subnets_count - Clarified
is_aggregatoras an ENR field in validator config with example YAML - Updated attester role documentation to clarify subscription behavior for bandwidth optimization
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| - New gossipsub topic: `attestation_{subnet_id}` for propagating `SignedAttestation` | ||
| - New gossipsub topic: `aggregated_attestation` for propagating `SignedAggregatedAttestation` | ||
| - Note: attesters should still propate its attestations to the global `attestation` topic for safe target computation | ||
| - Note: attesters should still propagate its attestations to the global `attestation` topic for safe target computation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it is better to explicitly say that attesters should subscribe to the global attestation topic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Clarified a bit
| - **Networking:** | ||
| - New ENR metadata field: `is_aggregator`, follows `validator.is_aggregator` value | ||
| - New ENR metadata field: `is_aggregator`, follows previous explanation | ||
| - Every validator now belongs to one of the attestation subnets. `subnet_id` is defined by `validator_id % subnets_count` formula to ease debugging. In future devnets it will be replaced by the random assignment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much better way to assign subnet thanks!
|
I think these are straightforward enough and definitely an improvement over the previous version so I'll merge it now. As always we can make more changes in new PRs. |
Slight updates to existing plan to:
is_aggregatorENR record