Release: develop -> main - #127
Merged
Merged
Conversation
…s watchdog reconnects
On Mainnet (10 min mean block time), the existing 90 s liveness watchdog
fires every ~2 min on quiet stretches between blocks because nothing
flows on the WebSocket between block events. The scanner reconnects ~27
times per hour — operationally harmless (HTTP fallback handles block
fetches, scanner recovers) but noisy and trips log alerts.
RFC 6455 §5.5 mandates the peer respond to every Ping with a Pong; the
Pong arrives on the same reader the watchdog observes. Adding a periodic
client-side Ping makes the watchdog reset every ~30 s regardless of
block-event cadence, restoring its original meaning ("no pong + no event
in 90 s = connection genuinely dead, reconnect").
- New `DEFAULT_PING_INTERVAL: Duration = 30s` and `ScannerWsConfig::ping_interval`.
- Compile-time assertion that `ping_interval * 2 < liveness_timeout` so any
future tweak to either constant trips the build instead of silently
drifting back into the spurious-reconnect regime.
- `connect_and_drain` now splits the WebSocketStream (`StreamExt::split`)
and runs reader + ping ticker inside one `tokio::select!`. The liveness
deadline is tracked manually via `tokio::time::sleep_until(deadline)`
rather than wrapping each `next()` in `timeout(...)`, so select-arm
cancellation cannot silently reset the watchdog (only inbound frames do).
- Marker `scanner-polling-ok:` added on the `tokio::time::interval` /
`sleep_until` lines per the CI lint enforcing CONTRIBUTING.md § "No
polling — events only".
Tests:
- `ping_interval_is_strictly_below_half_liveness_timeout` — const sanity.
- `run_scanner_ws_pongs_keep_connection_alive_past_liveness_timeout` — quiet
server (no blocks, draining reader auto-pongs); scanner stays on a single
connection through multiple watchdog windows.
- `run_scanner_ws_watchdog_fires_when_pongs_are_dropped` — server stops
reading after subscribe; no pongs come back; watchdog still fires and
the scanner reconnects (count ≥ 2 in a 1.5 s window).
Existing tests get an explicit `ping_interval` field; values are set well
above the test budget so the keepalive does not interfere with the
behaviour each test is targeting.
…ect path Three findings from independent review of the keepalive PR: 1. MAJOR - sink.send(Ping).await inside tokio::select! was not cancel-safe. SinkExt::send is not cancel-safe by contract: if the read arm wins a race against a half-completed send, the future is dropped and the sink can be left in a torn state mid-frame. Risk was low in practice (tiny payload, biased branch order) but a real correctness footgun. Switched to a dedicated writer task that owns the SplitSink and drains a 1-slot tokio::sync::mpsc::Receiver<WsMessage> via feed + flush. The main loop's ping arm now does out_tx.send(msg) which IS cancel-safe (documented), and the actual wire-level send runs outside any select! boundary in the writer task. The 1-slot channel provides natural back-pressure against a stalled writer rather than letting an unbounded queue of pings grow against a slow peer. A scoped AbortOnDrop guard guarantees the writer task is torn down on every return path, so the underlying TCP socket is freed deterministically. 2. MINOR - no test for the send-error reconnect branch. Added run_scanner_ws_reconnects_when_ping_send_errors: the fake server accepts the WS, completes the subscribe handshake, then drops the stream. The scanner detects the closed socket via the close path (ping-send error or read error - both go through the cancel-safe plumbing) and reconnects well within liveness_timeout. The test pins liveness_timeout at 30 s and observes >= 2 connections inside a 2 s budget so the reconnect cannot be attributed to the watchdog. 3. NIT - _PING_INTERVAL_FITS_LIVENESS const assertion read as "interval * 2 < timeout" which is algebraically equivalent to but directionally inverse of the docstring. Flipped to "interval < timeout / 2" so the expression matches the prose.
…ue-capacity const
feat(scanner_ws): client-side WebSocket Ping/Pong keepalive
github-actions
Bot
added
the
ci:full
TaprootFreak
added
ci:full
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automatic Release PR
Commits: 5 new commit(s)