From 4dc6e6515ecf75cefa4de840edc7b57e76a8fc9e Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Fri, 30 Aug 2024 15:53:18 -0700 Subject: [PATCH] Add restrictions to RuntimeVariableList api --- consensus/types/src/blob_sidecar.rs | 1 - consensus/types/src/data_column_sidecar.rs | 5 +- consensus/types/src/runtime_var_list.rs | 73 +++++++++++++++++----- 3 files changed, 59 insertions(+), 20 deletions(-) diff --git a/consensus/types/src/blob_sidecar.rs b/consensus/types/src/blob_sidecar.rs index c2981e14cc..108c807b47 100644 --- a/consensus/types/src/blob_sidecar.rs +++ b/consensus/types/src/blob_sidecar.rs @@ -266,7 +266,6 @@ impl BlobSidecar { pub type BlobSidecarList = RuntimeVariableList>>; /// Alias for a non length-constrained list of `BlobSidecar`s. -pub type BlobSidecarVec = Vec>>; pub type FixedBlobSidecarList = RuntimeFixedList>>>; pub type BlobsList = VariableList, ::MaxBlobCommitmentsPerBlock>; diff --git a/consensus/types/src/data_column_sidecar.rs b/consensus/types/src/data_column_sidecar.rs index ea0fb2b14a..34bdc33886 100644 --- a/consensus/types/src/data_column_sidecar.rs +++ b/consensus/types/src/data_column_sidecar.rs @@ -1,7 +1,7 @@ use crate::beacon_block_body::{KzgCommitments, BLOB_KZG_COMMITMENTS_INDEX}; use crate::test_utils::TestRandom; -use crate::{BeaconBlockHeader, Epoch, EthSpec, Hash256, KzgProofs, SignedBeaconBlockHeader, Slot}; -use crate::{BeaconStateError, ChainSpec}; +use crate::BeaconStateError; +use crate::{BeaconBlockHeader, EthSpec, Hash256, KzgProofs, SignedBeaconBlockHeader, Slot}; use bls::Signature; use derivative::Derivative; use kzg::Error as KzgError; @@ -11,7 +11,6 @@ use safe_arith::ArithError; use serde::{Deserialize, Serialize}; use ssz::Encode; use ssz_derive::{Decode, Encode}; -use ssz_types::typenum::Unsigned; use ssz_types::Error as SszError; use ssz_types::{FixedVector, VariableList}; use std::hash::Hash; diff --git a/consensus/types/src/runtime_var_list.rs b/consensus/types/src/runtime_var_list.rs index 4c6d6e2d92..aa43e3fe75 100644 --- a/consensus/types/src/runtime_var_list.rs +++ b/consensus/types/src/runtime_var_list.rs @@ -2,7 +2,7 @@ use derivative::Derivative; use serde::{Deserialize, Serialize}; use ssz::Decode; use ssz_types::Error; -use std::ops::{Deref, DerefMut, Index, IndexMut}; +use std::ops::{Deref, Index, IndexMut}; use std::slice::SliceIndex; /// Emulates a SSZ `List`. @@ -41,8 +41,10 @@ use std::slice::SliceIndex; #[serde(transparent)] pub struct RuntimeVariableList { vec: Vec, + /// A `None` here indicates an uninitialized `Self`. + /// No mutating operation will be allowed until `max_len` is Some #[serde(skip)] - max_len: usize, + max_len: Option, } impl RuntimeVariableList { @@ -50,7 +52,10 @@ impl RuntimeVariableList { /// `Err(OutOfBounds { .. })`. pub fn new(vec: Vec, max_len: usize) -> Result { if vec.len() <= max_len { - Ok(Self { vec, max_len }) + Ok(Self { + vec, + max_len: Some(max_len), + }) } else { Err(Error::OutOfBounds { i: vec.len(), @@ -62,14 +67,17 @@ impl RuntimeVariableList { pub fn from_vec(mut vec: Vec, max_len: usize) -> Self { vec.truncate(max_len); - Self { vec, max_len } + Self { + vec, + max_len: Some(max_len), + } } /// Create an empty list. pub fn empty(max_len: usize) -> Self { Self { vec: vec![], - max_len, + max_len: Some(max_len), } } @@ -77,6 +85,28 @@ impl RuntimeVariableList { self.vec.as_slice() } + pub fn as_mut_slice(&mut self) -> Option<&mut [T]> { + if self.max_len.is_none() { + return None; + }; + Some(self.vec.as_mut_slice()) + } + + /// Returns an instance of `Self` with max_len = None. + /// + /// No mutating operation can be performed on an uninitialized instance + /// without first setting max_len. + pub fn empty_uninitialized() -> Self { + Self { + vec: vec![], + max_len: None, + } + } + + pub fn set_max_len(&mut self, max_len: usize) { + self.max_len = Some(max_len); + } + /// Returns the number of values presently in `self`. pub fn len(&self) -> usize { self.vec.len() @@ -88,7 +118,9 @@ impl RuntimeVariableList { } /// Returns the type-level maximum length. - pub fn max_len(&self) -> usize { + /// + /// Returns `None` if self is uninitialized with a max_len. + pub fn max_len(&self) -> Option { self.max_len } @@ -96,13 +128,17 @@ impl RuntimeVariableList { /// /// Returns `Err(())` when appending `value` would exceed the maximum length. pub fn push(&mut self, value: T) -> Result<(), Error> { - if self.vec.len() < self.max_len { + let Some(max_len) = self.max_len else { + // TODO(pawan): set a better error + return Err(Error::MissingLengthInformation); + }; + if self.vec.len() < max_len { self.vec.push(value); Ok(()) } else { Err(Error::OutOfBounds { i: self.vec.len().saturating_add(1), - len: self.max_len, + len: max_len, }) } } @@ -135,7 +171,10 @@ impl RuntimeVariableList { } else { ssz::decode_list_of_variable_length_items(bytes, Some(max_len))? }; - Ok(Self { vec, max_len }) + Ok(Self { + vec, + max_len: Some(max_len), + }) } } @@ -169,12 +208,6 @@ impl Deref for RuntimeVariableList { } } -impl DerefMut for RuntimeVariableList { - fn deref_mut(&mut self) -> &mut [T] { - &mut self.vec[..] - } -} - impl<'a, T> IntoIterator for &'a RuntimeVariableList { type Item = &'a T; type IntoIter = std::slice::Iter<'a, T>; @@ -221,7 +254,6 @@ pub struct RuntimeFixedList { } impl RuntimeFixedList { - // TODO(pawan): no need to take len pub fn new(vec: Vec) -> Self { let len = vec.len(); Self { vec, len } @@ -280,6 +312,7 @@ mod test { use super::*; use ssz::*; use std::fmt::Debug; + use tree_hash::TreeHash; #[test] fn new() { @@ -358,4 +391,12 @@ mod test { round_trip::(RuntimeVariableList::from_vec(vec![42; 8], 8)); round_trip::(RuntimeVariableList::from_vec(vec![0; 8], 8)); } + + #[test] + fn test_empty_list_encoding() { + use ssz_types::{typenum::U16, VariableList}; + + let a: RuntimeVariableList = RuntimeVariableList::from_vec(vec![], 16); + dbg!(a.tree_hash_root()); + } }