From 53a711956eb5c5ffeef277b2a13850bd4911946b Mon Sep 17 00:00:00 2001 From: Akihito Nakano Date: Sat, 14 Mar 2026 03:27:15 +0900 Subject: [PATCH] Fix flaky `test_same_subnet_unsubscription` (#8932) Co-Authored-By: figtracer <1gusredo@gmail.com> Co-Authored-By: ackintosh --- beacon_node/network/src/subnet_service/mod.rs | 7 ---- .../network/src/subnet_service/tests/mod.rs | 34 +++++++++---------- 2 files changed, 16 insertions(+), 25 deletions(-) diff --git a/beacon_node/network/src/subnet_service/mod.rs b/beacon_node/network/src/subnet_service/mod.rs index be491e56d3..008e7ab9ac 100644 --- a/beacon_node/network/src/subnet_service/mod.rs +++ b/beacon_node/network/src/subnet_service/mod.rs @@ -198,13 +198,6 @@ impl SubnetService { self.permanent_attestation_subscriptions.iter() } - /// Returns whether we are subscribed to a subnet for testing purposes. - #[cfg(test)] - pub(crate) fn is_subscribed(&self, subnet: &Subnet) -> bool { - self.subscriptions.contains_key(subnet) - || self.permanent_attestation_subscriptions.contains(subnet) - } - /// Returns whether we are subscribed to a permanent subnet for testing purposes. #[cfg(test)] pub(crate) fn is_subscribed_permanent(&self, subnet: &Subnet) -> bool { diff --git a/beacon_node/network/src/subnet_service/tests/mod.rs b/beacon_node/network/src/subnet_service/tests/mod.rs index bee6569b7b..619154d738 100644 --- a/beacon_node/network/src/subnet_service/tests/mod.rs +++ b/beacon_node/network/src/subnet_service/tests/mod.rs @@ -335,28 +335,26 @@ mod test { // submit the subscriptions subnet_service.validator_subscriptions(vec![sub1, sub2].into_iter()); - // Unsubscription event should happen at slot 2 (since subnet id's are the same, unsubscription event should be at higher slot + 1) - let expected = SubnetServiceMessage::Subscribe(Subnet::Attestation(subnet_id1)); + let subnet = Subnet::Attestation(subnet_id1); - if subnet_service.is_subscribed(&Subnet::Attestation(subnet_id1)) { - // If we are permanently subscribed to this subnet, we won't see a subscribe message - let _ = get_events_until_num_slots(&mut subnet_service, None, 1).await; + if subnet_service.is_subscribed_permanent(&subnet) { + // If permanently subscribed, no Subscribe/Unsubscribe events will be generated + let events = get_events_until_num_slots(&mut subnet_service, None, 3).await; + assert!(events.is_empty()); } else { - let subscription = get_events_until_num_slots(&mut subnet_service, None, 1).await; - assert_eq!(subscription, [expected]); + // Wait 1 slot: expect a single Subscribe event (no duplicate for the same subnet). + let events = get_events_until_num_slots(&mut subnet_service, None, 1).await; + assert_eq!(events, [SubnetServiceMessage::Subscribe(subnet)]); + + // Wait for the Unsubscribe event after subscription_slot2 expires. + // Use a longer timeout because the test doesn't start exactly at a slot + // boundary, so the previous 1-slot wait may end partway through slot 1, + // leaving insufficient time to catch the Unsubscribe within another 1 slot. + let events = get_events_until_num_slots(&mut subnet_service, Some(1), 3).await; + assert_eq!(events, [SubnetServiceMessage::Unsubscribe(subnet)]); } - // Get event for 1 more slot duration, we should get the unsubscribe event now. - let unsubscribe_event = get_events_until_num_slots(&mut subnet_service, None, 1).await; - - // If the long lived and short lived subnets are different, we should get an unsubscription - // event. - let expected = SubnetServiceMessage::Unsubscribe(Subnet::Attestation(subnet_id1)); - if !subnet_service.is_subscribed(&Subnet::Attestation(subnet_id1)) { - assert_eq!([expected], unsubscribe_event[..]); - } - - // Should no longer be subscribed to any short lived subnets after unsubscription. + // Should no longer be subscribed to any short lived subnets after unsubscription. assert_eq!(subnet_service.subscriptions().count(), 0); }