From 37e71d759e5bdf64031f02bf98e83ab82f98274d Mon Sep 17 00:00:00 2001 From: Tomer Weller Date: Fri, 21 Aug 2026 23:04:38 +0000 Subject: [PATCH 1/3] =?UTF-8?q?Regression=20test=20for=20#3723=20=E2=80=94?= =?UTF-8?q?=20fails=20on=20current=20main?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Assert that relaying a ready SCP envelope stamps event-loop phase=3 (broadcast) with a non-zero sub-phase. Fails on main: there is no App::relay_ready_scp_envelope and the inline relay arm never calls set_phase(3), so a freeze in the relay path is misattributed to phase=0 "waiting". Refs #3723 Co-authored-by: Claude Code --- crates/app/src/app/lifecycle.rs | 42 +++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/crates/app/src/app/lifecycle.rs b/crates/app/src/app/lifecycle.rs index f1974a34..372361ad 100644 --- a/crates/app/src/app/lifecycle.rs +++ b/crates/app/src/app/lifecycle.rs @@ -3605,6 +3605,48 @@ mod scp_dedup_pipeline_tests { assert_eq!(app.scp_scheduled.dedup_count(), 0, "no dedup rejections"); } + /// Regression (#3723): relaying a ready SCP envelope must stamp the event + /// loop phase as 3 ("broadcast") with a non-zero phase-3 sub-phase. + /// + /// The 07-12 mainnet freeze (~101 s) happened in the relay-broadcast path, + /// but the deployed build's `phase=3 broadcast` stamp was later lost: the + /// generic SCP-relay arm broadcasts without any `set_phase` call, and the + /// loop top stamps `set_phase(0)` before every `select!`, so a freeze in + /// the relay path would be misattributed to `phase=0 "waiting"`. This test + /// pins the invariant "relaying a ready SCP envelope marks the loop phase=3 + /// (broadcast)". It FAILS on `origin/main` (no `relay_ready_scp_envelope`, + /// phase stays 0). + /// + /// `set_phase(3)` and `set_phase_sub(PHASE_3_1_OVERLAY_READ)` are stamped + /// before the `if let Some(overlay)` guard, so the invariant holds even on + /// a fresh App whose overlay is unset (`None`). + #[tokio::test] + async fn test_relay_ready_scp_envelope_stamps_broadcast_phase() { + use henyey_herder::{ReadyPath, ScpRelayEnvelope}; + + let dir = tempfile::tempdir().expect("temp dir"); + let db_path = dir.path().join("rs-stellar-relay-phase.db"); + let config = crate::config::ConfigBuilder::new() + .database_path(db_path) + .build(); + let app = App::new(config).await.unwrap(); + + // Fresh App has no overlay wired, so relay_ready_scp_envelope returns + // early after stamping phase 3 + a phase-3 sub-phase. + assert!(app.overlay().await.is_none(), "fresh app has no overlay"); + + let relay_env = ScpRelayEnvelope { + envelope: make_test_envelope(100, 2_000_000_000), + received_at: None, + ready_path: ReadyPath::Immediate, + }; + app.relay_ready_scp_envelope(relay_env).await; + + let (phase, sub) = app.phase_snapshot_for_test(); + assert_eq!(phase, 3, "relay path must stamp phase=3 (broadcast)"); + assert_ne!(sub, 0, "relay path must stamp a non-zero phase-3 sub-phase"); + } + /// #3625 (app-side consumer touch): when the SCP consumer drains a /// token-bearing `OverlayMessage` via `pump_scp_intake`, the attached /// `FlowControlRelease` fires `end_message_processing`. Draining a full From b3a4cff482cfe01ae933a56cfc4bdea3e82a9e0a Mon Sep 17 00:00:00 2001 From: Tomer Weller Date: Fri, 21 Aug 2026 23:11:50 +0000 Subject: [PATCH 2/3] Restore phase=3 broadcast watchdog stamp on the SCP-relay path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The generic SCP-relay arm (fetching_relay_rx) — the sole relay path for all fetched-ready SCP envelopes — broadcast without any phase stamp, while the loop top stamps set_phase(0) before every select!. So a freeze in the relay path (the ~101 s 2026-07-12 mainnet freeze class) would be misattributed to phase=0 "waiting" instead of phase=3 "broadcast". Extract the arm body into App::relay_ready_scp_envelope (single source of truth, unit-testable without spinning the select! loop). Stamp phase=3 plus a 3.1/3.2 sub-phase split (overlay RwLock read vs in-broadcast work) and time the broadcast with warn_if_slow, so the next occurrence names the exact await it parked on. Relay semantics (message construction, receive-to-relay histogram guard/label, Failed-to-relay warn) are moved verbatim — diagnostics only, no observable-surface change. Also correct the stale #3218 cross-reference in monitor-tick (3e) docs to #3723 (the live broadcast-freeze tracker). Refs #3723 Co-authored-by: Claude Code --- .claude/skills/monitor-tick/SKILL.md | 6 +-- crates/app/src/app/lifecycle.rs | 81 ++++++++++++++++++---------- crates/app/src/app/mod.rs | 29 ++++++++++ crates/app/src/app/phase.rs | 30 +++++++++++ 4 files changed, 114 insertions(+), 32 deletions(-) diff --git a/.claude/skills/monitor-tick/SKILL.md b/.claude/skills/monitor-tick/SKILL.md index e855a482..345664d7 100644 --- a/.claude/skills/monitor-tick/SKILL.md +++ b/.claude/skills/monitor-tick/SKILL.md @@ -868,7 +868,7 @@ the residual mode left after (3c) (owns the fatal-state-wipe case) and (3b) (owns the UNRESPONSIVE / frozen-event-loop case). (3e) is the **mirror of (3b)**: (3b) fires when the admin port is dead/timed-out; (3e) requires a *live, answering* port — mutually exclusive by construction. It is a band-aid for root -cause #3218 (overlay SCP broadcast backpressure); the escalate-on-streak guard +cause #3723 (overlay SCP broadcast backpressure); the escalate-on-streak guard surfaces a restart loop as `urgent` rather than silently masking the defect. Evaluate (3e) **strictly last** in the alive-process path — order is @@ -946,8 +946,8 @@ cooldown** (`NOW - last_restart < 900` → `cooldown`, no restart this tick) and the legitimate replay that follows the restart. - On `STUCK_ALIVE_SYNC=escalate`: do **NOT** restart. File a single `urgent`-labeled issue (reusing the (3b)-wedge filing idiom) noting the - restart streak and pointing at the unfixed root cause #3218 (overlay SCP - broadcast backpressure / near-tip-stall recovery); if #3218 is closed, + restart streak and pointing at the unfixed root cause #3723 (overlay SCP + broadcast backpressure / near-tip-stall recovery); if #3723 is closed, escalate to the operator. A restart loop is itself the signal that the root cause is unaddressed. - On `STUCK_ALIVE_SYNC=cooldown` or `no`: report-only, no action this tick. diff --git a/crates/app/src/app/lifecycle.rs b/crates/app/src/app/lifecycle.rs index 372361ad..70b98339 100644 --- a/crates/app/src/app/lifecycle.rs +++ b/crates/app/src/app/lifecycle.rs @@ -1052,35 +1052,7 @@ impl App { // from FetchingEnvelopes (both immediate-ready and deferred-ready). // Parity: stellar-core PendingEnvelopes::envelopeReady(). Some(relay_env) = fetching_relay_rx.recv() => { - let slot = relay_env.envelope.statement.slot_index; - let received_at = relay_env.received_at; - let ready_path = relay_env.ready_path; - let relay_msg = StellarMessage::ScpMessage(relay_env.envelope); - if let Some(overlay) = self.overlay().await { - match overlay.broadcast(relay_msg).await { - Ok(count) => { - tracing::debug!(slot, peers = count, "Relayed SCP envelope"); - // Record receive-to-relay latency (#2648). - // Only sample on successful broadcast to ≥1 peer. - if count > 0 { - if let Some(t) = received_at { - let label = match ready_path { - henyey_herder::ReadyPath::Immediate => "immediate", - henyey_herder::ReadyPath::Deferred => "deferred", - }; - metrics::histogram!( - "henyey_scp_receive_to_relay_seconds", - "path" => label - ) - .record(t.elapsed().as_secs_f64()); - } - } - } - Err(e) => { - tracing::warn!(slot, error = %e, "Failed to relay SCP envelope"); - } - } - } + self.relay_ready_scp_envelope(relay_env).await; } // Process non-critical overlay messages (TX floods, etc.). @@ -1705,6 +1677,57 @@ impl App { Ok(()) } + /// Relay a fetched-ready SCP envelope to peers — the sole relay path for + /// all SCP envelopes (both immediate-ready and deferred-ready), driven by + /// the `fetching_relay_rx` arm of the event loop. + /// + /// Parity: stellar-core `PendingEnvelopes::envelopeReady()`. + /// + /// Watchdog attribution (#3723): stamps `phase=3` ("broadcast") and a + /// phase-3 sub-phase around the two `.await` points, so the ~101 s freeze + /// class observed on 2026-07-12 is captured at its true location instead + /// of being misattributed to `phase=0 "waiting"` (the loop-top stamp). + /// `set_phase(3)` + `set_phase_sub(PHASE_3_1_OVERLAY_READ)` are stamped + /// before the overlay guard so even the overlay-unset path records the + /// broadcast phase. Behavior of the relay itself is unchanged. + async fn relay_ready_scp_envelope(&self, relay_env: henyey_herder::ScpRelayEnvelope) { + self.set_phase(3); // 3 = broadcast + let slot = relay_env.envelope.statement.slot_index; + let received_at = relay_env.received_at; + let ready_path = relay_env.ready_path; + let relay_msg = StellarMessage::ScpMessage(relay_env.envelope); + self.set_phase_sub(super::phase::PHASE_3_1_OVERLAY_READ); + if let Some(overlay) = self.overlay().await { + self.set_phase_sub(super::phase::PHASE_3_2_BROADCAST); + let broadcast_start = std::time::Instant::now(); + let result = overlay.broadcast(relay_msg).await; + super::warn_if_slow(broadcast_start.elapsed(), "scp_relay_broadcast", 1); + match result { + Ok(count) => { + tracing::debug!(slot, peers = count, "Relayed SCP envelope"); + // Record receive-to-relay latency (#2648). + // Only sample on successful broadcast to ≥1 peer. + if count > 0 { + if let Some(t) = received_at { + let label = match ready_path { + henyey_herder::ReadyPath::Immediate => "immediate", + henyey_herder::ReadyPath::Deferred => "deferred", + }; + metrics::histogram!( + "henyey_scp_receive_to_relay_seconds", + "path" => label + ) + .record(t.elapsed().as_secs_f64()); + } + } + } + Err(e) => { + tracing::warn!(slot, error = %e, "Failed to relay SCP envelope"); + } + } + } + } + /// Dispatches the periodic known-peers refresh off the event-loop thread, /// mirroring dispatch_peer_maintenance's use for phase 28 (#3689). /// diff --git a/crates/app/src/app/mod.rs b/crates/app/src/app/mod.rs index 3034471c..9b5ccad0 100644 --- a/crates/app/src/app/mod.rs +++ b/crates/app/src/app/mod.rs @@ -10534,6 +10534,35 @@ mod tests { /// All `PHASE_13_*` sub-phase constants are distinct and within a /// sensible range. Prevents accidental constant collision during /// future edits. + /// All `PHASE_3_*` sub-phase constants are distinct and densely numbered. + /// Mirrors the `PHASE_6_*` / `PHASE_13_*` tests. Prevents accidental + /// constant collision during future edits (issue #3723). + #[test] + fn test_phase_3_constants_distinct_and_dense() { + use super::phase::*; + let all = [PHASE_3_1_OVERLAY_READ, PHASE_3_2_BROADCAST]; + let mut sorted = all.to_vec(); + sorted.sort_unstable(); + sorted.dedup(); + assert_eq!( + sorted.len(), + all.len(), + "phase-3 sub-phase constants must all be distinct" + ); + assert_eq!(sorted.first().copied(), Some(1)); + assert_eq!( + sorted.last().copied(), + Some(max_defined_phase_3_sub_phase()) + ); + for (i, v) in sorted.iter().enumerate() { + assert_eq!( + *v, + (i as u32) + 1, + "phase-3 sub-phase constants must be densely numbered 1..=N" + ); + } + } + #[test] fn test_phase_13_constants_distinct_and_dense() { use super::phase::*; diff --git a/crates/app/src/app/phase.rs b/crates/app/src/app/phase.rs index e000bcda..16ce88d5 100644 --- a/crates/app/src/app/phase.rs +++ b/crates/app/src/app/phase.rs @@ -16,6 +16,17 @@ //! The `PHASE_6_*` constants below provide coarse attribution within //! phase 6, narrowing a stall to one of 12 sub-regions. //! +//! ## Phase 3 (`broadcast`) +//! +//! Issue #3723 context: the coarse `phase=3 broadcast` label covers the +//! SCP-relay arm in `App::relay_ready_scp_envelope` (lifecycle.rs) — the +//! sole relay path for all fetched-ready SCP envelopes. The ~101 s +//! event-loop freeze on 2026-07-12 landed in this path but was later at +//! risk of being misattributed to `phase=0 waiting` because the relay arm +//! carried no phase stamp. The `PHASE_3_*` constants below split the two +//! `.await` points so the next freeze names whether the loop parked on the +//! overlay `RwLock` read or inside `broadcast` itself. +//! //! ## Phase 13 (`buffered_catchup`) //! //! Issue #1788 context: the coarse `phase=13 buffered_catchup` label @@ -30,6 +41,25 @@ //! Convention: constants are one-based dense integers within a coarse //! phase. Zero means "coarse phase entered, sub-phase not yet set". +// ── Phase 3: broadcast (issue #3723) ───────────────────────────────── + +/// `relay_ready_scp_envelope`: about to acquire the overlay handle via +/// `self.overlay().await` (a read on the overlay `RwLock`) before relaying. +/// A park here points at a long-held overlay writer (startup wiring / +/// shutdown `take()`). +pub(crate) const PHASE_3_1_OVERLAY_READ: u32 = 1; + +/// `relay_ready_scp_envelope`: about to call `overlay.broadcast(...).await` +/// for the ready SCP envelope. A park here points inside `broadcast` +/// (flood-gate accounting / `DashMap` contention), not the overlay read. +pub(crate) const PHASE_3_2_BROADCAST: u32 = 2; + +/// Test helper: return the highest phase-3 sub-phase constant. +#[cfg(test)] +pub(crate) const fn max_defined_phase_3_sub_phase() -> u32 { + PHASE_3_2_BROADCAST +} + // ── Phase 6: pending_close (issue #1921) ───────────────────────────── /// `handle_close_complete_inner`: about to acquire From 022b6600314ab5fccd396a12a6c14133606149fd Mon Sep 17 00:00:00 2001 From: Tomer Weller Date: Sat, 22 Aug 2026 01:06:56 +0000 Subject: [PATCH 3/3] Address review feedback - Fix doc-comment misplacement in crates/app/src/app/mod.rs: the PHASE_13 distinctness-test doc block was left attached to the new test_phase_3_constants_distinct_and_dense test (doubled, self- contradictory doc) while test_phase_13_constants_distinct_and_dense had none. Restored each doc block to its own test. Refs #3723 Co-authored-by: Claude Code --- crates/app/src/app/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/app/src/app/mod.rs b/crates/app/src/app/mod.rs index 9b5ccad0..5052fca2 100644 --- a/crates/app/src/app/mod.rs +++ b/crates/app/src/app/mod.rs @@ -10531,9 +10531,6 @@ mod tests { ); } - /// All `PHASE_13_*` sub-phase constants are distinct and within a - /// sensible range. Prevents accidental constant collision during - /// future edits. /// All `PHASE_3_*` sub-phase constants are distinct and densely numbered. /// Mirrors the `PHASE_6_*` / `PHASE_13_*` tests. Prevents accidental /// constant collision during future edits (issue #3723). @@ -10563,6 +10560,9 @@ mod tests { } } + /// All `PHASE_13_*` sub-phase constants are distinct and within a + /// sensible range. Prevents accidental constant collision during + /// future edits. #[test] fn test_phase_13_constants_distinct_and_dense() { use super::phase::*;