From 6d7ad6f0b57f02c49a54eec83ded2f48c1eb5f2b Mon Sep 17 00:00:00 2001 From: Nikolai Golub Date: Thu, 2 Nov 2023 14:53:57 +0100 Subject: [PATCH 01/13] [no ci] Adding Hierrarchical Prover Storage --- Cargo.lock | 14 + Cargo.toml | 1 + adapters/mock-da/src/types/mod.rs | 8 + full-node/db/sov-db/src/schema/tables.rs | 1 - full-node/db/sov-schema-db/src/lib.rs | 5 +- full-node/db/sov-schema-db/src/snapshot.rs | 6 +- .../db/sov-schema-db/tests/snapshot_test.rs | 2 +- .../sov-prover-storage-manager/Cargo.toml | 25 + .../sov-prover-storage-manager/README.md | 3 + .../src/dummy_storage.rs | 145 +++++ .../sov-prover-storage-manager/src/lib.rs | 502 ++++++++++++++++++ .../src/snapshot_manager.rs | 86 +++ rollup-interface/src/state_machine/da.rs | 2 + rollup-interface/src/state_machine/storage.rs | 29 + 14 files changed, 821 insertions(+), 8 deletions(-) create mode 100644 full-node/sov-prover-storage-manager/Cargo.toml create mode 100644 full-node/sov-prover-storage-manager/README.md create mode 100644 full-node/sov-prover-storage-manager/src/dummy_storage.rs create mode 100644 full-node/sov-prover-storage-manager/src/lib.rs create mode 100644 full-node/sov-prover-storage-manager/src/snapshot_manager.rs diff --git a/Cargo.lock b/Cargo.lock index ac515e88b..071a4fedf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8964,6 +8964,20 @@ dependencies = [ "tempfile", ] +[[package]] +name = "sov-prover-storage-manager" +version = "0.3.0" +dependencies = [ + "anyhow", + "byteorder", + "sov-db", + "sov-mock-da", + "sov-rollup-interface", + "sov-schema-db", + "sov-state", + "tempfile", +] + [[package]] name = "sov-risc0-adapter" version = "0.3.0" diff --git a/Cargo.toml b/Cargo.toml index 64a01906d..53c628355 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,6 +18,7 @@ members = [ "full-node/sov-ethereum", "full-node/sov-ledger-rpc", "full-node/sov-stf-runner", + "full-node/sov-prover-storage-manager", # Utils "utils/zk-cycle-macros", "utils/zk-cycle-utils", diff --git a/adapters/mock-da/src/types/mod.rs b/adapters/mock-da/src/types/mod.rs index 3a0761649..2f7293ddd 100644 --- a/adapters/mock-da/src/types/mod.rs +++ b/adapters/mock-da/src/types/mod.rs @@ -1,6 +1,7 @@ mod address; use std::fmt::Formatter; +use std::hash::Hasher; pub use address::{MockAddress, MOCK_SEQUENCER_DA_ADDRESS}; use borsh::{BorshDeserialize, BorshSerialize}; @@ -45,6 +46,13 @@ impl From for [u8; 32] { } } +impl std::hash::Hash for MockHash { + fn hash(&self, state: &mut H) { + state.write(&self.0); + state.finish(); + } +} + impl BlockHashTrait for MockHash {} /// A mock block header used for testing. diff --git a/full-node/db/sov-db/src/schema/tables.rs b/full-node/db/sov-db/src/schema/tables.rs index 6f267d971..95a0fd49f 100644 --- a/full-node/db/sov-db/src/schema/tables.rs +++ b/full-node/db/sov-db/src/schema/tables.rs @@ -208,7 +208,6 @@ macro_rules! define_table_with_seek_key_codec { }; } -// fn deser(target: &mut &[u8]) -> Result; define_table_with_seek_key_codec!( /// The primary source for slot data (SlotByNumber) SlotNumber => StoredSlot diff --git a/full-node/db/sov-schema-db/src/lib.rs b/full-node/db/sov-schema-db/src/lib.rs index 7cd85d261..1957141e9 100644 --- a/full-node/db/sov-schema-db/src/lib.rs +++ b/full-node/db/sov-schema-db/src/lib.rs @@ -311,14 +311,13 @@ impl SchemaBatch { column_writes.insert(key, operation); } - #[allow(dead_code)] pub(crate) fn read( &self, key: &impl KeyCodec, - ) -> anyhow::Result> { + ) -> anyhow::Result> { let key = key.encode_key()?; if let Some(column_writes) = self.last_writes.get(&S::COLUMN_FAMILY_NAME) { - return Ok(column_writes.get(&key).cloned()); + return Ok(column_writes.get(&key)); } Ok(None) } diff --git a/full-node/db/sov-schema-db/src/snapshot.rs b/full-node/db/sov-schema-db/src/snapshot.rs index 6bfbe3f7b..2d9479894 100644 --- a/full-node/db/sov-schema-db/src/snapshot.rs +++ b/full-node/db/sov-schema-db/src/snapshot.rs @@ -107,7 +107,7 @@ pub struct FrozenDbSnapshot { impl FrozenDbSnapshot { /// Get value from its own cache - pub fn get(&self, key: &impl KeyCodec) -> anyhow::Result> { + pub fn get(&self, key: &impl KeyCodec) -> anyhow::Result> { self.cache.read(key) } @@ -135,10 +135,10 @@ impl From for SchemaBatch { } } -fn decode_operation(operation: Operation) -> anyhow::Result> { +fn decode_operation(operation: &Operation) -> anyhow::Result> { match operation { Operation::Put { value } => { - let value = S::Value::decode_value(&value)?; + let value = S::Value::decode_value(value)?; Ok(Some(value)) } Operation::Delete => Ok(None), diff --git a/full-node/db/sov-schema-db/tests/snapshot_test.rs b/full-node/db/sov-schema-db/tests/snapshot_test.rs index 24df1a7a2..c07691e36 100644 --- a/full-node/db/sov-schema-db/tests/snapshot_test.rs +++ b/full-node/db/sov-schema-db/tests/snapshot_test.rs @@ -69,7 +69,7 @@ impl QueryManager for LinearSnapshotManager { for snapshot in self.snapshots[..snapshot_id as usize].iter().rev() { if let Some(operation) = snapshot.get(key)? { return match operation { - Operation::Put { value } => Ok(Some(S::Value::decode_value(&value)?)), + Operation::Put { value } => Ok(Some(S::Value::decode_value(value)?)), Operation::Delete => Ok(None), }; } diff --git a/full-node/sov-prover-storage-manager/Cargo.toml b/full-node/sov-prover-storage-manager/Cargo.toml new file mode 100644 index 000000000..f86da10b5 --- /dev/null +++ b/full-node/sov-prover-storage-manager/Cargo.toml @@ -0,0 +1,25 @@ +[package] +name = "sov-prover-storage-manager" +description = "Storage manager for prover storage" +license = { workspace = true } +edition = { workspace = true } +authors = { workspace = true } +homepage = { workspace = true } +repository = { workspace = true } + +version = { workspace = true } +readme = "README.md" +resolver = "2" + +[dependencies] +anyhow = { workspace = true } +sov-rollup-interface = { path = "../../rollup-interface" } +sov-db = { path = "../db/sov-db" } +sov-schema-db = { path = "../db/sov-schema-db" } +sov-state = { path = "../../module-system/sov-state", features = ["native"] } +# TODO: Remove this after integrated with `sov-db` fully +byteorder = { workspace = true, default-features = true } + +[dev-dependencies] +sov-mock-da = { path = "../../adapters/mock-da", features = ["native"] } +tempfile = { workspace = true } \ No newline at end of file diff --git a/full-node/sov-prover-storage-manager/README.md b/full-node/sov-prover-storage-manager/README.md new file mode 100644 index 000000000..5412b6458 --- /dev/null +++ b/full-node/sov-prover-storage-manager/README.md @@ -0,0 +1,3 @@ +# `sov-prover-storage-manager` + +Implementation of `StorageManager` for `ProverStorage` that can handle forks and re-orgs \ No newline at end of file diff --git a/full-node/sov-prover-storage-manager/src/dummy_storage.rs b/full-node/sov-prover-storage-manager/src/dummy_storage.rs new file mode 100644 index 000000000..08ce22839 --- /dev/null +++ b/full-node/sov-prover-storage-manager/src/dummy_storage.rs @@ -0,0 +1,145 @@ +use std::marker::PhantomData; + +use byteorder::{BigEndian, ReadBytesExt}; +use sov_schema_db::schema::{KeyDecoder, KeyEncoder, Result as CodecResult, ValueCodec}; +use sov_schema_db::snapshot::{DbSnapshot, QueryManager}; +use sov_schema_db::{define_schema, CodecError}; +use sov_state::MerkleProofSpec; + +/// Oversimplified representation of [`sov_state::ProverStorage`] +pub struct NewProverStorage { + state_db: DbSnapshot, + native_db: DbSnapshot, + p: PhantomData, +} + +impl NewProverStorage { + pub(crate) fn with_db_handlers( + state_db_snapshot: DbSnapshot, + native_db_snapshot: DbSnapshot, + ) -> Self { + NewProverStorage { + state_db: state_db_snapshot, + native_db: native_db_snapshot, + p: Default::default(), + } + } + + pub(crate) fn freeze(self) -> (DbSnapshot, DbSnapshot) { + let NewProverStorage { + state_db, + native_db, + .. + } = self; + (state_db, native_db) + } + + #[allow(dead_code)] + pub(crate) fn read_state(&self, key: u64) -> anyhow::Result> { + let key = DummyField(key); + Ok(self + .state_db + .read::(&key)? + .map(Into::into)) + } + + #[allow(dead_code)] + pub(crate) fn write_state(&self, key: u64, value: u64) -> anyhow::Result<()> { + let key = DummyField(key); + let value = DummyField(value); + self.state_db.put::(&key, &value) + } + + #[allow(dead_code)] + pub(crate) fn read_native(&self, key: u64) -> anyhow::Result> { + let key = DummyField(key); + Ok(self + .native_db + .read::(&key)? + .map(Into::into)) + } + + #[allow(dead_code)] + pub(crate) fn write_native(&self, key: u64, value: u64) -> anyhow::Result<()> { + let key = DummyField(key); + let value = DummyField(value); + self.native_db.put::(&key, &value) + } +} + +// -------------- +// The code below used to emulate native and state db, but on oversimplified level + +pub(crate) const DUMMY_STATE_CF: &str = "DummyStateCF"; +pub(crate) const DUMMY_NATIVE_CF: &str = "DummyNativeCF"; + +define_schema!(DummyStateSchema, DummyField, DummyField, DUMMY_STATE_CF); +define_schema!(DummyNativeSchema, DummyField, DummyField, DUMMY_NATIVE_CF); + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) struct DummyField(pub(crate) u64); + +impl From for u64 { + fn from(value: DummyField) -> Self { + value.0 + } +} + +impl DummyField { + fn as_bytes(&self) -> Vec { + self.0.to_be_bytes().to_vec() + } + + fn from_bytes(data: &[u8]) -> CodecResult { + let mut reader = std::io::Cursor::new(data); + Ok(Self( + reader + .read_u64::() + .map_err(|e| CodecError::Wrapped(e.into()))?, + )) + } +} + +impl KeyEncoder for DummyField { + fn encode_key(&self) -> CodecResult> { + Ok(self.as_bytes()) + } +} + +impl KeyDecoder for DummyField { + fn decode_key(data: &[u8]) -> CodecResult { + Self::from_bytes(data) + } +} + +impl ValueCodec for DummyField { + fn encode_value(&self) -> CodecResult> { + Ok(self.as_bytes()) + } + + fn decode_value(data: &[u8]) -> CodecResult { + Self::from_bytes(data) + } +} + +impl KeyEncoder for DummyField { + fn encode_key(&self) -> CodecResult> { + Ok(self.as_bytes()) + } +} + +impl KeyDecoder for DummyField { + fn decode_key(data: &[u8]) -> CodecResult { + Self::from_bytes(data) + } +} + +impl ValueCodec for DummyField { + fn encode_value(&self) -> CodecResult> { + Ok(self.as_bytes()) + } + + fn decode_value(data: &[u8]) -> CodecResult { + Self::from_bytes(data) + } +} diff --git a/full-node/sov-prover-storage-manager/src/lib.rs b/full-node/sov-prover-storage-manager/src/lib.rs new file mode 100644 index 000000000..a1ff75db9 --- /dev/null +++ b/full-node/sov-prover-storage-manager/src/lib.rs @@ -0,0 +1,502 @@ +mod dummy_storage; +mod snapshot_manager; + +use std::collections::HashMap; +use std::hash::Hash; +use std::marker::PhantomData; +use std::sync::{Arc, RwLock}; + +use sov_rollup_interface::da::{BlockHeaderTrait, DaSpec}; +use sov_rollup_interface::storage::HierarchicalStorageManager; +use sov_schema_db::snapshot::{DbSnapshot, FrozenDbSnapshot, ReadOnlyLock, SnapshotId}; +use sov_state::MerkleProofSpec; + +use crate::dummy_storage::NewProverStorage; +use crate::snapshot_manager::SnapshotManager; + +struct NewProverStorageManager { + // L1 forks representation + // Chain: prev_block -> child_blocks + chain_forks: HashMap>, + // Reverse: child_block -> parent + blocks_to_parent: HashMap, + + latest_snapshot_id: SnapshotId, + block_hash_to_snapshot_id: HashMap, + // Same reference for individual managers + snapshot_id_to_parent: Arc>>, + + state_snapshot_manager: Arc>, + native_snapshot_manager: Arc>, + + phantom_mp_spec: PhantomData, +} + +impl NewProverStorageManager { + #[allow(dead_code)] + pub fn new(state_db: sov_schema_db::DB, native_db: sov_schema_db::DB) -> Self { + let snapshot_id_to_parent = Arc::new(RwLock::new(HashMap::new())); + + let state_snapshot_manager = SnapshotManager::new(state_db, snapshot_id_to_parent.clone()); + let native_snapshot_manager = + SnapshotManager::new(native_db, snapshot_id_to_parent.clone()); + + Self { + chain_forks: Default::default(), + blocks_to_parent: Default::default(), + latest_snapshot_id: 0, + block_hash_to_snapshot_id: Default::default(), + snapshot_id_to_parent, + state_snapshot_manager: Arc::new(RwLock::new(state_snapshot_manager)), + native_snapshot_manager: Arc::new(RwLock::new(native_snapshot_manager)), + phantom_mp_spec: Default::default(), + } + } + + #[cfg(test)] + fn is_empty(&self) -> bool { + self.chain_forks.is_empty() + && self.blocks_to_parent.is_empty() + && self.block_hash_to_snapshot_id.is_empty() + && self.snapshot_id_to_parent.read().unwrap().is_empty() + && self.state_snapshot_manager.read().unwrap().is_empty() + && self.native_snapshot_manager.read().unwrap().is_empty() + } +} + +impl HierarchicalStorageManager + for NewProverStorageManager +where + Da::SlotHash: Hash, +{ + type NativeStorage = NewProverStorage; + type NativeChangeSet = NewProverStorage; + + fn get_native_storage_on( + &mut self, + block_header: &Da::BlockHeader, + ) -> anyhow::Result { + let current_block_hash = block_header.hash(); + let prev_block_hash = block_header.prev_hash(); + assert_ne!( + current_block_hash, prev_block_hash, + "Cannot provide storage for corrupt block" + ); + + let new_snapshot_id = match self.block_hash_to_snapshot_id.get(¤t_block_hash) { + // Storage for this block has been requested before + Some(snapshot_id) => { + // TODO: Do consistency checks here? + + *snapshot_id + } + // Storage requested first time + None => { + let new_snapshot_id = self.latest_snapshot_id + 1; + if let Some(parent_snapshot_id) = + self.block_hash_to_snapshot_id.get(&prev_block_hash) + { + let mut snapshot_id_to_parent = self.snapshot_id_to_parent.write().unwrap(); + snapshot_id_to_parent.insert(new_snapshot_id, *parent_snapshot_id); + } + + self.block_hash_to_snapshot_id + .insert(current_block_hash.clone(), new_snapshot_id); + + self.chain_forks + .entry(prev_block_hash.clone()) + .or_default() + .push(current_block_hash.clone()); + + self.blocks_to_parent + .insert(current_block_hash, prev_block_hash); + + // Update latest snapshot id + self.latest_snapshot_id = new_snapshot_id; + new_snapshot_id + } + }; + println!( + "BLOCK HEIGHT={} SNAP_ID={}", + block_header.height(), + new_snapshot_id + ); + + let state_db_snapshot = DbSnapshot::new( + new_snapshot_id, + ReadOnlyLock::new(self.state_snapshot_manager.clone()), + ); + + let native_db_snapshot = DbSnapshot::new( + new_snapshot_id, + ReadOnlyLock::new(self.native_snapshot_manager.clone()), + ); + + Ok(NewProverStorage::with_db_handlers( + state_db_snapshot, + native_db_snapshot, + )) + } + + fn save_change_set( + &mut self, + block_header: &Da::BlockHeader, + change_set: Self::NativeChangeSet, + ) -> anyhow::Result<()> { + if !self.chain_forks.contains_key(&block_header.prev_hash()) { + anyhow::bail!("Attempt to save changeset for unknown block header"); + } + let (state_db, native_db) = change_set.freeze(); + let state_snapshot: FrozenDbSnapshot = state_db.into(); + let native_snapshot: FrozenDbSnapshot = native_db.into(); + let snapshot_id = state_snapshot.get_id(); + if snapshot_id != native_snapshot.get_id() { + anyhow::bail!( + "State id={} and Native id={} snapshots have different are not matching", + snapshot_id, + native_snapshot.get_id() + ); + } + + // Obviously alien + println!("L={} S={}", self.latest_snapshot_id, snapshot_id); + if snapshot_id > self.latest_snapshot_id { + anyhow::bail!("Attempt to save unknown snapshot with id={}", snapshot_id); + } + + { + let existing_snapshot_id = self + .block_hash_to_snapshot_id + .get(&block_header.hash()) + .expect("Inconsistent block_hash_to_snapshot_id"); + if *existing_snapshot_id != snapshot_id { + anyhow::bail!("Attempt to save unknown snapshot with id={}", snapshot_id); + } + } + + { + let mut state_manager = self.state_snapshot_manager.write().unwrap(); + let mut native_manager = self.native_snapshot_manager.write().unwrap(); + + state_manager.add_snapshot(state_snapshot); + native_manager.add_snapshot(native_snapshot); + } + + Ok(()) + } + + fn finalize(&mut self, block_header: &Da::BlockHeader) -> anyhow::Result<()> { + let current_block_hash = block_header.hash(); + let prev_block_hash = block_header.prev_hash(); + + let snapshot_id = self + .block_hash_to_snapshot_id + .remove(¤t_block_hash) + .ok_or(anyhow::anyhow!("Attempt to finalize non existing snapshot"))?; + + let mut state_manager = self.state_snapshot_manager.write().unwrap(); + let mut native_manager = self.native_snapshot_manager.write().unwrap(); + + // Return error here, as underlying database can return error + state_manager.commit_snapshot(&snapshot_id)?; + native_manager.commit_snapshot(&snapshot_id)?; + + // All siblings of current snapshot + let mut to_discard: Vec<_> = self + .chain_forks + .remove(&prev_block_hash) + .expect("Inconsistent chain_forks") + .into_iter() + .filter(|bh| bh != ¤t_block_hash) + .collect(); + + while let Some(block_hash) = to_discard.pop() { + let child_block_hashes = self.chain_forks.remove(&block_hash).unwrap_or_default(); + self.blocks_to_parent.remove(&block_hash).unwrap(); + let snapshot_id = self.block_hash_to_snapshot_id.remove(&block_hash).unwrap(); + + state_manager.discard_snapshot(&snapshot_id); + native_manager.discard_snapshot(&snapshot_id); + + to_discard.extend(child_block_hashes); + } + + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use std::path; + + use sov_db::rocks_db_config::gen_rocksdb_options; + use sov_mock_da::{MockBlockHeader, MockHash}; + use sov_schema_db::snapshot::FrozenDbSnapshot; + + use super::*; + use crate::dummy_storage::{ + DummyField, DummyNativeSchema, DummyStateSchema, DUMMY_NATIVE_CF, DUMMY_STATE_CF, + }; + + type Da = sov_mock_da::MockDaSpec; + type S = sov_state::DefaultStorageSpec; + + fn build_dbs( + state_path: &path::Path, + native_path: &path::Path, + ) -> (sov_schema_db::DB, sov_schema_db::DB) { + let state_tables = vec![DUMMY_STATE_CF.to_string()]; + let state_db = sov_schema_db::DB::open( + state_path, + "state_db", + state_tables, + &gen_rocksdb_options(&Default::default(), false), + ) + .unwrap(); + let native_tables = vec![DUMMY_NATIVE_CF.to_string()]; + let native_db = sov_schema_db::DB::open( + native_path, + "native_db", + native_tables, + &gen_rocksdb_options(&Default::default(), false), + ) + .unwrap(); + + (state_db, native_db) + } + + #[test] + fn initiate_new() { + let state_tmpdir = tempfile::tempdir().unwrap(); + let native_tmpdir = tempfile::tempdir().unwrap(); + + let (state_db, native_db) = build_dbs(state_tmpdir.path(), native_tmpdir.path()); + + let storage_manager = NewProverStorageManager::::new(state_db, native_db); + assert!(storage_manager.is_empty()); + } + + #[test] + fn get_new_storage() { + let state_tmpdir = tempfile::tempdir().unwrap(); + let native_tmpdir = tempfile::tempdir().unwrap(); + + let (state_db, native_db) = build_dbs(state_tmpdir.path(), native_tmpdir.path()); + + let mut storage_manager = NewProverStorageManager::::new(state_db, native_db); + assert!(storage_manager.is_empty()); + + let block_header = MockBlockHeader { + prev_hash: MockHash::from([1; 32]), + hash: MockHash::from([2; 32]), + height: 1, + }; + + let _storage = storage_manager + .get_native_storage_on(&block_header) + .unwrap(); + + assert!(!storage_manager.is_empty()); + assert!(!storage_manager.chain_forks.is_empty()); + assert!(!storage_manager.block_hash_to_snapshot_id.is_empty()); + assert!(storage_manager + .snapshot_id_to_parent + .read() + .unwrap() + .is_empty()); + assert!(storage_manager + .state_snapshot_manager + .read() + .unwrap() + .is_empty()); + assert!(storage_manager + .native_snapshot_manager + .read() + .unwrap() + .is_empty()); + } + + #[test] + fn try_get_new_storage_same_block() { + let state_tmpdir = tempfile::tempdir().unwrap(); + let native_tmpdir = tempfile::tempdir().unwrap(); + + let (state_db, native_db) = build_dbs(state_tmpdir.path(), native_tmpdir.path()); + + let mut storage_manager = NewProverStorageManager::::new(state_db, native_db); + assert!(storage_manager.is_empty()); + + let block_header = MockBlockHeader { + prev_hash: MockHash::from([1; 32]), + hash: MockHash::from([2; 32]), + height: 1, + }; + + let storage_1 = storage_manager + .get_native_storage_on(&block_header) + .unwrap(); + + let storage_2 = storage_manager + .get_native_storage_on(&block_header) + .unwrap(); + + // We just check, that both storage have same underlying id. + // This is more tight with implementation. + // More black box way to check would be: + // - have some data in db + // - have some parent snapshots + // - make sure that writing to each individual storage do not propagate to another + // - both storage have same view of the previous state, for example they don't look into siblings + let (state_db_1, native_db_1) = storage_1.freeze(); + let state_snapshot_1 = FrozenDbSnapshot::from(state_db_1); + let native_snapshot_1 = FrozenDbSnapshot::from(native_db_1); + let (state_db_2, native_db_2) = storage_2.freeze(); + let state_snapshot_2 = FrozenDbSnapshot::from(state_db_2); + let native_snapshot_2 = FrozenDbSnapshot::from(native_db_2); + + assert_eq!(state_snapshot_1.get_id(), state_snapshot_2.get_id()); + assert_eq!(native_snapshot_1.get_id(), native_snapshot_2.get_id()); + } + + #[test] + #[should_panic(expected = "Cannot provide storage for corrupt block")] + fn try_get_new_storage_corrupt_block() { + let state_tmpdir = tempfile::tempdir().unwrap(); + let native_tmpdir = tempfile::tempdir().unwrap(); + + let (state_db, native_db) = build_dbs(state_tmpdir.path(), native_tmpdir.path()); + + let mut storage_manager = NewProverStorageManager::::new(state_db, native_db); + assert!(storage_manager.is_empty()); + + let block_header = MockBlockHeader { + prev_hash: MockHash::from([1; 32]), + hash: MockHash::from([1; 32]), + height: 1, + }; + + let _storage_1 = storage_manager + .get_native_storage_on(&block_header) + .unwrap(); + } + + #[test] + #[ignore = "TBD"] + fn save_change_set() {} + + #[test] + fn try_save_unknown_changeset() { + let state_tmpdir_1 = tempfile::tempdir().unwrap(); + let native_tmpdir_1 = tempfile::tempdir().unwrap(); + + let state_tmpdir_2 = tempfile::tempdir().unwrap(); + let native_tmpdir_2 = tempfile::tempdir().unwrap(); + + let block_a = MockBlockHeader { + prev_hash: MockHash::from([1; 32]), + hash: MockHash::from([2; 32]), + height: 1, + }; + + let snapshot_1 = { + let (state_db, native_db) = build_dbs(state_tmpdir_1.path(), native_tmpdir_1.path()); + let mut storage_manager_temp = + NewProverStorageManager::::new(state_db, native_db); + storage_manager_temp + .get_native_storage_on(&block_a) + .unwrap() + }; + + let (state_db, native_db) = build_dbs(state_tmpdir_2.path(), native_tmpdir_2.path()); + let mut storage_manager = NewProverStorageManager::::new(state_db, native_db); + + let unknown_id = storage_manager.save_change_set(&block_a, snapshot_1); + assert!(unknown_id.is_err()); + assert!(unknown_id + .err() + .unwrap() + .to_string() + .starts_with("Attempt to save unknown snapshot with id=")); + + // TODO: Unknown block + + // TODO: Block / snapshot_id mismatch + } + + #[test] + fn lifecycle_simulation() { + let state_tmpdir = tempfile::tempdir().unwrap(); + let native_tmpdir = tempfile::tempdir().unwrap(); + + let (state_db, native_db) = build_dbs(state_tmpdir.path(), native_tmpdir.path()); + + // State DB has following values initially: + // x = 1 + // y = 2 + + let x = DummyField(1); + let y = DummyField(2); + let _z = DummyField(3); + + state_db + .put::(&x, &DummyField(1)) + .unwrap(); + state_db + .put::(&y, &DummyField(2)) + .unwrap(); + + // Native DB has following values initially + // 10 = 10 + // 20 = 20 + + native_db + .put::(&x, &DummyField(100)) + .unwrap(); + native_db + .put::(&y, &DummyField(200)) + .unwrap(); + + let mut storage_manager = NewProverStorageManager::::new(state_db, native_db); + assert!(storage_manager.is_empty()); + + // / -> D + // A -> B -> C -> D -> E + // | \ -> G -> H + // \ -> F -> K + // + + // Block A + let block_a = MockBlockHeader { + prev_hash: MockHash::from([0; 32]), + hash: MockHash::from([1; 32]), + height: 1, + }; + + let storage_a = storage_manager.get_native_storage_on(&block_a).unwrap(); + + let state_x_actual = storage_a.read_state(x.0).unwrap(); + assert_eq!(Some(1), state_x_actual); + + let native_x_actual = storage_a.read_native(x.0).unwrap(); + assert_eq!(Some(100), native_x_actual); + + storage_a.write_state(x.0, 2).unwrap(); + storage_a.write_native(x.0, 20).unwrap(); + + storage_manager + .save_change_set(&block_a, storage_a) + .unwrap(); + + // Block B + let block_b = MockBlockHeader { + prev_hash: MockHash::from([1; 32]), + hash: MockHash::from([2; 32]), + height: 1, + }; + + let storage_b = storage_manager.get_native_storage_on(&block_b).unwrap(); + + assert_eq!(Some(2), storage_b.read_state(x.0).unwrap()); + assert_eq!(Some(20), storage_b.read_native(x.0).unwrap()); + } +} diff --git a/full-node/sov-prover-storage-manager/src/snapshot_manager.rs b/full-node/sov-prover-storage-manager/src/snapshot_manager.rs new file mode 100644 index 000000000..04a87a903 --- /dev/null +++ b/full-node/sov-prover-storage-manager/src/snapshot_manager.rs @@ -0,0 +1,86 @@ +use std::collections::HashMap; +use std::sync::{Arc, RwLock}; + +use sov_schema_db::schema::{KeyCodec, ValueCodec}; +use sov_schema_db::snapshot::{FrozenDbSnapshot, QueryManager, SnapshotId}; +use sov_schema_db::{Operation, Schema}; + +/// Snapshot manager holds snapshots associated with particular DB and can traverse them backwards +/// down to DB level +/// Managed externally by [`NewProverStorageManager`] +pub struct SnapshotManager { + db: sov_schema_db::DB, + snapshots: HashMap, + /// Hierarchical + to_parent: Arc>>, +} + +impl SnapshotManager { + pub(crate) fn new( + db: sov_schema_db::DB, + to_parent: Arc>>, + ) -> Self { + Self { + db, + snapshots: HashMap::new(), + to_parent, + } + } + + pub(crate) fn add_snapshot(&mut self, snapshot: FrozenDbSnapshot) { + let snapshot_id = snapshot.get_id(); + if self.snapshots.insert(snapshot_id, snapshot).is_some() { + panic!("Attempt to double save same snapshot"); + } + } + + pub(crate) fn discard_snapshot(&mut self, snapshot_id: &SnapshotId) { + self.snapshots + .remove(snapshot_id) + .expect("Attempt to remove unknown snapshot"); + } + + pub(crate) fn commit_snapshot(&mut self, snapshot_id: &SnapshotId) -> anyhow::Result<()> { + if !self.snapshots.contains_key(snapshot_id) { + anyhow::bail!("Attempt to commit unknown snapshot"); + } + + let snapshot = self.snapshots.remove(snapshot_id).unwrap(); + self.db.write_schemas(snapshot.into()) + } + + #[cfg(test)] + pub(crate) fn is_empty(&self) -> bool { + self.snapshots.is_empty() + } +} + +impl QueryManager for SnapshotManager { + fn get( + &self, + mut snapshot_id: SnapshotId, + key: &impl KeyCodec, + ) -> anyhow::Result> { + while let Some(parent_snapshot_id) = self.to_parent.read().unwrap().get(&snapshot_id) { + let parent_snapshot = self + .snapshots + .get(parent_snapshot_id) + .expect("Inconsistent snapshots tree"); + + // Some operation has been found + if let Some(operation) = parent_snapshot.get(key)? { + return match operation { + Operation::Put { value } => Ok(Some(S::Value::decode_value(value)?)), + Operation::Delete => Ok(None), + }; + } + + snapshot_id = *parent_snapshot_id; + } + + self.db.get(key) + } +} + +#[cfg(test)] +mod tests {} diff --git a/rollup-interface/src/state_machine/da.rs b/rollup-interface/src/state_machine/da.rs index 6afd6dc20..b915ed14c 100644 --- a/rollup-interface/src/state_machine/da.rs +++ b/rollup-interface/src/state_machine/da.rs @@ -179,6 +179,8 @@ pub trait BlobReaderTrait: Serialize + DeserializeOwned + Send + Sync + 'static /// Trait with collection of trait bounds for a block hash. pub trait BlockHashTrait: + // TODO: Can we replace `Into<[u8; 32]>` with std::hash::Hash + // so it is compatible with StorageManager implementation? Serialize + DeserializeOwned + PartialEq + Debug + Send + Sync + Clone + Eq + Into<[u8; 32]> { } diff --git a/rollup-interface/src/state_machine/storage.rs b/rollup-interface/src/state_machine/storage.rs index 35ecea4e3..110eb4f2c 100644 --- a/rollup-interface/src/state_machine/storage.rs +++ b/rollup-interface/src/state_machine/storage.rs @@ -1,6 +1,8 @@ //! Trait that represents life time of the state //! +use crate::da::DaSpec; + /// Storage manager persistence and allows to work on state /// Temporal placeholder for ForkManager pub trait StorageManager { @@ -12,3 +14,30 @@ pub trait StorageManager { /// Get latest native state fn get_native_storage(&self) -> Self::NativeStorage; } + +/// Storage manager, that supports tree-like hierarchy of snapshots +/// So different rollup state can be mapped to DA state 1 to 1, including chain forks. +pub trait HierarchicalStorageManager { + /// Type that can be consumed by `[crate::state_machine::stf::StateTransitionFunction]` in native context. + type NativeStorage; + /// Type that is produced by `[crate::state_machine::stf::StateTransitionFunction]`. + type NativeChangeSet; + + /// Creates storage based on given Da block header, + /// meaning that at will have access to previous blocks state in same fork. + fn get_native_storage_on( + &mut self, + block_header: &Da::BlockHeader, + ) -> anyhow::Result; + + /// Adds [`Self::ChangeSet`] to the storage. + /// [`Da::BlockHeader`] must be provided for efficient consistency checking. + fn save_change_set( + &mut self, + block_header: &Da::BlockHeader, + change_set: Self::NativeChangeSet, + ) -> anyhow::Result<()>; + + /// Finalizes snapshot on given block header + fn finalize(&mut self, block_header: &Da::BlockHeader) -> anyhow::Result<()>; +} From f971a688d78c86a299d02c3a55be9457276084ec Mon Sep 17 00:00:00 2001 From: Nikolai Golub Date: Mon, 20 Nov 2023 19:50:19 +0100 Subject: [PATCH 02/13] [no ci] Adding more tests for QueryManager --- full-node/db/sov-schema-db/src/snapshot.rs | 19 +++ .../src/snapshot_manager.rs | 153 +++++++++++++++++- 2 files changed, 170 insertions(+), 2 deletions(-) diff --git a/full-node/db/sov-schema-db/src/snapshot.rs b/full-node/db/sov-schema-db/src/snapshot.rs index 2d9479894..36ab80d4d 100644 --- a/full-node/db/sov-schema-db/src/snapshot.rs +++ b/full-node/db/sov-schema-db/src/snapshot.rs @@ -35,6 +35,12 @@ impl ReadOnlyLock { } } +impl From>> for ReadOnlyLock { + fn from(value: Arc>) -> Self { + Self::new(value) + } +} + /// Wrapper around [`QueryManager`] that allows to read from snapshots pub struct DbSnapshot { id: SnapshotId, @@ -144,3 +150,16 @@ fn decode_operation(operation: &Operation) -> anyhow::Result Ok(None), } } + +/// QueryManager, which never returns any values +pub struct NoopQueryManager; + +impl QueryManager for NoopQueryManager { + fn get( + &self, + _snapshot_id: SnapshotId, + _key: &impl KeyCodec, + ) -> anyhow::Result> { + Ok(None) + } +} diff --git a/full-node/sov-prover-storage-manager/src/snapshot_manager.rs b/full-node/sov-prover-storage-manager/src/snapshot_manager.rs index 04a87a903..56cfcd555 100644 --- a/full-node/sov-prover-storage-manager/src/snapshot_manager.rs +++ b/full-node/sov-prover-storage-manager/src/snapshot_manager.rs @@ -37,7 +37,7 @@ impl SnapshotManager { pub(crate) fn discard_snapshot(&mut self, snapshot_id: &SnapshotId) { self.snapshots .remove(snapshot_id) - .expect("Attempt to remove unknown snapshot"); + .expect("Attempt to discard unknown snapshot"); } pub(crate) fn commit_snapshot(&mut self, snapshot_id: &SnapshotId) -> anyhow::Result<()> { @@ -83,4 +83,153 @@ impl QueryManager for SnapshotManager { } #[cfg(test)] -mod tests {} +mod tests { + use std::collections::HashMap; + use std::sync::{Arc, RwLock}; + + use sov_db::rocks_db_config::gen_rocksdb_options; + use sov_schema_db::snapshot::{DbSnapshot, NoopQueryManager}; + + use crate::dummy_storage::DUMMY_STATE_CF; + use crate::snapshot_manager::SnapshotManager; + + fn create_test_db(path: &std::path::Path) -> sov_schema_db::DB { + let tables = vec![DUMMY_STATE_CF.to_string()]; + let db = sov_schema_db::DB::open( + path, + "test_db", + tables, + &gen_rocksdb_options(&Default::default(), false), + ) + .unwrap(); + db + } + + #[test] + fn test_empty() { + let tempdir = tempfile::tempdir().unwrap(); + let db = create_test_db(tempdir.path()); + let snapshot_manager = SnapshotManager::new(db, Arc::new(RwLock::new(HashMap::new()))); + assert!(snapshot_manager.is_empty()); + } + + #[test] + fn test_add_and_discard_snapshot() { + let tempdir = tempfile::tempdir().unwrap(); + let db = create_test_db(tempdir.path()); + let to_parent = Arc::new(RwLock::new(HashMap::new())); + let mut snapshot_manager = SnapshotManager::new(db, to_parent.clone()); + let query_manager = Arc::new(RwLock::new(NoopQueryManager)); + + let snapshot_id = 1; + let db_snapshot = DbSnapshot::new(snapshot_id, query_manager.clone().into()); + + snapshot_manager.add_snapshot(db_snapshot.into()); + assert!(!snapshot_manager.is_empty()); + snapshot_manager.discard_snapshot(&snapshot_id); + assert!(snapshot_manager.is_empty()); + } + + #[test] + #[should_panic(expected = "Attempt to double save same snapshot")] + fn test_add_twice() { + let tempdir = tempfile::tempdir().unwrap(); + let db = create_test_db(tempdir.path()); + let to_parent = Arc::new(RwLock::new(HashMap::new())); + let mut snapshot_manager = SnapshotManager::new(db, to_parent.clone()); + let query_manager = Arc::new(RwLock::new(NoopQueryManager)); + + let snapshot_id = 1; + // Both share the same ID + let db_snapshot_1 = DbSnapshot::new(snapshot_id, query_manager.clone().into()); + let db_snapshot_2 = DbSnapshot::new(snapshot_id, query_manager.clone().into()); + + snapshot_manager.add_snapshot(db_snapshot_1.into()); + assert!(!snapshot_manager.is_empty()); + snapshot_manager.add_snapshot(db_snapshot_2.into()); + } + + #[test] + #[should_panic(expected = "Attempt to commit unknown snapshot")] + fn test_commit_unknown() { + let tempdir = tempfile::tempdir().unwrap(); + let db = create_test_db(tempdir.path()); + let to_parent = Arc::new(RwLock::new(HashMap::new())); + let mut snapshot_manager = SnapshotManager::new(db, to_parent.clone()); + + snapshot_manager.commit_snapshot(&1).unwrap(); + } + + #[test] + #[should_panic(expected = "Attempt to discard unknown snapshot")] + fn test_discard_unknown() { + let tempdir = tempfile::tempdir().unwrap(); + let db = create_test_db(tempdir.path()); + let to_parent = Arc::new(RwLock::new(HashMap::new())); + let mut snapshot_manager = SnapshotManager::new(db, to_parent.clone()); + + snapshot_manager.discard_snapshot(&1); + } + + #[test] + fn test_commit_snapshot() { + let tempdir = tempfile::tempdir().unwrap(); + let db = create_test_db(tempdir.path()); + let to_parent = Arc::new(RwLock::new(HashMap::new())); + let mut snapshot_manager = SnapshotManager::new(db, to_parent.clone()); + let query_manager = Arc::new(RwLock::new(NoopQueryManager)); + + let snapshot_id = 1; + let db_snapshot = DbSnapshot::new(snapshot_id, query_manager.clone().into()); + + snapshot_manager.add_snapshot(db_snapshot.into()); + let result = snapshot_manager.commit_snapshot(&snapshot_id); + assert!(result.is_ok()); + assert!(snapshot_manager.is_empty()); + } + + #[test] + #[ignore = "TBD"] + fn test_query_lifecycle() { + let tempdir = tempfile::tempdir().unwrap(); + let db = create_test_db(tempdir.path()); + let to_parent = Arc::new(RwLock::new(HashMap::new())); + { + // / -> 6 -> 7 + // DB -> 1 -> 2 -> 3 + // \ -> 4 -> 5 + let mut edit = to_parent.write().unwrap(); + edit.insert(3, 2); + edit.insert(2, 1); + edit.insert(4, 1); + edit.insert(5, 4); + edit.insert(6, 2); + edit.insert(7, 6); + } + let _snapshot_manager = SnapshotManager::new(db, to_parent.clone()); + let _query_manager = Arc::new(RwLock::new(NoopQueryManager)); + + // Operations: + // | snapshot_id | key | operation | + // | DB | 1 | write(1) | + // | 1 | 1 | write(2) | + // | 1 | 2 | write(3) | + // | 2 | 1 | delete | + // | 2 | 2 | write(4) | + // | 4 | 1 | write(5) | + // | 4 | 3 | write(6) | + // | 6 | 1 | write(7) | + + // View: + // | from s_id | key | value | + // | 3 | 1 | None | + // | 3 | 2 | 3 | + // | 3 | 3 | None | + // | 5 | 1 | 5 | + // | 5 | 2 | 4 | + // | 5 | 3 | 6 | + // | 7 | 1 | 7 | + // | 7 | 2 | 4 | + // | 7 | 3 | None | + } +} From fb3748d179563d17163dc3dff4dd2b1f1160da70 Mon Sep 17 00:00:00 2001 From: Nikolai Golub Date: Mon, 20 Nov 2023 23:47:43 +0100 Subject: [PATCH 03/13] [no ci] Tests for SnapshotManager --- full-node/db/sov-schema-db/src/snapshot.rs | 3 +- .../src/snapshot_manager.rs | 166 ++++++++++++++++-- 2 files changed, 150 insertions(+), 19 deletions(-) diff --git a/full-node/db/sov-schema-db/src/snapshot.rs b/full-node/db/sov-schema-db/src/snapshot.rs index 36ab80d4d..91d32602d 100644 --- a/full-node/db/sov-schema-db/src/snapshot.rs +++ b/full-node/db/sov-schema-db/src/snapshot.rs @@ -10,7 +10,8 @@ pub type SnapshotId = u64; /// A trait to make nested calls to several [`SchemaBatch`]s and eventually [`crate::DB`] pub trait QueryManager { - /// Get a value from snapshot or its parents + /// Get a value from parents of given [`SnapshotId`] + /// In case of unknown [`SnapshotId`] return `Ok(None)` fn get( &self, snapshot_id: SnapshotId, diff --git a/full-node/sov-prover-storage-manager/src/snapshot_manager.rs b/full-node/sov-prover-storage-manager/src/snapshot_manager.rs index 56cfcd555..7fdfd71cd 100644 --- a/full-node/sov-prover-storage-manager/src/snapshot_manager.rs +++ b/full-node/sov-prover-storage-manager/src/snapshot_manager.rs @@ -65,7 +65,7 @@ impl QueryManager for SnapshotManager { let parent_snapshot = self .snapshots .get(parent_snapshot_id) - .expect("Inconsistent snapshots tree"); + .expect("Inconsistency between snapshots and to_parent"); // Some operation has been found if let Some(operation) = parent_snapshot.get(key)? { @@ -88,11 +88,14 @@ mod tests { use std::sync::{Arc, RwLock}; use sov_db::rocks_db_config::gen_rocksdb_options; - use sov_schema_db::snapshot::{DbSnapshot, NoopQueryManager}; + use sov_schema_db::snapshot::{DbSnapshot, NoopQueryManager, QueryManager}; + use sov_schema_db::SchemaBatch; - use crate::dummy_storage::DUMMY_STATE_CF; + use crate::dummy_storage::{DummyField, DummyStateSchema, DUMMY_STATE_CF}; use crate::snapshot_manager::SnapshotManager; + type Schema = DummyStateSchema; + fn create_test_db(path: &std::path::Path) -> sov_schema_db::DB { let tables = vec![DUMMY_STATE_CF.to_string()]; let db = sov_schema_db::DB::open( @@ -189,7 +192,51 @@ mod tests { } #[test] - #[ignore = "TBD"] + fn test_query_unknown_snapshot_id() { + let tempdir = tempfile::tempdir().unwrap(); + let db = create_test_db(tempdir.path()); + let to_parent = Arc::new(RwLock::new(HashMap::new())); + let snapshot_manager = SnapshotManager::new(db, to_parent.clone()); + assert_eq!( + None, + snapshot_manager.get::(1, &DummyField(1)).unwrap() + ); + } + + #[test] + fn test_query_genesis_snapshot() { + let tempdir = tempfile::tempdir().unwrap(); + let db = create_test_db(tempdir.path()); + let to_parent = Arc::new(RwLock::new(HashMap::new())); + + let one = DummyField(1); + let two = DummyField(2); + let three = DummyField(3); + + let mut db_data = SchemaBatch::new(); + db_data.put::(&one, &one).unwrap(); + db_data.put::(&three, &three).unwrap(); + db.write_schemas(db_data).unwrap(); + + let mut snapshot_manager = SnapshotManager::new(db, to_parent.clone()); + let query_manager = Arc::new(RwLock::new(NoopQueryManager)); + + let db_snapshot = DbSnapshot::new(1, query_manager.clone().into()); + db_snapshot.put::(&two, &two).unwrap(); + db_snapshot.delete::(&three).unwrap(); + + snapshot_manager.add_snapshot(db_snapshot.into()); + + // Effectively querying database: + assert_eq!(Some(one), snapshot_manager.get::(1, &one).unwrap()); + assert_eq!(None, snapshot_manager.get::(1, &two).unwrap()); + assert_eq!( + Some(three), + snapshot_manager.get::(1, &three).unwrap() + ); + } + + #[test] fn test_query_lifecycle() { let tempdir = tempfile::tempdir().unwrap(); let db = create_test_db(tempdir.path()); @@ -206,30 +253,113 @@ mod tests { edit.insert(6, 2); edit.insert(7, 6); } - let _snapshot_manager = SnapshotManager::new(db, to_parent.clone()); - let _query_manager = Arc::new(RwLock::new(NoopQueryManager)); + + let one = DummyField(1); + let two = DummyField(2); + let three = DummyField(3); + let four = DummyField(4); + let five = DummyField(5); + let six = DummyField(6); + let seven = DummyField(7); + let eight = DummyField(8); + + let mut db_data = SchemaBatch::new(); + db_data.put::(&one, &one).unwrap(); + db.write_schemas(db_data).unwrap(); + + let mut snapshot_manager = SnapshotManager::new(db, to_parent.clone()); + let query_manager = Arc::new(RwLock::new(NoopQueryManager)); // Operations: // | snapshot_id | key | operation | // | DB | 1 | write(1) | - // | 1 | 1 | write(2) | - // | 1 | 2 | write(3) | - // | 2 | 1 | delete | - // | 2 | 2 | write(4) | - // | 4 | 1 | write(5) | + // | 1 | 2 | write(2) | + // | 1 | 3 | write(4) | + // | 2 | 1 | write(5) | + // | 2 | 2 | delete | // | 4 | 3 | write(6) | // | 6 | 1 | write(7) | + // | 6 | 2 | write(8) | + + // 1 + let db_snapshot = DbSnapshot::new(1, query_manager.clone().into()); + db_snapshot.put::(&two, &two).unwrap(); + db_snapshot.put::(&three, &four).unwrap(); + snapshot_manager.add_snapshot(db_snapshot.into()); + + // 2 + let db_snapshot = DbSnapshot::new(2, query_manager.clone().into()); + db_snapshot.put::(&one, &five).unwrap(); + db_snapshot.delete::(&two).unwrap(); + snapshot_manager.add_snapshot(db_snapshot.into()); + + // 3 + let db_snapshot = DbSnapshot::new(3, query_manager.clone().into()); + snapshot_manager.add_snapshot(db_snapshot.into()); + + // 4 + let db_snapshot = DbSnapshot::new(4, query_manager.clone().into()); + db_snapshot.put::(&three, &six).unwrap(); + snapshot_manager.add_snapshot(db_snapshot.into()); + + // 5 + let db_snapshot = DbSnapshot::new(5, query_manager.clone().into()); + snapshot_manager.add_snapshot(db_snapshot.into()); + + // 6 + let db_snapshot = DbSnapshot::new(6, query_manager.clone().into()); + db_snapshot.put::(&one, &seven).unwrap(); + db_snapshot.put::(&two, &eight).unwrap(); + snapshot_manager.add_snapshot(db_snapshot.into()); + + // 7 + let db_snapshot = DbSnapshot::new(7, query_manager.clone().into()); + snapshot_manager.add_snapshot(db_snapshot.into()); // View: // | from s_id | key | value | - // | 3 | 1 | None | - // | 3 | 2 | 3 | - // | 3 | 3 | None | - // | 5 | 1 | 5 | - // | 5 | 2 | 4 | + // | 3 | 1 | 5 | + // | 3 | 2 | None | + // | 3 | 3 | 4 | + // | 5 | 1 | 1 | + // | 5 | 2 | 2 | // | 5 | 3 | 6 | // | 7 | 1 | 7 | - // | 7 | 2 | 4 | - // | 7 | 3 | None | + // | 7 | 2 | 8 | + // | 7 | 3 | 4 | + assert_eq!( + Some(five.clone()), + snapshot_manager.get::(3, &one).unwrap() + ); + assert_eq!(None, snapshot_manager.get::(3, &two).unwrap()); + assert_eq!( + Some(four.clone()), + snapshot_manager.get::(3, &three).unwrap() + ); + assert_eq!( + Some(one.clone()), + snapshot_manager.get::(5, &one).unwrap() + ); + assert_eq!( + Some(two.clone()), + snapshot_manager.get::(5, &two).unwrap() + ); + assert_eq!( + Some(six.clone()), + snapshot_manager.get::(5, &three).unwrap() + ); + + assert_eq!( + Some(seven), + snapshot_manager.get::(7, &one).unwrap() + ); + assert_eq!( + Some(eight), + snapshot_manager.get::(7, &two).unwrap() + ); + assert_eq!( + Some(four), + snapshot_manager.get::(7, &three).unwrap() + ); } } From 83f4e3721f147dd8952859162da91afafd533bc3 Mon Sep 17 00:00:00 2001 From: Nikolai Golub Date: Tue, 21 Nov 2023 17:14:00 +0100 Subject: [PATCH 04/13] [no ci] more bigger tests! --- .../src/dummy_storage.rs | 19 +- .../sov-prover-storage-manager/src/lib.rs | 389 +++++++++++++++--- 2 files changed, 340 insertions(+), 68 deletions(-) diff --git a/full-node/sov-prover-storage-manager/src/dummy_storage.rs b/full-node/sov-prover-storage-manager/src/dummy_storage.rs index 08ce22839..b9801c5a4 100644 --- a/full-node/sov-prover-storage-manager/src/dummy_storage.rs +++ b/full-node/sov-prover-storage-manager/src/dummy_storage.rs @@ -34,7 +34,7 @@ impl NewProverStorage { (state_db, native_db) } - #[allow(dead_code)] + #[cfg(test)] pub(crate) fn read_state(&self, key: u64) -> anyhow::Result> { let key = DummyField(key); Ok(self @@ -43,14 +43,20 @@ impl NewProverStorage { .map(Into::into)) } - #[allow(dead_code)] + #[cfg(test)] pub(crate) fn write_state(&self, key: u64, value: u64) -> anyhow::Result<()> { let key = DummyField(key); let value = DummyField(value); self.state_db.put::(&key, &value) } - #[allow(dead_code)] + #[cfg(test)] + pub(crate) fn delete_state(&self, key: u64) -> anyhow::Result<()> { + let key = DummyField(key); + self.state_db.delete::(&key) + } + + #[cfg(test)] pub(crate) fn read_native(&self, key: u64) -> anyhow::Result> { let key = DummyField(key); Ok(self @@ -59,12 +65,17 @@ impl NewProverStorage { .map(Into::into)) } - #[allow(dead_code)] + #[cfg(test)] pub(crate) fn write_native(&self, key: u64, value: u64) -> anyhow::Result<()> { let key = DummyField(key); let value = DummyField(value); self.native_db.put::(&key, &value) } + + pub(crate) fn delete_native(&self, key: u64) -> anyhow::Result<()> { + let key = DummyField(key); + self.native_db.delete::(&key) + } } // -------------- diff --git a/full-node/sov-prover-storage-manager/src/lib.rs b/full-node/sov-prover-storage-manager/src/lib.rs index a1ff75db9..82ecfa58b 100644 --- a/full-node/sov-prover-storage-manager/src/lib.rs +++ b/full-node/sov-prover-storage-manager/src/lib.rs @@ -116,11 +116,6 @@ where new_snapshot_id } }; - println!( - "BLOCK HEIGHT={} SNAP_ID={}", - block_header.height(), - new_snapshot_id - ); let state_db_snapshot = DbSnapshot::new( new_snapshot_id, @@ -144,7 +139,10 @@ where change_set: Self::NativeChangeSet, ) -> anyhow::Result<()> { if !self.chain_forks.contains_key(&block_header.prev_hash()) { - anyhow::bail!("Attempt to save changeset for unknown block header"); + anyhow::bail!( + "Attempt to save changeset for unknown block header {:?}", + block_header + ); } let (state_db, native_db) = change_set.freeze(); let state_snapshot: FrozenDbSnapshot = state_db.into(); @@ -159,7 +157,6 @@ where } // Obviously alien - println!("L={} S={}", self.latest_snapshot_id, snapshot_id); if snapshot_id > self.latest_snapshot_id { anyhow::bail!("Attempt to save unknown snapshot with id={}", snapshot_id); } @@ -342,11 +339,6 @@ mod tests { // We just check, that both storage have same underlying id. // This is more tight with implementation. - // More black box way to check would be: - // - have some data in db - // - have some parent snapshots - // - make sure that writing to each individual storage do not propagate to another - // - both storage have same view of the previous state, for example they don't look into siblings let (state_db_1, native_db_1) = storage_1.freeze(); let state_snapshot_1 = FrozenDbSnapshot::from(state_db_1); let native_snapshot_1 = FrozenDbSnapshot::from(native_db_1); @@ -356,6 +348,13 @@ mod tests { assert_eq!(state_snapshot_1.get_id(), state_snapshot_2.get_id()); assert_eq!(native_snapshot_1.get_id(), native_snapshot_2.get_id()); + + // TODO: Do more checks + // More black box way to check would be: + // - have some data in db + // - have some parent snapshots + // - make sure that writing to each individual storage do not propagate to another + // - both storage have same view of the previous state, for example they don't look into siblings } #[test] @@ -375,17 +374,43 @@ mod tests { height: 1, }; - let _storage_1 = storage_manager + storage_manager .get_native_storage_on(&block_header) .unwrap(); } #[test] - #[ignore = "TBD"] - fn save_change_set() {} + fn save_change_set() { + let state_tmpdir = tempfile::tempdir().unwrap(); + let native_tmpdir = tempfile::tempdir().unwrap(); + + let (state_db, native_db) = build_dbs(state_tmpdir.path(), native_tmpdir.path()); + + let mut storage_manager = NewProverStorageManager::::new(state_db, native_db); + assert!(storage_manager.is_empty()); + + let block_header = MockBlockHeader { + prev_hash: MockHash::from([1; 32]), + hash: MockHash::from([2; 32]), + height: 1, + }; + + assert!(storage_manager.is_empty()); + let storage = storage_manager + .get_native_storage_on(&block_header) + .unwrap(); + assert!(!storage_manager.is_empty()); + + // We can save empty storage as well + storage_manager + .save_change_set(&block_header, storage) + .unwrap(); + + assert!(!storage_manager.is_empty()); + } #[test] - fn try_save_unknown_changeset() { + fn try_save_unknown_block_header() { let state_tmpdir_1 = tempfile::tempdir().unwrap(); let native_tmpdir_1 = tempfile::tempdir().unwrap(); @@ -410,17 +435,84 @@ mod tests { let (state_db, native_db) = build_dbs(state_tmpdir_2.path(), native_tmpdir_2.path()); let mut storage_manager = NewProverStorageManager::::new(state_db, native_db); - let unknown_id = storage_manager.save_change_set(&block_a, snapshot_1); - assert!(unknown_id.is_err()); - assert!(unknown_id - .err() - .unwrap() - .to_string() - .starts_with("Attempt to save unknown snapshot with id=")); + let result = storage_manager.save_change_set(&block_a, snapshot_1); + assert!(result.is_err()); + let expected_error_msg = format!( + "Attempt to save changeset for unknown block header {:?}", + &block_a + ); + assert_eq!(expected_error_msg, result.err().unwrap().to_string()); + } - // TODO: Unknown block + #[test] + fn try_save_unknown_snapshot() { + // This test we create 2 snapshot managers and try to save snapshots from first manager + // in another + // First it checks for yet unknown id 2. It is larger that last known snapshot 1. + // Then we commit own snapshot 1, and then try to save alien snapshot with id 1 + let state_tmpdir_1 = tempfile::tempdir().unwrap(); + let native_tmpdir_1 = tempfile::tempdir().unwrap(); + + let state_tmpdir_2 = tempfile::tempdir().unwrap(); + let native_tmpdir_2 = tempfile::tempdir().unwrap(); + + let block_a = MockBlockHeader { + prev_hash: MockHash::from([0; 32]), + hash: MockHash::from([1; 32]), + height: 1, + }; - // TODO: Block / snapshot_id mismatch + let block_b = MockBlockHeader { + prev_hash: MockHash::from([2; 32]), + hash: MockHash::from([3; 32]), + height: 2, + }; + + let (snapshot_alien_1, snapshot_alien_2) = { + let (state_db, native_db) = build_dbs(state_tmpdir_1.path(), native_tmpdir_1.path()); + let mut storage_manager_temp = + NewProverStorageManager::::new(state_db, native_db); + // ID = 1 + let snapshot_a = storage_manager_temp + .get_native_storage_on(&block_a) + .unwrap(); + // ID = 2 + let snapshot_b = storage_manager_temp + .get_native_storage_on(&block_b) + .unwrap(); + (snapshot_a, snapshot_b) + }; + + let (state_db, native_db) = build_dbs(state_tmpdir_2.path(), native_tmpdir_2.path()); + let mut storage_manager = NewProverStorageManager::::new(state_db, native_db); + + let snapshot_own_a = storage_manager.get_native_storage_on(&block_a).unwrap(); + let _snapshot_own_b = storage_manager.get_native_storage_on(&block_b).unwrap(); + + let result = storage_manager.save_change_set(&block_a, snapshot_alien_2); + assert!(result.is_err()); + let err_msg = result.err().unwrap().to_string(); + assert_eq!("Attempt to save unknown snapshot with id=2", err_msg); + + storage_manager + .save_change_set(&block_a, snapshot_own_a) + .unwrap(); + + storage_manager.finalize(&block_a).unwrap(); + + let result = storage_manager.save_change_set(&block_b, snapshot_alien_1); + assert!(result.is_err()); + let err_msg = result.err().unwrap().to_string(); + assert_eq!("Attempt to save unknown snapshot with id=1", err_msg); + } + + #[test] + #[ignore = "TBD"] + fn read_state_before_parent_is_added() { + // Blocks A -> B + // create snapshot A from block A + // create snapshot B from block B + // query data from block B, before adding snapshot A back to the manager! } #[test] @@ -431,72 +523,241 @@ mod tests { let (state_db, native_db) = build_dbs(state_tmpdir.path(), native_tmpdir.path()); // State DB has following values initially: - // x = 1 - // y = 2 + // 1 = 1 + // 2 = 2 + let one = DummyField(1); + let two = DummyField(2); - let x = DummyField(1); - let y = DummyField(2); - let _z = DummyField(3); - - state_db - .put::(&x, &DummyField(1)) - .unwrap(); - state_db - .put::(&y, &DummyField(2)) - .unwrap(); + state_db.put::(&one, &one).unwrap(); + state_db.put::(&two, &two).unwrap(); // Native DB has following values initially - // 10 = 10 - // 20 = 20 + // 1 = 100 + // 2 = 200 native_db - .put::(&x, &DummyField(100)) + .put::(&one, &DummyField(100)) .unwrap(); native_db - .put::(&y, &DummyField(200)) + .put::(&two, &DummyField(200)) .unwrap(); let mut storage_manager = NewProverStorageManager::::new(state_db, native_db); assert!(storage_manager.is_empty()); - // / -> D + // Chains: + // 1 2 3 4 5 + // / -> L -> M // A -> B -> C -> D -> E // | \ -> G -> H // \ -> F -> K - // + // M, E, H, K: Observability snapshots. - // Block A let block_a = MockBlockHeader { prev_hash: MockHash::from([0; 32]), hash: MockHash::from([1; 32]), height: 1, }; - - let storage_a = storage_manager.get_native_storage_on(&block_a).unwrap(); - - let state_x_actual = storage_a.read_state(x.0).unwrap(); - assert_eq!(Some(1), state_x_actual); - - let native_x_actual = storage_a.read_native(x.0).unwrap(); - assert_eq!(Some(100), native_x_actual); - - storage_a.write_state(x.0, 2).unwrap(); - storage_a.write_native(x.0, 20).unwrap(); - - storage_manager - .save_change_set(&block_a, storage_a) - .unwrap(); - - // Block B let block_b = MockBlockHeader { prev_hash: MockHash::from([1; 32]), hash: MockHash::from([2; 32]), - height: 1, + height: 2, + }; + let block_c = MockBlockHeader { + prev_hash: MockHash::from([2; 32]), + hash: MockHash::from([3; 32]), + height: 3, + }; + let block_d = MockBlockHeader { + prev_hash: MockHash::from([3; 32]), + hash: MockHash::from([4; 32]), + height: 4, + }; + let block_e = MockBlockHeader { + prev_hash: MockHash::from([4; 32]), + hash: MockHash::from([5; 32]), + height: 5, + }; + let block_f = MockBlockHeader { + prev_hash: MockHash::from([1; 32]), + hash: MockHash::from([32; 32]), + height: 2, + }; + let block_g = MockBlockHeader { + prev_hash: MockHash::from([2; 32]), + hash: MockHash::from([23; 32]), + height: 3, + }; + let block_h = MockBlockHeader { + prev_hash: MockHash::from([23; 32]), + hash: MockHash::from([24; 32]), + height: 4, + }; + let block_k = MockBlockHeader { + prev_hash: MockHash::from([32; 32]), + hash: MockHash::from([33; 32]), + height: 3, + }; + let block_l = MockBlockHeader { + prev_hash: MockHash::from([2; 32]), + hash: MockHash::from([13; 32]), + height: 3, + }; + let block_m = MockBlockHeader { + prev_hash: MockHash::from([13; 32]), + hash: MockHash::from([14; 32]), + height: 4, }; + // Data + // | Block | DB | Key | Operation | + // | A | state | 1 | write(3) | + // | A | state | 3 | write(4) | + // | A | native | 3 | write(400) | + // | B | state | 3 | write(4) | + // | B | native | 3 | write(500) | + // | C | state | 1 | delete | + // | C | state | 4 | write(5) | + // | C | native | 1 | write(600) | + // | D | state | 3 | write(6) | + // | F | state | 1 | write(7) | + // | F | native | 3 | write(700) | + // | F | state | 3 | delete | + // | F | native | 1 | delete | + // | G | state | 1 | write(8) | + // | G | native | 2 | write(9) | + // | L | state | 1 | write(10) | + + // A + let storage_a = storage_manager.get_native_storage_on(&block_a).unwrap(); + storage_a.write_state(1, 3).unwrap(); + storage_a.write_state(3, 4).unwrap(); + storage_a.write_native(3, 400).unwrap(); + storage_manager + .save_change_set(&block_a, storage_a) + .unwrap(); + // B let storage_b = storage_manager.get_native_storage_on(&block_b).unwrap(); + storage_b.write_state(3, 4).unwrap(); + storage_b.write_native(3, 500).unwrap(); + storage_manager + .save_change_set(&block_b, storage_b) + .unwrap(); + // C + let storage_c = storage_manager.get_native_storage_on(&block_c).unwrap(); + storage_c.delete_state(1).unwrap(); + storage_c.write_state(4, 5).unwrap(); + storage_c.write_native(1, 600).unwrap(); + storage_manager + .save_change_set(&block_c, storage_c) + .unwrap(); + // D + let storage_d = storage_manager.get_native_storage_on(&block_d).unwrap(); + storage_d.write_state(3, 6).unwrap(); + storage_manager + .save_change_set(&block_d, storage_d) + .unwrap(); + // F + let storage_f = storage_manager.get_native_storage_on(&block_f).unwrap(); + storage_f.write_state(1, 7).unwrap(); + storage_f.write_native(3, 700).unwrap(); + storage_f.delete_state(3).unwrap(); + storage_f.delete_native(1).unwrap(); + storage_manager + .save_change_set(&block_f, storage_f) + .unwrap(); + // G + let storage_g = storage_manager.get_native_storage_on(&block_g).unwrap(); + storage_g.write_state(1, 8).unwrap(); + storage_g.write_native(2, 9).unwrap(); + storage_manager + .save_change_set(&block_g, storage_g) + .unwrap(); + // L + let storage_l = storage_manager.get_native_storage_on(&block_l).unwrap(); + storage_l.write_state(1, 10).unwrap(); + storage_manager + .save_change_set(&block_l, storage_l) + .unwrap(); - assert_eq!(Some(2), storage_b.read_state(x.0).unwrap()); - assert_eq!(Some(20), storage_b.read_native(x.0).unwrap()); + // VIEW: Before finalization of A + // | snapshot | DB | Key | Value | + // | E | state | 1 | None | + // | E | state | 2 | 2 | + // | E | state | 3 | 6 | + // | E | state | 4 | 5 | + // | E | native | 1 | 600 | + // | E | native | 2 | 200 | + // | E | native | 3 | 500 | + // | E | native | 4 | None | + // | M | state | 1 | 10 | + // | M | state | 2 | 2 | + // | M | state | 3 | 4 | + // | M | state | 4 | None | + // | M | native | 1 | 100 | + // | M | native | 2 | 200 | + // | M | native | 3 | 500 | + // | M | native | 4 | None | + // | H | state | 1 | 8 | + // | H | state | 2 | 2 | + // | H | state | 3 | 4 | + // | H | state | 4 | None | + // | H | native | 1 | 100 | + // | H | native | 2 | 9 | + // | H | native | 3 | 500 | + // | H | native | 4 | None | + // | K | state | 1 | 7 | + // | K | state | 2 | 2 | + // | K | state | 3 | None | + // | K | state | 4 | None | + // | K | native | 1 | None | + // | K | native | 2 | 200 | + // | K | native | 3 | 700 | + // | K | native | 4 | None | + + let storage_e = storage_manager.get_native_storage_on(&block_e).unwrap(); + assert_eq!(None, storage_e.read_state(1).unwrap()); + assert_eq!(Some(2), storage_e.read_state(2).unwrap()); + assert_eq!(Some(6), storage_e.read_state(3).unwrap()); + assert_eq!(Some(5), storage_e.read_state(4).unwrap()); + assert_eq!(Some(600), storage_e.read_native(1).unwrap()); + assert_eq!(Some(200), storage_e.read_native(2).unwrap()); + assert_eq!(Some(500), storage_e.read_native(3).unwrap()); + assert_eq!(None, storage_e.read_native(4).unwrap()); + + let storage_m = storage_manager.get_native_storage_on(&block_m).unwrap(); + assert_eq!(Some(10), storage_m.read_state(1).unwrap()); + assert_eq!(Some(2), storage_m.read_state(2).unwrap()); + assert_eq!(Some(4), storage_m.read_state(3).unwrap()); + assert_eq!(None, storage_m.read_state(4).unwrap()); + assert_eq!(Some(100), storage_m.read_native(1).unwrap()); + assert_eq!(Some(200), storage_m.read_native(2).unwrap()); + assert_eq!(Some(500), storage_m.read_native(3).unwrap()); + assert_eq!(None, storage_m.read_native(4).unwrap()); + + let storage_h = storage_manager.get_native_storage_on(&block_h).unwrap(); + assert_eq!(Some(8), storage_h.read_state(1).unwrap()); + assert_eq!(Some(2), storage_h.read_state(2).unwrap()); + assert_eq!(Some(4), storage_h.read_state(3).unwrap()); + assert_eq!(None, storage_h.read_state(4).unwrap()); + assert_eq!(Some(100), storage_h.read_native(1).unwrap()); + assert_eq!(Some(9), storage_h.read_native(2).unwrap()); + assert_eq!(Some(500), storage_h.read_native(3).unwrap()); + assert_eq!(None, storage_h.read_native(4).unwrap()); + + let storage_k = storage_manager.get_native_storage_on(&block_k).unwrap(); + assert_eq!(Some(7), storage_k.read_state(1).unwrap()); + assert_eq!(Some(2), storage_k.read_state(2).unwrap()); + assert_eq!(None, storage_k.read_state(3).unwrap()); + assert_eq!(None, storage_k.read_state(4).unwrap()); + assert_eq!(None, storage_k.read_native(1).unwrap()); + assert_eq!(Some(200), storage_k.read_native(2).unwrap()); + assert_eq!(Some(700), storage_k.read_native(3).unwrap()); + assert_eq!(None, storage_k.read_native(4).unwrap()); + + // After finalization + storage_manager.finalize(&block_a).unwrap(); + + storage_manager.finalize(&block_b).unwrap(); } } From 6187cbe02dddb29264a17a5f53992c302ee0b269 Mon Sep 17 00:00:00 2001 From: Nikolai Golub Date: Tue, 21 Nov 2023 19:13:55 +0100 Subject: [PATCH 05/13] [no ci] Still working on the test --- .../sov-prover-storage-manager/src/lib.rs | 122 +++++++++++++----- .../src/snapshot_manager.rs | 9 +- 2 files changed, 96 insertions(+), 35 deletions(-) diff --git a/full-node/sov-prover-storage-manager/src/lib.rs b/full-node/sov-prover-storage-manager/src/lib.rs index 82ecfa58b..e8399b873 100644 --- a/full-node/sov-prover-storage-manager/src/lib.rs +++ b/full-node/sov-prover-storage-manager/src/lib.rs @@ -186,6 +186,9 @@ where let current_block_hash = block_header.hash(); let prev_block_hash = block_header.prev_hash(); + self.blocks_to_parent.remove(&prev_block_hash); + self.blocks_to_parent.remove(¤t_block_hash); + let snapshot_id = self .block_hash_to_snapshot_id .remove(¤t_block_hash) @@ -193,6 +196,8 @@ where let mut state_manager = self.state_snapshot_manager.write().unwrap(); let mut native_manager = self.native_snapshot_manager.write().unwrap(); + let mut snapshot_id_to_parent = self.snapshot_id_to_parent.write().unwrap(); + snapshot_id_to_parent.remove(&snapshot_id); // Return error here, as underlying database can return error state_manager.commit_snapshot(&snapshot_id)?; @@ -210,8 +215,9 @@ where while let Some(block_hash) = to_discard.pop() { let child_block_hashes = self.chain_forks.remove(&block_hash).unwrap_or_default(); self.blocks_to_parent.remove(&block_hash).unwrap(); - let snapshot_id = self.block_hash_to_snapshot_id.remove(&block_hash).unwrap(); + let snapshot_id = self.block_hash_to_snapshot_id.remove(&block_hash).unwrap(); + snapshot_id_to_parent.remove(&snapshot_id); state_manager.discard_snapshot(&snapshot_id); native_manager.discard_snapshot(&snapshot_id); @@ -515,6 +521,40 @@ mod tests { // query data from block B, before adding snapshot A back to the manager! } + #[test] + fn linear_progression() { + let state_tmpdir = tempfile::tempdir().unwrap(); + let native_tmpdir = tempfile::tempdir().unwrap(); + + let (state_db, native_db) = build_dbs(state_tmpdir.path(), native_tmpdir.path()); + let mut storage_manager = NewProverStorageManager::::new(state_db, native_db); + assert!(storage_manager.is_empty()); + + let block_from_i = |i: u8| MockBlockHeader { + prev_hash: MockHash::from([i; 32]), + hash: MockHash::from([i + 1; 32]), + height: i as u64 + 1, + }; + + for i in 0u8..10 { + let block = block_from_i(i); + let storage = storage_manager.get_native_storage_on(&block).unwrap(); + storage_manager.save_change_set(&block, storage).unwrap(); + } + + for i in 0u8..10 { + let block = block_from_i(i); + storage_manager.finalize(&block).unwrap(); + } + assert!(storage_manager.is_empty()); + } + + #[test] + #[ignore = "TBD"] + fn finalize_non_earliest_block() { + // All previous states should be finalized + } + #[test] fn lifecycle_simulation() { let state_tmpdir = tempfile::tempdir().unwrap(); @@ -716,36 +756,45 @@ mod tests { // | K | native | 4 | None | let storage_e = storage_manager.get_native_storage_on(&block_e).unwrap(); - assert_eq!(None, storage_e.read_state(1).unwrap()); - assert_eq!(Some(2), storage_e.read_state(2).unwrap()); - assert_eq!(Some(6), storage_e.read_state(3).unwrap()); - assert_eq!(Some(5), storage_e.read_state(4).unwrap()); - assert_eq!(Some(600), storage_e.read_native(1).unwrap()); - assert_eq!(Some(200), storage_e.read_native(2).unwrap()); - assert_eq!(Some(500), storage_e.read_native(3).unwrap()); - assert_eq!(None, storage_e.read_native(4).unwrap()); - let storage_m = storage_manager.get_native_storage_on(&block_m).unwrap(); - assert_eq!(Some(10), storage_m.read_state(1).unwrap()); - assert_eq!(Some(2), storage_m.read_state(2).unwrap()); - assert_eq!(Some(4), storage_m.read_state(3).unwrap()); - assert_eq!(None, storage_m.read_state(4).unwrap()); - assert_eq!(Some(100), storage_m.read_native(1).unwrap()); - assert_eq!(Some(200), storage_m.read_native(2).unwrap()); - assert_eq!(Some(500), storage_m.read_native(3).unwrap()); - assert_eq!(None, storage_m.read_native(4).unwrap()); - let storage_h = storage_manager.get_native_storage_on(&block_h).unwrap(); - assert_eq!(Some(8), storage_h.read_state(1).unwrap()); - assert_eq!(Some(2), storage_h.read_state(2).unwrap()); - assert_eq!(Some(4), storage_h.read_state(3).unwrap()); - assert_eq!(None, storage_h.read_state(4).unwrap()); - assert_eq!(Some(100), storage_h.read_native(1).unwrap()); - assert_eq!(Some(9), storage_h.read_native(2).unwrap()); - assert_eq!(Some(500), storage_h.read_native(3).unwrap()); - assert_eq!(None, storage_h.read_native(4).unwrap()); - let storage_k = storage_manager.get_native_storage_on(&block_k).unwrap(); + + let assert_main_fork = || { + println!("E"); + assert_eq!(None, storage_e.read_state(1).unwrap()); + println!("1"); + assert_eq!(Some(2), storage_e.read_state(2).unwrap()); + println!("2"); + assert_eq!(Some(6), storage_e.read_state(3).unwrap()); + assert_eq!(Some(5), storage_e.read_state(4).unwrap()); + assert_eq!(Some(600), storage_e.read_native(1).unwrap()); + assert_eq!(Some(200), storage_e.read_native(2).unwrap()); + assert_eq!(Some(500), storage_e.read_native(3).unwrap()); + assert_eq!(None, storage_e.read_native(4).unwrap()); + + println!("M"); + assert_eq!(Some(10), storage_m.read_state(1).unwrap()); + assert_eq!(Some(2), storage_m.read_state(2).unwrap()); + assert_eq!(Some(4), storage_m.read_state(3).unwrap()); + assert_eq!(None, storage_m.read_state(4).unwrap()); + assert_eq!(Some(100), storage_m.read_native(1).unwrap()); + assert_eq!(Some(200), storage_m.read_native(2).unwrap()); + assert_eq!(Some(500), storage_m.read_native(3).unwrap()); + assert_eq!(None, storage_m.read_native(4).unwrap()); + + println!("H"); + assert_eq!(Some(8), storage_h.read_state(1).unwrap()); + assert_eq!(Some(2), storage_h.read_state(2).unwrap()); + assert_eq!(Some(4), storage_h.read_state(3).unwrap()); + assert_eq!(None, storage_h.read_state(4).unwrap()); + assert_eq!(Some(100), storage_h.read_native(1).unwrap()); + assert_eq!(Some(9), storage_h.read_native(2).unwrap()); + assert_eq!(Some(500), storage_h.read_native(3).unwrap()); + assert_eq!(None, storage_h.read_native(4).unwrap()); + println!("DONE!"); + }; + assert_main_fork(); assert_eq!(Some(7), storage_k.read_state(1).unwrap()); assert_eq!(Some(2), storage_k.read_state(2).unwrap()); assert_eq!(None, storage_k.read_state(3).unwrap()); @@ -755,9 +804,22 @@ mod tests { assert_eq!(Some(700), storage_k.read_native(3).unwrap()); assert_eq!(None, storage_k.read_native(4).unwrap()); - // After finalization + // After finalization of A storage_manager.finalize(&block_a).unwrap(); - + assert_main_fork(); + // Storage K is now unknown snapshot, as it was created from block F, which was discarded + // assert_eq!(Some(7), storage_k.read_state(1).unwrap()); + // assert_eq!(Some(2), storage_k.read_state(2).unwrap()); + // assert_eq!(None, storage_k.read_state(3).unwrap()); + // assert_eq!(None, storage_k.read_state(4).unwrap()); + // assert_eq!(None, storage_k.read_native(1).unwrap()); + // assert_eq!(Some(200), storage_k.read_native(2).unwrap()); + // assert_eq!(Some(700), storage_k.read_native(3).unwrap()); + // assert_eq!(None, storage_k.read_native(4).unwrap()); + + // Finalizing the rest storage_manager.finalize(&block_b).unwrap(); + + // TODO: Check that values are in the database } } diff --git a/full-node/sov-prover-storage-manager/src/snapshot_manager.rs b/full-node/sov-prover-storage-manager/src/snapshot_manager.rs index 7fdfd71cd..72a38eb65 100644 --- a/full-node/sov-prover-storage-manager/src/snapshot_manager.rs +++ b/full-node/sov-prover-storage-manager/src/snapshot_manager.rs @@ -35,9 +35,7 @@ impl SnapshotManager { } pub(crate) fn discard_snapshot(&mut self, snapshot_id: &SnapshotId) { - self.snapshots - .remove(snapshot_id) - .expect("Attempt to discard unknown snapshot"); + self.snapshots.remove(snapshot_id); } pub(crate) fn commit_snapshot(&mut self, snapshot_id: &SnapshotId) -> anyhow::Result<()> { @@ -65,7 +63,7 @@ impl QueryManager for SnapshotManager { let parent_snapshot = self .snapshots .get(parent_snapshot_id) - .expect("Inconsistency between snapshots and to_parent"); + .expect("Inconsistency between `self.snapshots` and `self.to_parent`"); // Some operation has been found if let Some(operation) = parent_snapshot.get(key)? { @@ -164,8 +162,9 @@ mod tests { } #[test] - #[should_panic(expected = "Attempt to discard unknown snapshot")] fn test_discard_unknown() { + // Discarding unknown snapshots are fine. + // As it possible that caller didn't save it previously. let tempdir = tempfile::tempdir().unwrap(); let db = create_test_db(tempdir.path()); let to_parent = Arc::new(RwLock::new(HashMap::new())); From 042d785bce67343e7bb32d66b353535b91463d60 Mon Sep 17 00:00:00 2001 From: Nikolai Golub Date: Wed, 22 Nov 2023 00:53:01 +0100 Subject: [PATCH 06/13] Fixing misconfigured snapshot_to_parent map --- .../sov-prover-storage-manager/src/lib.rs | 143 +++++++++++++++++- .../src/snapshot_manager.rs | 5 + 2 files changed, 141 insertions(+), 7 deletions(-) diff --git a/full-node/sov-prover-storage-manager/src/lib.rs b/full-node/sov-prover-storage-manager/src/lib.rs index e8399b873..0efd22069 100644 --- a/full-node/sov-prover-storage-manager/src/lib.rs +++ b/full-node/sov-prover-storage-manager/src/lib.rs @@ -32,7 +32,10 @@ struct NewProverStorageManager { phantom_mp_spec: PhantomData, } -impl NewProverStorageManager { +impl NewProverStorageManager +where + Da::SlotHash: Hash, +{ #[allow(dead_code)] pub fn new(state_db: sov_schema_db::DB, native_db: sov_schema_db::DB) -> Self { let snapshot_id_to_parent = Arc::new(RwLock::new(HashMap::new())); @@ -62,6 +65,68 @@ impl NewProverStorageManager { && self.state_snapshot_manager.read().unwrap().is_empty() && self.native_snapshot_manager.read().unwrap().is_empty() } + + #[cfg(test)] + fn print_internal_state(&self) { + println!("============================="); + println!("chain_forks: {:?}", self.chain_forks); + println!("blocks_to_parent: {:?}", self.blocks_to_parent); + println!( + "snapshot_id_to_parent: {:?}", + self.snapshot_id_to_parent.read().unwrap() + ); + println!( + "block_hash_to_snapshot_id: {:?}", + self.block_hash_to_snapshot_id + ); + println!("============================="); + } + + #[cfg(test)] + fn validate_internal_consistency(&self) { + let snapshot_id_to_parent = self.snapshot_id_to_parent.read().unwrap(); + let state_snapshot_manager = self.state_snapshot_manager.read().unwrap(); + let native_snapshot_manager = self.state_snapshot_manager.read().unwrap(); + + for (block_hash, parent_block_hash) in self.blocks_to_parent.iter() { + // For each block hash there should be snapshot id + let snapshot_id = self + .block_hash_to_snapshot_id + .get(block_hash) + .expect("Missing snapshot_id"); + // For each snapshot id there should be snapshot in the manager + assert!( + state_snapshot_manager.contains_snapshot(snapshot_id), + "snapshot id={} is missing in state_snapshot_manager", + snapshot_id + ); + assert!( + native_snapshot_manager.contains_snapshot(snapshot_id), + "snapshot id={} is missing in native_snapshot_manager", + snapshot_id + ); + + // If there's reference to parent snapshot id, it should be consistent + match snapshot_id_to_parent.get(snapshot_id) { + None => { + assert!(self + .block_hash_to_snapshot_id + .get(parent_block_hash) + .is_none()); + } + Some(parent_snapshot_id) => { + let parent_snapshot_id_from_block_hash = self + .block_hash_to_snapshot_id + .get(parent_block_hash) + .expect(&format!( + "Missing parent snapshot_id for block_hash={:?}, parent_block_hash={:?}, snapshot_id={}, expected_parent_snapshot_id={}", + block_hash, parent_block_hash, snapshot_id, parent_snapshot_id, + )); + assert_eq!(parent_snapshot_id, parent_snapshot_id_from_block_hash); + } + } + } + } } impl HierarchicalStorageManager @@ -183,25 +248,29 @@ where } fn finalize(&mut self, block_header: &Da::BlockHeader) -> anyhow::Result<()> { + println!("FINALIZING BH: {:?}", block_header); let current_block_hash = block_header.hash(); let prev_block_hash = block_header.prev_hash(); self.blocks_to_parent.remove(&prev_block_hash); self.blocks_to_parent.remove(¤t_block_hash); - let snapshot_id = self + // Removing previous + self.block_hash_to_snapshot_id.remove(&prev_block_hash); + let snapshot_id = &self .block_hash_to_snapshot_id .remove(¤t_block_hash) .ok_or(anyhow::anyhow!("Attempt to finalize non existing snapshot"))?; + println!("FINALIZING SNAPSHOT ID: {:?}", snapshot_id); let mut state_manager = self.state_snapshot_manager.write().unwrap(); let mut native_manager = self.native_snapshot_manager.write().unwrap(); let mut snapshot_id_to_parent = self.snapshot_id_to_parent.write().unwrap(); - snapshot_id_to_parent.remove(&snapshot_id); + snapshot_id_to_parent.remove(snapshot_id); // Return error here, as underlying database can return error - state_manager.commit_snapshot(&snapshot_id)?; - native_manager.commit_snapshot(&snapshot_id)?; + state_manager.commit_snapshot(snapshot_id)?; + native_manager.commit_snapshot(snapshot_id)?; // All siblings of current snapshot let mut to_discard: Vec<_> = self @@ -224,6 +293,9 @@ where to_discard.extend(child_block_hashes); } + // TODO: O(n) operation, can be optimized + snapshot_id_to_parent.retain(|_, parent_snapshot_id| parent_snapshot_id != snapshot_id); + Ok(()) } } @@ -536,19 +608,72 @@ mod tests { height: i as u64 + 1, }; - for i in 0u8..10 { + for i in 0u8..4 { let block = block_from_i(i); let storage = storage_manager.get_native_storage_on(&block).unwrap(); storage_manager.save_change_set(&block, storage).unwrap(); } - for i in 0u8..10 { + storage_manager.print_internal_state(); + println!("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"); + for i in 0u8..4 { let block = block_from_i(i); + println!("Finalizing block {}", block); storage_manager.finalize(&block).unwrap(); + storage_manager.print_internal_state(); + storage_manager.validate_internal_consistency(); } assert!(storage_manager.is_empty()); } + #[test] + fn parallel_forks() { + let state_tmpdir = tempfile::tempdir().unwrap(); + let native_tmpdir = tempfile::tempdir().unwrap(); + + let (state_db, native_db) = build_dbs(state_tmpdir.path(), native_tmpdir.path()); + let mut storage_manager = NewProverStorageManager::::new(state_db, native_db); + assert!(storage_manager.is_empty()); + + // 1 2 3 + // / -> D -> E + // A -> B -> C + // \ -> F -> G + + // (height, prev_hash, current_hash) + let blocks: Vec<(u8, u8, u8)> = vec![ + (1, 0, 1), // A + (2, 1, 2), // B + (2, 1, 12), // D + (2, 1, 22), // F + (3, 2, 3), // C + (3, 12, 13), // E + (3, 22, 23), // G + ]; + + for (height, prev_hash, next_hash) in blocks { + let block = MockBlockHeader { + prev_hash: MockHash::from([prev_hash; 32]), + hash: MockHash::from([next_hash; 32]), + height: height as u64, + }; + let storage = storage_manager.get_native_storage_on(&block).unwrap(); + storage_manager.save_change_set(&block, storage).unwrap(); + } + + for prev_hash in 0..3 { + let block = MockBlockHeader { + prev_hash: MockHash::from([prev_hash; 32]), + hash: MockHash::from([prev_hash + 1; 32]), + height: prev_hash as u64 + 1, + }; + storage_manager.finalize(&block).unwrap(); + storage_manager.validate_internal_consistency(); + } + + assert!(storage_manager.is_empty()); + } + #[test] #[ignore = "TBD"] fn finalize_non_earliest_block() { @@ -805,7 +930,11 @@ mod tests { assert_eq!(None, storage_k.read_native(4).unwrap()); // After finalization of A + // storage_manager.print_internal_state(); + // storage_manager.validate_internal_consistency(); storage_manager.finalize(&block_a).unwrap(); + storage_manager.print_internal_state(); + storage_manager.validate_internal_consistency(); assert_main_fork(); // Storage K is now unknown snapshot, as it was created from block F, which was discarded // assert_eq!(Some(7), storage_k.read_state(1).unwrap()); diff --git a/full-node/sov-prover-storage-manager/src/snapshot_manager.rs b/full-node/sov-prover-storage-manager/src/snapshot_manager.rs index 72a38eb65..e90f91e73 100644 --- a/full-node/sov-prover-storage-manager/src/snapshot_manager.rs +++ b/full-node/sov-prover-storage-manager/src/snapshot_manager.rs @@ -51,6 +51,11 @@ impl SnapshotManager { pub(crate) fn is_empty(&self) -> bool { self.snapshots.is_empty() } + + #[cfg(test)] + pub(crate) fn contains_snapshot(&self, snapshot_id: &SnapshotId) -> bool { + self.snapshots.contains_key(snapshot_id) + } } impl QueryManager for SnapshotManager { From 76308859d20a69dea16f6ab7de2fb406a26430bf Mon Sep 17 00:00:00 2001 From: Nikolai Golub Date: Wed, 22 Nov 2023 13:20:02 +0100 Subject: [PATCH 07/13] Fixed tests and bugs! --- .../src/dummy_storage.rs | 1 + .../sov-prover-storage-manager/src/lib.rs | 160 ++++++++---------- .../src/snapshot_manager.rs | 81 +++------ 3 files changed, 96 insertions(+), 146 deletions(-) diff --git a/full-node/sov-prover-storage-manager/src/dummy_storage.rs b/full-node/sov-prover-storage-manager/src/dummy_storage.rs index b9801c5a4..f618eadff 100644 --- a/full-node/sov-prover-storage-manager/src/dummy_storage.rs +++ b/full-node/sov-prover-storage-manager/src/dummy_storage.rs @@ -72,6 +72,7 @@ impl NewProverStorage { self.native_db.put::(&key, &value) } + #[cfg(test)] pub(crate) fn delete_native(&self, key: u64) -> anyhow::Result<()> { let key = DummyField(key); self.native_db.delete::(&key) diff --git a/full-node/sov-prover-storage-manager/src/lib.rs b/full-node/sov-prover-storage-manager/src/lib.rs index 0efd22069..59d3ff839 100644 --- a/full-node/sov-prover-storage-manager/src/lib.rs +++ b/full-node/sov-prover-storage-manager/src/lib.rs @@ -65,68 +65,6 @@ where && self.state_snapshot_manager.read().unwrap().is_empty() && self.native_snapshot_manager.read().unwrap().is_empty() } - - #[cfg(test)] - fn print_internal_state(&self) { - println!("============================="); - println!("chain_forks: {:?}", self.chain_forks); - println!("blocks_to_parent: {:?}", self.blocks_to_parent); - println!( - "snapshot_id_to_parent: {:?}", - self.snapshot_id_to_parent.read().unwrap() - ); - println!( - "block_hash_to_snapshot_id: {:?}", - self.block_hash_to_snapshot_id - ); - println!("============================="); - } - - #[cfg(test)] - fn validate_internal_consistency(&self) { - let snapshot_id_to_parent = self.snapshot_id_to_parent.read().unwrap(); - let state_snapshot_manager = self.state_snapshot_manager.read().unwrap(); - let native_snapshot_manager = self.state_snapshot_manager.read().unwrap(); - - for (block_hash, parent_block_hash) in self.blocks_to_parent.iter() { - // For each block hash there should be snapshot id - let snapshot_id = self - .block_hash_to_snapshot_id - .get(block_hash) - .expect("Missing snapshot_id"); - // For each snapshot id there should be snapshot in the manager - assert!( - state_snapshot_manager.contains_snapshot(snapshot_id), - "snapshot id={} is missing in state_snapshot_manager", - snapshot_id - ); - assert!( - native_snapshot_manager.contains_snapshot(snapshot_id), - "snapshot id={} is missing in native_snapshot_manager", - snapshot_id - ); - - // If there's reference to parent snapshot id, it should be consistent - match snapshot_id_to_parent.get(snapshot_id) { - None => { - assert!(self - .block_hash_to_snapshot_id - .get(parent_block_hash) - .is_none()); - } - Some(parent_snapshot_id) => { - let parent_snapshot_id_from_block_hash = self - .block_hash_to_snapshot_id - .get(parent_block_hash) - .expect(&format!( - "Missing parent snapshot_id for block_hash={:?}, parent_block_hash={:?}, snapshot_id={}, expected_parent_snapshot_id={}", - block_hash, parent_block_hash, snapshot_id, parent_snapshot_id, - )); - assert_eq!(parent_snapshot_id, parent_snapshot_id_from_block_hash); - } - } - } - } } impl HierarchicalStorageManager @@ -248,7 +186,6 @@ where } fn finalize(&mut self, block_header: &Da::BlockHeader) -> anyhow::Result<()> { - println!("FINALIZING BH: {:?}", block_header); let current_block_hash = block_header.hash(); let prev_block_hash = block_header.prev_hash(); @@ -261,7 +198,6 @@ where .block_hash_to_snapshot_id .remove(¤t_block_hash) .ok_or(anyhow::anyhow!("Attempt to finalize non existing snapshot"))?; - println!("FINALIZING SNAPSHOT ID: {:?}", snapshot_id); let mut state_manager = self.state_snapshot_manager.write().unwrap(); let mut native_manager = self.native_snapshot_manager.write().unwrap(); @@ -293,8 +229,14 @@ where to_discard.extend(child_block_hashes); } - // TODO: O(n) operation, can be optimized - snapshot_id_to_parent.retain(|_, parent_snapshot_id| parent_snapshot_id != snapshot_id); + // Removing snapshot id pointers for children of this one + for child_block_hash in self.chain_forks.get(¤t_block_hash).unwrap_or(&vec![]) { + let child_snapshot_id = self + .block_hash_to_snapshot_id + .get(child_block_hash) + .unwrap(); + snapshot_id_to_parent.remove(child_snapshot_id); + } Ok(()) } @@ -316,6 +258,59 @@ mod tests { type Da = sov_mock_da::MockDaSpec; type S = sov_state::DefaultStorageSpec; + fn validate_internal_consistency(storage_manager: &NewProverStorageManager) { + let snapshot_id_to_parent = storage_manager.snapshot_id_to_parent.read().unwrap(); + let state_snapshot_manager = storage_manager.state_snapshot_manager.read().unwrap(); + let native_snapshot_manager = storage_manager.state_snapshot_manager.read().unwrap(); + + for (block_hash, parent_block_hash) in storage_manager.blocks_to_parent.iter() { + // For each block hash there should be snapshot id + let snapshot_id = storage_manager + .block_hash_to_snapshot_id + .get(block_hash) + .expect("Missing snapshot_id"); + + // For each snapshot id, that is not head, there should be parent snapshot id + if !storage_manager + .chain_forks + .get(block_hash) + .unwrap_or(&vec![]) + .is_empty() + { + assert!( + state_snapshot_manager.contains_snapshot(snapshot_id), + "snapshot id={} is missing in state_snapshot_manager", + snapshot_id + ); + assert!( + native_snapshot_manager.contains_snapshot(snapshot_id), + "snapshot id={} is missing in native_snapshot_manager", + snapshot_id + ); + } + + // If there's reference to parent snapshot id, it should be consistent with block hash i + match snapshot_id_to_parent.get(snapshot_id) { + None => { + assert!(storage_manager + .block_hash_to_snapshot_id + .get(parent_block_hash) + .is_none()); + } + Some(parent_snapshot_id) => { + let parent_snapshot_id_from_block_hash = storage_manager + .block_hash_to_snapshot_id + .get(parent_block_hash) + .unwrap_or_else(|| panic!( + "Missing parent snapshot_id for block_hash={:?}, parent_block_hash={:?}, snapshot_id={}, expected_parent_snapshot_id={}", + block_hash, parent_block_hash, snapshot_id, parent_snapshot_id, + )); + assert_eq!(parent_snapshot_id, parent_snapshot_id_from_block_hash); + } + } + } + } + fn build_dbs( state_path: &path::Path, native_path: &path::Path, @@ -349,6 +344,7 @@ mod tests { let storage_manager = NewProverStorageManager::::new(state_db, native_db); assert!(storage_manager.is_empty()); + validate_internal_consistency(&storage_manager); } #[test] @@ -614,14 +610,10 @@ mod tests { storage_manager.save_change_set(&block, storage).unwrap(); } - storage_manager.print_internal_state(); - println!("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"); for i in 0u8..4 { let block = block_from_i(i); - println!("Finalizing block {}", block); storage_manager.finalize(&block).unwrap(); - storage_manager.print_internal_state(); - storage_manager.validate_internal_consistency(); + validate_internal_consistency(&storage_manager); } assert!(storage_manager.is_empty()); } @@ -668,7 +660,7 @@ mod tests { height: prev_hash as u64 + 1, }; storage_manager.finalize(&block).unwrap(); - storage_manager.validate_internal_consistency(); + validate_internal_consistency(&storage_manager); } assert!(storage_manager.is_empty()); @@ -886,11 +878,8 @@ mod tests { let storage_k = storage_manager.get_native_storage_on(&block_k).unwrap(); let assert_main_fork = || { - println!("E"); assert_eq!(None, storage_e.read_state(1).unwrap()); - println!("1"); assert_eq!(Some(2), storage_e.read_state(2).unwrap()); - println!("2"); assert_eq!(Some(6), storage_e.read_state(3).unwrap()); assert_eq!(Some(5), storage_e.read_state(4).unwrap()); assert_eq!(Some(600), storage_e.read_native(1).unwrap()); @@ -898,7 +887,6 @@ mod tests { assert_eq!(Some(500), storage_e.read_native(3).unwrap()); assert_eq!(None, storage_e.read_native(4).unwrap()); - println!("M"); assert_eq!(Some(10), storage_m.read_state(1).unwrap()); assert_eq!(Some(2), storage_m.read_state(2).unwrap()); assert_eq!(Some(4), storage_m.read_state(3).unwrap()); @@ -908,7 +896,6 @@ mod tests { assert_eq!(Some(500), storage_m.read_native(3).unwrap()); assert_eq!(None, storage_m.read_native(4).unwrap()); - println!("H"); assert_eq!(Some(8), storage_h.read_state(1).unwrap()); assert_eq!(Some(2), storage_h.read_state(2).unwrap()); assert_eq!(Some(4), storage_h.read_state(3).unwrap()); @@ -917,7 +904,6 @@ mod tests { assert_eq!(Some(9), storage_h.read_native(2).unwrap()); assert_eq!(Some(500), storage_h.read_native(3).unwrap()); assert_eq!(None, storage_h.read_native(4).unwrap()); - println!("DONE!"); }; assert_main_fork(); assert_eq!(Some(7), storage_k.read_state(1).unwrap()); @@ -929,26 +915,14 @@ mod tests { assert_eq!(Some(700), storage_k.read_native(3).unwrap()); assert_eq!(None, storage_k.read_native(4).unwrap()); + validate_internal_consistency(&storage_manager); // After finalization of A - // storage_manager.print_internal_state(); - // storage_manager.validate_internal_consistency(); storage_manager.finalize(&block_a).unwrap(); - storage_manager.print_internal_state(); - storage_manager.validate_internal_consistency(); + validate_internal_consistency(&storage_manager); assert_main_fork(); - // Storage K is now unknown snapshot, as it was created from block F, which was discarded - // assert_eq!(Some(7), storage_k.read_state(1).unwrap()); - // assert_eq!(Some(2), storage_k.read_state(2).unwrap()); - // assert_eq!(None, storage_k.read_state(3).unwrap()); - // assert_eq!(None, storage_k.read_state(4).unwrap()); - // assert_eq!(None, storage_k.read_native(1).unwrap()); - // assert_eq!(Some(200), storage_k.read_native(2).unwrap()); - // assert_eq!(Some(700), storage_k.read_native(3).unwrap()); - // assert_eq!(None, storage_k.read_native(4).unwrap()); - // Finalizing the rest storage_manager.finalize(&block_b).unwrap(); - + validate_internal_consistency(&storage_manager); // TODO: Check that values are in the database } } diff --git a/full-node/sov-prover-storage-manager/src/snapshot_manager.rs b/full-node/sov-prover-storage-manager/src/snapshot_manager.rs index e90f91e73..0d07c4544 100644 --- a/full-node/sov-prover-storage-manager/src/snapshot_manager.rs +++ b/full-node/sov-prover-storage-manager/src/snapshot_manager.rs @@ -101,14 +101,13 @@ mod tests { fn create_test_db(path: &std::path::Path) -> sov_schema_db::DB { let tables = vec![DUMMY_STATE_CF.to_string()]; - let db = sov_schema_db::DB::open( + sov_schema_db::DB::open( path, "test_db", tables, &gen_rocksdb_options(&Default::default(), false), ) - .unwrap(); - db + .unwrap() } #[test] @@ -258,17 +257,17 @@ mod tests { edit.insert(7, 6); } - let one = DummyField(1); - let two = DummyField(2); - let three = DummyField(3); - let four = DummyField(4); - let five = DummyField(5); - let six = DummyField(6); - let seven = DummyField(7); - let eight = DummyField(8); + let f1 = DummyField(1); + let f2 = DummyField(2); + let f3 = DummyField(3); + let f4 = DummyField(4); + let f5 = DummyField(5); + let f6 = DummyField(6); + let f7 = DummyField(7); + let f8 = DummyField(8); let mut db_data = SchemaBatch::new(); - db_data.put::(&one, &one).unwrap(); + db_data.put::(&f1, &f1).unwrap(); db.write_schemas(db_data).unwrap(); let mut snapshot_manager = SnapshotManager::new(db, to_parent.clone()); @@ -287,14 +286,14 @@ mod tests { // 1 let db_snapshot = DbSnapshot::new(1, query_manager.clone().into()); - db_snapshot.put::(&two, &two).unwrap(); - db_snapshot.put::(&three, &four).unwrap(); + db_snapshot.put::(&f2, &f2).unwrap(); + db_snapshot.put::(&f3, &f4).unwrap(); snapshot_manager.add_snapshot(db_snapshot.into()); // 2 let db_snapshot = DbSnapshot::new(2, query_manager.clone().into()); - db_snapshot.put::(&one, &five).unwrap(); - db_snapshot.delete::(&two).unwrap(); + db_snapshot.put::(&f1, &f5).unwrap(); + db_snapshot.delete::(&f2).unwrap(); snapshot_manager.add_snapshot(db_snapshot.into()); // 3 @@ -303,7 +302,7 @@ mod tests { // 4 let db_snapshot = DbSnapshot::new(4, query_manager.clone().into()); - db_snapshot.put::(&three, &six).unwrap(); + db_snapshot.put::(&f3, &f6).unwrap(); snapshot_manager.add_snapshot(db_snapshot.into()); // 5 @@ -312,8 +311,8 @@ mod tests { // 6 let db_snapshot = DbSnapshot::new(6, query_manager.clone().into()); - db_snapshot.put::(&one, &seven).unwrap(); - db_snapshot.put::(&two, &eight).unwrap(); + db_snapshot.put::(&f1, &f7).unwrap(); + db_snapshot.put::(&f2, &f8).unwrap(); snapshot_manager.add_snapshot(db_snapshot.into()); // 7 @@ -331,39 +330,15 @@ mod tests { // | 7 | 1 | 7 | // | 7 | 2 | 8 | // | 7 | 3 | 4 | - assert_eq!( - Some(five.clone()), - snapshot_manager.get::(3, &one).unwrap() - ); - assert_eq!(None, snapshot_manager.get::(3, &two).unwrap()); - assert_eq!( - Some(four.clone()), - snapshot_manager.get::(3, &three).unwrap() - ); - assert_eq!( - Some(one.clone()), - snapshot_manager.get::(5, &one).unwrap() - ); - assert_eq!( - Some(two.clone()), - snapshot_manager.get::(5, &two).unwrap() - ); - assert_eq!( - Some(six.clone()), - snapshot_manager.get::(5, &three).unwrap() - ); - - assert_eq!( - Some(seven), - snapshot_manager.get::(7, &one).unwrap() - ); - assert_eq!( - Some(eight), - snapshot_manager.get::(7, &two).unwrap() - ); - assert_eq!( - Some(four), - snapshot_manager.get::(7, &three).unwrap() - ); + assert_eq!(Some(f5), snapshot_manager.get::(3, &f1).unwrap()); + assert_eq!(None, snapshot_manager.get::(3, &f2).unwrap()); + assert_eq!(Some(f4), snapshot_manager.get::(3, &f3).unwrap()); + assert_eq!(Some(f1), snapshot_manager.get::(5, &f1).unwrap()); + assert_eq!(Some(f2), snapshot_manager.get::(5, &f2).unwrap()); + assert_eq!(Some(f6), snapshot_manager.get::(5, &f3).unwrap()); + + assert_eq!(Some(f7), snapshot_manager.get::(7, &f1).unwrap()); + assert_eq!(Some(f8), snapshot_manager.get::(7, &f2).unwrap()); + assert_eq!(Some(f4), snapshot_manager.get::(7, &f3).unwrap()); } } From 4d25a326ead376a829a8b402dad7af9ae916c6a9 Mon Sep 17 00:00:00 2001 From: Nikolai Golub Date: Wed, 22 Nov 2023 13:44:37 +0100 Subject: [PATCH 08/13] More tests --- .../sov-prover-storage-manager/src/lib.rs | 66 +++++++++++++++---- .../src/snapshot_manager.rs | 1 - 2 files changed, 54 insertions(+), 13 deletions(-) diff --git a/full-node/sov-prover-storage-manager/src/lib.rs b/full-node/sov-prover-storage-manager/src/lib.rs index 59d3ff839..c3539688f 100644 --- a/full-node/sov-prover-storage-manager/src/lib.rs +++ b/full-node/sov-prover-storage-manager/src/lib.rs @@ -83,8 +83,14 @@ where let prev_block_hash = block_header.prev_hash(); assert_ne!( current_block_hash, prev_block_hash, - "Cannot provide storage for corrupt block" + "Cannot provide storage for corrupt block: prev_hash == current_hash" ); + if let Some(prev_snapshot_id) = self.block_hash_to_snapshot_id.get(&prev_block_hash) { + let state_snapshot_manager = self.state_snapshot_manager.read().unwrap(); + if !state_snapshot_manager.contains_snapshot(prev_snapshot_id) { + anyhow::bail!("Snapshot for previous block has been saved yet"); + } + } let new_snapshot_id = match self.block_hash_to_snapshot_id.get(¤t_block_hash) { // Storage for this block has been requested before @@ -287,6 +293,11 @@ mod tests { "snapshot id={} is missing in native_snapshot_manager", snapshot_id ); + } else { + assert_eq!( + state_snapshot_manager.contains_snapshot(snapshot_id), + native_snapshot_manager.contains_snapshot(snapshot_id), + ); } // If there's reference to parent snapshot id, it should be consistent with block hash i @@ -398,8 +409,8 @@ mod tests { assert!(storage_manager.is_empty()); let block_header = MockBlockHeader { - prev_hash: MockHash::from([1; 32]), - hash: MockHash::from([2; 32]), + prev_hash: MockHash::from([0; 32]), + hash: MockHash::from([1; 32]), height: 1, }; @@ -453,6 +464,42 @@ mod tests { .unwrap(); } + #[test] + fn read_state_before_parent_is_added() { + // Blocks A -> B + // create snapshot A from block A + // create snapshot B from block B + // query data from block B, before adding snapshot A back to the manager! + let state_tmpdir = tempfile::tempdir().unwrap(); + let native_tmpdir = tempfile::tempdir().unwrap(); + + let (state_db, native_db) = build_dbs(state_tmpdir.path(), native_tmpdir.path()); + + let mut storage_manager = NewProverStorageManager::::new(state_db, native_db); + assert!(storage_manager.is_empty()); + + let block_a = MockBlockHeader { + prev_hash: MockHash::from([1; 32]), + hash: MockHash::from([2; 32]), + height: 1, + }; + let block_b = MockBlockHeader { + prev_hash: MockHash::from([2; 32]), + hash: MockHash::from([1; 32]), + height: 2, + }; + + let _storage_a = storage_manager.get_native_storage_on(&block_a).unwrap(); + + // new storage can be crated only on top of saved snapshot. + let result = storage_manager.get_native_storage_on(&block_b); + assert!(result.is_err()); + assert_eq!( + "Snapshot for previous block has been saved yet", + result.err().unwrap().to_string() + ); + } + #[test] fn save_change_set() { let state_tmpdir = tempfile::tempdir().unwrap(); @@ -580,15 +627,6 @@ mod tests { assert_eq!("Attempt to save unknown snapshot with id=1", err_msg); } - #[test] - #[ignore = "TBD"] - fn read_state_before_parent_is_added() { - // Blocks A -> B - // create snapshot A from block A - // create snapshot B from block B - // query data from block B, before adding snapshot A back to the manager! - } - #[test] fn linear_progression() { let state_tmpdir = tempfile::tempdir().unwrap(); @@ -923,6 +961,10 @@ mod tests { // Finalizing the rest storage_manager.finalize(&block_b).unwrap(); validate_internal_consistency(&storage_manager); + storage_manager.finalize(&block_c).unwrap(); + validate_internal_consistency(&storage_manager); + storage_manager.finalize(&block_d).unwrap(); + validate_internal_consistency(&storage_manager); // TODO: Check that values are in the database } } diff --git a/full-node/sov-prover-storage-manager/src/snapshot_manager.rs b/full-node/sov-prover-storage-manager/src/snapshot_manager.rs index 0d07c4544..63572ea59 100644 --- a/full-node/sov-prover-storage-manager/src/snapshot_manager.rs +++ b/full-node/sov-prover-storage-manager/src/snapshot_manager.rs @@ -52,7 +52,6 @@ impl SnapshotManager { self.snapshots.is_empty() } - #[cfg(test)] pub(crate) fn contains_snapshot(&self, snapshot_id: &SnapshotId) -> bool { self.snapshots.contains_key(snapshot_id) } From 62b36c41370832ce6969ec662b6e489100645e23 Mon Sep 17 00:00:00 2001 From: Nikolai Golub Date: Wed, 22 Nov 2023 14:22:32 +0100 Subject: [PATCH 09/13] Adding hash bound to BlockHash trait --- adapters/avail/src/spec/hash.rs | 2 +- rollup-interface/src/state_machine/da.rs | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/adapters/avail/src/spec/hash.rs b/adapters/avail/src/spec/hash.rs index aa0b1546c..f3d52a354 100644 --- a/adapters/avail/src/spec/hash.rs +++ b/adapters/avail/src/spec/hash.rs @@ -2,7 +2,7 @@ use primitive_types::H256; use serde::{Deserialize, Serialize}; use sov_rollup_interface::da::BlockHashTrait; -#[derive(Serialize, Deserialize, Default, Clone, Debug, PartialEq, Eq)] +#[derive(Serialize, Deserialize, Default, Clone, Debug, PartialEq, Eq, Hash)] pub struct AvailHash(H256); impl AvailHash { diff --git a/rollup-interface/src/state_machine/da.rs b/rollup-interface/src/state_machine/da.rs index b915ed14c..2bb137cdb 100644 --- a/rollup-interface/src/state_machine/da.rs +++ b/rollup-interface/src/state_machine/da.rs @@ -179,9 +179,8 @@ pub trait BlobReaderTrait: Serialize + DeserializeOwned + Send + Sync + 'static /// Trait with collection of trait bounds for a block hash. pub trait BlockHashTrait: - // TODO: Can we replace `Into<[u8; 32]>` with std::hash::Hash // so it is compatible with StorageManager implementation? - Serialize + DeserializeOwned + PartialEq + Debug + Send + Sync + Clone + Eq + Into<[u8; 32]> + Serialize + DeserializeOwned + PartialEq + Debug + Send + Sync + Clone + Eq + Into<[u8; 32]> + std::hash::Hash { } From 4c176b7c9eb9d20654f90bdbd8df212c54a3405f Mon Sep 17 00:00:00 2001 From: Nikolai Golub Date: Wed, 22 Nov 2023 14:48:04 +0100 Subject: [PATCH 10/13] Fixing CI issues --- full-node/sov-prover-storage-manager/Cargo.toml | 2 +- full-node/sov-prover-storage-manager/src/lib.rs | 16 ++++++++-------- packages_to_publish.yml | 1 + rollup-interface/src/state_machine/da.rs | 2 +- rollup-interface/src/state_machine/storage.rs | 2 +- 5 files changed, 12 insertions(+), 11 deletions(-) diff --git a/full-node/sov-prover-storage-manager/Cargo.toml b/full-node/sov-prover-storage-manager/Cargo.toml index f86da10b5..afd950340 100644 --- a/full-node/sov-prover-storage-manager/Cargo.toml +++ b/full-node/sov-prover-storage-manager/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "sov-prover-storage-manager" -description = "Storage manager for prover storage" +description = "Hierarchical storage manager for prover storage" license = { workspace = true } edition = { workspace = true } authors = { workspace = true } diff --git a/full-node/sov-prover-storage-manager/src/lib.rs b/full-node/sov-prover-storage-manager/src/lib.rs index c3539688f..aa6eebd83 100644 --- a/full-node/sov-prover-storage-manager/src/lib.rs +++ b/full-node/sov-prover-storage-manager/src/lib.rs @@ -27,7 +27,7 @@ struct NewProverStorageManager { snapshot_id_to_parent: Arc>>, state_snapshot_manager: Arc>, - native_snapshot_manager: Arc>, + accessory_snapshot_manager: Arc>, phantom_mp_spec: PhantomData, } @@ -41,7 +41,7 @@ where let snapshot_id_to_parent = Arc::new(RwLock::new(HashMap::new())); let state_snapshot_manager = SnapshotManager::new(state_db, snapshot_id_to_parent.clone()); - let native_snapshot_manager = + let accessory_snapshot_manager = SnapshotManager::new(native_db, snapshot_id_to_parent.clone()); Self { @@ -51,7 +51,7 @@ where block_hash_to_snapshot_id: Default::default(), snapshot_id_to_parent, state_snapshot_manager: Arc::new(RwLock::new(state_snapshot_manager)), - native_snapshot_manager: Arc::new(RwLock::new(native_snapshot_manager)), + accessory_snapshot_manager: Arc::new(RwLock::new(accessory_snapshot_manager)), phantom_mp_spec: Default::default(), } } @@ -63,7 +63,7 @@ where && self.block_hash_to_snapshot_id.is_empty() && self.snapshot_id_to_parent.read().unwrap().is_empty() && self.state_snapshot_manager.read().unwrap().is_empty() - && self.native_snapshot_manager.read().unwrap().is_empty() + && self.accessory_snapshot_manager.read().unwrap().is_empty() } } @@ -133,7 +133,7 @@ where let native_db_snapshot = DbSnapshot::new( new_snapshot_id, - ReadOnlyLock::new(self.native_snapshot_manager.clone()), + ReadOnlyLock::new(self.accessory_snapshot_manager.clone()), ); Ok(NewProverStorage::with_db_handlers( @@ -182,7 +182,7 @@ where { let mut state_manager = self.state_snapshot_manager.write().unwrap(); - let mut native_manager = self.native_snapshot_manager.write().unwrap(); + let mut native_manager = self.accessory_snapshot_manager.write().unwrap(); state_manager.add_snapshot(state_snapshot); native_manager.add_snapshot(native_snapshot); @@ -206,7 +206,7 @@ where .ok_or(anyhow::anyhow!("Attempt to finalize non existing snapshot"))?; let mut state_manager = self.state_snapshot_manager.write().unwrap(); - let mut native_manager = self.native_snapshot_manager.write().unwrap(); + let mut native_manager = self.accessory_snapshot_manager.write().unwrap(); let mut snapshot_id_to_parent = self.snapshot_id_to_parent.write().unwrap(); snapshot_id_to_parent.remove(snapshot_id); @@ -392,7 +392,7 @@ mod tests { .unwrap() .is_empty()); assert!(storage_manager - .native_snapshot_manager + .accessory_snapshot_manager .read() .unwrap() .is_empty()); diff --git a/packages_to_publish.yml b/packages_to_publish.yml index 298f84c22..1ca2df684 100644 --- a/packages_to_publish.yml +++ b/packages_to_publish.yml @@ -7,6 +7,7 @@ - sov-zk-cycle-utils - sov-modules-core - sov-state +- sov-prover-storage-manager - sov-modules-macros # Requires --no-verify because it writes outside of OUT_DIR. - sov-modules-api - sov-sequencer diff --git a/rollup-interface/src/state_machine/da.rs b/rollup-interface/src/state_machine/da.rs index 2bb137cdb..18725863c 100644 --- a/rollup-interface/src/state_machine/da.rs +++ b/rollup-interface/src/state_machine/da.rs @@ -180,7 +180,7 @@ pub trait BlobReaderTrait: Serialize + DeserializeOwned + Send + Sync + 'static /// Trait with collection of trait bounds for a block hash. pub trait BlockHashTrait: // so it is compatible with StorageManager implementation? - Serialize + DeserializeOwned + PartialEq + Debug + Send + Sync + Clone + Eq + Into<[u8; 32]> + std::hash::Hash + Serialize + DeserializeOwned + PartialEq + Debug + Send + Sync + Clone + Eq + Into<[u8; 32]> + core::hash::Hash { } diff --git a/rollup-interface/src/state_machine/storage.rs b/rollup-interface/src/state_machine/storage.rs index 110eb4f2c..def99458f 100644 --- a/rollup-interface/src/state_machine/storage.rs +++ b/rollup-interface/src/state_machine/storage.rs @@ -31,7 +31,7 @@ pub trait HierarchicalStorageManager { ) -> anyhow::Result; /// Adds [`Self::ChangeSet`] to the storage. - /// [`Da::BlockHeader`] must be provided for efficient consistency checking. + /// [`DaSpec::BlockHeader`] must be provided for efficient consistency checking. fn save_change_set( &mut self, block_header: &Da::BlockHeader, From 3798f650f49dadd1b06183ae3b2d9921e036fc28 Mon Sep 17 00:00:00 2001 From: Nikolai Golub Date: Wed, 22 Nov 2023 14:57:07 +0100 Subject: [PATCH 11/13] Remove TODO that is checked in tests --- full-node/sov-prover-storage-manager/src/lib.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/full-node/sov-prover-storage-manager/src/lib.rs b/full-node/sov-prover-storage-manager/src/lib.rs index aa6eebd83..2c5c9a4fd 100644 --- a/full-node/sov-prover-storage-manager/src/lib.rs +++ b/full-node/sov-prover-storage-manager/src/lib.rs @@ -94,11 +94,7 @@ where let new_snapshot_id = match self.block_hash_to_snapshot_id.get(¤t_block_hash) { // Storage for this block has been requested before - Some(snapshot_id) => { - // TODO: Do consistency checks here? - - *snapshot_id - } + Some(snapshot_id) => *snapshot_id, // Storage requested first time None => { let new_snapshot_id = self.latest_snapshot_id + 1; From d757cfa242f3ccee91b25f04725b95c0bd1a381b Mon Sep 17 00:00:00 2001 From: Nikolai Golub Date: Wed, 22 Nov 2023 17:05:46 +0100 Subject: [PATCH 12/13] Fix doc, again --- rollup-interface/src/state_machine/storage.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rollup-interface/src/state_machine/storage.rs b/rollup-interface/src/state_machine/storage.rs index def99458f..50bc51e19 100644 --- a/rollup-interface/src/state_machine/storage.rs +++ b/rollup-interface/src/state_machine/storage.rs @@ -30,7 +30,7 @@ pub trait HierarchicalStorageManager { block_header: &Da::BlockHeader, ) -> anyhow::Result; - /// Adds [`Self::ChangeSet`] to the storage. + /// Adds [`Self::NativeChangeSet`] to the storage. /// [`DaSpec::BlockHeader`] must be provided for efficient consistency checking. fn save_change_set( &mut self, From d718432c0a24e071939fb94d7bf035f5a1441631 Mon Sep 17 00:00:00 2001 From: Nikolai Golub Date: Thu, 23 Nov 2023 12:23:20 +0100 Subject: [PATCH 13/13] Addressing comment about removing dummy_storage.rs --- full-node/sov-prover-storage-manager/src/dummy_storage.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/full-node/sov-prover-storage-manager/src/dummy_storage.rs b/full-node/sov-prover-storage-manager/src/dummy_storage.rs index f618eadff..e47b4be0c 100644 --- a/full-node/sov-prover-storage-manager/src/dummy_storage.rs +++ b/full-node/sov-prover-storage-manager/src/dummy_storage.rs @@ -1,3 +1,5 @@ +// This module is used as a temporary filler for `ProverStorage`, until this module is integrated. +// It is going to be deleted after integration has been completed use std::marker::PhantomData; use byteorder::{BigEndian, ReadBytesExt}; @@ -6,7 +8,7 @@ use sov_schema_db::snapshot::{DbSnapshot, QueryManager}; use sov_schema_db::{define_schema, CodecError}; use sov_state::MerkleProofSpec; -/// Oversimplified representation of [`sov_state::ProverStorage`] +// Oversimplified representation of [`sov_state::ProverStorage`] pub struct NewProverStorage { state_db: DbSnapshot, native_db: DbSnapshot,