Add jittered Electrum client connection max age

[DeviaVir]

What changed

• add an opt-in --electrum-rpc-conn-max-age <seconds> flag
• keep the existing unlimited connection lifetime when the flag is unset or 0
• assign each accepted client a lifetime jittered between 50% and 100% of the configured maximum
• register the connection with the reaper in the acceptor immediately after the accept, so the deadline is anchored to the accept and enforced even while the socket waits in the accept queue during a connection burst
• enforce each deadline with a single RPC-wide reaper thread that shuts the socket down at the absolute deadline — no timer thread per connection

Why

Electrum clients keep TCP sessions open for long periods. With connection-level L4 balancing, sequential replica restarts can pin most sessions to one backend and saturate it. Bounded, jittered lifetimes let clients reconnect gradually while all backends are available, allowing the load balancer to redistribute sessions without a synchronized reconnect storm.

How the deadline is enforced

The reaper tracks all connection deadlines in a min-heap and calls shutdown() on the connection's socket at the absolute deadline. Shutting the socket down unblocks both peer threads even when the writer is stuck in a blocking write_all to a client that requested a large response and stopped reading — a case an in-band expiry check between messages can never catch.

The reaper holds only a Weak reference to each stream, so a connection that ends before its deadline releases its file descriptor immediately, and an amortized compaction pass drops the queue entries of ended connections so the queue stays proportional to the number of live connections even under high connection churn with a large max age. On RPC shutdown the reaper is stopped with an explicit Shutdown message and joined by the RPC thread, since the acceptor's sender clone can outlive it while blocked in accept().

Expected teardown noise is silenced: a reader that cannot deliver its final Done, that fails sending a request into an already-closed reply channel, or that fails with a disconnect-rooted error after the socket was shut down under it, now logs at debug instead of warn/error.

Validation

• cargo test --lib: 29 passed, including a non-reading-client test (blocked write_all forced to fail at the deadline), an fd-release test, a queue-compaction test (dead entries dropped, live entry survives the pass), a reaper-shutdown test, a channel-teardown classification test, and a deadline-overflow test
• cargo check --all-targets and --features liquid / --features electrum-discovery checks clean
• CLI help verified for --electrum-rpc-conn-max-age
• git diff --check: clean

[Copilot]

🟡 Not ready to approve

It introduces a potential panic via Instant overflow and an expected-expiry path that can cause avoidable warn-level log noise.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

Adds an opt-in, jittered maximum lifetime for inbound Electrum RPC TCP connections to help L4 load balancers redistribute long-lived client sessions gradually and avoid backend pinning.

Changes:

• Introduces --electrum-rpc-conn-max-age <seconds> config/CLI flag (0/unset keeps unlimited lifetime).
• Assigns each accepted Electrum client a randomized lifetime (50–100% of configured max) and enforces expiry via recv_timeout rather than per-connection timer threads.
• Adds unit tests for lifetime selection and expiry behavior.

File summaries

File	Description
tests/common.rs	Updates test Config construction to include the new Electrum connection max-age field.
src/electrum/server.rs	Implements per-connection expiry (jittered lifetime + receive-with-deadline) and adds unit tests.
src/config.rs	Adds CLI flag parsing and plumbs electrum_rpc_conn_max_age into runtime configuration.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 2
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟢 Ready to approve

The new behavior is opt-in (default remains unlimited) and the connection-expiry logic is bounded and covered by unit tests.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 0 new
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟡 Not ready to approve

The reaper’s heap can retain many stale expiry entries until their (potentially far-future) deadlines, which can cause avoidable memory growth under high connection churn with large max-age values.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (1)

src/electrum/server.rs:894

• The reaper keeps a ConnectionExpiry entry in the heap until its deadline even if the connection ends early (the Weak just fails to upgrade when it eventually pops). With a large max-age and high connection churn, this can accumulate many stale entries and grow memory usage until their far-future deadlines are reached. Consider adding an explicit deregistration path (e.g., send a "finished" message on connection shutdown) or periodic heap compaction based on Weak::strong_count() == 0 so ended connections are removed promptly.

fn reap_expired_connections(registrations: Receiver<ConnectionExpiry>) {
    let mut expiries = BinaryHeap::new();
    loop {
        let now = Instant::now();
        while expiries
            .peek()
            .map_or(false, |e: &ConnectionExpiry| e.expires_at <= now)

• Files reviewed: 3/3 changed files
• Comments generated: 0 new
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟢 Ready to approve

The changes appear correct and well-tested, with only a minor reaper-loop timing optimization suggested in review comments.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (1)

src/electrum/server.rs:954

• reap_expired_connections calls Instant::now() twice per loop iteration (once for reap_due and again when computing the recv_timeout duration). If the deadline is very near, the second now() can be >= deadline, producing a 0-duration timeout and an avoidable tight extra loop. Capture now once per iteration and reuse it for both reaping and computing the timeout duration.

        queue.reap_due(Instant::now());
        let next = match queue.next_deadline() {
            Some(deadline) => {
                registrations.recv_timeout(deadline.saturating_duration_since(Instant::now()))
            }

• Files reviewed: 3/3 changed files
• Comments generated: 0 new
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟢 Ready to approve

The change is well-scoped, preserves default behavior, and includes focused tests covering key expiry and teardown scenarios.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 0 new
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟢 Ready to approve

The implementation matches the stated design (single reaper thread, checked deadline arithmetic, accept-time anchoring) and is backed by targeted unit tests for key expiry and shutdown behaviors.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 0 new
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.