Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
304 changes: 261 additions & 43 deletions Cargo.lock

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ bytes = "1"
clap = { version = "4.5", features = ["derive"] }
crc = "3"
directories-next = "2"
fastbloom = "0.14"
fastbloom = { version = "0.17", default-features = false }
futures-io = "0.3.19"
getrandom = { version = "0.3", default-features = false }
getrandom = { version = "0.4", default-features = false }
hdrhistogram = { version = "7.2", default-features = false }
hex-literal = "0.4"
identity-hash = "0.1.0"
Expand All @@ -34,7 +34,7 @@ log = "0.4"
pin-project-lite = "0.2"
proptest = { version = "1.9.0", default-features = false, features = ["std"] }
qlog = { package = "n0-qlog", version = "0.1.0" }
rand = "0.9"
rand = "0.10"
rcgen = "0.14"
ring = "0.17"
rustc-hash = "2"
Expand Down Expand Up @@ -66,7 +66,7 @@ async-global-executor = "2.4.1"
async-fs = "2.1"
async-executor = "1.13.0"
aws-lc-fips-sys = "0.13.10"
aws-lc-sys = "0.34.0"
aws-lc-sys = "0.39.0"
gcc = "0.3.55"
http = "1.1.0"
lazy_static = "1.5.0"
Expand Down
6 changes: 4 additions & 2 deletions deny.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,19 @@ allow = [
"NCSA",
"OpenSSL",
"Unicode-3.0",
"Zlib", # foldhash, dependency of fastbloom
]
private = { ignore = true }

[bans]
multiple-versions = "warn"
skip = [
# aws-lc-rs 1.x uses aws-lc-sys 0.32.x, our workaround deps use 0.34.x
{ crate = "aws-lc-sys", reason = "aws-lc-rs uses 0.32.x, minimal-versions workaround uses 0.34.x" },
# hdrhistogram uses base64 0.21, newer crates use 0.22
{ crate = "base64", reason = "hdrhistogram uses 0.21, newer crates use 0.22" },
{ crate = "cpufeatures", reason = "rand 0.10 pulls in newer chacha (which depends on this), but aes-gcm hasn't updated yet" },
# ring uses getrandom 0.2, newer crates use 0.3
{ crate = "getrandom", reason = "ring depends on 0.2, newer ecosystem uses 0.3" },
{ crate = "r-efi", reason = "proptest dev-dependency pulls in old getrandom" },
# jni and redox_users use thiserror 1.x
{ crate = "thiserror", reason = "transitive deps use thiserror 1.x" },
{ crate = "thiserror-impl", reason = "follows thiserror" },
Expand All @@ -43,4 +44,5 @@ skip = [
{ crate = "windows_x86_64_gnu", reason = "follows windows-sys" },
{ crate = "windows_x86_64_gnullvm", reason = "follows windows-sys" },
{ crate = "windows_x86_64_msvc", reason = "follows windows-sys" },
{ crate = "wit-bindgen", reason = "proptest dev-dependency pulls in old getrandom" },
]
6 changes: 5 additions & 1 deletion noq-proto/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,17 @@ web-time = { workspace = true }
assert_matches = { workspace = true }
hex-literal = { workspace = true }
proptest = { workspace = true }
rand_pcg = "0.9"
rand_pcg = "0.10"
rcgen = { workspace = true }
test-strategy = { workspace = true }
testresult = "0.4.1"
tracing-subscriber = { workspace = true }
wasm-bindgen-test = { workspace = true }

[target.'cfg(all(target_family = "wasm", target_os = "unknown"))'.dev-dependencies]
# ensure we set the wasm_js feature even for the older version of getrandom used in proptest in Wasm
getrandom_0_3 = { package = "getrandom", version = "0.3", features = ["wasm_js"] }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • so we have to enforce using both getrandom 0.3 and 0.4 in our tree atm?
  • is there now flag on getrandom 0.4 we need to set anymore?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only in dev-dependencies, and only because we still depend on proptest, and we run some tests in Wasm that depend on proptest (although we don't run proptests themselves). This is a trade-off we made a while ago to make cfg-ing proptest things everywhere not necessary.


[[bench]]
name = "send_buffer"
harness = false
Expand Down
3 changes: 2 additions & 1 deletion noq-proto/src/cid_generator.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::hash::Hasher;

use rand::{Rng, RngCore};
use rand::Rng;
use rand::RngExt;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's with our formatter? It is on strike?

(genuinely the dig is at our tooling, this is not something you should have to handle)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah - weirdly enough running cargo make format doesn't seem to change this code 🤔


use crate::Duration;
use crate::MAX_CID_SIZE;
Expand Down
2 changes: 1 addition & 1 deletion noq-proto/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ impl Default for EndpointConfig {
fn default() -> Self {
#[cfg(all(feature = "aws-lc-rs", not(feature = "ring")))]
use aws_lc_rs::hmac;
use rand::RngCore;
use rand::Rng;
#[cfg(feature = "ring")]
use ring::hmac;

Expand Down
8 changes: 2 additions & 6 deletions noq-proto/src/congestion/bbr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::any::Any;
use std::fmt::Debug;
use std::sync::Arc;

use rand::{Rng, SeedableRng};
use rand::RngExt;

use crate::congestion::ControllerMetrics;
use crate::congestion::bbr::bw_estimation::BandwidthEstimation;
Expand Down Expand Up @@ -56,7 +56,6 @@ pub struct Bbr {
bw_at_last_round: u64,
round_wo_bw_gain: u64,
ack_aggregation: AckAggregationState,
random_number_generator: rand::rngs::StdRng,
}

impl Bbr {
Expand Down Expand Up @@ -97,7 +96,6 @@ impl Bbr {
bw_at_last_round: 0,
round_wo_bw_gain: 0,
ack_aggregation: AckAggregationState::default(),
random_number_generator: rand::rngs::StdRng::from_os_rng(),
}
}

Expand All @@ -114,9 +112,7 @@ impl Bbr {
// Pick a random offset for the gain cycle out of {0, 2..7} range. 1 is
// excluded because in that case increased gain and decreased gain would not
// follow each other.
let mut rand_index = self
.random_number_generator
.random_range(0..K_PACING_GAIN.len() as u8 - 1);
let mut rand_index = rand::rng().random_range(0..K_PACING_GAIN.len() as u8 - 1);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is very unfortunate, I know we are planning on removing it, but we would need this back later, for proper seeded testing :/

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know - but the previous version of this code didn't allow passing in the randomness or a seed either, so this is no regression in that sense.

I'm aware of this issue in general, but fixing this would be annoying and blow up the diff in this PR.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you not losing the option to be deterministic by doing this?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You never had the option to configure this congestion controller to be deterministic in the first place, so no.

if rand_index >= 1 {
rand_index += 1;
}
Expand Down
3 changes: 2 additions & 1 deletion noq-proto/src/connection/assembler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -676,6 +676,7 @@ mod test {
#[cfg(all(test, not(target_family = "wasm")))]
mod proptests {
use proptest::prelude::*;
use rand::RngExt;
use test_strategy::{Arbitrary, proptest};

use super::*;
Expand Down Expand Up @@ -743,7 +744,7 @@ mod proptests {
}

fn make_data() -> Vec<u8> {
use rand::{Rng, SeedableRng};
use rand::SeedableRng;
let mut rng = rand::rngs::StdRng::seed_from_u64(0xDEADBEEF);
let mut data = vec![0u8; MAX_OFFSET as usize];
rng.fill(data.as_mut_slice());
Expand Down
2 changes: 1 addition & 1 deletion noq-proto/src/connection/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use std::{
use bytes::{Bytes, BytesMut};
use frame::StreamMetaVec;

use rand::{Rng, SeedableRng, rngs::StdRng};
use rand::{RngExt, SeedableRng, rngs::StdRng};
use rustc_hash::{FxHashMap, FxHashSet};
use thiserror::Error;
use tracing::{debug, error, trace, trace_span, warn};
Expand Down
2 changes: 1 addition & 1 deletion noq-proto/src/connection/packet_builder.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use bytes::{BufMut, Bytes};
use rand::Rng;
use rand::RngExt;
use tracing::{debug, trace, trace_span};

use super::{Connection, PathId, SentFrames, TransmitBuf, spaces::SentPacket};
Expand Down
2 changes: 1 addition & 1 deletion noq-proto/src/connection/packet_crypto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::connection::assembler::Assembler;
use crate::crypto::{self, HeaderKey, KeyPair, Keys, PacketKey};
use crate::packet::{Packet, PartialDecode};
use crate::token::ResetToken;
use rand::{CryptoRng, Rng};
use rand::{CryptoRng, RngExt};

use crate::{ConnectionId, Instant, Side};
use crate::{RESET_TOKEN_SIZE, TransportError};
Expand Down
4 changes: 2 additions & 2 deletions noq-proto/src/connection/spaces.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::{
ops::{Bound, Index, IndexMut},
};

use rand::{CryptoRng, Rng};
use rand::{CryptoRng, RngExt};
use rustc_hash::{FxHashMap, FxHashSet};
use sorted_index_buffer::SortedIndexBuffer;
use tracing::trace;
Expand Down Expand Up @@ -1279,7 +1279,7 @@ const MAX_ACK_BLOCKS: usize = 64;

#[cfg(test)]
mod test {
use rand::RngCore;
use rand::Rng;
use rand::seq::SliceRandom;

use crate::token::ResetToken;
Expand Down
4 changes: 2 additions & 2 deletions noq-proto/src/endpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use std::{
};

use bytes::{Buf, BufMut, Bytes, BytesMut};
use rand::{Rng, RngCore, SeedableRng, rngs::StdRng};
use rand::{Rng, RngExt, SeedableRng, rngs::StdRng};
use rustc_hash::FxHashMap;
use slab::Slab;
use thiserror::Error;
Expand Down Expand Up @@ -69,7 +69,7 @@ impl Endpoint {
Self {
rng: config
.rng_seed
.map_or_else(StdRng::from_os_rng, StdRng::from_seed),
.map_or_else(|| StdRng::from_rng(&mut rand::rng()), StdRng::from_seed),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the benefit of StdRng over rand::rng?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StdRng is generally meant to be a fast in-memory rng. At the moment internally it's implemented using chacha. IIUC, rand::rng() pulls in randomness from the OS, if there wasn't any randomness on this thread yet.

index: ConnectionIndex::default(),
connections: Slab::new(),
local_cid_generator: (config.connection_id_generator_factory.as_ref())(),
Expand Down
2 changes: 1 addition & 1 deletion noq-proto/src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use assert_matches::assert_matches;
use aws_lc_rs::hmac;
use bytes::{Bytes, BytesMut};
use hex_literal::hex;
use rand::RngCore;
use rand::Rng;
#[cfg(feature = "ring")]
use ring::hmac;
use rustls::{
Expand Down
5 changes: 3 additions & 2 deletions noq-proto/src/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::{
};

use bytes::{Buf, BufMut, Bytes};
use rand::{CryptoRng, Rng};
use rand::{CryptoRng, RngExt};

use crate::{
Duration, RESET_TOKEN_SIZE, ServerConfig, SystemTime, UNIX_EPOCH,
Expand Down Expand Up @@ -403,10 +403,11 @@ impl fmt::Display for ResetToken {

#[cfg(all(test, any(feature = "aws-lc-rs", feature = "ring")))]
mod test {
use rand::Rng;

use crate::crypto::ring_like::RetryTokenKey;

use super::*;
use rand::prelude::*;

fn token_round_trip(payload: TokenPayload) -> TokenPayload {
let rng = &mut rand::rng();
Expand Down
23 changes: 13 additions & 10 deletions noq-proto/src/transport_parameters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use std::{
};

use bytes::{Buf, BufMut};
use rand::{Rng as _, RngCore, seq::SliceRandom as _};
use rand::{Rng, RngExt, seq::SliceRandom as _};
use thiserror::Error;

use crate::{
Expand Down Expand Up @@ -163,7 +163,7 @@ impl TransportParameters {
cid_gen: &dyn ConnectionIdGenerator,
initial_src_cid: ConnectionId,
server_config: Option<&ServerConfig>,
rng: &mut impl RngCore,
rng: &mut impl Rng,
) -> Self {
Self {
initial_src_cid: Some(initial_src_cid),
Expand Down Expand Up @@ -639,7 +639,7 @@ impl ReservedTransportParameter {
/// The implementation is inspired by quic-go and quiche:
/// 1. <https://github.com/quic-go/quic-go/blob/3e0a67b2476e1819752f04d75968de042b197b56/internal/wire/transport_parameters.go#L338-L344>
/// 2. <https://github.com/google/quiche/blob/cb1090b20c40e2f0815107857324e99acf6ec567/quiche/quic/core/crypto/transport_parameters.cc#L843-L860>
fn random(rng: &mut impl RngCore) -> Self {
fn random(rng: &mut impl Rng) -> Self {
let id = Self::generate_reserved_id(rng);

let payload_len = rng.random_range(0..Self::MAX_PAYLOAD_LEN);
Expand Down Expand Up @@ -667,7 +667,7 @@ impl ReservedTransportParameter {
/// Reserved transport parameter identifiers are used to test compliance with the requirement
/// that unknown transport parameters must be ignored by peers.
/// See: <https://www.rfc-editor.org/rfc/rfc9000.html#section-18.1> and <https://www.rfc-editor.org/rfc/rfc9000.html#section-22.3>
fn generate_reserved_id(rng: &mut impl RngCore) -> VarInt {
fn generate_reserved_id(rng: &mut impl Rng) -> VarInt {
let id = {
let rand = rng.random_range(0u64..(1 << 62) - 27);
let n = rand / 31;
Expand Down Expand Up @@ -886,21 +886,23 @@ mod test {

struct StepRng(u64);

impl RngCore for StepRng {
impl rand::TryRng for StepRng {
type Error = std::convert::Infallible;

#[inline]
fn next_u32(&mut self) -> u32 {
self.next_u64() as u32
fn try_next_u32(&mut self) -> Result<u32, Self::Error> {
Ok(self.next_u64() as u32)
}

#[inline]
fn next_u64(&mut self) -> u64 {
fn try_next_u64(&mut self) -> Result<u64, Self::Error> {
let res = self.0;
self.0 = self.0.wrapping_add(1);
res
Ok(res)
}

#[inline]
fn fill_bytes(&mut self, dst: &mut [u8]) {
fn try_fill_bytes(&mut self, dst: &mut [u8]) -> Result<(), Self::Error> {
let mut left = dst;
while left.len() >= 8 {
let (l, r) = left.split_at_mut(8);
Expand All @@ -911,6 +913,7 @@ mod test {
if n > 0 {
left.copy_from_slice(&self.next_u32().to_le_bytes()[..n]);
}
Ok(())
}
}

Expand Down
2 changes: 1 addition & 1 deletion noq/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::runtime::TokioRuntime;
use crate::{Duration, Instant};
use bytes::Bytes;
use proto::{ConnectionError, RandomConnectionIdGenerator, crypto::rustls::QuicClientConfig};
use rand::{RngCore, SeedableRng, rngs::StdRng};
use rand::{Rng, SeedableRng, rngs::StdRng};
use rustls::{
RootCertStore,
pki_types::{CertificateDer, PrivateKeyDer, PrivatePkcs8KeyDer},
Expand Down
2 changes: 1 addition & 1 deletion noq/tests/many_connections.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use std::{

use crc::Crc;
use noq::{self, ConnectionError, ReadError, StoppedError, TransportConfig, WriteError};
use rand::{self, RngCore};
use rand::{self, Rng};
use rustls::pki_types::{CertificateDer, PrivatePkcs8KeyDer};
use tokio::runtime::Builder;

Expand Down
Loading