From e6bbff4e72f0d7df3a61e9f903ddc4a67f4c7d06 Mon Sep 17 00:00:00 2001 From: Chase Date: Fri, 19 Jun 2026 07:35:21 +0200 Subject: [PATCH 1/3] feat(node): opt-in flag to disable HTTP keep-alive pool to the node MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add --node-disable-conn-pool (default false: keep pooling). When set, the reqwest client uses pool_max_idle_per_host(0) so every request to the node opens a fresh connection. Needed when the node is reached through a locality-aware L4 proxy (nginx local-primary/remote-backup bitcoin-proxy in front of a GKE MCS ClusterSetIP): a pooled keep-alive connection gets pinned by the proxy to whichever backend it first hit, so after a transient local-node outage it stays stuck on the remote region. Disabling the pool makes each request re-evaluate local-first (sub-ms against a local node) — the waterfalls analog of electrs --daemon-rpc-conn-max-age. Default stays pooled so remote/esplora backends and single-region deploys keep keep-alive (no TCP+TLS handshake per request). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/fetch.rs | 9 +++++++-- src/server/mod.rs | 5 +++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/fetch.rs b/src/fetch.rs index a8935f5..d3b44ff 100644 --- a/src/fetch.rs +++ b/src/fetch.rs @@ -97,9 +97,14 @@ impl Client { node_url.unwrap_or(format!("{LOCAL}:{port}")) }; log::info!("connecting to {base_url}"); - let client = reqwest::Client::builder() + let mut builder = reqwest::Client::builder() .timeout(Duration::from_secs(args.request_timeout_seconds)) - .connect_timeout(Duration::from_secs(args.request_timeout_seconds)) // Connection establishment timeout + .connect_timeout(Duration::from_secs(args.request_timeout_seconds)); // Connection establishment timeout + if args.node_disable_conn_pool { + // No keep-alive reuse: each request opens a fresh connection. + builder = builder.pool_max_idle_per_host(0); + } + let client = builder .build() .with_context(|| "Failed to create HTTP client with timeout")?; diff --git a/src/server/mod.rs b/src/server/mod.rs index 82dfbbb..7cb26cf 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -140,6 +140,10 @@ pub struct Arguments { #[arg(env, long, default_value = "30")] pub request_timeout_seconds: u64, + /// Disable HTTP keep-alive connection pooling to the node, forcing a fresh connection per request. + #[arg(env, long)] + pub node_disable_conn_pool: bool, + /// Timeout in seconds for reading incoming HTTP request headers (protects against slowloris attacks) #[arg(env, long, default_value = "10")] pub header_read_timeout_seconds: u64, @@ -187,6 +191,7 @@ impl std::fmt::Debug for Arguments { .field("enable_db_statistics", &self.enable_db_statistics) .field("cache_control_seconds", &self.cache_control_seconds) .field("request_timeout_seconds", &self.request_timeout_seconds) + .field("node_disable_conn_pool", &self.node_disable_conn_pool) .field( "header_read_timeout_seconds", &self.header_read_timeout_seconds, From 1a6f7869d59b17430cee50b6105d70a2a00c92de Mon Sep 17 00:00:00 2001 From: Chase Date: Fri, 19 Jun 2026 08:07:06 +0200 Subject: [PATCH 2/3] test(node): cover --node-disable-conn-pool flag and pooling behavior MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds three unit tests in fetch.rs (no node/network needed, run under `cargo test --lib`): - test_node_disable_conn_pool_flag: locks the CLI contract — defaults to pooled, --node-disable-conn-pool flips it on. - test_conn_pool_enabled_reuses_connection: against a loopback HTTP/1.1 server that counts accepted TCP connections, three sequential requests ride a single reused keep-alive connection (default behavior). - test_conn_pool_disabled_opens_fresh_connection_each_request: with the flag set, each request opens its own connection — proving the proxy re-evaluates the upstream every request instead of staying pinned. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/fetch.rs | 123 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 123 insertions(+) diff --git a/src/fetch.rs b/src/fetch.rs index d3b44ff..d0b3745 100644 --- a/src/fetch.rs +++ b/src/fetch.rs @@ -693,6 +693,129 @@ mod test { assert_eq!(result[&6], 2.0); } + #[test] + fn test_node_disable_conn_pool_flag() { + use clap::Parser; + + // Defaults to pooled (keep-alive) when the flag is absent. + let args = Arguments::try_parse_from(["waterfalls", "--network", "bitcoin"]).unwrap(); + assert!(!args.node_disable_conn_pool); + + // The long flag opts into a fresh connection per request. + let args = Arguments::try_parse_from([ + "waterfalls", + "--network", + "bitcoin", + "--node-disable-conn-pool", + ]) + .unwrap(); + assert!(args.node_disable_conn_pool); + } + + /// Spawn a minimal HTTP/1.1 server on loopback that counts the TCP + /// connections it accepts and answers every request with the same canned + /// blockhash, keeping the connection alive for any follow-up requests. + /// Returns the bound address and the accept counter. + async fn spawn_counting_node() -> ( + std::net::SocketAddr, + std::sync::Arc, + ) { + use std::sync::atomic::{AtomicUsize, Ordering}; + use std::sync::Arc; + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + use tokio::net::TcpListener; + + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + let connections = Arc::new(AtomicUsize::new(0)); + let counter = connections.clone(); + + tokio::spawn(async move { + // A valid 64-hex blockhash so `block_hash` parses the body successfully. + const HASH_HEX: &str = + "000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f"; + let response = format!( + "HTTP/1.1 200 OK\r\nContent-Length: {}\r\n\r\n{HASH_HEX}", + HASH_HEX.len() + ); + loop { + let (mut socket, _) = match listener.accept().await { + Ok(pair) => pair, + Err(_) => break, + }; + counter.fetch_add(1, Ordering::SeqCst); + let response = response.clone(); + tokio::spawn(async move { + let mut buf = [0u8; 1024]; + // Serve each request on this connection until the client closes it, + // so a reused keep-alive connection is counted only once. + 'conn: loop { + let mut req = Vec::new(); + loop { + match socket.read(&mut buf).await { + Ok(0) | Err(_) => break 'conn, // client closed the connection + Ok(n) => { + req.extend_from_slice(&buf[..n]); + if req.windows(4).any(|w| w == b"\r\n\r\n") { + break; // full request headers received + } + } + } + } + if socket.write_all(response.as_bytes()).await.is_err() { + break; + } + } + }); + } + }); + + (addr, connections) + } + + fn node_client(addr: std::net::SocketAddr, disable_conn_pool: bool) -> Client { + let mut args = Arguments::default(); + args.network = Network::Bitcoin; + args.use_esplora = false; + args.node_url = Some(format!("http://{addr}")); + args.rpc_user_password = Some("user:pass".to_string()); // satisfies is_valid() + args.request_timeout_seconds = 30; // Default::default() leaves this 0, which is_valid() rejects + args.node_disable_conn_pool = disable_conn_pool; + Client::new(&args).unwrap() + } + + /// Default (pooled) behavior: sequential requests reuse a single keep-alive connection. + #[tokio::test] + async fn test_conn_pool_enabled_reuses_connection() { + use std::sync::atomic::Ordering; + + let (addr, connections) = spawn_counting_node().await; + let client = node_client(addr, false); + + for _ in 0..3 { + client.block_hash(0).await.unwrap().unwrap(); + } + + assert_eq!(connections.load(Ordering::SeqCst), 1); + } + + /// With the pool disabled, every request establishes its own connection, so a + /// locality-aware L4 proxy re-evaluates the upstream on each request instead of + /// staying pinned to whichever backend the first connection landed on. + #[tokio::test] + async fn test_conn_pool_disabled_opens_fresh_connection_each_request() { + use std::sync::atomic::Ordering; + + let (addr, connections) = spawn_counting_node().await; + let client = node_client(addr, true); + + for _ in 0..3 { + client.block_hash(0).await.unwrap().unwrap(); + } + + assert_eq!(connections.load(Ordering::SeqCst), 3); + } + #[tokio::test] #[ignore = "connects to prod server"] async fn test_client_esplora() { From 10878912dc5f2e8b0ac4ff47c02a746dc49f5ba9 Mon Sep 17 00:00:00 2001 From: Chase Date: Fri, 19 Jun 2026 10:36:31 +0200 Subject: [PATCH 3/3] fix(node): gate conn-pool toggle to node-only, warn under esplora Address review feedback on PR #10: - node_disable_conn_pool no longer disables keep-alive pooling for the Esplora backend (would force TCP+TLS per request, a regression). When the flag is set together with --use-esplora it is ignored and a loud warning is logged instead of being silently dropped. - Document the node-only scope in the flag help text. - Drop the clap flag-parse test; the behavior is covered by the connection-reuse tests. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/fetch.rs | 37 ++++++++++++++++--------------------- src/server/mod.rs | 1 + 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/src/fetch.rs b/src/fetch.rs index d0b3745..5679836 100644 --- a/src/fetch.rs +++ b/src/fetch.rs @@ -101,8 +101,22 @@ impl Client { .timeout(Duration::from_secs(args.request_timeout_seconds)) .connect_timeout(Duration::from_secs(args.request_timeout_seconds)); // Connection establishment timeout if args.node_disable_conn_pool { - // No keep-alive reuse: each request opens a fresh connection. - builder = builder.pool_max_idle_per_host(0); + if use_esplora { + // The flag is node-only; applying it to Esplora would force a fresh + // TCP + TLS handshake per request — a real regression — so ignore it + // here, but make the ignored setting loud rather than silent. + log::warn!( + "**********************************************************************\n\ + * --node-disable-conn-pool (NODE_DISABLE_CONN_POOL) is set together \n\ + * with --use-esplora. This flag only affects the local node \n\ + * connection and is being IGNORED for the Esplora backend; keep-alive \n\ + * pooling stays ENABLED. \n\ + **********************************************************************" + ); + } else { + // No keep-alive reuse: each request opens a fresh connection. + builder = builder.pool_max_idle_per_host(0); + } } let client = builder .build() @@ -693,25 +707,6 @@ mod test { assert_eq!(result[&6], 2.0); } - #[test] - fn test_node_disable_conn_pool_flag() { - use clap::Parser; - - // Defaults to pooled (keep-alive) when the flag is absent. - let args = Arguments::try_parse_from(["waterfalls", "--network", "bitcoin"]).unwrap(); - assert!(!args.node_disable_conn_pool); - - // The long flag opts into a fresh connection per request. - let args = Arguments::try_parse_from([ - "waterfalls", - "--network", - "bitcoin", - "--node-disable-conn-pool", - ]) - .unwrap(); - assert!(args.node_disable_conn_pool); - } - /// Spawn a minimal HTTP/1.1 server on loopback that counts the TCP /// connections it accepts and answers every request with the same canned /// blockhash, keeping the connection alive for any follow-up requests. diff --git a/src/server/mod.rs b/src/server/mod.rs index 7cb26cf..17dfc50 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -141,6 +141,7 @@ pub struct Arguments { pub request_timeout_seconds: u64, /// Disable HTTP keep-alive connection pooling to the node, forcing a fresh connection per request. + /// Node-only: ignored (with a warning) when --use-esplora is set. #[arg(env, long)] pub node_disable_conn_pool: bool,