Skip to content

fix(network): backport QUIC loss recovery progress - #941

Closed
czarcas7ic wants to merge 2 commits into
mainfrom
adam/getblocks-quic-prerequisite
Closed

fix(network): backport QUIC loss recovery progress#941
czarcas7ic wants to merge 2 commits into
mainfrom
adam/getblocks-quic-prerequisite

Conversation

@czarcas7ic

@czarcas7ic czarcas7ic commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Closed in favor of the separate Iroh upgrade in #935. The backport was removed from the remaining GetBlocks drafts; #942 now targets main.

Motivation

QUIC loss recovery can stop making progress when a socket cannot send a batch of datagrams. This is the transport prerequisite extracted from #892.

Solution

Pin the complete QUIC crate family to the reviewed backport through the workspace patch table, retaining registry version requirements for downstream consumers. Downstream workspaces must apply the same patch themselves. This does not activate the new block-sync protocol.

Testing

Formatting, patch and changelog checks pass. The focused QUIC transfer regression passes. The regression transfers more data than the flow-control windows in both directions. Prepared with Codex assistance.

Changelog

Added the QUIC recovery fragment for this PR.

@v12-auditor

v12-auditor Bot commented Sep 9, 2026

Copy link
Copy Markdown

Note

Complete: Audit complete. V12 found three issues worth reviewing.

Open the full results here.

FindingSeverityDetails
F-271270 🟠 High
Stale IP after QUIC migration

Zakura snapshots a peer's UDP source IP once after the native control handshake and then treats that address as the connection's permanent identity for active_by_ip accounting. direct_endpoint_builder and ZakuraLocalLimits::transport_config() never disable QUIC connection migration, so Iroh/Quinn still accept a validated path change from address A to address B. confirmed_remote_ip reads endpoint.conn_type() / remote_info() only at accept or outbound-dial registration; ZakuraPeerConnectionEntry.remote_ip is never refreshed. After a malicious peer completes path validation on B, traffic rides on B while the per-IP cap, duplicate-eviction same-IP shortcut, and registration metadata continue to charge A. The same hole the inbound accept path recently closed — one current source IP filling the global budget with distinct node ids — reopens for any peer that can control two reachable addresses.

F-271271 🟠 High
QUIC windows enable memory DoS

When Zakura P2P v2 is enabled, every admitted QUIC connection is configured with 32 MiB stream, connection-receive, and send windows, while admission only counts connections. ZakuraLocalLimits::transport_config() writes DEFAULT_ZAKURA_STREAM_RECEIVE_WINDOW, DEFAULT_ZAKURA_RECEIVE_WINDOW, and DEFAULT_ZAKURA_SEND_WINDOW (all 32 MiB) into Iroh's TransportConfig for the endpoint. The inbound gate is Semaphore::new(limits.max_connections) with default 256, plus an exact-IP bucket of 16. Application admit_inbound_message() enforces the 4 MiB LOCAL_MAX_MESSAGE_BYTES cap only after QUIC has already buffered stream data up to the transport window. Testnet and other non-Mainnet defaults resolve P2pStack::Default to Dual, so a public 0.0.0.0:8234 listener starts with these windows unless the operator opts out.

F-271272 🟡 Medium
Custom testnets share handshake identity

Zakura P2P v2 uses one ALPN (p2p-v2/1) for every network and then binds peers with ZakuraNetworkId plus chain_id. ZakuraNetworkId::from_network maps every non-default, non-regtest Network::Testnet to the same Configured wire value, and ZakuraHandshakeConfig::for_network sets chain_id to network.genesis_hash(). The testnet parameter builder defaults genesis_hash to the public Testnet genesis while with_network_magic independently allows a distinct magic. Two custom testnets that keep that default genesis therefore advertise identical network_id and chain_id, so ZakuraControlHello::validate accepts the peer. Magic is never carried on the v2 hello or ack. Dev-cohort isolation only applies when dev_network is set; ordinary configured testnets have no extra discriminator.

Analyzed one file, diff 03d33fc...1ec3d01.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant