ralexstokes / ethereum-consensus

Apache License 2.0
136 stars 51 forks source link

Revisit use of const generics #372

Open ralexstokes opened 5 months ago

ralexstokes commented 5 months ago

This repo uses const generics to implement "presets" from the consensus-specs.

However, there are many of them making the code hard to read, extend, and maintain.

Here's a sample refactoring we could consider instead:

// in ssz primitives
pub mod ssz_rs {
    pub trait Bounded {
        const VALUE: usize;
    }

    #[derive(Debug)]
    pub struct Bound<const VALUE: usize>;

    impl<const BOUND: usize> Bounded for Bound<BOUND> {
        const VALUE: usize = BOUND;
    }

    #[derive(Debug)]
    pub struct Vector<T, B: Bounded> {
        pub data: Vec<T>,
        _p: std::marker::PhantomData<B>,
    }

    impl<T, B: Bounded> Vector<T, B> {
        pub fn new(data: Vec<T>) -> Self {
            if B::VALUE > data.len() {
                panic!("not in bounds")
            } else {
                Self {
                    data,
                    _p: std::marker::PhantomData,
                }
            }
        }
    }
}

// in ethereum-consensus
pub mod consensus {
    use super::ssz_rs::*;

    pub trait Preset {
        type ValidatorsBound: Bounded;
    }

    pub mod presets {
        use super::*;

        const MAINNET_VALIDATORS_BOUND: usize = 3;

        #[derive(Debug)]
        pub struct SomePreset<const BAR: usize>;

        impl<const BAR: usize> Preset for SomePreset<BAR> {
            type ValidatorsBound = Bound<BAR>;
        }

        pub type Mainnet = SomePreset<MAINNET_VALIDATORS_BOUND>;
    }

    #[derive(Debug)]
    pub struct State<P: Preset> {
        pub validators: Vector<u8, P::ValidatorsBound>,
        pub b: usize,
    }

    pub fn do_state<P: Preset>(state: &mut State<P>) {
        dbg!(state.validators.data.len());
        state.b += 1;
    }
}

fn main() {
    use consensus::{do_state, presets::Mainnet, State};
    use ssz_rs::Vector;

    let mut state = State::<Mainnet> {
        validators: Vector::new([23u8, 14u8, 32u8].to_vec()),
        b: 234,
    };
    dbg!(&state);
    do_state(&mut state);
    dbg!(state);
}

This approach uses another step of "type indirection" to pass the const bound of the SSZ type from the consumer's definition to the underlying target (bounded) type.

This would require some changes to ssz-rs, and make that code a bit more complex with the new Bounded abstraction; however, the consumer code replaces a long list of const generics with a single generic with bound Preset to group them all behind a single trait. As far as I can tell, this "indirection" through Bounded is required to avoid using the const value until the very end (use earlier is currently not supported by the Rust compiler...).

The upside is all of the types in this repo, and the functions that operate on them, become much simpler.