Skip to content

Commit 0fda8d4

Browse files
committed
fix: block outgoing messages during entire doppelgänger protection window
Fixes critical bug where outgoing messages were not blocked during grace period, allowing competition with potential twin operators and defeating the purpose of doppelgänger protection. Problem: - is_monitoring() returned false during grace period - Message sender only checked is_monitoring() - Node sent messages during grace period, competing with potential twins - Validation masked duplicate messages (only one propose per round accepted) - Twin detection could fail Solution: - Add is_active() method checking both GracePeriod and Monitoring states - Update message sender to use is_active() instead of is_monitoring() - Block ALL outgoing messages during entire protection window - Keep is_monitoring() for detection logic (only active after grace period) Behavior: - GracePeriod: Block outgoing messages, skip twin detection - Monitoring: Block outgoing messages, actively detect twins - Completed: Allow messages, skip detection This addresses feedback in PR #692 discussion r2459937843 about validation masking and the need to focus on message blocking during startup. Tests: - Added test_state_is_active() to verify state machine behavior - Added test_service_is_active_during_grace_period() for service API - All 9 operator_doppelganger tests passing
1 parent ff3c4a9 commit 0fda8d4

File tree

2 files changed

+67
-7
lines changed

2 files changed

+67
-7
lines changed

anchor/message_sender/src/network.rs

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,13 @@ impl<S: SlotClock + 'static, D: DutiesProvider> MessageSender for Arc<NetworkMes
5353
committee_id: CommitteeId,
5454
additional_message_callback: Option<Box<MessageCallback>>,
5555
) -> Result<(), Error> {
56-
// Check if in doppelgänger monitoring period - silently drop
56+
// Check if doppelgänger protection is active - block outgoing messages
57+
// This includes both grace period (waiting for old messages to expire) and
58+
// monitoring period (actively detecting twins)
5759
if let Some(dg) = &self.doppelganger_service
58-
&& dg.is_monitoring()
60+
&& dg.is_active()
5961
{
60-
trace!("Dropping message send - in doppelgänger monitoring period");
62+
trace!("Dropping message send - doppelgänger protection active");
6163
return Ok(());
6264
}
6365

@@ -106,11 +108,13 @@ impl<S: SlotClock + 'static, D: DutiesProvider> MessageSender for Arc<NetworkMes
106108
}
107109

108110
fn send(&self, message: SignedSSVMessage, committee_id: CommitteeId) -> Result<(), Error> {
109-
// Check if in doppelgänger monitoring period - silently drop
111+
// Check if doppelgänger protection is active - block outgoing messages
112+
// This includes both grace period (waiting for old messages to expire) and
113+
// monitoring period (actively detecting twins)
110114
if let Some(dg) = &self.doppelganger_service
111-
&& dg.is_monitoring()
115+
&& dg.is_active()
112116
{
113-
trace!("Dropping message send - in doppelgänger monitoring period");
117+
trace!("Dropping message send - doppelgänger protection active");
114118
return Ok(());
115119
}
116120

anchor/operator_doppelganger/src/service.rs

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,11 +66,19 @@ impl DoppelgangerState {
6666
Self::GracePeriod
6767
}
6868

69-
/// Check if actively monitoring
69+
/// Check if actively monitoring (excludes grace period)
7070
const fn is_monitoring(self) -> bool {
7171
matches!(self, Self::Monitoring)
7272
}
7373

74+
/// Check if doppelgänger protection is active (includes grace period and monitoring)
75+
///
76+
/// Returns `true` during the entire protection window (grace period + monitoring).
77+
/// Use this to block outgoing messages during startup to prevent competition with twins.
78+
const fn is_active(self) -> bool {
79+
matches!(self, Self::GracePeriod | Self::Monitoring)
80+
}
81+
7482
/// Transition from grace period to monitoring
7583
fn end_grace_period(&mut self) {
7684
*self = Self::Monitoring;
@@ -247,6 +255,17 @@ impl OperatorDoppelgangerService {
247255
pub fn is_monitoring(&self) -> bool {
248256
self.state.read().is_monitoring()
249257
}
258+
259+
/// Check if doppelgänger protection is active
260+
///
261+
/// Returns `true` during the entire protection window (grace period + monitoring).
262+
/// Returns `false` after monitoring completes.
263+
///
264+
/// Use this to determine if outgoing messages should be blocked to prevent
265+
/// competition with potential twin operators during startup.
266+
pub fn is_active(&self) -> bool {
267+
self.state.read().is_active()
268+
}
250269
}
251270

252271
#[cfg(test)]
@@ -392,6 +411,43 @@ mod tests {
392411
assert!(!state.is_monitoring());
393412
}
394413

414+
#[test]
415+
fn test_state_is_active() {
416+
let mut state = DoppelgangerState::new();
417+
418+
// Grace period: is_active should be true, is_monitoring should be false
419+
assert!(state.is_active(), "Should be active during grace period");
420+
assert!(
421+
!state.is_monitoring(),
422+
"Should not be monitoring during grace period"
423+
);
424+
425+
// Transition to monitoring
426+
state.end_grace_period();
427+
assert!(state.is_active(), "Should be active during monitoring");
428+
assert!(state.is_monitoring(), "Should be monitoring");
429+
430+
// Complete monitoring
431+
state.end_monitoring();
432+
assert!(!state.is_active(), "Should not be active after completion");
433+
assert!(
434+
!state.is_monitoring(),
435+
"Should not be monitoring after completion"
436+
);
437+
}
438+
439+
#[test]
440+
fn test_service_is_active_during_grace_period() {
441+
let service = create_service();
442+
443+
// Start in grace period - should be active but not monitoring
444+
assert!(service.is_active(), "Should be active during grace period");
445+
assert!(
446+
!service.is_monitoring(),
447+
"Should not be monitoring during grace period"
448+
);
449+
}
450+
395451
#[test]
396452
fn test_service_creation() {
397453
let service = create_service();

0 commit comments

Comments
 (0)