Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revisit use of const generics #372

Open
ralexstokes opened this issue Apr 4, 2024 · 0 comments
Open

Revisit use of const generics #372

ralexstokes opened this issue Apr 4, 2024 · 0 comments

Comments

@ralexstokes
Copy link
Owner

ralexstokes commented Apr 4, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant