Skip to content

Fix: Add Electrum timeout to connections#147

Merged
mononaut merged 2 commits into
mempoolfrom
junderw/electrum-limits
May 9, 2026
Merged

Fix: Add Electrum timeout to connections#147
mononaut merged 2 commits into
mempoolfrom
junderw/electrum-limits

Conversation

@junderw
Copy link
Copy Markdown
Member

@junderw junderw commented May 8, 2026

For now I think the default of 600 seconds sounds fine.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a configurable idle timeout for Electrum client connections, aiming to automatically close connections that have been inactive for a specified period (defaulting to 600 seconds) to reduce resource usage from lingering clients.

Changes:

  • Added a new CLI/config option --electrum-idle-timeout and corresponding Config::electrum_idle_timeout field.
  • Tracked per-connection last request time and added a periodic idle check loop.
  • Closed Electrum connections that exceed the configured idle timeout since the last client request.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/electrum/server.rs Tracks last client request time per connection and closes connections that exceed the configured idle timeout.
src/config.rs Adds electrum_idle_timeout to configuration, CLI arg parsing, and default value (600s).
Comments suppressed due to low confidence (1)

src/electrum/server.rs:589

  • Idle timeout is measured only from the last client request (last_request_at is updated only for Message::Request). For clients that rely on long-lived subscriptions (server sends periodic notifications but the client may not send further requests), this will disconnect an otherwise active connection after the timeout. If the goal is to close truly idle sockets, consider tracking last activity (read or write) and updating the timestamp on PeriodicUpdate/send_values, or clarify via config/flag semantics that subscriptions require explicit client keepalives (e.g., server.ping).
                        Message::Request(line) => {
                            self.last_request_at = Instant::now();
                            let result = self.handle_line(&line);
                            self.send_values(&[result])?
                        }
                        Message::PeriodicUpdate => {
                            let values = self
                                .update_subscriptions()
                                .chain_err(|| "failed to update subscriptions")?;
                            self.send_values(&values)?
                        }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/electrum/server.rs
Copy link
Copy Markdown
Contributor

@mononaut mononaut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested ACK @ [d6d580c]

@mononaut mononaut merged commit 6dfe529 into mempool May 9, 2026
12 checks passed
@mononaut mononaut deleted the junderw/electrum-limits branch May 9, 2026 04:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants