From dad616b883c76a9ede8ea0275fdae8cfdbe1089f Mon Sep 17 00:00:00 2001 From: brentstone Date: Wed, 14 Sep 2022 20:07:58 +0200 Subject: [PATCH] WIP: StateMachine tests for lazy_vec validation update Cargo.lock --- .../storage_api/collections/lazy_vec.rs | 322 +-------- shared/src/types/storage.rs | 11 +- .../storage_api/collections/lazy_vec.txt | 7 + tests/src/lib.rs | 2 + tests/src/storage_api/collections/lazy_vec.rs | 631 ++++++++++++++++++ tests/src/storage_api/collections/mod.rs | 1 + tests/src/storage_api/mod.rs | 1 + tests/src/vm_host_env/tx.rs | 25 + 8 files changed, 684 insertions(+), 316 deletions(-) create mode 100644 tests/proptest-regressions/storage_api/collections/lazy_vec.txt create mode 100644 tests/src/storage_api/collections/lazy_vec.rs create mode 100644 tests/src/storage_api/collections/mod.rs create mode 100644 tests/src/storage_api/mod.rs diff --git a/shared/src/ledger/storage_api/collections/lazy_vec.rs b/shared/src/ledger/storage_api/collections/lazy_vec.rs index 79967cb9c3..d86f33ec08 100644 --- a/shared/src/ledger/storage_api/collections/lazy_vec.rs +++ b/shared/src/ledger/storage_api/collections/lazy_vec.rs @@ -55,7 +55,7 @@ pub enum SubKeyWithData { /// Possible actions that can modify a [`LazyVec`]. This roughly corresponds to /// the methods that have `StorageWrite` access. -#[derive(Debug)] +#[derive(Clone, Debug)] pub enum Action { /// Push a value `T` into a [`LazyVec`] Push(T), @@ -86,10 +86,10 @@ pub enum ValidationError { #[error("Pop at a wrong index. Got {got}, expected {expected}.")] UnexpectedPopIndex { got: Index, expected: Index }, #[error( - "Update (combination of pop and push) at a wrong index. Got {got}, \ - expected {expected}." + "Update (or a combination of pop and push) at a wrong index. Got \ + {got}, expected maximum {max}." )] - UnexpectedUpdateIndex { got: Index, expected: Index }, + UnexpectedUpdateIndex { got: Index, max: Index }, #[error("An index has overflown its representation: {0}")] IndexOverflow(>::Error), #[error("Unexpected underflow in `{0} - {0}`")] @@ -323,8 +323,8 @@ where let builder = builder.get_or_insert(ValidationBuilder::default()); builder.changes.push(change); - return Ok(true); } + return Ok(true); } Ok(false) } @@ -474,34 +474,14 @@ impl ValidationBuilder { } } - // And finally iterate updates in increasing order of indices - let mut last_updated = Option::None; + // And finally iterate updates for index in updated { - if let Some(last_updated) = last_updated { - // Following additions should be at monotonically increasing - // indices - let expected = last_updated + 1; - if expected != index { - return Err(ValidationError::UnexpectedUpdateIndex { - got: index, - expected, - }); - } - } - last_updated = Some(index); - } - if let Some(index) = last_updated { - let expected = len_pre.checked_sub(deleted_len).ok_or( - ValidationError::UnexpectedUnderflow(len_pre, deleted_len), - )?; - if index != expected { - // The last update must be at the pre length value minus - // deleted_len. - // If something is added and then deleted in a - // single tx, it will never be visible here. + // Update index has to be within the length bounds + let max = len_pre + len_diff; + if index >= max { return Err(ValidationError::UnexpectedUpdateIndex { got: index, - expected: len_pre, + max, }); } } @@ -512,12 +492,6 @@ impl ValidationBuilder { #[cfg(test)] mod test { - use proptest::prelude::*; - use proptest::prop_state_machine; - use proptest::state_machine::{AbstractStateMachine, StateMachineTest}; - use proptest::test_runner::Config; - use test_log::test; - use super::*; use crate::ledger::storage::testing::TestStorage; @@ -556,280 +530,4 @@ mod test { Ok(()) } - - prop_state_machine! { - #![proptest_config(Config { - // Instead of the default 256, we only run 5 because otherwise it - // takes too long and it's preferable to crank up the number of - // transitions instead, to allow each case to run for more epochs as - // some issues only manifest once the model progresses further. - // Additionally, more cases will be explored every time this test is - // executed in the CI. - cases: 5, - .. Config::default() - })] - #[test] - /// A `StateMachineTest` implemented on `LazyVec` that manipulates - /// it with `Transition`s and checks its state against an in-memory - /// `std::collections::Vec`. - fn lazy_vec_api_state_machine_test(sequential 1..100 => ConcreteLazyVecState); - - } - - /// Some borsh-serializable type with arbitrary fields to be used inside - /// LazyVec state machine test - #[derive( - Clone, - Debug, - BorshSerialize, - BorshDeserialize, - PartialEq, - Eq, - PartialOrd, - Ord, - )] - struct TestVecItem { - x: u64, - y: bool, - } - - #[derive(Debug)] - struct ConcreteLazyVecState { - // The eager vec in `AbstractLazyVecState` is not visible in `impl - // StateMachineTest for ConcreteLazyVecState`, it's only used to drive - // transition generation, so we duplicate it here and apply the - // transitions on it the same way (with - // `fn apply_transition_on_eager_vec`) - eager_vec: Vec, - lazy_vec: LazyVec, - storage: TestStorage, - } - - #[derive(Clone, Debug)] - struct AbstractLazyVecState(Vec); - - /// Possible transitions that can modify a [`LazyVec`]. This roughly - /// corresponds to the methods that have `StorageWrite` access and is very - /// similar to [`Action`] - #[derive(Clone, Debug)] - pub enum Transition { - /// Push a value `T` into a [`LazyVec`] - Push(T), - /// Pop a value from a [`LazyVec`] - Pop, - /// Update a value `T` at index from pre to post state in a - /// [`LazyVec`] - Update { - /// index at which the value is updated - index: Index, - /// value to update the element to - value: T, - }, - } - - impl AbstractStateMachine for AbstractLazyVecState { - type State = Self; - type Transition = Transition; - - fn init_state() -> BoxedStrategy { - Just(Self(vec![])).boxed() - } - - // Apply a random transition to the state - fn transitions(state: &Self::State) -> BoxedStrategy { - if state.0.is_empty() { - prop_oneof![arb_test_vec_item().prop_map(Transition::Push)] - .boxed() - } else { - let indices: Vec = - (0_usize..state.0.len()).map(|ix| ix as Index).collect(); - let arb_index = proptest::sample::select(indices); - prop_oneof![ - Just(Transition::Pop), - arb_test_vec_item().prop_map(Transition::Push), - (arb_index, arb_test_vec_item()).prop_map( - |(index, value)| Transition::Update { index, value } - ) - ] - .boxed() - } - } - - fn apply_abstract( - mut state: Self::State, - transition: &Self::Transition, - ) -> Self::State { - apply_transition_on_eager_vec(&mut state.0, transition); - state - } - - fn preconditions( - state: &Self::State, - transition: &Self::Transition, - ) -> bool { - if state.0.is_empty() { - // Ensure that the pop or update transitions are not applied to - // an empty state - !matches!( - transition, - Transition::Pop | Transition::Update { .. } - ) - } else if let Transition::Update { index, .. } = transition { - // Ensure that the update index is a valid one - *index < (state.0.len() - 1) as Index - } else { - true - } - } - } - - impl StateMachineTest for ConcreteLazyVecState { - type Abstract = AbstractLazyVecState; - type ConcreteState = Self; - - fn init_test( - _initial_state: ::State, - ) -> Self::ConcreteState { - Self { - eager_vec: vec![], - lazy_vec: LazyVec::open( - storage::Key::parse("key_path/arbitrary").unwrap(), - ), - storage: TestStorage::default(), - } - } - - fn apply_concrete( - mut state: Self::ConcreteState, - transition: ::Transition, - ) -> Self::ConcreteState { - // Transition application on lazy vec and post-conditions: - match dbg!(&transition) { - Transition::Push(value) => { - let old_len = state.lazy_vec.len(&state.storage).unwrap(); - - state - .lazy_vec - .push(&mut state.storage, value.clone()) - .unwrap(); - - // Post-conditions: - let new_len = state.lazy_vec.len(&state.storage).unwrap(); - let stored_value = state - .lazy_vec - .get(&state.storage, new_len - 1) - .unwrap() - .unwrap(); - assert_eq!( - &stored_value, value, - "the new item must be added to the back" - ); - assert_eq!(old_len + 1, new_len, "length must increment"); - } - Transition::Pop => { - let old_len = state.lazy_vec.len(&state.storage).unwrap(); - - let popped = state - .lazy_vec - .pop(&mut state.storage) - .unwrap() - .unwrap(); - - // Post-conditions: - let new_len = state.lazy_vec.len(&state.storage).unwrap(); - assert_eq!(old_len, new_len + 1, "length must decrement"); - assert_eq!( - &popped, - state.eager_vec.last().unwrap(), - "popped element matches the last element in eager vec \ - before it's updated" - ); - } - Transition::Update { index, value } => { - let old_len = state.lazy_vec.len(&state.storage).unwrap(); - let old_val = state - .lazy_vec - .get(&state.storage, *index) - .unwrap() - .unwrap(); - - state - .lazy_vec - .update(&mut state.storage, *index, value.clone()) - .unwrap(); - - // Post-conditions: - let new_len = state.lazy_vec.len(&state.storage).unwrap(); - let new_val = state - .lazy_vec - .get(&state.storage, *index) - .unwrap() - .unwrap(); - assert_eq!(old_len, new_len, "length must not change"); - assert_eq!( - &old_val, - state.eager_vec.get(*index as usize).unwrap(), - "old value must match the value at the same index in \ - the eager vec before it's updated" - ); - assert_eq!( - &new_val, value, - "new value must match that which was passed into the \ - Transition::Update" - ); - } - } - - // Apply transition in the eager vec for comparison - apply_transition_on_eager_vec(&mut state.eager_vec, &transition); - - // Global post-conditions: - - // All items in eager vec must be present in lazy vec - for (ix, expected_item) in state.eager_vec.iter().enumerate() { - let got = state - .lazy_vec - .get(&state.storage, ix as Index) - .unwrap() - .expect("The expected item must be present in lazy vec"); - assert_eq!(expected_item, &got, "at index {ix}"); - } - - // All items in lazy vec must be present in eager vec - for (ix, expected_item) in - state.lazy_vec.iter(&state.storage).unwrap().enumerate() - { - let expected_item = expected_item.unwrap(); - let got = state - .eager_vec - .get(ix) - .expect("The expected item must be present in eager vec"); - assert_eq!(&expected_item, got, "at index {ix}"); - } - - state - } - } - - /// Generate an arbitrary `TestVecItem` - fn arb_test_vec_item() -> impl Strategy { - (any::(), any::()).prop_map(|(x, y)| TestVecItem { x, y }) - } - - /// Apply `Transition` on an eager `Vec`. - fn apply_transition_on_eager_vec( - vec: &mut Vec, - transition: &Transition, - ) { - match transition { - Transition::Push(value) => vec.push(value.clone()), - Transition::Pop => { - let _popped = vec.pop(); - } - Transition::Update { index, value } => { - let entry = vec.get_mut(*index as usize).unwrap(); - *entry = value.clone(); - } - } - } } diff --git a/shared/src/types/storage.rs b/shared/src/types/storage.rs index b5d72b7b02..1c3f6b1313 100644 --- a/shared/src/types/storage.rs +++ b/shared/src/types/storage.rs @@ -378,15 +378,18 @@ impl Key { /// - `Some(None)` if the prefix is matched, but it has no suffix, or /// - `None` if it doesn't match pub fn split_prefix(&self, prefix: &Self) -> Option> { - if self.segments.len() < prefix.len() { + if self.segments.len() < prefix.segments.len() { return None; } else if self == prefix { return Some(None); } - let mut self_prefix = self.segments.clone(); - let rest = self_prefix.split_off(prefix.len()); + // This is safe, because we check that the length of segments in self >= + // in prefix above + let (self_prefix, rest) = self.segments.split_at(prefix.segments.len()); if self_prefix == prefix.segments { - Some(Some(Key { segments: rest })) + Some(Some(Key { + segments: rest.to_vec(), + })) } else { None } diff --git a/tests/proptest-regressions/storage_api/collections/lazy_vec.txt b/tests/proptest-regressions/storage_api/collections/lazy_vec.txt new file mode 100644 index 0000000000..97a16dcbeb --- /dev/null +++ b/tests/proptest-regressions/storage_api/collections/lazy_vec.txt @@ -0,0 +1,7 @@ +# Seeds for failure cases proptest has generated in the past. It is +# automatically read and these particular cases re-run before any +# novel cases are generated. +# +# It is recommended to check this file in to source control so that +# everyone who runs the test benefits from these saved cases. +cc 4330a283e32b5ff3f38d0af2298e1e98c30b1901c1027b572070a1af3356688e # shrinks to (initial_state, transitions) = (AbstractLazyVecState { valid_transitions: [], committed_transitions: [] }, [Push(TestVecItem { x: 15352583996758053781, y: true }), Pop, CommitTx, Push(TestVecItem { x: 6904067244182623445, y: false }), CommitTx, Pop, Push(TestVecItem { x: 759762287021483883, y: true }), Push(TestVecItem { x: 7885704082671389345, y: true }), Pop, Pop, Push(TestVecItem { x: 2762344561419437403, y: false }), Push(TestVecItem { x: 11448034977049028254, y: false }), Update { index: 0, value: TestVecItem { x: 7097339541298715775, y: false } }, Pop, Pop, Push(TestVecItem { x: 457884036257686887, y: true }), CommitTx, Push(TestVecItem { x: 17719281119971095810, y: true }), CommitTx, Push(TestVecItem { x: 4612681906563857058, y: false }), CommitTx, CommitTx, Pop, CommitTx, Pop, Push(TestVecItem { x: 4269537158299505726, y: false }), CommitTx, Pop, Pop, CommitTx, CommitTx, CommitTx, CommitTx, Push(TestVecItem { x: 9020889554694833528, y: true }), Push(TestVecItem { x: 4022797489860699620, y: false }), Update { index: 0, value: TestVecItem { x: 6485081152860611495, y: true } }, Pop, CommitTx, Push(TestVecItem { x: 14470031031894733310, y: false }), Push(TestVecItem { x: 1113274973965556867, y: true }), Push(TestVecItem { x: 4122902042678339346, y: false }), Push(TestVecItem { x: 9672639635189564637, y: true }), Pop, Pop, Pop, CommitTx, Update { index: 0, value: TestVecItem { x: 6372193991838429158, y: false } }, Push(TestVecItem { x: 15140852824102579010, y: false }), Pop, Pop, Pop, Push(TestVecItem { x: 4012218522073776592, y: false }), Push(TestVecItem { x: 10637893847792386454, y: true }), Push(TestVecItem { x: 3357788278949652885, y: false }), CommitTx, CommitTx, Pop, Pop, CommitTx, Pop, Push(TestVecItem { x: 11768518086398350214, y: true }), Push(TestVecItem { x: 4361685178396183644, y: true }), Pop, CommitTx, Push(TestVecItem { x: 2450907664540456425, y: false }), Push(TestVecItem { x: 18184919885943118586, y: true }), Update { index: 1, value: TestVecItem { x: 10611906658537706503, y: false } }, Push(TestVecItem { x: 4887827541279511396, y: false }), Update { index: 0, value: TestVecItem { x: 13021774003761931172, y: false } }, Push(TestVecItem { x: 3644118228573898014, y: false }), CommitTx, Update { index: 0, value: TestVecItem { x: 1276840798381751183, y: false } }, Pop, Pop]) diff --git a/tests/src/lib.rs b/tests/src/lib.rs index 1b75f83bdc..78ebf2473c 100644 --- a/tests/src/lib.rs +++ b/tests/src/lib.rs @@ -12,6 +12,8 @@ mod e2e; #[cfg(test)] mod native_vp; pub mod storage; +#[cfg(test)] +mod storage_api; /// Using this import requires `tracing` and `tracing-subscriber` dependencies. /// Set env var `RUST_LOG=info` to see the logs from a test run (and diff --git a/tests/src/storage_api/collections/lazy_vec.rs b/tests/src/storage_api/collections/lazy_vec.rs new file mode 100644 index 0000000000..20ee80592d --- /dev/null +++ b/tests/src/storage_api/collections/lazy_vec.rs @@ -0,0 +1,631 @@ +#[cfg(test)] +mod tests { + use std::convert::TryInto; + + use borsh::{BorshDeserialize, BorshSerialize}; + use namada::types::address::{self, Address}; + use namada::types::storage; + use namada_tx_prelude::storage::KeySeg; + use namada_tx_prelude::storage_api::collections::{ + lazy_vec, LazyCollection, LazyVec, + }; + use proptest::prelude::*; + use proptest::prop_state_machine; + use proptest::state_machine::{AbstractStateMachine, StateMachineTest}; + use proptest::test_runner::Config; + use test_log::test; + + use crate::tx::tx_host_env; + use crate::vp::vp_host_env; + + prop_state_machine! { + #![proptest_config(Config { + // Instead of the default 256, we only run 5 because otherwise it + // takes too long and it's preferable to crank up the number of + // transitions instead, to allow each case to run for more epochs as + // some issues only manifest once the model progresses further. + // Additionally, more cases will be explored every time this test is + // executed in the CI. + cases: 5, + .. Config::default() + })] + #[test] + fn lazy_vec_api_state_machine_test(sequential 1..100 => ConcreteLazyVecState); + } + + /// Some borsh-serializable type with arbitrary fields to be used inside + /// LazyVec state machine test + #[derive( + Clone, + Debug, + BorshSerialize, + BorshDeserialize, + PartialEq, + Eq, + PartialOrd, + Ord, + )] + struct TestVecItem { + x: u64, + y: bool, + } + + /// A `StateMachineTest` implemented on this struct manipulates it with + /// `Transition`s, which are also being accumulated into + /// `current_transitions`. It then: + /// + /// - checks its state against an in-memory `std::collections::Vec` + /// - runs validation and checks that the `LazyVec::Action`s reported from + /// validation match with transitions that were applied + /// + /// Additionally, one of the transitions is to commit a block and/or + /// transaction, during which the currently accumulated state changes are + /// persisted, or promoted from transaction write log to block's write log. + #[derive(Debug)] + struct ConcreteLazyVecState { + /// Address is used to prefix the storage key of the `lazy_vec` in + /// order to simulate a transaction and a validity predicate + /// check from changes on the `lazy_vec` + address: Address, + /// In the test, we apply the same transitions on the `lazy_vec` as on + /// `eager_vec` to check that `lazy_vec`'s state is consistent with + /// `eager_vec`. + eager_vec: Vec, + /// Handle to a lazy vec + lazy_vec: LazyVec, + /// Valid LazyVec changes in the current transaction + current_transitions: Vec>, + } + + #[derive(Clone, Debug)] + struct AbstractLazyVecState { + /// Valid LazyVec changes in the current transaction + valid_transitions: Vec>, + /// Valid LazyVec changes committed to storage + committed_transitions: Vec>, + } + + /// Possible transitions that can modify a [`LazyVec`]. This roughly + /// corresponds to the methods that have `StorageWrite` access and is very + /// similar to [`Action`] + #[derive(Clone, Debug)] + pub enum Transition { + /// Commit all valid transitions in the current transaction + CommitTx, + /// Commit all valid transitions in the current transaction and also + /// commit the current block + CommitTxAndBlock, + /// Push a value `T` into a [`LazyVec`] + Push(T), + /// Pop a value from a [`LazyVec`] + Pop, + /// Update a value `T` at index from pre to post state in a + /// [`LazyVec`] + Update { + /// index at which the value is updated + index: lazy_vec::Index, + /// value to update the element to + value: T, + }, + } + + impl AbstractStateMachine for AbstractLazyVecState { + type State = Self; + type Transition = Transition; + + fn init_state() -> BoxedStrategy { + Just(Self { + valid_transitions: vec![], + committed_transitions: vec![], + }) + .boxed() + } + + // Apply a random transition to the state + fn transitions(state: &Self::State) -> BoxedStrategy { + let length = state.len(); + if length == 0 { + prop_oneof![ + 1 => Just(Transition::CommitTx), + 1 => Just(Transition::CommitTxAndBlock), + 3 => arb_test_vec_item().prop_map(Transition::Push) + ] + .boxed() + } else { + let arb_index = || { + let indices: Vec = (0..length).collect(); + proptest::sample::select(indices) + }; + prop_oneof![ + 1 => Just(Transition::CommitTx), + 1 => Just(Transition::CommitTxAndBlock), + 3 => (arb_index(), arb_test_vec_item()).prop_map( + |(index, value)| Transition::Update { index, value } + ), + 3 => Just(Transition::Pop), + 5 => arb_test_vec_item().prop_map(Transition::Push), + ] + .boxed() + } + } + + fn apply_abstract( + mut state: Self::State, + transition: &Self::Transition, + ) -> Self::State { + match transition { + Transition::CommitTx => { + let valid_actions_to_commit = + std::mem::take(&mut state.valid_transitions); + state + .committed_transitions + .extend(valid_actions_to_commit.into_iter()); + } + _ => state.valid_transitions.push(transition.clone()), + } + state + } + + fn preconditions( + state: &Self::State, + transition: &Self::Transition, + ) -> bool { + let length = state.len(); + if length == 0 { + // Ensure that the pop or update transitions are not applied to + // an empty state + !matches!( + transition, + Transition::Pop | Transition::Update { .. } + ) + } else if let Transition::Update { index, .. } = transition { + // Ensure that the update index is a valid one + *index < (length - 1) + } else { + true + } + } + } + + impl StateMachineTest for ConcreteLazyVecState { + type Abstract = AbstractLazyVecState; + type ConcreteState = Self; + + fn init_test( + _initial_state: ::State, + ) -> Self::ConcreteState { + // Init transaction env in which we'll be applying the transitions + tx_host_env::init(); + + // The lazy_vec's path must be prefixed by the address to be able + // to trigger a validity predicate on it + let address = address::testing::established_address_1(); + tx_host_env::with(|env| env.spawn_accounts([&address])); + let lazy_vec_prefix: storage::Key = address.to_db_key().into(); + + Self { + address, + eager_vec: vec![], + lazy_vec: LazyVec::open( + lazy_vec_prefix.push(&"arbitrary".to_string()).unwrap(), + ), + current_transitions: vec![], + } + } + + fn apply_concrete( + mut state: Self::ConcreteState, + transition: ::Transition, + ) -> Self::ConcreteState { + // Apply transitions in transaction env + let ctx = tx_host_env::ctx(); + + // Persist the transitions in the current tx, or clear previous ones + // if we're committing a tx + match &transition { + Transition::CommitTx | Transition::CommitTxAndBlock => { + state.current_transitions = vec![]; + } + _ => { + state.current_transitions.push(transition.clone()); + } + } + + // Transition application on lazy vec and post-conditions: + match &transition { + Transition::CommitTx => { + // commit the tx without committing the block + tx_host_env::with(|env| env.write_log.commit_tx()); + } + Transition::CommitTxAndBlock => { + // commit the tx and the block + tx_host_env::commit_tx_and_block(); + } + Transition::Push(value) => { + let old_len = state.lazy_vec.len(ctx).unwrap(); + + state.lazy_vec.push(ctx, value.clone()).unwrap(); + + // Post-conditions: + let new_len = state.lazy_vec.len(ctx).unwrap(); + let stored_value = + state.lazy_vec.get(ctx, new_len - 1).unwrap().unwrap(); + assert_eq!( + &stored_value, value, + "the new item must be added to the back" + ); + assert_eq!(old_len + 1, new_len, "length must increment"); + + state.assert_validation_accepted(new_len); + } + Transition::Pop => { + let old_len = state.lazy_vec.len(ctx).unwrap(); + + let popped = state.lazy_vec.pop(ctx).unwrap().unwrap(); + + // Post-conditions: + let new_len = state.lazy_vec.len(ctx).unwrap(); + assert_eq!(old_len, new_len + 1, "length must decrement"); + assert_eq!( + &popped, + state.eager_vec.last().unwrap(), + "popped element matches the last element in eager vec \ + before it's updated" + ); + + state.assert_validation_accepted(new_len); + } + Transition::Update { index, value } => { + let old_len = state.lazy_vec.len(ctx).unwrap(); + let old_val = + state.lazy_vec.get(ctx, *index).unwrap().unwrap(); + + state.lazy_vec.update(ctx, *index, value.clone()).unwrap(); + + // Post-conditions: + let new_len = state.lazy_vec.len(ctx).unwrap(); + let new_val = + state.lazy_vec.get(ctx, *index).unwrap().unwrap(); + assert_eq!(old_len, new_len, "length must not change"); + assert_eq!( + &old_val, + state.eager_vec.get(*index as usize).unwrap(), + "old value must match the value at the same index in \ + the eager vec before it's updated" + ); + assert_eq!( + &new_val, value, + "new value must match that which was passed into the \ + Transition::Update" + ); + + state.assert_validation_accepted(new_len); + } + } + + // Apply transition in the eager vec for comparison + apply_transition_on_eager_vec(&mut state.eager_vec, &transition); + + // Global post-conditions: + + // All items in eager vec must be present in lazy vec + for (ix, expected_item) in state.eager_vec.iter().enumerate() { + let got = state + .lazy_vec + .get(ctx, ix as lazy_vec::Index) + .unwrap() + .expect("The expected item must be present in lazy vec"); + assert_eq!(expected_item, &got, "at index {ix}"); + } + + // All items in lazy vec must be present in eager vec + for (ix, expected_item) in + state.lazy_vec.iter(ctx).unwrap().enumerate() + { + let expected_item = expected_item.unwrap(); + let got = state + .eager_vec + .get(ix) + .expect("The expected item must be present in eager vec"); + assert_eq!(&expected_item, got, "at index {ix}"); + } + + state + } + } + + impl AbstractLazyVecState { + /// Find the length of the vector from the applied transitions + fn len(&self) -> u64 { + (vec_len_diff_from_transitions(self.committed_transitions.iter()) + + vec_len_diff_from_transitions(self.valid_transitions.iter())) + .try_into() + .expect( + "It shouldn't be possible to underflow length from all \ + transactions applied in abstract state", + ) + } + } + + /// Find the difference in length of the vector from the applied transitions + fn vec_len_diff_from_transitions<'a>( + all_transitions: impl Iterator>, + ) -> i64 { + let mut push_count: i64 = 0; + let mut pop_count: i64 = 0; + + for trans in all_transitions { + match trans { + Transition::CommitTx + | Transition::CommitTxAndBlock + | Transition::Update { .. } => {} + Transition::Push(_) => push_count += 1, + Transition::Pop => pop_count += 1, + } + } + push_count - pop_count + } + + impl ConcreteLazyVecState { + fn assert_validation_accepted(&self, new_vec_len: u64) { + // Init the VP env from tx env in which we applied the vec + // transitions + let tx_env = tx_host_env::take(); + vp_host_env::init_from_tx(self.address.clone(), tx_env, |_| {}); + + // Simulate a validity predicate run using the lazy vec's validation + // helpers + let changed_keys = + vp_host_env::with(|env| env.all_touched_storage_keys()); + + let mut validation_builder = None; + + // Push followed by pop is a no-op, in which case we'd still see the + // changed keys for these actions, but they wouldn't affect the + // validation result and they never get persisted, but we'd still + // them as changed key here. To guard against this case, + // we check that `vec_len_from_transitions` is not empty. + let vec_len_diff = + vec_len_diff_from_transitions(self.current_transitions.iter()); + + // To help debug validation issues... + dbg!( + &self.current_transitions, + &changed_keys + .iter() + .map(storage::Key::to_string) + .collect::>() + ); + + for key in &changed_keys { + let is_sub_key = self + .lazy_vec + .accumulate( + vp_host_env::ctx(), + &mut validation_builder, + key, + ) + .unwrap(); + + assert!( + is_sub_key, + "We're only modifying the lazy_vec's keys here. Key: \ + \"{key}\", vec length diff {vec_len_diff}" + ); + } + if !changed_keys.is_empty() && vec_len_diff != 0 { + assert!( + validation_builder.is_some(), + "If some keys were changed, the builder must get filled in" + ); + let actions = validation_builder.unwrap().build().expect( + "With valid transitions only, validation should always \ + pass", + ); + let mut actions_to_check = actions.clone(); + + // Check that every transition has a corresponding action from + // validation. We drop the found actions to check that all + // actions are matched too. + let current_transitions = normalize_transitions( + &self.current_transitions, + new_vec_len, + ); + for transition in ¤t_transitions { + match transition { + Transition::CommitTx | Transition::CommitTxAndBlock => { + } + Transition::Push(expected_val) => { + let mut ix = 0; + while ix < actions_to_check.len() { + if let lazy_vec::Action::Push(val) = + &actions_to_check[ix] + { + if expected_val == val { + actions_to_check.remove(ix); + break; + } + } + ix += 1; + } + } + Transition::Pop => { + let mut ix = 0; + while ix < actions_to_check.len() { + if let lazy_vec::Action::Pop(_val) = + &actions_to_check[ix] + { + actions_to_check.remove(ix); + break; + } + ix += 1; + } + } + Transition::Update { + index: expected_index, + value, + } => { + let mut ix = 0; + while ix < actions_to_check.len() { + if let lazy_vec::Action::Update { + index, + pre: _, + post, + } = &actions_to_check[ix] + { + if expected_index == index && post == value + { + actions_to_check.remove(ix); + break; + } + } + ix += 1; + } + } + } + } + + assert!( + actions_to_check.is_empty(), + "All the actions reported from validation {actions:#?} \ + should have been matched with SM transitions \ + {current_transitions:#?}, but these actions didn't \ + match: {actions_to_check:#?}", + ) + } + + // Put the tx_env back before checking the result + tx_host_env::set_from_vp_env(vp_host_env::take()); + } + } + + /// Generate an arbitrary `TestVecItem` + fn arb_test_vec_item() -> impl Strategy { + (any::(), any::()).prop_map(|(x, y)| TestVecItem { x, y }) + } + + /// Apply `Transition` on an eager `Vec`. + fn apply_transition_on_eager_vec( + vec: &mut Vec, + transition: &Transition, + ) { + match transition { + Transition::CommitTx | Transition::CommitTxAndBlock => {} + Transition::Push(value) => vec.push(value.clone()), + Transition::Pop => { + let _popped = vec.pop(); + } + Transition::Update { index, value } => { + let entry = vec.get_mut(*index as usize).unwrap(); + *entry = value.clone(); + } + } + } + + /// Normalize transitions: + /// - pop at ix + push(val) at ix -> update(ix, val) + /// - push(val) at ix + update(ix, new_val) -> push(new_val) at ix + /// - update(ix, val) + update(ix, new_val) -> update(ix, new_val) + /// + /// Note that the normalizable transitions pairs do not have to be directly + /// next to each other, but their order does matter. + fn normalize_transitions( + transitions: &[Transition], + new_vec_len: u64, + ) -> Vec> { + let stack_start_pos = ((new_vec_len as i64) + - vec_len_diff_from_transitions(transitions.iter())) + as u64; + let mut stack_pos = stack_start_pos; + let mut collapsed = vec![]; + 'outer: for transition in transitions { + match transition { + Transition::CommitTx | Transition::CommitTxAndBlock => { + collapsed.push(transition.clone()) + } + Transition::Push(value) => { + // If there are some pops, the last one can be collapsed + // with this push + if stack_pos < stack_start_pos { + // Find the pop from the back + let mut found_ix = None; + for (ix, transition) in + collapsed.iter().enumerate().rev() + { + if let Transition::Pop = transition { + found_ix = Some(ix); + break; + } + } + let ix = found_ix.expect("Pop must be found"); + // pop at ix + push(val) at ix -> update(ix, val) + + // Replace the Pop with an Update and don't insert the + // Push + *collapsed.get_mut(ix).unwrap() = Transition::Update { + index: stack_pos, + value: value.clone(), + }; + } else { + collapsed.push(transition.clone()); + } + stack_pos += 1; + } + Transition::Pop => { + collapsed.push(transition.clone()); + stack_pos -= 1; + } + Transition::Update { index, value } => { + // If there are some pushes, check if one of them is at the + // same index as this update + if stack_pos > stack_start_pos { + let mut current_pos = stack_start_pos; + for (ix, collapsed_transition) in + collapsed.iter().enumerate() + { + match collapsed_transition { + Transition::CommitTx + | Transition::CommitTxAndBlock => {} + Transition::Push(_) => { + if ¤t_pos == index { + // push(val) at `ix` + update(ix, + // new_val) -> + // push(new_val) at `ix` + + // Replace the Push with the new Push of + // Update's + // value and don't insert the Update + *collapsed.get_mut(ix).unwrap() = + Transition::Push(value.clone()); + continue 'outer; + } + current_pos += 1; + } + Transition::Pop => { + current_pos -= 1; + } + Transition::Update { + index: prev_update_index, + value: _, + } => { + if index == prev_update_index { + // update(ix, val) + update(ix, new_val) + // -> update(ix, new_val) + + // Replace the Update with the new + // Update instead of inserting it + *collapsed.get_mut(ix).unwrap() = + transition.clone(); + continue 'outer; + } + } + } + } + } + collapsed.push(transition.clone()) + } + } + } + collapsed + } +} diff --git a/tests/src/storage_api/collections/mod.rs b/tests/src/storage_api/collections/mod.rs new file mode 100644 index 0000000000..d874b88e22 --- /dev/null +++ b/tests/src/storage_api/collections/mod.rs @@ -0,0 +1 @@ +mod lazy_vec; diff --git a/tests/src/storage_api/mod.rs b/tests/src/storage_api/mod.rs new file mode 100644 index 0000000000..bc487bd59e --- /dev/null +++ b/tests/src/storage_api/mod.rs @@ -0,0 +1 @@ +mod collections; diff --git a/tests/src/vm_host_env/tx.rs b/tests/src/vm_host_env/tx.rs index 728c488bca..6e23ced06b 100644 --- a/tests/src/vm_host_env/tx.rs +++ b/tests/src/vm_host_env/tx.rs @@ -18,6 +18,8 @@ use namada::vm::{self, WasmCacheRwAccess}; use namada_tx_prelude::{BorshSerialize, Ctx}; use tempfile::TempDir; +use crate::vp::TestVpEnv; + /// Tx execution context provides access to host env functions static mut CTX: Ctx = unsafe { Ctx::new() }; @@ -235,6 +237,29 @@ mod native_tx_host_env { with(|env| env.commit_tx_and_block()) } + /// Set the [`TestTxEnv`] back from a [`TestVpEnv`]. This is useful when + /// testing validation with multiple transactions that accumulate some state + /// changes. + pub fn set_from_vp_env(vp_env: TestVpEnv) { + let TestVpEnv { + storage, + write_log, + tx, + vp_wasm_cache, + vp_cache_dir, + .. + } = vp_env; + let tx_env = TestTxEnv { + storage, + write_log, + vp_wasm_cache, + vp_cache_dir, + tx, + ..Default::default() + }; + set(tx_env); + } + /// A helper macro to create implementations of the host environment /// functions exported to wasm, which uses the environment from the /// `ENV` variable.