Conversation
Signed-off-by: turuslan <turuslan.devbox@gmail.com>
Signed-off-by: turuslan <turuslan.devbox@gmail.com> # Conflicts: # src/app/impl/chain_spec_impl.cpp # src/modules/networking/networking.cpp
Signed-off-by: turuslan <turuslan.devbox@gmail.com> # Conflicts: # src/loaders/impl/networking_loader.hpp
Signed-off-by: turuslan <turuslan.devbox@gmail.com>
Signed-off-by: turuslan <turuslan.devbox@gmail.com>
Signed-off-by: turuslan <turuslan.devbox@gmail.com>
Signed-off-by: turuslan <turuslan.devbox@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the devnet networking/consensus flow to operate on 5 sub-slot intervals (800ms each), introduces attestation aggregation gossiping, and adds subnet/aggregator-aware peer selection via ENR/bootnode metadata.
Changes:
- Introduce
Interval-based timekeeping (milliseconds + 5 intervals/slot) across timeline scheduling, fork-choice ticking, and tests. - Add signed aggregated attestation type + networking/production plumbing to broadcast and ingest aggregations.
- Add “aggregator” role metadata (ENR + bootnodes + CLI) and subnet-based topic/peer-connection behavior.
Reviewed changes
Copilot reviewed 43 out of 43 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/serde/enr_test.cpp | Update ENR round-trip tests for is_aggregator field. |
| tests/unit/blockchain/fork_choice_test.cpp | Adapt fork-choice tests to new Interval time type. |
| tests/unit/app/bootnodes_test.cpp | Update bootnode info construction for is_aggregator. |
| tests/mock/app/chain_spec_mock.hpp | Extend ChainSpec mock with isAggregator(). |
| src/utils/sample_peer.hpp | Add is_aggregator into generated ENR for sample peers. |
| src/types/slot.hpp | Replace raw interval typedef with Interval struct and conversions. |
| src/types/signed_attestation.hpp | Add helper constructor SignedAttestation::from(...). |
| src/types/signed_aggregated_attestation.hpp | New SSZ container for broadcasting aggregated attestations. |
| src/types/hash.hpp | New Hash alias type. |
| src/types/constants.hpp | Change timing constants to 5 intervals/slot and 800ms interval duration. |
| src/types/config.hpp | Add genesisTimeMs() helper. |
| src/serde/enr.hpp | Extend ENR with optional is_aggregator and accessor; update encode signature. |
| src/serde/enr.cpp | Implement RLP bool encoding/decoding and encode/decode of is_aggregator. |
| src/modules/shared/networking_types.tmp.hpp | Add broadcast notification type for aggregated attestations. |
| src/modules/production/read_config_yaml.hpp | Minor formatting updates while reading genesis config. |
| src/modules/production/production.cpp | Dispatch aggregated attestation gossip notifications from fork-choice tick actions. |
| src/modules/production/interfaces.hpp | Add loader dispatch API for aggregated attestations. |
| src/modules/networking/networking.hpp | Add deps/state for subnet/aggregator handling + aggregation topic. |
| src/modules/networking/networking.cpp | Subnet-scoped attestation topic, aggregation gossip handling, and aggregator-pref connections. |
| src/modules/networking/module.cpp | Extend module factory wiring with validator registry + genesis config. |
| src/modules/networking/interfaces.hpp | Add handler for sending aggregated attestations. |
| src/loaders/impl/production_loader.hpp | Wire new production dispatch for aggregated attestations. |
| src/loaders/impl/networking_loader.hpp | Wire new networking deps and subscription for aggregated attestation sends. |
| src/injector/boost_di_inject_traits_many.hpp | Add DI macro helper for constructors with >10 args. |
| src/commands/generate_genesis.hpp | Add subnet count arg/output and mark initial aggregators in generated nodes. |
| src/clock/impl/clock_impl.hpp | Change nowMsec() return type to std::chrono::milliseconds. |
| src/clock/impl/clock_impl.cpp | Implement nowMsec() returning a chrono duration (not integer count). |
| src/clock/clock.hpp | Update clock interface for chrono-based milliseconds. |
| src/blockchain/validator_subnet.hpp | Add subnet assignment helper (validator index → subnet). |
| src/blockchain/genesis_config.hpp | Track validator count from anchor state. |
| src/blockchain/genesis_config.cpp | Initialize validator_count from state. |
| src/blockchain/fork_choice.hpp | Add aggregation APIs/state and switch onTick to chrono milliseconds. |
| src/blockchain/fork_choice.cpp | Implement aggregation collection/broadcast/import, pruning, and interval-based ticking. |
| src/app/impl/timeline_impl.cpp | Schedule slot intervals using Interval timing (5 phases). |
| src/app/impl/chain_spec_impl.hpp | Add isAggregator() implementation state. |
| src/app/impl/chain_spec_impl.cpp | Derive aggregator role from bootnodes/ENR + CLI override. |
| src/app/configurator.cpp | Add CLI flags for aggregator role and subnet count. |
| src/app/configuration.hpp | Add accessors for CLI aggregator role and subnet count. |
| src/app/configuration.cpp | Implement new CLI accessors. |
| src/app/chain_spec.hpp | Extend chain spec interface with isAggregator(). |
| src/app/bootnodes.hpp | Extend BootnodeInfo with is_aggregator. |
| src/app/bootnodes.cpp | Implement extended BootnodeInfo constructor. |
| example/1-network/README.md | Document --is-aggregator usage in example. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| } else if (time_.phase() == 2) { | ||
| SL_TRACE(logger_, "Interval 2 of slot {}: aggregate", current_slot); | ||
| if (is_aggregator_) { | ||
| auto aggregated_attestations = aggregateSignatures(); | ||
| for (auto &aggregated_attestation : aggregated_attestations) { | ||
| auto res = onGossipAggregatedAttestation(aggregated_attestation); | ||
| if (not res.has_value()) { | ||
| SL_WARN( | ||
| logger_, "failed to import own aggregation: {}", res.error()); | ||
| continue; | ||
| } | ||
| result.emplace_back(aggregated_attestation); | ||
| } | ||
| } |
There was a problem hiding this comment.
This PR introduces a new interval-2 aggregation path (aggregateSignatures() + onGossipAggregatedAttestation() import + broadcast). The existing fork-choice unit tests don’t appear to cover this new behavior (e.g., that only aggregators aggregate, that same-subnet signatures are collected, and that aggregated attestations update latest_*_attestations_ / block production input as intended). Adding a focused unit test around this interval-2 flow would help prevent regressions.
Signed-off-by: turuslan <turuslan.devbox@gmail.com> # Conflicts: # src/modules/networking/networking.cpp # src/modules/networking/networking.hpp
Signed-off-by: turuslan <turuslan.devbox@gmail.com>
Signed-off-by: turuslan <turuslan.devbox@gmail.com>
Signed-off-by: turuslan <turuslan.devbox@gmail.com>
Signed-off-by: turuslan <turuslan.devbox@gmail.com>
Signed-off-by: turuslan <turuslan.devbox@gmail.com>
Signed-off-by: turuslan <turuslan.devbox@gmail.com>
Signed-off-by: turuslan <turuslan.devbox@gmail.com>
Signed-off-by: turuslan <turuslan.devbox@gmail.com>
Signed-off-by: turuslan <turuslan.devbox@gmail.com>
enr.is_aggregator == truefrom same subnet (validator_index % subnet_count) ignoring connection limit.