Skip to content

P2P controller hardening#60

Open
kalabukdima wants to merge 9 commits into
masterfrom
net-stream-server
Open

P2P controller hardening#60
kalabukdima wants to merge 9 commits into
masterfrom
net-stream-server

Conversation

@kalabukdima

@kalabukdima kalabukdima commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Closes #25

Transport layer

Replace request-response behaviour with a much simpler and more flexible stream behaviour. Benefits:

  • Protobuf parsing used to run deep inside the transport lib. Now the worker parses the messages, simplifying the flow.
  • Instead of passing the result back to the transport under the global swarm lock, each request handler can now write the response or drop the stream independently.
  • No hidden limits and dropped connections.
  • Manual timeout handling for reads and writes separately.
  • It opens the road for multipart/streaming responses instead of a single protobuf message.

See subsquid/sqd-network#211

Handling backpressure

Now there is a synchronous event loop pulling events (stream requests) from the transport layer. It quickly checks if the client is allowed to send the request and spawns a processing tokio task or sends the rejection to the client. If this loop can't keep up with the incoming requests, extra requests will get dropped before being parsed.

This is not perfect because it doesn't propagate the backpressure to cut the early handshake. The full incoming stream is still read to memory. But making it perfect requires a significant reorg of the libp2p stack and isn't directly supported by existing libp2p behaviours.

Graceful rejections

In many cases the stream was dropped without sending any bytes. The client just saw a "connection reset" which was difficult to diagnose. Try to return a meaningful rejection now. Only drop the stream if there are too many in-flight rejections or the response couldn't be written.

Scenario Response Log Used CU
Undecodable query connection reset (no query_id to sign) none 0
Invalid signature connection reset → BadRequest("invalid query signature") none 0
Timestamp out of range connection reset → BadRequest("timestamp out of allowed range") none 0
Queue full (backpressure) connection reset → ServerOverloaded + backoff none 0
No allocation TooManyRequests none 0
Rate limited (paused) TooManyRequests + retry none 0
Malformed params (bad compression / missing block_range) BadRequest BadRequest 0 → 1
Success, delivered Ok Ok summary chip²
Success, signing fails connection reset → ServerError("failed to sign query result") Ok summary → ServerError chip²
Success, result too large connection reset → ServerError("query result too large") Ok summary → ServerError chip²
Success, transport send-buffer full connection reset → Ok (delivered) ¹ Ok summary chip²
NotFound (no chunks in range) NotFound NotFound chip²
Engine overloaded (ServiceOverloaded) ServerOverloaded + backoff ServerOverloaded 0 → chip²
Internal engine error (Other) ServerError ServerError chip²

¹ The old transport had an internal send queue that could reject a perfectly good result. stream_server's ResponseSender writes to the stream directly, so that queue no longer exists; the result is delivered (a reset now only happens if the client itself is gone or the write times out).
² chip = the query's overlap fraction with the chunk (1 for a full-chunk query); the flat 1.0 is claimed at admission and the unused part refunded, so a served query's charge is chip.

@kalabukdima kalabukdima self-assigned this Jul 8, 2026
@kalabukdima
kalabukdima marked this pull request as ready for review July 9, 2026 06:52
kalabukdima and others added 9 commits July 15, 2026 18:25
Extract the compute-unit accounting (`process_query`) and log building
(`generate_log`) into free functions generic over two dependency traits,
`QueryRunner` and `CuChecker`, and add mock implementations. This lets
the query pipeline be unit-tested without standing up the transport,
storage, or a real query engine.

Tests cover the current behavior: an overloaded engine is fully
refunded, malformed params are rejected before the CU spend, partial
chunks refund the unused fraction, and no-allocation queries are not
logged.
Bump sqd-network to the stream_server server behaviour (libp2p-stream
instead of request_response): responses are written to the stream
directly, so the send_* handle methods are now async.

On a full processing queue the worker now returns a signed
too_many_requests error instead of dropping the stream, which had left
the client with an opaque transport error.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The transport now hands over raw request bytes and takes raw response
bytes. Decode Query/LogsRequest in the event loop (off the swarm loop)
and encode responses via ResponseSender::send directly, replacing the
removed WorkerTransportHandle::send_* methods.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The log entry a worker stores (and gets rewarded on) was built
independently of the response the client received, so an oversized or
unsignable result was sent as a `server_error` while still being logged
as a successful query. Make the compute-unit spend the single boundary:
a query is logged iff it consumed a CU, and the log is a projection of
the response that was actually sent.

- Move admission (verify, reserve slot, spend one CU) into the event
  loop; reject pre-admission queries with signed typed errors and a
  backoff hint instead of silently dropping the stream
- Build the wire response and its log from one outcome in `build_delivery`,
  so an oversized/unsignable result downgrades to `server_error` in both
- Charge and log malformed params and engine overload; only unprovable or
  unbudgeted rejects (bad signature/timestamp, no allocation, rate limit)
  stay out of the log store, which bounds a log-flood DoS
- Bound concurrent reject responses with a semaphore
- Replace the run_all! macro and hand-wired cancellation token tree
  with a subsystem tree that drains gracefully and cancels stragglers
  after a 5s timeout instead of hanging forever
- Leak the controller for &'static access, removing the Arc clone
  per spawned task
- Report a loop that dies on its own as a named subsystem error
  instead of a silent all-Ok shutdown
- Pin the dependency to an exact version; the audited tarball hash
  is recorded in Cargo.lock

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.

P2P controller issues

1 participant