-
Notifications
You must be signed in to change notification settings - Fork 34
[WIP] Committee aggregation #282
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
base: main
Are you sure you want to change the base?
Conversation
use committee aggregated signature proofs
also rename gossip_signatures to gossip_committee_signatures
jihoonsong
left a comment
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.
Great work! Please excuse leaving some comments while it's still in draft. Just wanted to help iterate faster :)
| # Configure the genesis state. | ||
| genesis_config = Config( | ||
| genesis_time=genesis_time, | ||
| attestation_subnet_count=AGGREGATION_COMMITTEE_COUNT, |
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.
I admit that I was the one who advocated for attestation committee, but based on the fact that validators only push their attestations to aggregators in their subnet without subscribing to it, I now think aggregation committee gives us slightly better description.
I don't mind whichever we choose—either attestation committee or aggregation committee—but I do think we need to stick to one thing consistently in the Lean spec and pq-devnet-3.md in the pm repo.
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.
I don't have a strong preference too, however I think the rationale for topic names in beacon chain spec is based on the type of messages that are being propagated to this topic. For consistency we should probably stick to the same logic and keep using attestation subnets and attestation committees
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.
Sounds great! In the same vein, what do you think about this one?
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.
looks good to me, applied your suggestion
# Conflicts: # src/lean_spec/subspecs/forkchoice/store.py # src/lean_spec/subspecs/networking/__init__.py
| Validators participate in consensus by proposing blocks and producing attestations. This | ||
| document describes what honest validators do. | ||
| Validators participate in consensus by proposing blocks and producing attestations. | ||
| Optionally validators can opt-in to behave as aggregators in a single or multiple |
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.
A validator is assigned to a certain subnet but can they perform an aggregator role in multiple subnets?
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.
Good question. I think we may add this possibility in future. But lets start with one subnet per aggregator.
I think we may consider similar logic to how validators self-assign themselves for sampling of single or multiple columns data in PeerDAS
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.
Sounds great! We can update this description to a single committee and modify it later once we introduce multiple subnets aggregator.
docs/client/validator.md
Outdated
|
|
||
| When aggregation is added, aggregators will collect attestations and combine them. | ||
| Aggregated attestations will be broadcast separately. | ||
| Devnet-2 introduces signatures aggregation. Aggregators will collect attestations and combine them. Aggregated attestations will be broadcast separately. |
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.
| Devnet-2 introduces signatures aggregation. Aggregators will collect attestations and combine them. Aggregated attestations will be broadcast separately. | |
| Devnet-3 introduces signatures aggregation. Aggregators will collect attestations and combine them. Aggregated attestations will be broadcast separately. |
…nt in configuration
Aggregate during interval 2 if more threshold signatures were received
🗒️ Description
Introduces aggregator role and subnet aggregation.
Remaining work
🔗 Related Issues or PRs
leanEthereum/pm#56
leanEthereum/pm#58
✅ Checklist
toxchecks to avoid unnecessary CI fails:uvx tox