Skip to content

Commit 3aeaa2f

Browse files
committed
fix(zebrad): key the mempool per-peer download cap on IpAddr
The mempool inbound-download manager keyed its per-peer concurrency cap (`MAX_INBOUND_CONCURRENCY_PER_PEER`) on the full `SocketAddr`. The `source` is a transient `(IP, ephemeral port)`, so two connections from the same host landed in two distinct `pending_per_peer` buckets, each with its own 5-slot budget, and a single host could exceed the intended per-host bound. `get_transient_addr` is explicitly not a permanent identifier. Re-key `pending_per_peer` on `IpAddr` at the cap check, the increment, and `release_peer_slot`, mirroring the inbound-block download path (`in_flight_ips`). Adds a regression test asserting a sixth transaction from the same IP on a different port is rejected once the IP holds `MAX_INBOUND_CONCURRENCY_PER_PEER`.
1 parent 31e555e commit 3aeaa2f

3 files changed

Lines changed: 83 additions & 13 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,10 @@ and this project adheres to [Semantic Versioning](https://semver.org).
6868
peer's IPv4 address did not disconnect it while it stayed connected, and the same peer counted
6969
twice towards the per-IP inbound connection limit
7070
([#10695](https://github.com/ZcashFoundation/zebra/issues/10695)).
71+
- The mempool per-peer inbound-download cap is now keyed on the peer's IP address instead of its
72+
full socket address, so a single host can no longer exceed the cap by opening connections from
73+
multiple source ports. This matches the inbound-block download cap
74+
([#10685](https://github.com/ZcashFoundation/zebra/issues/10685)).
7175

7276
## [Zebra 6.2.3](https://github.com/ZcashFoundation/zebra/releases/tag/v6.2.3) - 2026-07-27
7377

zebrad/src/components/mempool/downloads.rs

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
//! [`Mempool::poll_ready`]: super::Mempool::poll_ready
2828
use std::{
2929
collections::{HashMap, HashSet},
30-
net::SocketAddr,
30+
net::{IpAddr, SocketAddr},
3131
pin::Pin,
3232
task::{Context, Poll},
3333
time::Duration,
@@ -206,12 +206,19 @@ where
206206
),
207207
>,
208208

209-
/// The number of currently in-flight download tasks per advertising peer.
209+
/// The number of currently in-flight download tasks per advertising host,
210+
/// keyed on the peer's [`IpAddr`].
210211
///
211-
/// Invariant: a peer is present here iff some entry in [`Self::cancel_handles`]
212-
/// has it as the third tuple element. Enforces
212+
/// Keyed on the host IP rather than the full `SocketAddr` so that all
213+
/// connections from one host share a single budget: the `source` is a
214+
/// transient `(IP, ephemeral port)`, so per-`SocketAddr` keying would give
215+
/// each connection its own bucket. This matches the inbound-block download
216+
/// cap (`in_flight_ips`). See #10685.
217+
///
218+
/// Invariant: an IP is present here with count `n` iff exactly `n` entries in
219+
/// [`Self::cancel_handles`] have a source with that IP. Enforces
213220
/// [`MAX_INBOUND_CONCURRENCY_PER_PEER`]. See `GHSA-4fc2-h7jh-287c`.
214-
pending_per_peer: HashMap<SocketAddr, usize>,
221+
pending_per_peer: HashMap<IpAddr, usize>,
215222
}
216223

217224
impl<ZN, ZV, ZS> Stream for Downloads<ZN, ZV, ZS>
@@ -371,10 +378,10 @@ where
371378
return Err(MempoolError::FullQueue);
372379
}
373380

374-
// Per-peer cap: a single advertising peer cannot saturate the queue
375-
// with attacker-supplied fake txids. See `GHSA-4fc2-h7jh-287c`.
381+
// Per-peer cap: a single advertising host (keyed by IP) cannot saturate
382+
// the queue with attacker-supplied fake txids. See `GHSA-4fc2-h7jh-287c`.
376383
if let Some(source) = source {
377-
let count = self.pending_per_peer.get(&source).copied().unwrap_or(0);
384+
let count = self.pending_per_peer.get(&source.ip()).copied().unwrap_or(0);
378385
if count >= MAX_INBOUND_CONCURRENCY_PER_PEER {
379386
debug!(
380387
?txid,
@@ -550,7 +557,7 @@ where
550557
if let Some(source) = source {
551558
// The per-peer cap check above ensures this can't exceed
552559
// `MAX_INBOUND_CONCURRENCY_PER_PEER`.
553-
*self.pending_per_peer.entry(source).or_insert(0) += 1;
560+
*self.pending_per_peer.entry(source.ip()).or_insert(0) += 1;
554561
}
555562

556563
debug!(
@@ -604,13 +611,14 @@ where
604611
metrics::gauge!("mempool.currently.queued.transactions",).set(self.pending.len() as f64);
605612
}
606613

607-
/// Decrement the per-peer pending count for `source`, removing the entry
614+
/// Decrement the per-host pending count for `source`'s IP, removing the entry
608615
/// when it reaches zero.
609-
fn release_peer_slot(pending_per_peer: &mut HashMap<SocketAddr, usize>, source: SocketAddr) {
610-
if let Some(count) = pending_per_peer.get_mut(&source) {
616+
fn release_peer_slot(pending_per_peer: &mut HashMap<IpAddr, usize>, source: SocketAddr) {
617+
let ip = source.ip();
618+
if let Some(count) = pending_per_peer.get_mut(&ip) {
611619
*count = count.saturating_sub(1);
612620
if *count == 0 {
613-
pending_per_peer.remove(&source);
621+
pending_per_peer.remove(&ip);
614622
}
615623
}
616624
}

zebrad/src/components/mempool/tests/vector.rs

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2366,3 +2366,61 @@ async fn verification_timeout_releases_peer_slot() {
23662366
.download_if_needed_and_verify(Gossip::Tx(tx), Some(source), None)
23672367
.expect("a further transaction from the same source should queue after slots are freed");
23682368
}
2369+
2370+
/// Regression test for #10685: the mempool per-peer download cap must be keyed
2371+
/// on the peer's `IpAddr`, not the full `SocketAddr`.
2372+
///
2373+
/// The `source` is a transient `(IP, ephemeral port)`, so keying on the whole
2374+
/// `SocketAddr` gave every connection from one host its own budget. Two sources
2375+
/// with the same IP and different ports must share a single
2376+
/// `MAX_INBOUND_CONCURRENCY_PER_PEER` bucket.
2377+
#[tokio::test(flavor = "current_thread")]
2378+
async fn per_peer_cap_is_keyed_on_ip_not_socket_addr() {
2379+
use std::net::SocketAddr;
2380+
2381+
use tower::timeout::Timeout;
2382+
use zebra_node_services::mempool::Gossip;
2383+
2384+
use crate::components::mempool::downloads::{
2385+
Downloads, MAX_INBOUND_CONCURRENCY_PER_PEER, TRANSACTION_DOWNLOAD_TIMEOUT,
2386+
TRANSACTION_VERIFY_TIMEOUT,
2387+
};
2388+
2389+
let _init_guard = zebra_test::init();
2390+
2391+
let peer_set: MockPeerSet = MockService::build().for_unit_tests();
2392+
let state: MockService<zs::Request, zs::Response, PanicAssertion> =
2393+
MockService::build().for_unit_tests();
2394+
let tx_verifier: MockTxVerifier = MockService::build().for_unit_tests();
2395+
2396+
let mut downloads = Box::pin(Downloads::new(
2397+
Timeout::new(peer_set, TRANSACTION_DOWNLOAD_TIMEOUT),
2398+
Timeout::new(tx_verifier, TRANSACTION_VERIFY_TIMEOUT),
2399+
state,
2400+
));
2401+
2402+
let mut iter = Network::Mainnet.unmined_transactions_in_blocks(1..=10);
2403+
2404+
// Fill the per-host budget from one `(IP, port)`.
2405+
let first: SocketAddr = "127.0.0.1:8233".parse().expect("valid socket addr");
2406+
for i in 0..MAX_INBOUND_CONCURRENCY_PER_PEER {
2407+
let tx = iter.next().expect("enough vector txs").transaction;
2408+
downloads
2409+
.as_mut()
2410+
.download_if_needed_and_verify(Gossip::Tx(tx), Some(first), None)
2411+
.unwrap_or_else(|e| panic!("queue tx {i} failed: {e:?}"));
2412+
}
2413+
2414+
// A further transaction from the SAME IP but a DIFFERENT port must be
2415+
// rejected, because the cap is keyed on the host IP. Before the fix it
2416+
// landed in a separate `SocketAddr` bucket and was accepted.
2417+
let tx = iter.next().expect("enough vector txs").transaction;
2418+
let same_ip_other_port: SocketAddr = "127.0.0.1:9999".parse().expect("valid socket addr");
2419+
let result = downloads
2420+
.as_mut()
2421+
.download_if_needed_and_verify(Gossip::Tx(tx), Some(same_ip_other_port), None);
2422+
assert!(
2423+
matches!(result, Err(MempoolError::FullQueue)),
2424+
"a transaction from the same IP on a different port must share the per-peer bucket, got {result:?}"
2425+
);
2426+
}

0 commit comments

Comments
 (0)