From bd5677e53529abdcd19bcbea8a588af1da4523fd Mon Sep 17 00:00:00 2001 From: Adrian Chifor Date: Fri, 30 Oct 2026 17:43:34 +0200 Subject: [PATCH 1/5] Parse a request head only once its end can have arrived dispatch ran httparse over the whole buffered head on every read. A head arriving in small reads was rescanned from its start each time. The cost grew with the square of the head size, up to the 16 MiB read cap. dispatch now scans each new byte once for a possible end of head. It parses when one is found, or when the buffered bytes have doubled. The doubling keeps the early 400 for bytes that cannot start a request. Leading empty lines are dropped first, as httparse would skip them. Once the head is parsed, its request length is kept. The head is not parsed again until the body is complete. A head is now limited to 4 MiB and answered with 431 past it. A GET may list 32768 validator ids by pubkey, about 3.2 MiB, so a smaller limit would refuse it. A const assert keeps the two limits in step. The 431 now arrives before the read buffer can fill, so reject_exhausted is removed. --- crates/beacon_api/src/beacon/validators.rs | 6 +- crates/beacon_api/src/server.rs | 30 +-- crates/httpcore/src/lib.rs | 3 +- crates/httpcore/src/server.rs | 232 ++++++++++++++++----- 4 files changed, 199 insertions(+), 72 deletions(-) diff --git a/crates/beacon_api/src/beacon/validators.rs b/crates/beacon_api/src/beacon/validators.rs index 93ffe2d0a..9be25566f 100644 --- a/crates/beacon_api/src/beacon/validators.rs +++ b/crates/beacon_api/src/beacon/validators.rs @@ -3,7 +3,7 @@ use silver_beacon_state_data::{ BLSPubkey, Epoch, FAR_FUTURE_EPOCH, PinnedState, StateReadView, ValidatorsView, Withdrawals, }; use silver_common::json_fields::parse_uint64; -use silver_httpcore::{ChunkedResponse, Query}; +use silver_httpcore::{ChunkedResponse, Query, REQUEST_HEAD_MAX}; use crate::{ ctx::{ApiCtx, read_flags}, @@ -17,6 +17,10 @@ use crate::{ const MAX_VALIDATOR_IDS: usize = 32 * 1024; +// A GET may name every id by pubkey. +const _: () = + assert!(MAX_VALIDATOR_IDS * ("&id=0x".len() + 2 * size_of::()) < REQUEST_HEAD_MAX); + /// About 0.25 MB of JSON and 40 µs of rendering per batch. const VALIDATORS_PER_BATCH: usize = 512; diff --git a/crates/beacon_api/src/server.rs b/crates/beacon_api/src/server.rs index 82041477c..fdbff3c82 100644 --- a/crates/beacon_api/src/server.rs +++ b/crates/beacon_api/src/server.rs @@ -401,17 +401,10 @@ impl Requests { if event.is_readable() { // A full buffer always leaves something to answer with: a body // declared past the cap is answered from headers already buffered, - // and a head that outgrows the whole buffer is answered 431. The - // answer, not the exhaustion, is what ends the connection. - let mut exhausted = false; + // and a head past its limit is answered 431 long before the cap. + // The answer, not the exhaustion, is what ends the connection. loop { - let space = match self.http.read_space() { - Ok(space) => space, - Err(_) => { - exhausted = true; - break; - } - }; + let Ok(space) = self.http.read_space() else { break }; match stream.read(space) { Ok(0) => return Err(io::Error::from(io::ErrorKind::UnexpectedEof)), Ok(n) => { @@ -424,12 +417,7 @@ impl Requests { } } - let mut answered = self.http.dispatch(request_handler); - if !answered && exhausted { - self.http.reject_exhausted(); - answered = true; - } - if answered { + if self.http.dispatch(request_handler) { registry.reregister(stream, event.token(), Interest::WRITABLE)?; } return Ok(false); @@ -2071,12 +2059,12 @@ mod tests { assert_identity_ok(&response); } - /// A head that outgrows the whole read buffer declares no length to - /// answer from, so filling the buffer is the verdict: a 431, delivered - /// like every other reject, where dropping the socket mid-send would be - /// the reset that costs a client its node. + /// A head with no end declares no length to answer from, so passing the + /// head limit is the verdict: a 431, delivered like every other reject, + /// where dropping the socket mid-send would be the reset that costs a + /// client its node. #[test] - fn head_that_outgrows_the_read_buffer_is_answered_with_431() { + fn head_past_its_limit_is_answered_with_431() { let mut server = server_with(64, LONG_TIMEOUT); let addr = tcp_addr(&server); diff --git a/crates/httpcore/src/lib.rs b/crates/httpcore/src/lib.rs index 42ab3bdea..a9d9672fb 100644 --- a/crates/httpcore/src/lib.rs +++ b/crates/httpcore/src/lib.rs @@ -16,7 +16,8 @@ pub use pool::{BufferCapacity, Endpoint, HttpPool}; pub use query::Query; pub use readiness::Readiness; pub use server::{ - AfterResponse, ParsedRequest, ServerConnection, frame_response, frame_response_with_headers, + AfterResponse, ParsedRequest, REQUEST_HEAD_MAX, ServerConnection, frame_response, + frame_response_with_headers, }; pub use stream::{Bind, Listener, Stream}; pub use token_range::TokenRange; diff --git a/crates/httpcore/src/server.rs b/crates/httpcore/src/server.rs index 573317812..5049e505c 100644 --- a/crates/httpcore/src/server.rs +++ b/crates/httpcore/src/server.rs @@ -11,6 +11,9 @@ const READ_BUF_MAX: usize = 16 << 20; const READ_BUF_INIT: usize = 4096; const WRITE_BUF_INIT: usize = 4096; +// Long enough for a GET whose query lists validators by pubkey. +pub const REQUEST_HEAD_MAX: usize = 4 << 20; + pub struct ParsedRequest<'a> { pub method: &'a str, pub path: &'a str, @@ -25,12 +28,13 @@ pub struct ParsedRequest<'a> { pub keep_alive: bool, } -/// `Incomplete` means "no verdict yet, feed me more bytes"; `Malformed` and -/// `TooLarge` mean the bytes can never become a request this connection -/// serves, so no amount of waiting helps. +/// `Incomplete` and `AwaitingBody` mean "no verdict yet, feed me more bytes"; +/// `Malformed` and `TooLarge` mean the bytes can never become a request this +/// connection serves, so no amount of waiting helps. enum ParseOutcome<'a> { Complete { consumed: usize, request: ParsedRequest<'a> }, Incomplete, + AwaitingBody { request_len: usize }, Malformed, TooLarge, } @@ -82,7 +86,7 @@ impl<'a> ParsedRequest<'a> { return ParseOutcome::TooLarge; } if buf.len() < total { - return ParseOutcome::Incomplete; + return ParseOutcome::AwaitingBody { request_len: total }; } ParseOutcome::Complete { @@ -127,10 +131,49 @@ enum Continuation { Linger, } +/// Offsets count from the first unconsumed byte. Without them, a request +/// arriving in many small reads is parsed from its start on every read. +enum Awaiting { + /// No end of head starts before `searched`. Parsing again each time the + /// bytes double still rejects a malformed head early, at linear cost. + Head { + searched: usize, + parsed_len: usize, + }, + Body { + request_len: usize, + }, +} + +impl Awaiting { + const HEAD: Self = Self::Head { searched: 0, parsed_len: 0 }; + + fn worth_parsing(&mut self, buffered: &[u8]) -> bool { + match self { + Self::Body { request_len } => buffered.len() >= *request_len, + Self::Head { searched, parsed_len } => { + let head = &buffered[..buffered.len().min(REQUEST_HEAD_MAX)]; + // httparse also ends a line on a bare LF. + let ended = (*searched..head.len()).any(|at| { + head[at] == b'\n' && matches!(head[at + 1..], [b'\n', ..] | [b'\r', b'\n', ..]) + }); + *searched = (*searched).max(head.len().saturating_sub(2)); + let doubled = buffered.len() >= 2 * *parsed_len; + if !ended && (buffered.len() > REQUEST_HEAD_MAX || !doubled) { + return false; + } + *parsed_len = buffered.len(); + true + } + } + } +} + pub struct ServerConnection { read_buf: Vec, read_pos: usize, read_end: usize, + awaiting: Awaiting, write_buf: Vec, write_pos: usize, continuation: Continuation, @@ -142,6 +185,7 @@ impl ServerConnection { read_buf: vec![0u8; READ_BUF_INIT], read_pos: 0, read_end: 0, + awaiting: Awaiting::HEAD, write_buf: Vec::with_capacity(WRITE_BUF_INIT), write_pos: 0, continuation: Continuation::KeepAlive, @@ -185,19 +229,39 @@ impl ServerConnection { &[("Connection", "close")], &[], ); - self.read_pos = 0; - self.read_end = 0; + self.consume(self.read_end - self.read_pos); true } - /// The buffer is full yet holds no complete request: the head alone - /// outgrew it, which is the one overrun `dispatch` cannot answer — a body - /// declared past the cap is a 413 from its headers and garbage is a 400, - /// both before the cap is spent. RFC 6585's 431 covers a head with no end - /// in sight. - pub fn reject_exhausted(&mut self) { - debug_assert!(self.read_end == READ_BUF_MAX, "space to read from remains"); - self.reject("431 Request Header Fields Too Large"); + fn consume(&mut self, n: usize) { + self.read_pos += n; + self.awaiting = Awaiting::HEAD; + if self.read_pos == self.read_end { + self.read_pos = 0; + self.read_end = 0; + } + } + + /// httparse skips these too. Dropping them first keeps every later end of + /// head a real one, so a run of them is never parsed over and over. + fn skip_empty_lines(&mut self) { + loop { + let line_len = match self.read_buf[self.read_pos..self.read_end] { + [b'\n', ..] => 1, + [b'\r', b'\n', ..] => 2, + _ => return, + }; + self.consume(line_len); + } + } + + /// RFC 6585's 431 covers a head with no end in sight. + fn wait_for_more(&mut self) -> bool { + let head_unended = matches!(self.awaiting, Awaiting::Head { .. }); + if head_unended && self.read_end - self.read_pos > REQUEST_HEAD_MAX { + return self.reject("431 Request Header Fields Too Large"); + } + false } /// Scratch for a lingering connection's drain: the bytes are read only to @@ -211,13 +275,21 @@ impl ServerConnection { &mut self, handler: &mut F, ) -> bool { - let (consumed, req) = - match ParsedRequest::parse(&self.read_buf[self.read_pos..self.read_end]) { - ParseOutcome::Complete { consumed, request } => (consumed, request), - ParseOutcome::Incomplete => return false, - ParseOutcome::Malformed => return self.reject("400 Bad Request"), - ParseOutcome::TooLarge => return self.reject("413 Payload Too Large"), - }; + self.skip_empty_lines(); + let buffered = &self.read_buf[self.read_pos..self.read_end]; + if !self.awaiting.worth_parsing(buffered) { + return self.wait_for_more(); + } + let (consumed, req) = match ParsedRequest::parse(buffered) { + ParseOutcome::Complete { consumed, request } => (consumed, request), + ParseOutcome::Incomplete => return self.wait_for_more(), + ParseOutcome::AwaitingBody { request_len } => { + self.awaiting = Awaiting::Body { request_len }; + return false; + } + ParseOutcome::Malformed => return self.reject("400 Bad Request"), + ParseOutcome::TooLarge => return self.reject("413 Payload Too Large"), + }; if req.version != 1 { silver_log::warn!("rejecting HTTP/1.0 request"); self.continuation = Continuation::Close; @@ -227,11 +299,7 @@ impl ServerConnection { if req.keep_alive { Continuation::KeepAlive } else { Continuation::Close }; handler(&req, &mut self.write_buf); } - self.read_pos += consumed; - if self.read_pos == self.read_end { - self.read_pos = 0; - self.read_end = 0; - } + self.consume(consumed); true } @@ -326,7 +394,7 @@ pub fn frame_response_with_headers( #[cfg(test)] mod tests { - use std::cell::RefCell; + use std::{cell::RefCell, time::Duration}; use super::*; @@ -370,9 +438,6 @@ mod tests { } Err(e) => return e, } - assert!(!conn.dispatch(&mut |_, _: &mut Vec| { - panic!("incomplete request must not dispatch") - })); } } @@ -390,6 +455,9 @@ mod tests { match ParsedRequest::parse(buf) { ParseOutcome::Complete { consumed, request } => (consumed, request), ParseOutcome::Incomplete => panic!("expected a complete request, got Incomplete"), + ParseOutcome::AwaitingBody { .. } => { + panic!("expected a complete request, got AwaitingBody") + } ParseOutcome::Malformed => panic!("expected a complete request, got Malformed"), ParseOutcome::TooLarge => panic!("expected a complete request, got TooLarge"), } @@ -463,7 +531,13 @@ mod tests { body.len() ); let mut buf = req.into_bytes(); - assert!(matches!(ParsedRequest::parse(&buf), ParseOutcome::Incomplete), "body not arrived"); + assert!( + matches!( + ParsedRequest::parse(&buf), + ParseOutcome::AwaitingBody { request_len } if request_len == buf.len() + body.len() + ), + "body not arrived" + ); buf.extend_from_slice(body); let (consumed, r) = parsed(&buf); assert_eq!(r.method, "POST"); @@ -592,30 +666,24 @@ mod tests { } } - /// A head that never ends reaches no verdict `dispatch` can deliver, so - /// the buffer filling up is itself the verdict: the 431 of RFC 6585, - /// answered and lingered like every other reject rather than a bare drop. #[test] - fn exhausted_buffer_holding_no_request_writes_431_then_lingers() { + fn head_without_end_past_the_limit_writes_431_then_lingers() { let mut conn = ServerConnection::new(); - feed(&mut conn, b"GET /eth/v1/node/health HTTP/1.1\r\nCookie: "); + let start = b"GET /eth/v1/node/health HTTP/1.1\r\nCookie: "; + feed(&mut conn, start); + let chunk = vec![b'c'; 64 << 10]; + let mut buffered = start.len(); loop { - match conn.read_space() { - Ok(space) => { - let n = space.len(); - space.fill(b'c'); - conn.commit_read(n); - } - Err(_) => break, + feed_all(&mut conn, &chunk); + buffered += chunk.len(); + let answered = + conn.dispatch(&mut |_, _: &mut Vec| panic!("no request can reach the handler")); + assert_eq!(answered, buffered > REQUEST_HEAD_MAX, "{buffered} bytes buffered"); + if answered { + break; } } - assert!( - !conn.dispatch(&mut |_, _: &mut Vec| panic!("no request can reach the handler")), - "an unfinished head must not dispatch" - ); - - conn.reject_exhausted(); assert_eq!( conn.pending_write(), b"HTTP/1.1 431 Request Header Fields Too Large\r\nConnection: close\r\n\ @@ -626,6 +694,55 @@ mod tests { assert_eq!(conn.after_response(&mut echo_path), AfterResponse::Linger); } + #[test] + fn whole_head_past_the_limit_writes_431() { + let mut request = b"GET /eth/v1/node/health HTTP/1.1\r\nCookie: ".to_vec(); + request.resize(REQUEST_HEAD_MAX + 1, b'c'); + request.extend_from_slice(b"\r\n\r\n"); + let mut conn = ServerConnection::new(); + feed_all(&mut conn, &request); + + assert!( + conn.dispatch(&mut |_, _: &mut Vec| panic!("no request can reach the handler")) + ); + assert!(conn.pending_write().starts_with(b"HTTP/1.1 431 ")); + } + + #[test] + fn query_near_the_head_limit_dispatches() { + let query = "id=1,".repeat((REQUEST_HEAD_MAX - 1024) / "id=1,".len()); + let request = + get_req(&format!("/eth/v1/beacon/states/head/validators?{query}"), "HTTP/1.1"); + let (last, unended) = request.split_last().unwrap(); + let mut conn = ServerConnection::new(); + for chunk in unended.chunks(64 << 10) { + feed_all(&mut conn, chunk); + assert!(!conn.dispatch(&mut |_, _: &mut Vec| panic!("the head has no end yet"))); + } + feed(&mut conn, &[*last]); + assert!( + conn.dispatch(&mut |_, out: &mut Vec| frame_response(out, "200 OK", None, b"")) + ); + assert!(conn.pending_write().starts_with(b"HTTP/1.1 200 OK\r\n")); + } + + #[test] + fn empty_lines_before_a_request_are_skipped() { + let mut conn = ServerConnection::new(); + for _ in 0..1024 { + feed(&mut conn, b"\r\n"); + assert!(!conn.dispatch(&mut |_, _: &mut Vec| panic!("no request has arrived"))); + } + feed(&mut conn, b"\n\r\nGET /metrics HTTP/1.1\r\nHost: x\r\n\r\n"); + assert!(conn.dispatch(&mut echo_path)); + assert_eq!(conn.pending_write(), b"HTTP/1.1 200 OK\r\nContent-Length: 8\r\n\r\n/metrics"); + } + + #[test] + fn bytes_that_cannot_start_a_request_are_rejected_before_any_line_ends() { + reject_and_linger(b"\x16\x03\x01\x02\x00\x01\x00\x01\xfc\x03\x03", "400 Bad Request"); + } + /// The body the client is still sending is read for one reason only — to /// keep the answer from being lost — so it must cost nothing to read. #[test] @@ -963,4 +1080,21 @@ mod tests { })); assert_eq!(*seen.borrow(), request[header_len..]); } + + /// Parsing the whole head again on each of 64Ki reads costs about 64 GiB + /// of scanning, which no machine finishes inside the bound. + #[test] + fn head_arriving_in_small_reads_is_parsed_in_linear_time() { + let mut conn = ServerConnection::new(); + feed(&mut conn, b"GET /eth/v1/node/health HTTP/1.1\r\nCookie: "); + let started = Instant::now(); + for _ in 0..(2 << 20) / 32 { + feed_all(&mut conn, &[b'c'; 32]); + assert!(!conn.dispatch(&mut |_, _: &mut Vec| panic!("the head has no end yet"))); + } + feed_all(&mut conn, b"\r\n\r\n"); + assert!(conn.dispatch(&mut echo_path)); + let elapsed = started.elapsed(); + assert!(elapsed < Duration::from_secs(1), "took {elapsed:?}"); + } } From d759da69fb0257a0c6d24b9910aa8f4e1ce52dad Mon Sep 17 00:00:00 2001 From: Adrian Chifor Date: Fri, 30 Oct 2026 17:44:40 +0200 Subject: [PATCH 2/5] Release a grown read buffer once its request is consumed read_buf doubles up to the 16 MiB read cap and kept its peak size for the life of the connection. Only write_buf was shrunk after a response. A keep-alive connection that once received a large body held that memory until it closed. When the read buffer empties and has grown past 64 KiB, it now shrinks back to its initial 4 KiB. Typical request bodies stay within 64 KiB, so they keep reusing the buffer without regrowing it. --- crates/httpcore/src/server.rs | 43 +++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/crates/httpcore/src/server.rs b/crates/httpcore/src/server.rs index 5049e505c..4e49c6e14 100644 --- a/crates/httpcore/src/server.rs +++ b/crates/httpcore/src/server.rs @@ -9,6 +9,8 @@ use crate::chunked_response::ChunkedResponse; // production maximums (21 blobs × 128 KiB plus block fields). const READ_BUF_MAX: usize = 16 << 20; const READ_BUF_INIT: usize = 4096; +// Typical request bodies fit, so they never pay to regrow the buffer. +const READ_BUF_KEPT_MAX: usize = 64 << 10; const WRITE_BUF_INIT: usize = 4096; // Long enough for a GET whose query lists validators by pubkey. @@ -239,6 +241,12 @@ impl ServerConnection { if self.read_pos == self.read_end { self.read_pos = 0; self.read_end = 0; + // The connection outlives the request that grew the buffer, and + // keeping its peak would pin up to the read cap per connection. + if self.read_buf.len() > READ_BUF_KEPT_MAX { + self.read_buf.truncate(READ_BUF_INIT); + self.read_buf.shrink_to(READ_BUF_INIT); + } } } @@ -1056,6 +1064,41 @@ mod tests { ); } + #[test] + fn large_request_does_not_leave_the_connection_inflated() { + let mut conn = ServerConnection::new(); + let mut request = oversized_post(1 << 20); + request.resize(request.len() + (1 << 20), b'p'); + feed_all(&mut conn, &request); + assert!(conn.read_buf.len() > READ_BUF_KEPT_MAX, "the body was buffered whole"); + + assert!(conn.dispatch(&mut echo_path)); + assert!( + conn.read_buf.capacity() <= READ_BUF_INIT, + "{} bytes still held", + conn.read_buf.capacity() + ); + + drain(&mut conn); + assert_eq!(conn.after_response(&mut echo_path), AfterResponse::AwaitRequest); + feed(&mut conn, &get_req("/next", "HTTP/1.1")); + assert!(conn.dispatch(&mut echo_path)); + assert_eq!(drain(&mut conn), b"HTTP/1.1 200 OK\r\nContent-Length: 5\r\n\r\n/next"); + } + + #[test] + fn request_buffer_within_the_kept_size_is_reused() { + let mut conn = ServerConnection::new(); + let mut request = oversized_post(READ_BUF_KEPT_MAX / 2); + request.resize(request.len() + READ_BUF_KEPT_MAX / 2, b'p'); + feed_all(&mut conn, &request); + let grown = conn.read_buf.len(); + assert!(grown > READ_BUF_INIT); + + assert!(conn.dispatch(&mut echo_path)); + assert_eq!(conn.read_buf.len(), grown); + } + #[test] fn request_split_across_growth_boundary_not_corrupted() { let mut conn = ServerConnection::new(); From c50c0bb0aba17c529159d5e95e7638daa9d29909 Mon Sep 17 00:00:00 2001 From: Adrian Chifor Date: Fri, 30 Oct 2026 17:46:52 +0200 Subject: [PATCH 3/5] Bound the time to receive one request The only connection timeout was the 75 s idle deadline, and every byte refreshed it. A request whose head or body kept arriving a few bytes at a time could hold a connection slot for as long as it kept sending. Each request now has a 60 s deadline, from its first byte until it is complete. The idle sweep answers an overdue request with 408 and lingers, like other rejects. The wait between keep-alive requests is unchanged and still governed by the idle deadline. A body at the 16 MiB read cap needs about 2.2 Mbit/s to meet the deadline. Bodies that matter, such as blocks, are useless after one 12 s slot anyway. --- crates/beacon_api/src/server.rs | 129 ++++++++++++++++++++++++++++++++ crates/httpcore/src/server.rs | 20 +++++ 2 files changed, 149 insertions(+) diff --git a/crates/beacon_api/src/server.rs b/crates/beacon_api/src/server.rs index fdbff3c82..c5fd6cce5 100644 --- a/crates/beacon_api/src/server.rs +++ b/crates/beacon_api/src/server.rs @@ -49,6 +49,10 @@ use crate::{ const MAX_SWEEP_INTERVAL: Duration = Duration::from_secs(1); +// A body at the read cap needs about 2.2 Mbit/s to arrive in time. Bodies +// that matter, such as blocks, are useless after one 12 s slot anyway. +const REQUEST_DEADLINE: Duration = Duration::from_secs(60); + /// nginx's `lingering_close` caps: how long a connection that has already /// answered may wait between the peer's bytes, and how long the whole drain /// may run before the slot is taken back. @@ -90,6 +94,7 @@ enum State { struct Requests { http: ServerConnection, last_activity: Instant, + receiving_since: Option, linger_since: Option, pending: Option, } @@ -357,11 +362,30 @@ impl Requests { Self { http: ServerConnection::new(), last_activity: now, + receiving_since: None, linger_since: None, pending: None, } } + fn overdue(&self, now: Instant, deadline: Duration) -> bool { + self.receiving_since.is_some_and(|since| now.duration_since(since) > deadline) + } + + /// A 408 can always be framed: the deadline runs only while a request is + /// arriving, before anything is framed for it. + fn time_out( + &mut self, + stream: &mut Stream, + registry: &Registry, + token: Token, + ) -> io::Result<()> { + debug_assert!(self.pending.is_none(), "an answered request is not arriving"); + self.receiving_since = None; + self.http.reject_timed_out(); + registry.reregister(stream, token, Interest::WRITABLE) + } + /// A lingering connection has answered already, so it lives by the linger /// caps rather than by the idle deadline that holds a served connection /// open for its client's next request. @@ -409,6 +433,7 @@ impl Requests { Ok(0) => return Err(io::Error::from(io::ErrorKind::UnexpectedEof)), Ok(n) => { self.last_activity = now; + self.receiving_since.get_or_insert(now); self.http.commit_read(n); } Err(e) if would_block(&e) => break, @@ -418,6 +443,7 @@ impl Requests { } if self.http.dispatch(request_handler) { + self.receiving_since = None; registry.reregister(stream, event.token(), Interest::WRITABLE)?; } return Ok(false); @@ -616,6 +642,7 @@ pub struct BeaconApi { listeners: Vec, max_connections: usize, idle: IdleSweep, + request_deadline: Duration, linger: Linger, streams: StreamLimits, last_keep_alive: Instant, @@ -678,6 +705,7 @@ impl BeaconApi { tokens, max_connections, idle: IdleSweep::new(idle_timeout), + request_deadline: REQUEST_DEADLINE, linger: Linger::default(), streams: StreamLimits::default(), last_keep_alive: Instant::now(), @@ -1347,6 +1375,7 @@ impl BeaconApi { } if self.idle.due(now) { + did_work |= self.time_out_requests(now); did_work |= self.close_expired(now); // Subscribers also leave on failed writes and reads, so the // last one is noticed here, within a sweep interval. @@ -1520,6 +1549,30 @@ impl BeaconApi { } } + /// Any byte refreshes the idle deadline, so only this bounds a request + /// that keeps arriving a little at a time. + fn time_out_requests(&mut self, now: Instant) -> bool { + let Self { connections, registry, request_deadline, .. } = self; + let mut timed_out = false; + connections.retain(|token, conn| { + let State::Requests(requests) = &mut conn.state else { return true }; + if !requests.overdue(now, *request_deadline) { + return true; + } + timed_out = true; + silver_log::warn!( + "beacon api request still incomplete after {request_deadline:?}, answering 408" + ); + if let Err(e) = requests.time_out(&mut conn.stream, registry, *token) { + silver_log::warn!("beacon api 408 could not be queued, closing: {e}"); + let _ = registry.deregister(&mut conn.stream); + return false; + } + true + }); + timed_out + } + fn close_expired(&mut self, now: Instant) -> bool { let Self { connections, registry, idle, linger, streams, .. } = self; let before = connections.len(); @@ -2267,6 +2320,82 @@ mod tests { assert_eq!(server.api.connections.len(), 1, "an active connection must survive the sweep"); } + /// Each byte refreshes the idle deadline, so only the request deadline + /// can end this request. + #[test] + fn request_still_arriving_at_its_deadline_is_answered_408() { + let mut server = server_with(64, Duration::from_millis(400)); + let deadline = Duration::from_millis(300); + server.api.request_deadline = deadline; + let addr = tcp_addr(&server); + + let (received, sending_for) = serve( + &mut server, + std::thread::spawn(move || { + let mut stream = connect(addr); + stream.set_read_timeout(Some(Duration::from_millis(50))).unwrap(); + let started = Instant::now(); + write!( + stream, + "POST /eth/v1/beacon/pool/attestations HTTP/1.1\r\nHost: x\r\n\ + Content-Length: 4096\r\n\r\n" + ) + .unwrap(); + let mut received = Vec::new(); + let mut chunk = [0u8; 1024]; + while started.elapsed() < Duration::from_secs(5) { + if stream.write_all(b" ").is_err() { + break; + } + match stream.read(&mut chunk) { + Ok(0) => break, + Ok(n) => received.extend_from_slice(&chunk[..n]), + Err(e) if would_block(&e) => {} + Err(_) => break, + } + } + (received, started.elapsed()) + }), + "request answered at its deadline", + ); + + assert_eq!( + received, + b"HTTP/1.1 408 Request Timeout\r\nConnection: close\r\nContent-Length: 0\r\n\r\n", + "{}", + String::from_utf8_lossy(&received) + ); + assert!(sending_for >= deadline, "answered after {sending_for:?}"); + pump_until(&mut server, "timed out connection closed", |server| { + server.api.connections.is_empty() + }); + } + + #[test] + fn keep_alive_wait_between_requests_is_not_held_to_the_request_deadline() { + let mut server = server_with(64, Duration::from_secs(2)); + server.api.request_deadline = Duration::from_millis(100); + let addr = tcp_addr(&server); + + let _still_open = serve( + &mut server, + std::thread::spawn(move || { + let mut stream = connect(addr); + let mut chunk = [0u8; 1024]; + for i in 0..2 { + write!(stream, "GET /metrics HTTP/1.1\r\nHost: x\r\n\r\n").unwrap(); + let n = stream.read(&mut chunk).unwrap(); + assert!(chunk[..n].starts_with(b"HTTP/1.1 200 OK\r\n"), "request #{i}"); + std::thread::sleep(Duration::from_millis(600)); + } + stream + }), + "keep-alive client served across a wait past the request deadline", + ); + + assert_eq!(server.api.connections.len(), 1); + } + /// Connection exhaustion scenario end to end: a hung client owns the only /// slot, so every other client is refused until the sweep frees it. #[test] diff --git a/crates/httpcore/src/server.rs b/crates/httpcore/src/server.rs index 4e49c6e14..028744ed3 100644 --- a/crates/httpcore/src/server.rs +++ b/crates/httpcore/src/server.rs @@ -272,6 +272,11 @@ impl ServerConnection { false } + pub fn reject_timed_out(&mut self) { + debug_assert!(self.write_buf.is_empty(), "a response is already framed"); + self.reject("408 Request Timeout"); + } + /// Scratch for a lingering connection's drain: the bytes are read only to /// be dropped, so the buffer is reused as it stands and never grows. pub fn discard_space(&mut self) -> &mut [u8] { @@ -751,6 +756,21 @@ mod tests { reject_and_linger(b"\x16\x03\x01\x02\x00\x01\x00\x01\xfc\x03\x03", "400 Bad Request"); } + #[test] + fn timed_out_request_writes_408_then_lingers() { + let mut conn = ServerConnection::new(); + feed(&mut conn, b"POST /p HTTP/1.1\r\nHost: x\r\nContent-Length: 8\r\n\r\nhalf"); + assert!(!conn.dispatch(&mut |_, _: &mut Vec| panic!("the body has not arrived"))); + + conn.reject_timed_out(); + assert_eq!( + conn.pending_write(), + b"HTTP/1.1 408 Request Timeout\r\nConnection: close\r\nContent-Length: 0\r\n\r\n" + ); + drain(&mut conn); + assert_eq!(conn.after_response(&mut echo_path), AfterResponse::Linger); + } + /// The body the client is still sending is read for one reason only — to /// keep the answer from being lost — so it must cost nothing to read. #[test] From 0d34efbed97690f3b23201235f362bfc4f6ce60b Mon Sep 17 00:00:00 2001 From: Adrian Chifor Date: Fri, 30 Oct 2026 17:37:02 +0200 Subject: [PATCH 4/5] Drop proposer preparations for indices past the registry prepare_beacon_proposer recorded every well-formed validator_index. The BeaconState tile kept each one in a map for two epochs. Indices at or past the head registry length are now dropped. Such an index cannot propose within the two retained epochs. Validator clients resend preparations each epoch. The request still answers 200, as Lighthouse does for unknown indices. --- .../beacon_api/src/validator/registration.rs | 86 +++++++++++++++---- 1 file changed, 67 insertions(+), 19 deletions(-) diff --git a/crates/beacon_api/src/validator/registration.rs b/crates/beacon_api/src/validator/registration.rs index cdf1b92c7..970449f17 100644 --- a/crates/beacon_api/src/validator/registration.rs +++ b/crates/beacon_api/src/validator/registration.rs @@ -33,26 +33,37 @@ pub(crate) fn post_register_validator(req: &Request<'_>, ctx: &ApiCtx, resp: &mu resp.await_sidecar(SidecarCall::RegisterValidators); } +/// An index past the registry is dropped, as Lighthouse drops it: it cannot +/// propose within the epochs a preparation is kept, and clients resend each +/// epoch. pub(crate) fn post_prepare_beacon_proposer( req: &Request<'_>, - _ctx: &ApiCtx, + ctx: &ApiCtx, resp: &mut Response<'_>, ) { let Some(preparations) = received(req.body, resp, ProposerPreparation::well_formed) else { return; }; - silver_log::debug!(count = preparations.len(), "proposer preparations received"); - if preparations.is_empty() { + let registry_len = ctx.read_state(|view| view.validators.count()) as u64; + let records: Vec<_> = preparations + .iter() + .map(ProposerPreparation::record) + .filter(|record| record.validator_index < registry_len) + .collect(); + silver_log::debug!( + count = preparations.len(), + registered = records.len(), + "proposer preparations received" + ); + if records.is_empty() { return resp.ok(); } resp.submit( - preparations.len() * PreparationRecord::SIZE, + records.len() * PreparationRecord::SIZE, |out| { - for (record, preparation) in - out.chunks_exact_mut(PreparationRecord::SIZE).zip(&preparations) - { - preparation.record().encode(record); + for (chunk, record) in out.chunks_exact_mut(PreparationRecord::SIZE).zip(&records) { + record.encode(chunk); } }, |preparations| BeaconApiRequest::ProposerPreparations { preparations }, @@ -100,13 +111,15 @@ impl ProposerPreparation<'_> { #[cfg(test)] mod tests { - use silver_beacon_state_data::SLOTS_PER_EPOCH; + use silver_beacon_state_data::{ + BeaconState, BeaconStateOwner, EpochStateFinalized, SLOTS_PER_EPOCH, SpecConfig, ValSeed, + }; use silver_common::TCacheProducer; use silver_httpcore::ParsedRequest; use super::*; use crate::{ - ctx::anchor_ctx, + ctx::{anchor_ctx, test_ctx}, http::{ids::MAX_BODY_IDS, router::Outcome}, testing::{dispatch, dispatch_into, posting, submissions}, }; @@ -154,9 +167,8 @@ mod tests { } /// Every receipt endpoint answers the same bodyless 200: neither schema - /// declares content under it. Neither reads beacon state, so - /// the whole suite runs against a node that has published none — which is - /// when a validator client first sends these. + /// declares content under it. The suite runs against an empty anchor, + /// which is when a validator client first sends these. #[test] fn well_formed_body_is_acknowledged_with_a_bodyless_200() { for (path, entry) in entries() { @@ -238,16 +250,28 @@ mod tests { assert_bad_request(&json_post(PREPARE, &at_cap), "invalid entry in request body"); } + fn ctx_with_validators(count: usize) -> ApiCtx { + let seeds: Vec<_> = (0..count).map(|_| ValSeed::default()).collect(); + let state = BeaconState::for_test(EpochStateFinalized::default(), &seeds, 0); + let mut owner = BeaconStateOwner::new(state); + let anchor = owner.roll_fresh(); + owner.publish_state_id(anchor); + test_ctx(&SpecConfig::mainnet(), owner.reader()) + } + + fn preparation_of(validator_index: u64, fee_recipient: u8) -> String { + format!( + "{{\"validator_index\":\"{validator_index}\",\"fee_recipient\":\"0x{}\"}}", + hex::encode([fee_recipient; 20]) + ) + } + #[test] fn preparations_are_forwarded_through_the_cache() { - let other = format!( - "{{\"validator_index\":\"9\",\"fee_recipient\":\"0x{}\"}}", - hex::encode([0x11; 20]) - ); - let body = format!("[{},{other}]", preparation()); + let body = format!("[{},{}]", preparation(), preparation_of(9, 0x11)); let mut submissions = submissions(); let (outcome, response) = - dispatch_into(&anchor_ctx(), &posting(PREPARE, &body), &mut submissions); + dispatch_into(&ctx_with_validators(10), &posting(PREPARE, &body), &mut submissions); assert_eq!(response, BODYLESS_OK); let Outcome::Response(Some(BeaconApiRequest::ProposerPreparations { preparations })) = outcome @@ -261,6 +285,30 @@ mod tests { ])); } + #[test] + fn preparations_past_the_registry_are_dropped() { + let body = format!("[{},{}]", preparation_of(10, 0x11), preparation_of(9, 0x22)); + let mut submissions = submissions(); + let (outcome, response) = + dispatch_into(&ctx_with_validators(10), &posting(PREPARE, &body), &mut submissions); + assert_eq!(response, BODYLESS_OK); + let Outcome::Response(Some(BeaconApiRequest::ProposerPreparations { preparations })) = + outcome + else { + panic!("{outcome:?}") + }; + let written = submissions.read_buffer(preparations).unwrap(); + assert!( + PreparationRecord::decode_all(written) + .eq([PreparationRecord { validator_index: 9, fee_recipient: [0x22; 20] }]) + ); + + let body = format!("[{},{}]", preparation_of(10, 0x11), preparation_of(u64::MAX, 0x22)); + let (outcome, response) = dispatch(&ctx_with_validators(10), &posting(PREPARE, &body)); + assert_eq!(response, BODYLESS_OK); + assert_eq!(outcome, Outcome::Response(None), "nothing reaches the state tile"); + } + #[test] fn registrations_wait_on_the_sidecar() { let mut ctx = anchor_ctx(); From 968c01300906ad0e0de975a2925bbfb2e973dd7a Mon Sep 17 00:00:00 2001 From: Adrian Chifor Date: Fri, 30 Oct 2026 18:02:36 +0200 Subject: [PATCH 5/5] Bound submission entries awaiting their verdicts Each accepted submission entry becomes one message per topic on the beacon_api_requests queue, emitted in one burst. That queue holds 2^14 messages and is shared with block and other requests. A single body could carry up to 256K entries, more than the queue holds. Consumers that fall that far behind skip ahead, losing unrelated requests. Multi-entry submission bodies are now capped at 4096 entries. That covers one slot's attestations for 131,072 validators. The server also caps entries awaiting verdicts across all connections at 8192, half the queue. A body that would exceed it is answered 503. The count is derived from each connection's pending submission. It is released as verdicts arrive or when the connection is reaped. A body of one entry is always admitted. The connection cap bounds those, and refusing a block would lose its proposal. --- crates/beacon_api/src/beacon/operations.rs | 18 +++- crates/beacon_api/src/server.rs | 112 ++++++++++++++++++++- crates/beacon_api/src/submission.rs | 13 ++- 3 files changed, 135 insertions(+), 8 deletions(-) diff --git a/crates/beacon_api/src/beacon/operations.rs b/crates/beacon_api/src/beacon/operations.rs index 7f2f3cc3a..432ca3dff 100644 --- a/crates/beacon_api/src/beacon/operations.rs +++ b/crates/beacon_api/src/beacon/operations.rs @@ -280,7 +280,7 @@ pub(crate) mod tests { use crate::{ http::router::Outcome, submission::{ - SubmissionFailure, + MAX_SUBMISSION_ENTRIES, SubmissionFailure, tests::{SLOT, ctx, posted_committees}, }, testing::{body, dispatch, dispatch_into, posting, status_code, submissions}, @@ -344,6 +344,22 @@ pub(crate) mod tests { assert_eq!(SingleAttestationView::signature(ssz), &[0x44; 96]); } + #[test] + fn body_past_the_submission_entry_cap_is_a_400() { + let ctx = ctx(); + let at_cap = format!("[{}]", vec![entry(1, 0); MAX_SUBMISSION_ENTRIES].join(",")); + assert!(matches!(submit(&ctx, &at_cap).0, Outcome::AwaitingVerdicts(_))); + + let past_cap = format!("[{}]", vec![entry(1, 0); MAX_SUBMISSION_ENTRIES + 1].join(",")); + let (outcome, response) = submit(&ctx, &past_cap); + assert_eq!(outcome, Outcome::Response(None)); + assert_eq!(status_code(&response), "400"); + assert_eq!( + body(&response), + b"{\"code\":400,\"message\":\"too many entries in request body\"}" + ); + } + /// Entries the posted shuffling cannot place are reported by their place /// in the body, and the rest of the submission still goes out. #[test] diff --git a/crates/beacon_api/src/server.rs b/crates/beacon_api/src/server.rs index c5fd6cce5..5bfa36d3b 100644 --- a/crates/beacon_api/src/server.rs +++ b/crates/beacon_api/src/server.rs @@ -38,8 +38,8 @@ use crate::{ node::peers::Peer, routes::{ROUTES, SERVED_WHILE_SYNCING}, submission::{ - AcceptedEntry, BuilderForward, Integration, RecentImports, SubmissionFailure, - failure_message, + AcceptedEntry, BuilderForward, Integration, MAX_SUBMISSION_ENTRIES, RecentImports, + Submission, SubmissionFailure, failure_message, }, validator::{ aggregate_attestation::AggregateRequest, execution_payload_envelope::EnvelopeRequest, @@ -53,6 +53,10 @@ const MAX_SWEEP_INTERVAL: Duration = Duration::from_secs(1); // that matter, such as blocks, are useless after one 12 s slot anyway. const REQUEST_DEADLINE: Duration = Duration::from_secs(60); +/// Two of the largest bodies at once: half the `beacon_api_requests` queue, +/// leaving the rest to the blocks and requests that share it. +const MAX_OUTSTANDING_SUBMISSIONS: usize = 2 * MAX_SUBMISSION_ENTRIES; + /// nginx's `lingering_close` caps: how long a connection that has already /// answered may wait between the peer's bytes, and how long the whole drain /// may run before the slot is taken back. @@ -1506,14 +1510,23 @@ impl BeaconApi { self.want_payload_attributes(true, emit); } } + Outcome::AwaitingVerdicts(submission) if !self.admits(&submission) => { + silver_log::warn!( + entries = submission.accepted.len(), + "submission refused: too many entries awaiting their verdicts" + ); + let requests = self.deferring(token); + let mut resp = Response::new(requests.http.write_buf_mut(), submissions); + resp.error(503, "too many submissions awaiting publication, retry shortly"); + self.resume_writing(token); + } deferred => { let (pending, ids_used) = Pending::defer(deferred, self.next_request_id, emit); self.next_request_id += ids_used; if matches!(&pending, Pending::Submission(p) if p.integration.awaits_import()) { self.awaiting_import.push(token); } - let requests = conn.requests_mut().expect("only a request handler defers"); - requests.pending = Some(pending); + self.deferring(token).pending = Some(pending); } }, Ok(true) => { @@ -1529,6 +1542,33 @@ impl BeaconApi { true } + fn deferring(&mut self, token: Token) -> &mut Requests { + self.connections + .get_mut(&token) + .and_then(Connection::requests_mut) + .expect("only a request handler defers") + } + + /// A body of one entry is admitted past the budget: the connection cap + /// bounds those, and a refused block would cost its proposal. + fn admits(&self, submission: &Submission) -> bool { + submission.body_len() == 1 || + self.outstanding_submissions() + submission.accepted.len() <= + MAX_OUTSTANDING_SUBMISSIONS + } + + /// Counted from the connections' own waits, so a connection that closes + /// or times out gives its share back with it. + fn outstanding_submissions(&self) -> usize { + self.connections + .values() + .filter_map(|conn| match conn.pending()? { + Pending::Submission(pending) => Some(pending.unanswered), + _ => None, + }) + .sum() + } + /// Connections close in any order while the cursor only advances, so the /// offset it lands on may still be held. The range holds every socket the /// server can register at once, so probing forward ends on a free one. @@ -3786,6 +3826,70 @@ mod tests { ); } + const ATTESTATIONS: &str = "/eth/v2/beacon/pool/attestations"; + + fn post_attestations(server: &mut Server, count: usize) -> TcpStream { + let client = connect(tcp_addr(server)); + let body = format!("[{}]", vec![entry(1, 0); count].join(",")); + let writer = client.try_clone().unwrap(); + std::thread::spawn(move || post(writer, ATTESTATIONS, &body)); + client + } + + fn submitted_after(server: &mut Server, count: usize, msg: &str) { + pump_until(server, msg, |s| s.requests.len() == count); + } + + fn refused(server: &mut Server, count: usize) -> Vec { + let client = post_attestations(server, count); + let response = serve(server, std::thread::spawn(move || read_to_eof(client)), "refusal"); + assert!(response.starts_with(b"HTTP/1.1 503 Service Unavailable\r\n"), "{response:?}"); + response + } + + /// A body's share of the budget comes back when its entries are answered + /// or when its connection is reaped. + #[test] + fn submissions_past_the_outstanding_budget_are_answered_503() { + let mut server = server_with(64, LONG_TIMEOUT); + server.api.ctx = submission::ctx(); + server.submissions = TCache::producer(TCacheId::BoundaryProcessing, 1 << 24); + + let first = post_attestations(&mut server, MAX_SUBMISSION_ENTRIES); + submitted_after(&mut server, MAX_SUBMISSION_ENTRIES, "the first body"); + let second = post_attestations(&mut server, MAX_SUBMISSION_ENTRIES); + submitted_after(&mut server, MAX_OUTSTANDING_SUBMISSIONS, "the second body"); + + let response = refused(&mut server, 2); + assert!(body(&response).starts_with(b"{\"code\":503,")); + assert_eq!(server.requests.len(), MAX_OUTSTANDING_SUBMISSIONS, "none of it was sent"); + + let _lone = post_attestations(&mut server, 1); + submitted_after(&mut server, MAX_OUTSTANDING_SUBMISSIONS + 1, "a body of one entry"); + + let first_ids: Vec<_> = + server.requests.drain(..MAX_SUBMISSION_ENTRIES).map(|r| request_id(&r)).collect(); + for request_id in first_ids { + server.answer(BeaconApiResponse::LocalGossipResponse { + request_id, + response: Ok(()), + block_root: None, + }); + } + let response = serve(&mut server, std::thread::spawn(move || read_to_eof(first)), "200"); + assert!(response.starts_with(b"HTTP/1.1 200 OK\r\n"), "{response:?}"); + let _admitted = post_attestations(&mut server, 2); + submitted_after(&mut server, MAX_SUBMISSION_ENTRIES + 3, "a body within the budget"); + + refused(&mut server, MAX_SUBMISSION_ENTRIES); + drop(second); + server.api.idle.timeout = Duration::ZERO; + pump_until(&mut server, "idle connections reaped", |s| s.api.connections.is_empty()); + server.api.idle.timeout = LONG_TIMEOUT; + let _largest = post_attestations(&mut server, MAX_SUBMISSION_ENTRIES); + submitted_after(&mut server, 2 * MAX_SUBMISSION_ENTRIES + 3, "the largest body"); + } + #[test] fn entry_published_on_several_topics_takes_one_request_id() { let ssz = submissions().write_with(1, |_| {}).unwrap(); diff --git a/crates/beacon_api/src/submission.rs b/crates/beacon_api/src/submission.rs index 119106326..77e0ac601 100644 --- a/crates/beacon_api/src/submission.rs +++ b/crates/beacon_api/src/submission.rs @@ -4,13 +4,19 @@ use serde::Deserialize; use silver_beacon_state_data::{B256, Slot, SpecConfig}; use silver_common::{ BroadcastValidation, GossipTopic, LocalGossipFailure, PayloadValidationStatus, TCacheRead, + json_fields::each_array_entry, }; use crate::{ ctx::ApiCtx, - http::{ids::each_body_entry, response::Response, router::Request}, + http::{response::Response, router::Request}, }; +/// Each accepted entry is a message per topic on the 2^14-deep +/// `beacon_api_requests` queue. A body of single-topic entries is a quarter of +/// it at most: one slot's attestations for 131,072 validators. +pub(crate) const MAX_SUBMISSION_ENTRIES: usize = 4096; + pub(crate) fn post_submission<'a, T: Deserialize<'a> + SubmittedEntry>( req: &Request<'a>, ctx: &ApiCtx, @@ -19,8 +25,9 @@ pub(crate) fn post_submission<'a, T: Deserialize<'a> + SubmittedEntry>( if !reads_submission(req, resp) { return; } - let parsed = - each_body_entry(req.body, |body_index, entry: T| submit(ctx, resp, body_index, entry)); + let parsed = each_array_entry(req.body, MAX_SUBMISSION_ENTRIES, |body_index, entry: T| { + submit(ctx, resp, body_index, entry) + }); match parsed { Err(message) => resp.error(400, message), Ok(0) => resp.error(400, "the body must name at least one entry"),