node, networking, forkchoice: import-subnet-ids for subnet network topology#482
Conversation
…pology Add `import_subnet_ids` to control which attestation subnets a node subscribes to and collects signatures from, independently of aggregator role. Subscription changes (p2p layer): - Aggregator nodes subscribe to their validator-derived subnet only. - Non-aggregator nodes with no import subnets skip attestation subscriptions entirely, saving bandwidth on subnets they have no use for. - import_subnet_ids are always subscribed regardless of aggregator flag, allowing proposer nodes to gather attestations from specific subnets for block inclusion without enabling full aggregation mode. - The two sets are additive and deduplicated. Signature collection changes (forkchoice layer): - Aggregator path (existing): collects from validators sharing its subnet. - Import-subnet path (new): always collects from explicitly listed subnets regardless of is_aggregator. Enables proposers to gather raw signatures. The --import-subnet-ids CLI flag accepts comma-separated subnet IDs (e.g. "0,1,2") and is passed through run_node, _init_from_genesis, and _init_from_checkpoint into NodeConfig, SyncService, and NetworkService. Tests: three new cases in TestOnGossipAttestationSubnetFiltering covering import-only storage, subnet boundary enforcement, and additive combination with aggregator mode.
MockForkchoiceStore.on_gossip_attestation and the inline reject_unknown stub in the sync service tests were missing the import_subnet_ids keyword argument added to the real Store.on_gossip_attestation, causing TypeError in CI.
anshalshukla
left a comment
anshalshukla
left a comment
There was a problem hiding this comment.
The functionality is not what we had initially discussed, we want to add a flag to pass aggregate-subnet-ids so that a aggregator with a powerful machine can aggregate signatures from subnets outside it's default subnet. This flag should be protected for nodes with is_aggregator set.
Along with that the nodes should regardless of them being an aggregator should subscribe to their designated subnet as this would help create a mesh for the attestation gossiping, we should not overly rely on fanout for attestation gossiping. These attestations can be skipped and not be imported in the store though as for non aggregators populating store is not of much use at the moment although this can change in future.
My spec changes are based on this PR. blockblaz/zeam#685. However, I notice the discrepancy now. I am pushing a fix. |
Three behavioral changes to match the final merged zeam PR #685:
1. Subscription gating:
- All nodes with validators now subscribe to their validator-derived
subnets regardless of is_aggregator flag (forms mesh for propagation).
- aggregate_subnet_ids subscription is gated on is_aggregator (only
aggregators need explicit extra subnets for signature collection).
- Non-aggregator nodes with no validators skip attestation subscriptions.
2. Import gating (store + sync):
- Gossip attestation import is gated solely on is_aggregator.
- Remove the independent import_subnet_ids path in on_gossip_attestation
that stored signatures without aggregator role. No such path exists in
the zeam final code.
- on_gossip_attestation no longer accepts import_subnet_ids kwarg.
3. Rename import_subnet_ids -> aggregate_subnet_ids throughout:
- CLI flag: --import-subnet-ids -> --aggregate-subnet-ids
- NodeConfig, SyncService, NetworkService fields renamed.
- Tests updated: three import_subnet_ids tests replaced with two tests
that verify the correct aggregator-only import semantics.
…bnet-ids without aggregator Subnet filtering now happens at the p2p subscription layer. Anything that reaches on_gossip_attestation on an aggregator can be stored directly — no need to compare attester subnet against current validator. Also error out early in __main__ if --aggregate-subnet-ids is set without --is-aggregator, since that combination is meaningless. Update tests to reflect the new semantics: aggregators store all received attestations, non-aggregators drop all.
4 participants
Summary
import_subnet_idsto control which attestation subnets a node subscribes to and collects from, independently of the aggregator role.import_subnet_idsare always subscribed regardless of aggregator flag (for proposer nodes that need attestations for block inclusion); the two sets are additive and deduplicated.is_aggregator.--import-subnet-idsCLI flag (comma-separated, e.g."0,1,2") wired throughrun_node,_init_from_genesis,_init_from_checkpoint,NodeConfig,SyncService, andNetworkService.Test plan
uvx tox -e all-checkspasses (ruff, ty, codespell, mdformat)TestOnGossipAttestationSubnetFiltering:test_import_subnet_stores_without_aggregator— import subnets work withoutis_aggregatortest_import_subnet_ignored_for_other_subnets— only explicitly listed subnets are collectedtest_import_subnet_combined_with_aggregator— both paths are additive when used togetherTestOnGossipAttestationSubnetFilteringpass