Skip to content

Typed netdir params

Dennis requested to merge djackson/arti:typed-netdir-params into main

As I mentioned at the Hackathon last week, I set out to see if I could refactor the tor-netdir/params module to make more use of Rust's type system. This was partly inspired by this blog post on the newtype pattern in Rust.

As a disclaimer, I mostly wanted to learn more about rust and explore the different possibilities. I'm not sure whether the approach I've taken is the right one for arti and offers the right trade-offs between complexity, reliability and ergonomics.

Goal

Currently, the configuration parameters are stored as i32s in a map keyed by an enum and callers must use one of get, get_u16 and get_bool depending on the parameter. If the incorrect method is used, a runtime panic ensues. We'd like to replace this with distinct types that ensure that each configuration parameter is correct by construction, there is only one get method per parameter and hence we catch as many errors as possible at compile time.

Overview / Design

I made a new crate called tor-primitive-types which implements a number of macros for generating bounded integer types. It also defines a number of useful types to represent values in Milliseconds, Percentages, Bandwidth Weights. These types wrap a primitive type such as a u32 or f32 and automatically implement the standard arithmetic operations. E.g:

        let x = Percentage(50);
        let y = Percentaget(27);
        let z = x + y;
        assert!(z == Percentage(77));
        let Percentage(z_raw) = z; 
        assert!(z_raw == 77); 

I then modified tor-netdir/params, introducing types with appropriate bounds for each of the parameters. Netdir now stores each of its configuration values in (strongly-typed) public structs which share a uniform interface and are generated by macros in tor-primitive-types. The interface for these macros is fairly straightforward:

bounded_type! {MinCircuitPathThreshold(Percentage,Percentage(25),Percentage(95),test_mcpt_bounds)}
make_default_type! {MinCircuitPathThreshold(Percentage(60),test_mcpt_default)}
make_saturating_type! {MinCircuitPathThreshold(Percentage)}

This macro sequence generates a new bounded type called MinCircuitPathThreshold, which wraps the Percentage type. It is required to be between 25% and 95%, with a default value of 60%. Finally, it uses a 'saturating' constructor which clamps extreme values to the upper or lower bounds without returning an error. (A 'checked' variant is also implemented).

Finally, I fixed up all the places that fetch these parameters to use the new type system and wrote some crappy tests and documentation.

Questions / Requests for comment:

  • Does this look useful?
  • If yes, how do should we push the type system? Should all variables representing milliseconds use the millisecond type? What about bandwidth weights? etc.
  • Does the underlying approach (macro-based) look reasonable and maintainable? I tried alternative approaches using generics and traits but neither of them worked out. If I understand correctly, when 'const generics' lands in stable then it will be exactly the right feature for bounded types without macros.

TODOs

  • Better docs and tests
  • Sort out the error handling code... "You are in a twisty maze of box <dyn Error> return values, all alike."
  • Have a think about the right types to wrap. E.g. Should percentages be a float? a u32 counting in 100ths of a percentage? An integer in 1-100? Something else?

P.S. I'm pretty new to rust (and writing non-research code) so especially happy for any feedback/suggestions.

Edited by Dennis

Merge request reports