From 47d7dff178ba2806a085d1d608fddd8425e856b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Thu, 16 Apr 2026 17:24:33 -0300 Subject: [PATCH 1/4] fix: subscribe non-aggregator validators to their subnets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Non-aggregator validator nodes were not joining the attestation mesh, so their attestations could only propagate via gossipsub fanout. In small devnets this collapses the mesh whenever peer churn thins out the single subscriber — observed as a permanent finalization stall with three validators voting three different targets. Align with leanSpec (src/lean_spec/__main__.py): every validator subscribes to its own subnet; aggregators additionally subscribe to explicit aggregate-subnet-ids with a fallback to subnet 0 when they have no validators of their own. Also restores the pre-regression semantics of the lean_attestation_committee_subnet metric, which should reflect this node's committee membership and therefore derive from validator subnets only, not from aggregator-only subscriptions. Regression introduced by #265; prior correct behavior was in #249. --- crates/net/p2p/src/lib.rs | 60 ++++++++++++++++----------------------- 1 file changed, 25 insertions(+), 35 deletions(-) diff --git a/crates/net/p2p/src/lib.rs b/crates/net/p2p/src/lib.rs index 134efe28..638762e7 100644 --- a/crates/net/p2p/src/lib.rs +++ b/crates/net/p2p/src/lib.rs @@ -206,49 +206,39 @@ pub fn build_swarm( .subscribe(&aggregation_topic) .unwrap(); - // Aggregators subscribe to attestation subnets to receive unaggregated - // attestations. Non-aggregators don't need to subscribe; they publish via - // gossipsub fanout. + // Subscribe to attestation subnets per leanSpec (`src/lean_spec/__main__.py`): + // every validator subscribes to its own subnet for mesh health; aggregators + // additionally subscribe to explicit `aggregate_subnet_ids` and fall back to + // subnet 0 when they have no validators of their own. + let validator_subnets: HashSet = config + .validator_ids + .iter() + .map(|vid| vid % config.attestation_committee_count) + .collect(); + + // The committee metric should reflect validator membership only, not + // aggregator-only subscriptions. + let metric_subnet = validator_subnets.iter().copied().min().unwrap_or(0); + metrics::set_attestation_committee_subnet(metric_subnet); + + let mut subscription_subnets = validator_subnets; if config.is_aggregator { - let mut aggregate_subnets: HashSet = config - .validator_ids - .iter() - .map(|vid| vid % config.attestation_committee_count) - .collect(); - if let Some(ref explicit_ids) = config.aggregate_subnet_ids { - aggregate_subnets.extend(explicit_ids); - } - // Aggregator with no validators and no explicit subnets: fallback to subnet 0 - if aggregate_subnets.is_empty() { - aggregate_subnets.insert(0); + if subscription_subnets.is_empty() { + subscription_subnets.insert(0); } - for &subnet_id in &aggregate_subnets { - let topic = attestation_subnet_topic(subnet_id); - swarm.behaviour_mut().gossipsub.subscribe(&topic)?; - info!(subnet_id, "Subscribed to attestation subnet"); + if let Some(ref explicit_ids) = config.aggregate_subnet_ids { + subscription_subnets.extend(explicit_ids); } } - // Build topic cache (avoids string allocation on every publish). - // Includes validator subnets and any explicit aggregate_subnet_ids. let mut attestation_topics: HashMap = HashMap::new(); - for &vid in &config.validator_ids { - let subnet_id = vid % config.attestation_committee_count; - attestation_topics - .entry(subnet_id) - .or_insert_with(|| attestation_subnet_topic(subnet_id)); - } - if let Some(ref explicit_ids) = config.aggregate_subnet_ids { - for &subnet_id in explicit_ids { - attestation_topics - .entry(subnet_id) - .or_insert_with(|| attestation_subnet_topic(subnet_id)); - } + for &subnet_id in &subscription_subnets { + let topic = attestation_subnet_topic(subnet_id); + swarm.behaviour_mut().gossipsub.subscribe(&topic)?; + info!(subnet_id, "Subscribed to attestation subnet"); + attestation_topics.insert(subnet_id, topic); } - let metric_subnet = attestation_topics.keys().copied().min().unwrap_or(0); - metrics::set_attestation_committee_subnet(metric_subnet); - info!(socket=%config.listening_socket, "P2P node started"); Ok(BuiltSwarm { From 887ee2e8f3f9d5769ea2df00ed91fcdcc290ad66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Thu, 16 Apr 2026 17:30:33 -0300 Subject: [PATCH 2/4] fix(blockchain): drop warning for unaggregated attestations on non-aggregators MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Non-aggregator nodes subscribe to their attestation subnet so they participate in the gossipsub mesh. They will therefore receive unaggregated attestations from peers, which is expected — only aggregators need to store and aggregate them. The warning was high-volume noise; the early return stays, but the log is removed. --- crates/blockchain/src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/blockchain/src/lib.rs b/crates/blockchain/src/lib.rs index e03865cf..194b2a2e 100644 --- a/crates/blockchain/src/lib.rs +++ b/crates/blockchain/src/lib.rs @@ -471,8 +471,10 @@ impl BlockChainServer { } fn on_gossip_attestation(&mut self, attestation: &SignedAttestation) { + // Non-aggregators subscribe to their attestation subnet for mesh health, + // so they see unaggregated attestations arrive. Only aggregators need to + // store and aggregate them. if !self.is_aggregator { - warn!("Received unaggregated attestation but node is not an aggregator"); return; } let _ = store::on_gossip_attestation(&mut self.store, attestation) From 805eb3797505c8898623e529d323c0eec2ad294c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Thu, 16 Apr 2026 17:36:34 -0300 Subject: [PATCH 3/4] fix(blockchain): validate and verify on non-aggregators, skip store only leanSpec's on_gossip_attestation validates the attestation data and verifies the XMSS signature on every node that receives the message. Only the signature storage step (used later for aggregation at interval 2) is gated by is_aggregator. Non-aggregators validate, verify, and drop. Previous commit short-circuited before validation to save XMSS cost; aligning with the spec so bad signatures are still caught at the edge. The is_aggregator flag now flows into store::on_gossip_attestation. --- crates/blockchain/src/lib.rs | 10 ++-------- crates/blockchain/src/store.rs | 18 ++++++++++++------ 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/crates/blockchain/src/lib.rs b/crates/blockchain/src/lib.rs index 194b2a2e..9ee4ee8e 100644 --- a/crates/blockchain/src/lib.rs +++ b/crates/blockchain/src/lib.rs @@ -198,7 +198,7 @@ impl BlockChainServer { // this the aggregator never sees its own validator's signature in // gossip_signatures and it is excluded from aggregated proofs. if self.is_aggregator { - let _ = store::on_gossip_attestation(&mut self.store, &signed_attestation) + let _ = store::on_gossip_attestation(&mut self.store, &signed_attestation, true) .inspect_err(|err| { warn!(%slot, %validator_id, %err, "Self-delivery of attestation failed") }); @@ -471,13 +471,7 @@ impl BlockChainServer { } fn on_gossip_attestation(&mut self, attestation: &SignedAttestation) { - // Non-aggregators subscribe to their attestation subnet for mesh health, - // so they see unaggregated attestations arrive. Only aggregators need to - // store and aggregate them. - if !self.is_aggregator { - return; - } - let _ = store::on_gossip_attestation(&mut self.store, attestation) + let _ = store::on_gossip_attestation(&mut self.store, attestation, self.is_aggregator) .inspect_err(|err| warn!(%err, "Failed to process gossiped attestation")); } diff --git a/crates/blockchain/src/store.rs b/crates/blockchain/src/store.rs index 63905c53..04d3c0c6 100644 --- a/crates/blockchain/src/store.rs +++ b/crates/blockchain/src/store.rs @@ -528,11 +528,14 @@ pub fn on_tick( /// Process a gossiped attestation with signature verification. /// -/// Verifies the validator's XMSS signature and stores it for later aggregation -/// at interval 2. Only aggregator nodes receive unaggregated gossip attestations. +/// Every subscriber validates the attestation data and verifies the XMSS +/// signature so invalid messages get caught at the edge. Only aggregators +/// store the signature for later aggregation at interval 2; non-aggregators +/// drop it after verification. pub fn on_gossip_attestation( store: &mut Store, signed_attestation: &SignedAttestation, + is_aggregator: bool, ) -> Result<(), StoreError> { let validator_id = signed_attestation.validator_id; let attestation = Attestation { @@ -570,10 +573,13 @@ pub fn on_gossip_attestation( } metrics::inc_pq_sig_attestation_signatures_valid(); - // Store gossip signature unconditionally for later aggregation at interval 2. - // Subnet filtering is handled at the P2P subscription layer. - store.insert_gossip_signature(hashed, validator_id, signature); - metrics::update_gossip_signatures(store.gossip_signatures_count()); + // Only aggregators persist the signature for later aggregation at + // interval 2. Non-aggregators drop the validated attestation — they + // still participate in the mesh so peers see the message propagate. + if is_aggregator { + store.insert_gossip_signature(hashed, validator_id, signature); + metrics::update_gossip_signatures(store.gossip_signatures_count()); + } metrics::inc_attestations_valid(1); From 6610198163ec2687f9fc0ed3bb507a7e751afa04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Thu, 16 Apr 2026 18:15:38 -0300 Subject: [PATCH 4/4] fix(p2p): apply aggregator subnet-0 fallback after explicit ids merge An aggregator with no validators but with --aggregate-subnet-ids was picking up subnet 0 in addition to its explicit subnets, because the emptiness check fired before aggregate_subnet_ids were folded in. Move the fallback to run after the extend so it only applies when the set is truly empty, matching the pre-#265 semantics. --- crates/net/p2p/src/lib.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/net/p2p/src/lib.rs b/crates/net/p2p/src/lib.rs index 638762e7..0298a54a 100644 --- a/crates/net/p2p/src/lib.rs +++ b/crates/net/p2p/src/lib.rs @@ -223,12 +223,14 @@ pub fn build_swarm( let mut subscription_subnets = validator_subnets; if config.is_aggregator { - if subscription_subnets.is_empty() { - subscription_subnets.insert(0); - } if let Some(ref explicit_ids) = config.aggregate_subnet_ids { subscription_subnets.extend(explicit_ids); } + // Fall back to subnet 0 only when the aggregator has no validators + // and no explicit subnets — otherwise leave the set as configured. + if subscription_subnets.is_empty() { + subscription_subnets.insert(0); + } } let mut attestation_topics: HashMap = HashMap::new();