Rust implementation of protover code deviates from C in some cases

Teor found some cases where the Rust protover code deviates from the C protover code. We should fix all of these in 0.3.3.

changed milestone to %Tor: 0.3.3.x-final

added 033-must component::core tor/tor milestone::Tor: 0.3.3.x-final owner::nickm priority::high protover resolution::fixed rust severity::normal status::closed type::defect labels

Update: most of the issues here are solvable with a little tweaking. I've made changes in torspec in a8455f4d1a6ef7 to define the C behavior as "right" a little more.

The one issue that will be trickiest to handle is that the Rust code stores versions in a hashset of u32, whereas the C code stores an array of (lo,hi) pairs. As such, the Rust code needs to reject 1-4000000000 as a range, or else it will fill up RAM. C accepts such a range everywhere except during voting.

I propose that we handle that last issue separately from the others.

The easier issues are solved in my branch protover_rust_compat_v3, along with a fix for #25250 (moved).

Trac:
Status: assigned to needs_review
Priority: Medium to High

This looks great!

Before we run this code on the public authorities, we should fuzz it to make sure it matches (see #24265 (moved)).

Trac:
Status: needs_review to merge_ready

merging!

Trac:
Status: merge_ready to closed
Resolution: N/A to fixed

Replying to nickm:

Update: most of the issues here are solvable with a little tweaking. I've made changes in torspec in a8455f4d1a6ef7 to define the C behavior as "right" a little more.

The one issue that will be trickiest to handle is that the Rust code stores versions in a hashset of u32, whereas the C code stores an array of (lo,hi) pairs. As such, the Rust code needs to reject 1-4000000000 as a range, or else it will fill up RAM. C accepts such a range everywhere except during voting.

I propose that we handle that last issue separately from the others.

...

That last issue already has a ticket: #24031 (moved).

closed

moved to tpo/core/tor#25252 (closed)