diff --git a/src/error.rs b/src/error.rs index 499712a..40c2c71 100644 --- a/src/error.rs +++ b/src/error.rs @@ -29,6 +29,7 @@ pub enum Error { BuilderStackLeftover, BuilderFull, BulkUpdateUnclean, + UpdateMapMissingMaxIndex, CowMissingEntry, LevelIterPendingUpdates, IntraRebaseZeroHash, diff --git a/src/interface.rs b/src/interface.rs index 6f27952..600fcae 100644 --- a/src/interface.rs +++ b/src/interface.rs @@ -26,6 +26,7 @@ pub trait ImmList { pub trait MutList: ImmList { fn validate_push(current_len: usize) -> Result<(), Error>; + fn validate_bulk_update>(&self, updates: &U) -> Result<(), Error>; fn replace(&mut self, index: usize, value: T) -> Result<(), Error>; fn update>( &mut self, @@ -137,10 +138,21 @@ where self.len() == 0 } + /// Stage a new set of `updates`, while flushing existing ones. pub fn bulk_update(&mut self, updates: U) -> Result<(), Error> { - if !self.updates.is_empty() { - return Err(Error::BulkUpdateUnclean); + // Flush existing updates. + // + // This is required prior to `validate_bulk_update` to update the backing's internal + // length. It does mean that an invalid bulk update will mutate `self` by flushing the + // existing updates. + self.apply_updates()?; + + // Validate new updates (check against List/Vector capacity and ensure contiguous range). + if !updates.is_empty() { + self.backing.validate_bulk_update(&updates)?; } + + // Stage new updates. self.updates = updates; Ok(()) } diff --git a/src/list.rs b/src/list.rs index 069dcfb..44198a7 100644 --- a/src/list.rs +++ b/src/list.rs @@ -16,9 +16,11 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer, ser::SerializeSeq} use ssz::{BYTES_PER_LENGTH_OFFSET, Decode, Encode, SszEncoder, TryFromIter}; use std::collections::{BTreeMap, HashMap}; use std::marker::PhantomData; +use std::ops::ControlFlow; use tree_hash::{Hash256, PackedEncoding, TreeHash}; use typenum::Unsigned; use vec_map::VecMap; + #[derive(Debug, Clone, Educe)] #[educe(PartialEq(bound(T: Value, N: Unsigned, U: UpdateMap + PartialEq)))] #[cfg_attr( @@ -195,6 +197,14 @@ impl> List { self.interface.apply_updates() } + /// Replace the `update_map` for the list with the given `updates`. + /// + /// Any existing updates will be flushed to the underlying tree before the replacement occurs. + /// The new updates WILL NOT be flushed, unless `self.apply_updates` is called immediately + /// afterwards. + /// + /// Errors if the updates would create a gap (all updates at indices >= current length must be + /// contiguous without gaps). pub fn bulk_update(&mut self, updates: U) -> Result<(), Error> { self.interface.bulk_update(updates) } @@ -292,6 +302,38 @@ where } } + fn validate_bulk_update>(&self, updates: &U) -> Result<(), Error> { + // For a list, we need to check that all updates at indices >= the current length are + // contiguous. + let max_index = updates.max_index().ok_or(Error::UpdateMapMissingMaxIndex)?; + + if max_index >= N::to_usize() { + return Err(Error::OutOfBoundsUpdate { + index: max_index, + len: N::to_usize(), + }); + } + + let start = self.length.as_usize(); + let end = max_index.saturating_add(1); + + let mut previous_index = self.length.as_usize().checked_sub(1); + + updates.for_each_range(start, end, |index, _| { + // This neatly handles the length=0 case, by ensuring that we only accept `index == 0`, + // which yields `None` when `.checked_sub(1)` is applied. + if index.checked_sub(1) != previous_index { + ControlFlow::Continue(Err(Error::OutOfBoundsUpdate { + index, + len: self.length.as_usize(), + })) + } else { + previous_index = Some(index); + ControlFlow::Continue(Ok(())) + } + }) + } + fn replace(&mut self, index: usize, value: T) -> Result<(), Error> { if index > self.len().as_usize() { return Err(Error::OutOfBoundsUpdate { diff --git a/src/tests/bulk_update.rs b/src/tests/bulk_update.rs new file mode 100644 index 0000000..552c36a --- /dev/null +++ b/src/tests/bulk_update.rs @@ -0,0 +1,48 @@ +use crate::List; +use std::collections::BTreeMap; +use typenum::U8; + +#[test] +fn bulk_update_push_empty() { + let mut list: List> = List::empty(); + let updates = BTreeMap::from([(0, 0)]); + list.bulk_update(updates).unwrap(); + assert_eq!(list.len(), 1); + assert_eq!(*list.get(0).unwrap(), 0); +} + +#[test] +fn bulk_update_push_empty_fail() { + let mut list: List> = List::empty(); + let updates = BTreeMap::from([(1, 0)]); + list.bulk_update(updates).unwrap_err(); + assert_eq!(list.len(), 0); +} + +#[test] +fn bulk_update_push_one() { + let mut list: List> = List::new(vec![0, 1, 2, 3, 4]).unwrap(); + let updates = BTreeMap::from([(5, 5)]); + list.bulk_update(updates).unwrap(); + assert_eq!(list.len(), 6); + assert_eq!(*list.get(5).unwrap(), 5); +} + +#[test] +fn bulk_update_push_two_btreemap() { + let mut list: List> = List::new(vec![0, 1, 2, 3, 4]).unwrap(); + let updates = BTreeMap::from([(5, 5), (6, 6)]); + list.bulk_update(updates).unwrap(); + assert_eq!(list.len(), 7); + assert_eq!(*list.get(5).unwrap(), 5); + assert_eq!(*list.get(6).unwrap(), 6); +} + +#[test] +fn bulk_update_with_gap() { + let mut list: List> = List::new(vec![0, 1, 2, 3, 4]).unwrap(); + let updates = BTreeMap::from([(6, 6)]); + list.bulk_update(updates).unwrap_err(); + list.apply_updates().unwrap(); + assert_eq!(list.len(), 5); +} diff --git a/src/tests/mod.rs b/src/tests/mod.rs index b6010f3..1069a74 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -1,6 +1,7 @@ #![cfg(test)] mod builder; +mod bulk_update; mod iterator; mod mem; mod packed; diff --git a/src/tests/proptest/operations.rs b/src/tests/proptest/operations.rs index 699a2fa..01d4493 100644 --- a/src/tests/proptest/operations.rs +++ b/src/tests/proptest/operations.rs @@ -1,7 +1,8 @@ use super::{Large, arb_hash256, arb_index, arb_large, arb_list, arb_vect}; -use crate::{Error, List, Value, Vector}; +use crate::{Error, List, UpdateMap, Value, Vector, update_map::MaxMap}; use proptest::prelude::*; use ssz::{Decode, Encode}; +use std::cmp::Ordering; use std::fmt::Debug; use std::marker::PhantomData; use std::ops::Deref; @@ -90,6 +91,29 @@ impl Spec { Err(Error::PushNotSupported) } } + + pub fn bulk_update(&mut self, mut updates: Vec<(usize, T)>) -> Result<(), Error> { + let mut new_list = self.clone(); + updates.sort_by_key(|(index, _)| *index); + for (index, value) in updates { + match index.cmp(&new_list.values.len()) { + Ordering::Less => { + new_list.values[index] = value; + } + Ordering::Equal if self.allow_push => { + new_list.push(value)?; + } + _ => { + return Err(Error::OutOfBoundsUpdate { + index, + len: new_list.values.len(), + }); + } + } + } + *self = new_list; + Ok(()) + } } #[derive(Debug, Clone)] @@ -128,6 +152,8 @@ pub enum Op { FromIntoRoundtrip, /// Rebase the list on itself to exploit self-sharing. IntraRebase, + /// Apply a bulk update to the list/vector. + BulkUpdate(Vec<(usize, T)>), } fn arb_op<'a, T, S>(strategy: &'a S, n: usize) -> impl Strategy> + 'a @@ -158,10 +184,11 @@ where Just(Op::Debase), Just(Op::FromIntoRoundtrip), Just(Op::IntraRebase), + proptest::collection::vec((arb_index(n), strategy), 0..n).prop_map(Op::BulkUpdate), ]; prop_oneof![ 10 => a_block, - 7 => b_block + 8 => b_block ] } @@ -285,6 +312,21 @@ where assert_eq!(new_list, *list); *list = new_list; } + Op::BulkUpdate(updates) => { + let mut update_map = MaxMap::>::default(); + for &(index, ref value) in &updates { + update_map.insert(index, value.clone()); + } + + match spec.bulk_update(updates) { + Ok(()) => { + list.bulk_update(update_map).unwrap(); + } + Err(_) => { + list.bulk_update(update_map).unwrap_err(); + } + } + } } } } @@ -389,6 +431,21 @@ where assert_eq!(new_vect, *vect); *vect = new_vect; } + Op::BulkUpdate(updates) => { + let mut update_map = MaxMap::>::default(); + for &(index, ref value) in &updates { + update_map.insert(index, value.clone()); + } + + match spec.bulk_update(updates) { + Ok(()) => { + vect.bulk_update(update_map).unwrap(); + } + Err(_) => { + vect.bulk_update(update_map).unwrap_err(); + } + } + } } } } diff --git a/src/update_map.rs b/src/update_map.rs index a58056f..6e6a26d 100644 --- a/src/update_map.rs +++ b/src/update_map.rs @@ -20,6 +20,7 @@ pub trait UpdateMap: Default + Clone { fn insert(&mut self, k: usize, value: T) -> Option; + /// Iterate between `start` (inclusive) and `end` (exclusive). fn for_each_range(&self, start: usize, end: usize, f: F) -> Result<(), E> where F: FnMut(usize, &T) -> ControlFlow<(), Result<(), E>>; diff --git a/src/vector.rs b/src/vector.rs index 1e6cda7..801ec0a 100644 --- a/src/vector.rs +++ b/src/vector.rs @@ -130,6 +130,10 @@ impl> Vector { pub fn apply_updates(&mut self) -> Result<(), Error> { self.interface.apply_updates() } + + pub fn bulk_update(&mut self, updates: U) -> Result<(), Error> { + self.interface.bulk_update(updates) + } } impl> TryFrom> for Vector { @@ -249,6 +253,20 @@ where Err(Error::PushNotSupported) } + fn validate_bulk_update>(&self, updates: &U) -> Result<(), Error> { + // For a vector there is no possibility of pushing in a bulk update so it is sufficient to + // check the `max_index`. + let max_index = updates.max_index().ok_or(Error::UpdateMapMissingMaxIndex)?; + if max_index >= N::to_usize() { + Err(Error::OutOfBoundsUpdate { + index: max_index, + len: N::to_usize(), + }) + } else { + Ok(()) + } + } + fn replace(&mut self, index: usize, value: T) -> Result<(), Error> { if index >= self.len().as_usize() { return Err(Error::OutOfBoundsUpdate {