quinn-proto: Non-determinism in endpoint/connection/congestion

[michael-yxchen]

While debugging an issue I have, I found that quinn-proto protocol isn't fully deterministic. The source of randomness seems to be the call to StdRng::from_entropy in three places:

https://github.com/quinn-rs/quinn/blob/e1e1e6e392a382fbded42ca010505fecb8fe3655/quinn-proto/src/endpoint.rs#L62

https://github.com/quinn-rs/quinn/blob/e1e1e6e392a382fbded42ca010505fecb8fe3655/quinn-proto/src/connection/mod.rs#L270

https://github.com/quinn-rs/quinn/blob/e1e1e6e392a382fbded42ca010505fecb8fe3655/quinn-proto/src/congestion/bbr/mod.rs#L99

Are these intended?

Thank you for the help!

[djc]

It's more interesting to look at how those random number generators are used. Did we promise somewhere that it would be fully deterministic?

[michael-yxchen]

Sure, I'll circle back on how those RNGs are are used.

I got the idea that quinn-proto would be fully deterministic from here

https://github.com/quinn-rs/quinn/blob/e1e1e6e392a382fbded42ca010505fecb8fe3655/quinn-proto/src/lib.rs#L3-L5

I was thinking that it'd imply that there'd be no source of randomness from within, though it wasn't explicitly mentioned.

[djc]

Yeah, I suppose the "fully deterministic" is a bit of a lie!

[Ralith]

Yep, that comment is out of date. The randomized stuff is usually of little interest, though -- tokens and a few skipped packet numbers, and whatever BBR does if you opt into that.

We could allow a RNG seed to be passed in to recover perfect determinism if someone actually wants that.

[michael-yxchen]

For context, one of our nightly tests depends on quinn-proto. It failed one time but we were never able to reproduce the error. It would be really helpful if the protocol can accept some RNG seeds so the test is perfectly deterministic.

I can write up a patch if people don't have the capacity to do it.

[Ralith]

I'd be happy to review such a patch. I think adding an argument to quinn_proto::{Endpoint, Connection}::new should do the trick.