-
Notifications
You must be signed in to change notification settings - Fork 32
feat: implement committee aggregation #537
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
Signed-off-by: grapebaba <grapebaba@grapebabadeMacBook-Pro.local>
Signed-off-by: grapebaba <grapebaba@grapebabadeMacBook-Pro.local>
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
Implements committee-style attestation aggregation by introducing an aggregated-attestation gossip message, adding subnet-capable gossip topic specs, and splitting signature aggregation into (1) aggregating fresh gossip signatures vs (2) selecting stored aggregated proofs for block production.
Changes:
- Added
SignedAggregatedAttestationtype and new.aggregationgossip topic/message for disseminating aggregated attestation proofs. - Reworked attestation signature handling: aggregators build proofs from gossip signatures; proposers select proofs from stored payloads for inclusion in blocks.
- Introduced attestation committee/subnet configuration and updated node/network plumbing (topic specs, subscriptions, timing intervals).
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkgs/types/src/lib.zig | Re-exports the new aggregated attestation type for use across packages. |
| pkgs/types/src/block_signatures_testing.zig | Updates tests to cover the new aggregation helpers (aggregateGossipSignatures, selectAggregatedProofs). |
| pkgs/types/src/block.zig | Adds new aggregation helpers and refactors aggregation logic into separate phases (gossip aggregation vs stored-proof selection). |
| pkgs/types/src/attestation.zig | Introduces SignedAggregatedAttestation with JSON helpers and deinit. |
| pkgs/state-transition/src/mock.zig | Updates mock chain generation to use the new gossip-signature aggregation API. |
| pkgs/params/src/types.zig | Adds ATTESTATION_COMMITTEE_COUNT to preset config. |
| pkgs/params/src/presets/mainnet.zig | Sets ATTESTATION_COMMITTEE_COUNT for mainnet preset. |
| pkgs/params/src/lib.zig | Exposes ATTESTATION_COMMITTEE_COUNT as an active preset constant. |
| pkgs/node/src/validator_client.zig | Adjusts validator interval switch to match new intervals-per-slot. |
| pkgs/node/src/node.zig | Adds aggregator mode behavior, aggregated-attestation publishing, topic-spec subscriptions, and attestation subnet publish logic. |
| pkgs/node/src/network.zig | Switches gossip publish API to publishWithTopic. |
| pkgs/node/src/lib.zig | Re-exports buildGossipTopicSpecs. |
| pkgs/node/src/forkchoice.zig | Adds aggregator/subnet tracking, selective signature storage for committee aggregation, and ordered payload storage. |
| pkgs/node/src/constants.zig | Changes INTERVALS_PER_SLOT to 5 to introduce a dedicated aggregation interval. |
| pkgs/node/src/chain.zig | Updates block production to select stored aggregated proofs; adds committee aggregation + aggregated-attestation gossip handling. |
| pkgs/network/src/mock.zig | Updates mock gossip interface to accept topic specs and topic-aware publish. |
| pkgs/network/src/lib.zig | Exposes GossipTopicSpec from the network package. |
| pkgs/network/src/interface.zig | Adds subnet-aware topic encoding/decoding, new .aggregation topic/message, and updates gossip interface signatures. |
| pkgs/network/src/ethlibp2p.zig | Adds .aggregation message decoding/logging and uses configured topic specs for network subscription/publish. |
| pkgs/cli/src/node.zig | Adds is_aggregator option, builds gossip topic specs, and plumbs into node/network init. |
| pkgs/cli/src/main.zig | Updates ethlibp2p demo/init wiring to supply gossip topic specs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const subnet_id = | ||
| @as(usize, @intCast(validator_id)) % ATTESTATION_COMMITTEE_COUNT; | ||
| const subnet_topic = networks.GossipTopicSpec{ .topic = .attestation, .subnet_id = subnet_id }; | ||
| try self.network.publishWithTopic(subnet_topic, &gossip_msg); | ||
| try self.network.publishWithTopic(.{ .topic = .attestation }, &gossip_msg); |
Copilot
AI
Jan 29, 2026
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.
Publishing the same attestation to both the subnet topic and the global .attestation topic will cause aggregator nodes (which subscribe to both) to process duplicate messages. Since the subnet_id is not propagated to handlers, these duplicates are indistinguishable and can inflate work/metrics. Consider publishing to only one topic (e.g. only subnet when ATTESTATION_COMMITTEE_COUNT > 1, or only global until subnet routing is fully supported).
| const subnet_id = | |
| @as(usize, @intCast(validator_id)) % ATTESTATION_COMMITTEE_COUNT; | |
| const subnet_topic = networks.GossipTopicSpec{ .topic = .attestation, .subnet_id = subnet_id }; | |
| try self.network.publishWithTopic(subnet_topic, &gossip_msg); | |
| try self.network.publishWithTopic(.{ .topic = .attestation }, &gossip_msg); | |
| if (ATTESTATION_COMMITTEE_COUNT == 1) { | |
| // With a single committee, there is no meaningful subnetting; use the global topic only. | |
| try self.network.publishWithTopic(.{ .topic = .attestation }, &gossip_msg); | |
| } else { | |
| // With multiple committees, publish only to the subnet-specific topic to avoid duplicates. | |
| const subnet_id = | |
| @as(usize, @intCast(validator_id)) % ATTESTATION_COMMITTEE_COUNT; | |
| const subnet_topic = networks.GossipTopicSpec{ .topic = .attestation, .subnet_id = subnet_id }; | |
| try self.network.publishWithTopic(subnet_topic, &gossip_msg); | |
| } |
| try topics.append(.{ .topic = .block }); | ||
| try topics.append(.{ .topic = .attestation }); | ||
| try topics.append(.{ .topic = .aggregation }); | ||
|
|
||
| if (is_aggregator) { | ||
| if (validator_ids) |ids| { | ||
| if (ATTESTATION_COMMITTEE_COUNT == 0) return topics.toOwnedSlice(); | ||
| const LocalSubnetSet = std.StaticBitSet(ATTESTATION_COMMITTEE_COUNT); | ||
| var local_subnets = LocalSubnetSet.initEmpty(); | ||
|
|
||
| for (ids) |validator_id| { | ||
| const subnet_id = validator_id % ATTESTATION_COMMITTEE_COUNT; | ||
| local_subnets.set(subnet_id); | ||
| } | ||
|
|
||
| var subnet_it = local_subnets.iterator(.{}); | ||
| while (subnet_it.next()) |subnet_id| { | ||
| try topics.append(.{ .topic = .attestation, .subnet_id = subnet_id }); | ||
| } |
Copilot
AI
Jan 29, 2026
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.
buildGossipTopicSpecs always includes the global .attestation topic and (for aggregators) also adds per-subnet .attestation topics. Because inbound messages are decoded to GossipTopic.attestation without exposing subnet_id to the application layer, subscribing to both leads to duplicate processing if peers publish on both. Either avoid mixing global+subnet subscriptions, or plumb subnet_id through to the gossip callback so duplicates can be filtered intentionally.
| while (true) { | ||
| const candidate_attestations = | ||
| try buildAggregatedAttestationsFromAttestations(self.allocator, selected_attestations.items); | ||
| var candidate_block = types.BeamBlock{ | ||
| .slot = opts.slot, | ||
| .proposer_index = opts.proposer_index, | ||
| .parent_root = parent_root, | ||
| .state_root = undefined, | ||
| .body = types.BeamBlockBody{ | ||
| .attestations = candidate_attestations, | ||
| }, | ||
| }; | ||
| defer candidate_block.deinit(); | ||
|
|
||
| var temp_state_ptr_opt: ?*types.BeamState = try self.allocator.create(types.BeamState); | ||
| errdefer if (temp_state_ptr_opt) |ptr| self.allocator.destroy(ptr); | ||
|
|
||
| const temp_state_ptr = temp_state_ptr_opt.?; | ||
| try types.sszClone(self.allocator, types.BeamState, pre_state.*, temp_state_ptr); | ||
| temp_state_ptr_opt = null; | ||
| defer { | ||
| temp_state_ptr.deinit(); | ||
| self.allocator.destroy(temp_state_ptr); | ||
| } | ||
| try stf.apply_raw_block(self.allocator, temp_state_ptr, &candidate_block, self.block_building_logger); | ||
|
|
||
| var added_any = false; | ||
| var no_payloads = false; | ||
| self.forkChoice.signatures_mutex.lock(); | ||
| defer self.forkChoice.signatures_mutex.unlock(); | ||
| if (self.forkChoice.aggregated_payloads.count() == 0) { |
Copilot
AI
Jan 29, 2026
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.
selectAttestationsForBlock repeatedly clones pre_state and runs stf.apply_raw_block inside a while (true) loop. This makes block production potentially O(N) full state transitions per slot, and it also holds signatures_mutex while doing allocations (sszRoot, append, put). Consider narrowing the lock to only the aggregated_payloads.get calls and/or reworking the selection to avoid re-running the full state transition on each iteration (e.g., incremental updates or a bounded fixed-point loop).
| fn ensureAttestationAppendCapacity(self: *Self) !void { | ||
| const max = params.VALIDATOR_REGISTRY_LIMIT; | ||
| if (self.attestations.len() >= max or self.attestation_signatures.len() >= max) { | ||
| return error.Overflow; | ||
| } | ||
|
|
||
| // Group attestations by data root using bitsets for validator tracking | ||
| const AttestationGroup = struct { | ||
| data: attestation.AttestationData, | ||
| data_root: Root, | ||
| validator_bits: std.DynamicBitSet, | ||
| }; | ||
| try self.attestations.inner.ensureTotalCapacity(self.attestations.len() + 1); | ||
| try self.attestation_signatures.inner.ensureTotalCapacity(self.attestation_signatures.len() + 1); |
Copilot
AI
Jan 29, 2026
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.
params.VALIDATOR_REGISTRY_LIMIT is a u32, but self.attestations.len() / self.attestation_signatures.len() are usize. This comparison will not type-check in Zig; cast the limit to usize (or define const max: usize = @intCast(params.VALIDATOR_REGISTRY_LIMIT)) before comparing and reserving capacity.
No description provided.