ralexstokes / ssz-rs

Implementation of ethereum's `ssz`
Apache License 2.0
102 stars 40 forks source link

add API for initializing a new `List` #20

Closed dtbuchholz closed 1 year ago

dtbuchholz commented 2 years ago

Issue raised in comment from ethereum-consensus

Idea is to avoid something like the below and allow for a new List to be initialized without allocating a Vec and then converting into a List.

let rotate_participation = vec![ParticipationFlags::default(); state.validators.len()];
    state.current_epoch_participation = rotate_participation
        .try_into()
        .expect("should convert from Vec to List");

For example, add impl of new for something like List<T>::new(capacity: usize) (or, in the example above, just allow for state.current_epoch_participation = List::<u8>::new(state.validators.len()).

Based on some initial research, I'm not quite sure how to do this since List obviously takes List<T, N> where N is a const usize...so List<T> would be invalid. And without knowing N at compile time, that introduces concepts like raw pointers, unsafe rust, Box, Arc, etc. that I couldn't figure out how to use or if they even solve this problem.

dtbuchholz commented 2 years ago

E.g., this obviously doesn't work but captures the idea:

impl<T, const N: usize> List<T, N> // shown to make it "work" but shouldn't require `N` (if that's even possible to do...)
where
    T: SimpleSerialize + Clone,
{
    // naive impl but maybe something similar to `try_from` & return `Self { ... }`
    fn new(capacity: usize) -> Self {
        let data = vec![T::default(); capacity];
        data.try_into().unwrap()
    }
}

#[test]
fn create_new_list() {
    const SIZE: usize = 32; // shown to make it "work" but shouldn't be required and use `capacity` to init
    let capacity = 32;
    let value = List::<u8, SIZE>::new(capacity); // aka remove `SIZE` here
    ...
}
ralexstokes commented 2 years ago

I think we can just use this?

https://github.com/ralexstokes/ssz-rs/blob/main/ssz-rs/src/list.rs#L110

let M;
let N;
assert!(M <= N);
let inner = vec![T; M];
let list = List::<T, N>::try_from(inner).unwrap();
ralexstokes commented 1 year ago

this is a bit stale but I believe the concern was around an additional allocation of the backing Vec but the current implementation of TryFrom<Vec<T>> for List just takes ownership of the data, so ill close this issue