From dfd1359385917e0c8549c3c825fe06f511f9f0c0 Mon Sep 17 00:00:00 2001 From: yito88 Date: Wed, 1 Feb 2023 17:49:09 +0100 Subject: [PATCH 01/10] write Merkle tree stores less often --- apps/src/lib/node/ledger/shell/mod.rs | 36 +++--- apps/src/lib/node/ledger/storage/mod.rs | 101 ++++++++++++++++ apps/src/lib/node/ledger/storage/rocksdb.rs | 107 +++++++++++++---- core/src/ledger/storage/merkle_tree.rs | 29 ++++- core/src/ledger/storage/mockdb.rs | 20 +++- core/src/ledger/storage/mod.rs | 124 ++++++++++++++++---- core/src/types/storage.rs | 53 +++++++++ 7 files changed, 397 insertions(+), 73 deletions(-) diff --git a/apps/src/lib/node/ledger/shell/mod.rs b/apps/src/lib/node/ledger/shell/mod.rs index 40a3f5d115..95f8782cb9 100644 --- a/apps/src/lib/node/ledger/shell/mod.rs +++ b/apps/src/lib/node/ledger/shell/mod.rs @@ -772,7 +772,9 @@ mod test_utils { use namada::types::chain::ChainId; use namada::types::hash::Hash; use namada::types::key::*; - use namada::types::storage::{BlockHash, BlockResults, Epoch, Header}; + use namada::types::storage::{ + BlockHash, BlockResults, Epoch, Epochs, Header, + }; use namada::types::transaction::{Fee, WrapperTx}; use tempfile::tempdir; use tokio::sync::mpsc::UnboundedReceiver; @@ -1015,24 +1017,28 @@ mod test_utils { let merkle_tree = MerkleTree::::default(); let stores = merkle_tree.stores(); let hash = BlockHash([0; 32]); - let pred_epochs = Default::default(); + let mut pred_epochs: Epochs = Default::default(); + pred_epochs.new_epoch(BlockHeight(1), 1000); let address_gen = EstablishedAddressGen::new("test"); shell .storage .db - .write_block(BlockStateWrite { - merkle_tree_stores: stores, - header: None, - hash: &hash, - height: BlockHeight(1), - epoch: Epoch(0), - pred_epochs: &pred_epochs, - next_epoch_min_start_height: BlockHeight(3), - next_epoch_min_start_time: DateTimeUtc::now(), - address_gen: &address_gen, - results: &BlockResults::default(), - tx_queue: &shell.storage.tx_queue, - }) + .write_block( + BlockStateWrite { + merkle_tree_stores: stores, + header: None, + hash: &hash, + height: BlockHeight(1), + epoch: Epoch(1), + pred_epochs: &pred_epochs, + next_epoch_min_start_height: BlockHeight(3), + next_epoch_min_start_time: DateTimeUtc::now(), + address_gen: &address_gen, + results: &BlockResults::default(), + tx_queue: &shell.storage.tx_queue, + }, + true, + ) .expect("Test failed"); // Drop the shell diff --git a/apps/src/lib/node/ledger/storage/mod.rs b/apps/src/lib/node/ledger/storage/mod.rs index 1bd1446c51..39634774de 100644 --- a/apps/src/lib/node/ledger/storage/mod.rs +++ b/apps/src/lib/node/ledger/storage/mod.rs @@ -50,6 +50,8 @@ fn new_blake2b() -> Blake2b { #[cfg(test)] mod tests { + use std::collections::HashMap; + use borsh::BorshSerialize; use itertools::Itertools; use namada::ledger::storage::types; @@ -132,6 +134,8 @@ mod tests { storage .write(&key, value_bytes.clone()) .expect("write failed"); + storage.block.epoch = storage.block.epoch.next(); + storage.block.pred_epochs.new_epoch(BlockHeight(100), 1000); storage.commit().expect("commit failed"); // save the last state and drop the storage @@ -247,6 +251,11 @@ mod tests { fn test_read_with_height(blocks_write_value in vec(any::(), 20)) { test_read_with_height_aux(blocks_write_value).unwrap() } + + #[test] + fn test_get_merkle_tree(blocks_write_type in vec(0..3u64, 20)) { + test_get_merkle_tree_aux(blocks_write_type).unwrap() + } } /// Test reads at arbitrary block heights. @@ -348,6 +357,98 @@ mod tests { Ok(()) } + /// Test the restore of the merkle tree + fn test_get_merkle_tree_aux( + blocks_write_type: Vec, + ) -> namada::ledger::storage::Result<()> { + let db_path = + TempDir::new().expect("Unable to create a temporary DB directory"); + let mut storage = PersistentStorage::open( + db_path.path(), + ChainId::default(), + address::nam(), + None, + ); + + let num_keys = 3; + let blocks_write_type = blocks_write_type.into_iter().enumerate().map( + |(index, write_type)| { + // try to update some keys at each height + let height = BlockHeight::from(index as u64 / num_keys + 1); + let key = Key::parse(format!("key{}", index as u64 % num_keys)) + .unwrap(); + (height, key, write_type) + }, + ); + + let mut roots = HashMap::new(); + + // Update and commit + let hash = BlockHash::default(); + storage.begin_block(hash, BlockHeight(1))?; + for (height, key, write_type) in blocks_write_type.clone() { + if height != storage.block.height { + // to check the root later + roots.insert(storage.block.height, storage.merkle_root()); + if storage.block.height.0 % 5 == 0 { + // new epoch every 5 heights + storage.block.epoch = storage.block.epoch.next(); + storage + .block + .pred_epochs + .new_epoch(storage.block.height, 1000); + } + storage.commit()?; + let hash = BlockHash::default(); + storage + .begin_block(hash, storage.block.height.next_height())?; + } + match write_type { + 0 => { + // no update + } + 1 => { + storage.delete(&key)?; + } + _ => { + let value_bytes = types::encode(&storage.block.height); + storage.write(&key, value_bytes)?; + } + } + } + roots.insert(storage.block.height, storage.merkle_root()); + + let mut current_state = HashMap::new(); + for i in 0..num_keys { + let key = Key::parse(format!("key{}", i)).unwrap(); + current_state.insert(key, false); + } + // Check a Merkle tree + for (height, key, write_type) in blocks_write_type { + let tree = storage.get_merkle_tree(height)?; + assert_eq!(tree.root().0, roots.get(&height).unwrap().0); + match write_type { + 0 => { + if *current_state.get(&key).unwrap() { + assert!(tree.has_key(&key)?); + } else { + assert!(!tree.has_key(&key)?); + } + } + 1 => { + assert!(!tree.has_key(&key)?); + current_state.insert(key, false); + } + _ => { + assert!(tree.has_key(&key)?); + current_state.insert(key, true); + } + } + } + + Ok(()) + } + /// Test the prefix iterator with RocksDB. #[test] fn test_persistent_storage_prefix_iter() { diff --git a/apps/src/lib/node/ledger/storage/rocksdb.rs b/apps/src/lib/node/ledger/storage/rocksdb.rs index fa7c981697..bfcf53603c 100644 --- a/apps/src/lib/node/ledger/storage/rocksdb.rs +++ b/apps/src/lib/node/ledger/storage/rocksdb.rs @@ -39,7 +39,8 @@ use namada::ledger::storage::{ }; use namada::types::internal::TxQueue; use namada::types::storage::{ - BlockHeight, BlockResults, Header, Key, KeySeg, KEY_SEGMENT_SEPARATOR, + BlockHeight, BlockResults, Epochs, Header, Key, KeySeg, + KEY_SEGMENT_SEPARATOR, }; use namada::types::time::DateTimeUtc; use rocksdb::{ @@ -424,7 +425,11 @@ impl DB for RocksDB { } } - fn write_block(&mut self, state: BlockStateWrite) -> Result<()> { + fn write_block( + &mut self, + state: BlockStateWrite, + is_full_commit: bool, + ) -> Result<()> { let mut batch = WriteBatch::default(); let BlockStateWrite { merkle_tree_stores, @@ -484,23 +489,25 @@ impl DB for RocksDB { .push(&"tree".to_owned()) .map_err(Error::KeyError)?; for st in StoreType::iter() { - let prefix_key = prefix_key - .push(&st.to_string()) - .map_err(Error::KeyError)?; - let root_key = prefix_key - .push(&"root".to_owned()) - .map_err(Error::KeyError)?; - batch.put( - root_key.to_string(), - types::encode(merkle_tree_stores.root(st)), - ); - let store_key = prefix_key - .push(&"store".to_owned()) - .map_err(Error::KeyError)?; - batch.put( - store_key.to_string(), - merkle_tree_stores.store(st).encode(), - ); + if *st == StoreType::Base || is_full_commit { + let prefix_key = prefix_key + .push(&st.to_string()) + .map_err(Error::KeyError)?; + let root_key = prefix_key + .push(&"root".to_owned()) + .map_err(Error::KeyError)?; + batch.put( + root_key.to_string(), + types::encode(merkle_tree_stores.root(st)), + ); + let store_key = prefix_key + .push(&"store".to_owned()) + .map_err(Error::KeyError)?; + batch.put( + store_key.to_string(), + merkle_tree_stores.store(st).encode(), + ); + } } } // Block header @@ -580,12 +587,28 @@ impl DB for RocksDB { fn read_merkle_tree_stores( &self, height: BlockHeight, - ) -> Result> { - let mut merkle_tree_stores = MerkleTreeStoresRead::default(); + ) -> Result> { + // Get the latest height at which the tree stores were written let height_key = Key::from(height.to_db_key()); - let tree_key = height_key + let key = height_key + .push(&"pred_epochs".to_owned()) + .expect("Cannot obtain a storage key"); + let pred_epochs: Epochs = match self + .0 + .get(key.to_string()) + .map_err(|e| Error::DBError(e.into_string()))? + { + Some(b) => types::decode(b).map_err(Error::CodingError)?, + None => return Ok(None), + }; + let stored_height = pred_epochs + .get_epoch_start_height(height) + .ok_or(Error::NoMerkleTree { height })?; + + let tree_key = Key::from(stored_height.to_db_key()) .push(&"tree".to_owned()) .map_err(Error::KeyError)?; + let mut merkle_tree_stores = MerkleTreeStoresRead::default(); for st in StoreType::iter() { let prefix_key = tree_key.push(&st.to_string()).map_err(Error::KeyError)?; @@ -618,7 +641,7 @@ impl DB for RocksDB { None => return Ok(None), } } - Ok(Some(merkle_tree_stores)) + Ok(Some((stored_height, merkle_tree_stores))) } fn read_subspace_val(&self, key: &Key) -> Result>> { @@ -891,7 +914,7 @@ impl<'iter> DBIter<'iter> for RocksDB { &'iter self, prefix: &Key, ) -> PersistentPrefixIterator<'iter> { - iter_prefix(self, prefix) + iter_subspace_prefix(self, prefix) } fn iter_results(&'iter self) -> PersistentPrefixIterator<'iter> { @@ -913,15 +936,47 @@ impl<'iter> DBIter<'iter> for RocksDB { ); PersistentPrefixIterator(PrefixIterator::new(iter, db_prefix)) } + + fn iter_old_diffs( + &'iter self, + height: BlockHeight, + ) -> PersistentPrefixIterator<'iter> { + iter_diffs_prefix(self, height, true) + } + + fn iter_new_diffs( + &'iter self, + height: BlockHeight, + ) -> PersistentPrefixIterator<'iter> { + iter_diffs_prefix(self, height, false) + } } -fn iter_prefix<'iter>( +fn iter_subspace_prefix<'iter>( db: &'iter RocksDB, prefix: &Key, ) -> PersistentPrefixIterator<'iter> { let db_prefix = "subspace/".to_owned(); let prefix = format!("{}{}", db_prefix, prefix); + iter_prefix(db, db_prefix, prefix) +} + +fn iter_diffs_prefix( + db: &RocksDB, + height: BlockHeight, + is_old: bool, +) -> PersistentPrefixIterator { + let prefix = if is_old { "old" } else { "new" }; + let db_prefix = format!("{}/diffs/{}/", height.0.raw(), prefix); + // get keys without a prefix + iter_prefix(db, db_prefix.clone(), db_prefix) +} +fn iter_prefix( + db: &RocksDB, + db_prefix: String, + prefix: String, +) -> PersistentPrefixIterator { let mut read_opts = ReadOptions::default(); // don't use the prefix bloom filter read_opts.set_total_order_seek(true); @@ -1097,7 +1152,7 @@ mod test { tx_queue: &tx_queue, }; - db.write_block(block).unwrap(); + db.write_block(block, true).unwrap(); let _state = db .read_last_block() diff --git a/core/src/ledger/storage/merkle_tree.rs b/core/src/ledger/storage/merkle_tree.rs index dc65a12540..7c392793a0 100644 --- a/core/src/ledger/storage/merkle_tree.rs +++ b/core/src/ledger/storage/merkle_tree.rs @@ -248,16 +248,38 @@ impl core::fmt::Debug for MerkleTree { impl MerkleTree { /// Restore the tree from the stores - pub fn new(stores: MerkleTreeStoresRead) -> Self { + pub fn new(stores: MerkleTreeStoresRead) -> Result { let base = Smt::new(stores.base.0.into(), stores.base.1); let account = Smt::new(stores.account.0.into(), stores.account.1); let ibc = Amt::new(stores.ibc.0.into(), stores.ibc.1); let pos = Smt::new(stores.pos.0.into(), stores.pos.1); - Self { + let tree = Self { base, account, ibc, pos, + }; + + // validate + let account_key = H::hash(StoreType::Account.to_string()); + let account_root = tree.base.get(&account_key.into())?; + let ibc_key = H::hash(StoreType::Ibc.to_string()); + let ibc_root = tree.base.get(&ibc_key.into())?; + let pos_key = H::hash(StoreType::PoS.to_string()); + let pos_root = tree.base.get(&pos_key.into())?; + if (tree.base.root().is_zero() + && tree.account.root().is_zero() + && tree.ibc.root().is_zero() + && tree.pos.root().is_zero()) + || (account_root == tree.account.root().into() + && ibc_root == tree.ibc.root().into() + && pos_root == tree.pos.root().into()) + { + Ok(tree) + } else { + Err(Error::MerkleTree( + "Invalid MerkleTreeStoresRead".to_string(), + )) } } @@ -696,7 +718,8 @@ mod test { stores_read.set_root(st, stores_write.root(st).clone()); stores_read.set_store(stores_write.store(st).to_owned()); } - let restored_tree = MerkleTree::::new(stores_read); + let restored_tree = + MerkleTree::::new(stores_read).unwrap(); assert!(restored_tree.has_key(&ibc_key).unwrap()); assert!(restored_tree.has_key(&pos_key).unwrap()); } diff --git a/core/src/ledger/storage/mockdb.rs b/core/src/ledger/storage/mockdb.rs index eb8ae04543..47447dc79b 100644 --- a/core/src/ledger/storage/mockdb.rs +++ b/core/src/ledger/storage/mockdb.rs @@ -172,7 +172,11 @@ impl DB for MockDB { } } - fn write_block(&mut self, state: BlockStateWrite) -> Result<()> { + fn write_block( + &mut self, + state: BlockStateWrite, + _is_full_commit: bool, + ) -> Result<()> { let BlockStateWrite { merkle_tree_stores, header, @@ -310,7 +314,7 @@ impl DB for MockDB { fn read_merkle_tree_stores( &self, height: BlockHeight, - ) -> Result> { + ) -> Result> { let mut merkle_tree_stores = MerkleTreeStoresRead::default(); let height_key = Key::from(height.to_db_key()); let tree_key = height_key @@ -342,7 +346,7 @@ impl DB for MockDB { None => return Ok(None), } } - Ok(Some(merkle_tree_stores)) + Ok(Some((height, merkle_tree_stores))) } fn read_subspace_val(&self, key: &Key) -> Result>> { @@ -455,6 +459,16 @@ impl<'iter> DBIter<'iter> for MockDB { let iter = self.0.borrow().clone().into_iter(); MockPrefixIterator::new(MockIterator { prefix, iter }, db_prefix) } + + fn iter_old_diffs(&self, _height: BlockHeight) -> MockPrefixIterator { + // Mock DB can read only the latest value for now + unimplemented!() + } + + fn iter_new_diffs(&self, _height: BlockHeight) -> MockPrefixIterator { + // Mock DB can read only the latest value for now + unimplemented!() + } } /// A prefix iterator base for the [`MockPrefixIterator`]. diff --git a/core/src/ledger/storage/mod.rs b/core/src/ledger/storage/mod.rs index 45f145ec87..fd2d73e017 100644 --- a/core/src/ledger/storage/mod.rs +++ b/core/src/ledger/storage/mod.rs @@ -8,6 +8,7 @@ pub mod traits; pub mod types; use core::fmt::Debug; +use std::cmp::Ordering; use std::collections::BTreeMap; use borsh::{BorshDeserialize, BorshSerialize}; @@ -224,7 +225,11 @@ pub trait DB: std::fmt::Debug { fn read_last_block(&mut self) -> Result>; /// Write block's metadata - fn write_block(&mut self, state: BlockStateWrite) -> Result<()>; + fn write_block( + &mut self, + state: BlockStateWrite, + is_full_commit: bool, + ) -> Result<()>; /// Read the block header with the given height from the DB fn read_block_header(&self, height: BlockHeight) -> Result>; @@ -233,7 +238,7 @@ pub trait DB: std::fmt::Debug { fn read_merkle_tree_stores( &self, height: BlockHeight, - ) -> Result>; + ) -> Result>; /// Read the latest value for account subspace key from the DB fn read_subspace_val(&self, key: &Key) -> Result>>; @@ -307,6 +312,12 @@ pub trait DBIter<'iter> { /// Read results subspace key value pairs from the DB fn iter_results(&'iter self) -> Self::PrefixIter; + + /// Read subspace old diffs at a given height + fn iter_old_diffs(&'iter self, height: BlockHeight) -> Self::PrefixIter; + + /// Read subspace new diffs at a given height + fn iter_new_diffs(&'iter self, height: BlockHeight) -> Self::PrefixIter; } /// Atomic batch write. @@ -379,7 +390,6 @@ where tx_queue, }) = self.db.read_last_block()? { - self.block.tree = MerkleTree::new(merkle_tree_stores); self.block.hash = hash; self.block.height = height; self.block.epoch = epoch; @@ -407,6 +417,8 @@ where ) .expect("unable to decode conversion state") } + self.block.tree = MerkleTree::new(merkle_tree_stores) + .or_else(|_| self.get_merkle_tree(height))?; #[cfg(feature = "ferveo-tpke")] { self.tx_queue = tx_queue; @@ -430,6 +442,8 @@ where /// Persist the current block's state to the database pub fn commit(&mut self) -> Result<()> { + let is_full_commit = + self.block.height.0 == 1 || self.last_epoch != self.block.epoch; let state = BlockStateWrite { merkle_tree_stores: self.block.tree.stores(), header: self.header.as_ref(), @@ -444,7 +458,7 @@ where #[cfg(feature = "ferveo-tpke")] tx_queue: &self.tx_queue, }; - self.db.write_block(state)?; + self.db.write_block(state, is_full_commit)?; self.last_height = self.block.height; self.last_epoch = self.block.epoch; self.header = None; @@ -607,6 +621,75 @@ where (self.block.hash.clone(), BLOCK_HASH_LENGTH as _) } + /// Get the Merkle tree with stores and diffs in the DB + /// Use `self.block.tree` if you want that of the current block height + pub fn get_merkle_tree( + &self, + height: BlockHeight, + ) -> Result> { + let (stored_height, stores) = + self.db.read_merkle_tree_stores(height)?.unwrap_or(( + // restore from the first height + BlockHeight::default(), + MerkleTreeStoresRead::default(), + )); + // Restore the tree state with diffs + let mut tree = MerkleTree::::new(stores).expect("invalid stores"); + let mut target_height = stored_height; + while target_height < height { + target_height = target_height.next_height(); + let mut old_diff_iter = self.db.iter_old_diffs(target_height); + let mut new_diff_iter = self.db.iter_new_diffs(target_height); + + let mut old_diff = old_diff_iter.next(); + let mut new_diff = new_diff_iter.next(); + loop { + match (&old_diff, &new_diff) { + (Some(old), Some(new)) => { + let old_key = Key::parse(old.0.clone()) + .expect("the key should be parsable"); + let new_key = Key::parse(new.0.clone()) + .expect("the key should be parsable"); + match old_key.cmp(&new_key) { + Ordering::Equal => { + // the value was updated + tree.update(&new_key, new.1.clone())?; + old_diff = old_diff_iter.next(); + new_diff = new_diff_iter.next(); + } + Ordering::Less => { + // the value was deleted + tree.delete(&old_key)?; + old_diff = old_diff_iter.next(); + } + Ordering::Greater => { + // the value was inserted + tree.update(&new_key, new.1.clone())?; + new_diff = new_diff_iter.next(); + } + } + } + (Some(old), None) => { + // the value was deleted + let key = Key::parse(old.0.clone()) + .expect("the key should be parsable"); + tree.delete(&key)?; + old_diff = old_diff_iter.next(); + } + (None, Some(new)) => { + // the value was inserted + let key = Key::parse(new.0.clone()) + .expect("the key should be parsable"); + tree.update(&key, new.1.clone())?; + new_diff = new_diff_iter.next(); + } + (None, None) => break, + } + } + } + Ok(tree) + } + /// Get the existence proof #[cfg(any(feature = "tendermint", feature = "tendermint-abcipp"))] pub fn get_existence_proof( @@ -629,21 +712,13 @@ where .map(Into::into) .map_err(Error::MerkleTreeError) } else { - match self.db.read_merkle_tree_stores(height)? { - Some(stores) => { - let tree = MerkleTree::::new(stores); - let MembershipProof::ICS23(proof) = tree - .get_sub_tree_existence_proof( - array::from_ref(key), - vec![value], - ) - .map_err(Error::MerkleTreeError)?; - tree.get_sub_tree_proof(key, proof) - .map(Into::into) - .map_err(Error::MerkleTreeError) - } - None => Err(Error::NoMerkleTree { height }), - } + let tree = self.get_merkle_tree(height)?; + let MembershipProof::ICS23(proof) = tree + .get_sub_tree_existence_proof(array::from_ref(key), vec![value]) + .map_err(Error::MerkleTreeError)?; + tree.get_sub_tree_proof(key, proof) + .map(Into::into) + .map_err(Error::MerkleTreeError) } } @@ -661,13 +736,10 @@ where .map(Into::into) .map_err(Error::MerkleTreeError) } else { - match self.db.read_merkle_tree_stores(height)? { - Some(stores) => MerkleTree::::new(stores) - .get_non_existence_proof(key) - .map(Into::into) - .map_err(Error::MerkleTreeError), - None => Err(Error::NoMerkleTree { height }), - } + self.get_merkle_tree(height)? + .get_non_existence_proof(key) + .map(Into::into) + .map_err(Error::MerkleTreeError) } } diff --git a/core/src/types/storage.rs b/core/src/types/storage.rs index b27c1e250c..11a5cf70c4 100644 --- a/core/src/types/storage.rs +++ b/core/src/types/storage.rs @@ -1079,6 +1079,19 @@ impl Epochs { } None } + + /// Look-up the starting block height of an epoch before a given height. + pub fn get_epoch_start_height( + &self, + height: BlockHeight, + ) -> Option { + for start_height in self.first_block_heights.iter().rev() { + if *start_height <= height { + return Some(*start_height); + } + } + None + } } /// A value of a storage prefix iterator. @@ -1180,10 +1193,30 @@ mod tests { epochs.new_epoch(BlockHeight(10), max_age_num_blocks); println!("epochs {:#?}", epochs); assert_eq!(epochs.get_epoch(BlockHeight(0)), Some(Epoch(0))); + assert_eq!( + epochs.get_epoch_start_height(BlockHeight(0)), + Some(BlockHeight(0)) + ); assert_eq!(epochs.get_epoch(BlockHeight(9)), Some(Epoch(0))); + assert_eq!( + epochs.get_epoch_start_height(BlockHeight(9)), + Some(BlockHeight(0)) + ); assert_eq!(epochs.get_epoch(BlockHeight(10)), Some(Epoch(1))); + assert_eq!( + epochs.get_epoch_start_height(BlockHeight(10)), + Some(BlockHeight(10)) + ); assert_eq!(epochs.get_epoch(BlockHeight(11)), Some(Epoch(1))); + assert_eq!( + epochs.get_epoch_start_height(BlockHeight(11)), + Some(BlockHeight(10)) + ); assert_eq!(epochs.get_epoch(BlockHeight(100)), Some(Epoch(1))); + assert_eq!( + epochs.get_epoch_start_height(BlockHeight(100)), + Some(BlockHeight(10)) + ); // epoch 2 epochs.new_epoch(BlockHeight(20), max_age_num_blocks); @@ -1192,8 +1225,20 @@ mod tests { assert_eq!(epochs.get_epoch(BlockHeight(9)), Some(Epoch(0))); assert_eq!(epochs.get_epoch(BlockHeight(10)), Some(Epoch(1))); assert_eq!(epochs.get_epoch(BlockHeight(11)), Some(Epoch(1))); + assert_eq!( + epochs.get_epoch_start_height(BlockHeight(11)), + Some(BlockHeight(10)) + ); assert_eq!(epochs.get_epoch(BlockHeight(20)), Some(Epoch(2))); + assert_eq!( + epochs.get_epoch_start_height(BlockHeight(20)), + Some(BlockHeight(20)) + ); assert_eq!(epochs.get_epoch(BlockHeight(100)), Some(Epoch(2))); + assert_eq!( + epochs.get_epoch_start_height(BlockHeight(100)), + Some(BlockHeight(20)) + ); // epoch 3, epoch 0 and 1 should be trimmed epochs.new_epoch(BlockHeight(200), max_age_num_blocks); @@ -1204,7 +1249,15 @@ mod tests { assert_eq!(epochs.get_epoch(BlockHeight(11)), None); assert_eq!(epochs.get_epoch(BlockHeight(20)), Some(Epoch(2))); assert_eq!(epochs.get_epoch(BlockHeight(100)), Some(Epoch(2))); + assert_eq!( + epochs.get_epoch_start_height(BlockHeight(100)), + Some(BlockHeight(20)) + ); assert_eq!(epochs.get_epoch(BlockHeight(200)), Some(Epoch(3))); + assert_eq!( + epochs.get_epoch_start_height(BlockHeight(200)), + Some(BlockHeight(200)) + ); // increase the limit max_age_num_blocks = 200; From 5e4566dbd7f1951f20b76368b25e665abbb5e9cc Mon Sep 17 00:00:00 2001 From: yito88 Date: Wed, 1 Feb 2023 22:04:44 +0100 Subject: [PATCH 02/10] add changelog --- .changelog/unreleased/improvements/1113-write-tree-stores.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/improvements/1113-write-tree-stores.md diff --git a/.changelog/unreleased/improvements/1113-write-tree-stores.md b/.changelog/unreleased/improvements/1113-write-tree-stores.md new file mode 100644 index 0000000000..573166cb8a --- /dev/null +++ b/.changelog/unreleased/improvements/1113-write-tree-stores.md @@ -0,0 +1,2 @@ +- Write Merkle tree stores only when a new epoch + ([#1113](https://github.com/anoma/namada/issues/1113)) \ No newline at end of file From 98f7f415428675c0475f0da10842e1761012723d Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Wed, 1 Feb 2023 22:21:28 +0000 Subject: [PATCH 03/10] [ci] wasm checksums update --- wasm/checksums.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wasm/checksums.json b/wasm/checksums.json index c36511452a..2f2a607c0b 100644 --- a/wasm/checksums.json +++ b/wasm/checksums.json @@ -1,7 +1,7 @@ { "tx_bond.wasm": "tx_bond.c69b0b6b85a8340473dace3e927bc01be12cb5ae82d3367938d0005522a29479.wasm", "tx_change_validator_commission.wasm": "tx_change_validator_commission.97b0d6f07c9db41320f1006e12e726098c93aad75eb843a575222c8f305462e7.wasm", - "tx_ibc.wasm": "tx_ibc.3637ae4b46f854b288c01e74eccc63b7e45c9c2d1ee3098b698f4039a6010705.wasm", + "tx_ibc.wasm": "tx_ibc.fb29789c4591793d7658032ca2c7163a3212f5d71cde83117a1e65970860032d.wasm", "tx_init_account.wasm": "tx_init_account.7aa4dbbf0ecad2d079766c47bf6c6d210523d1c4d74d118a02cde6427460a8c8.wasm", "tx_init_proposal.wasm": "tx_init_proposal.4a977c3d205b68114c6ec8f4ae8d933b768f146759a35de21796395626ca5d43.wasm", "tx_init_validator.wasm": "tx_init_validator.34b54635942c83de4e4dc94445753ffe55942ebef8a95393ffb10d487e681822.wasm", From ae3eebc31ae75f1b81236dc60dfa6e1ef77cf9da Mon Sep 17 00:00:00 2001 From: yito88 Date: Thu, 2 Feb 2023 10:44:16 +0100 Subject: [PATCH 04/10] fix for the first height --- apps/src/lib/node/ledger/storage/mod.rs | 9 ++++++++- apps/src/lib/node/ledger/storage/rocksdb.rs | 8 +++++--- core/src/ledger/storage/mod.rs | 1 + 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/apps/src/lib/node/ledger/storage/mod.rs b/apps/src/lib/node/ledger/storage/mod.rs index 39634774de..637b64c161 100644 --- a/apps/src/lib/node/ledger/storage/mod.rs +++ b/apps/src/lib/node/ledger/storage/mod.rs @@ -383,6 +383,13 @@ mod tests { let mut roots = HashMap::new(); + // write values at Height 0 like init_storage + for i in 0..num_keys { + let key = Key::parse(format!("key{}", i)).unwrap(); + let value_bytes = types::encode(&storage.block.height); + storage.write(&key, value_bytes)?; + } + // Update and commit let hash = BlockHash::default(); storage.begin_block(hash, BlockHeight(1))?; @@ -421,7 +428,7 @@ mod tests { let mut current_state = HashMap::new(); for i in 0..num_keys { let key = Key::parse(format!("key{}", i)).unwrap(); - current_state.insert(key, false); + current_state.insert(key, true); } // Check a Merkle tree for (height, key, write_type) in blocks_write_type { diff --git a/apps/src/lib/node/ledger/storage/rocksdb.rs b/apps/src/lib/node/ledger/storage/rocksdb.rs index bfcf53603c..d73c8b9a6a 100644 --- a/apps/src/lib/node/ledger/storage/rocksdb.rs +++ b/apps/src/lib/node/ledger/storage/rocksdb.rs @@ -601,9 +601,11 @@ impl DB for RocksDB { Some(b) => types::decode(b).map_err(Error::CodingError)?, None => return Ok(None), }; - let stored_height = pred_epochs - .get_epoch_start_height(height) - .ok_or(Error::NoMerkleTree { height })?; + // Read the tree at the first height if no epoch update + let stored_height = match pred_epochs.get_epoch_start_height(height) { + Some(BlockHeight(0)) | None => BlockHeight(1), + Some(h) => h, + }; let tree_key = Key::from(stored_height.to_db_key()) .push(&"tree".to_owned()) diff --git a/core/src/ledger/storage/mod.rs b/core/src/ledger/storage/mod.rs index fd2d73e017..e63feb1fb1 100644 --- a/core/src/ledger/storage/mod.rs +++ b/core/src/ledger/storage/mod.rs @@ -442,6 +442,7 @@ where /// Persist the current block's state to the database pub fn commit(&mut self) -> Result<()> { + // All states are written only when the first height or a new epoch let is_full_commit = self.block.height.0 == 1 || self.last_epoch != self.block.epoch; let state = BlockStateWrite { From fad858adbd50148b73f0630c078d22103c7c1dc6 Mon Sep 17 00:00:00 2001 From: yito88 Date: Fri, 3 Feb 2023 19:16:07 +0100 Subject: [PATCH 05/10] compare keys as string --- core/src/ledger/storage/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/src/ledger/storage/mod.rs b/core/src/ledger/storage/mod.rs index e63feb1fb1..28914cc4c3 100644 --- a/core/src/ledger/storage/mod.rs +++ b/core/src/ledger/storage/mod.rs @@ -651,7 +651,8 @@ where .expect("the key should be parsable"); let new_key = Key::parse(new.0.clone()) .expect("the key should be parsable"); - match old_key.cmp(&new_key) { + // compare keys as String + match old.0.cmp(&new.0) { Ordering::Equal => { // the value was updated tree.update(&new_key, new.1.clone())?; From 695424aa8b6ce028ea13457c6ee6c9428ba27d3b Mon Sep 17 00:00:00 2001 From: yito88 Date: Fri, 3 Feb 2023 22:11:25 +0100 Subject: [PATCH 06/10] fix the unit test --- apps/src/lib/node/ledger/storage/mod.rs | 22 +++++++++++++++++---- apps/src/lib/node/ledger/storage/rocksdb.rs | 2 +- core/src/ledger/storage/mod.rs | 10 ++++------ 3 files changed, 23 insertions(+), 11 deletions(-) diff --git a/apps/src/lib/node/ledger/storage/mod.rs b/apps/src/lib/node/ledger/storage/mod.rs index 637b64c161..75d95985c9 100644 --- a/apps/src/lib/node/ledger/storage/mod.rs +++ b/apps/src/lib/node/ledger/storage/mod.rs @@ -253,7 +253,7 @@ mod tests { } #[test] - fn test_get_merkle_tree(blocks_write_type in vec(0..3u64, 20)) { + fn test_get_merkle_tree(blocks_write_type in vec(0..5_u64, 50)) { test_get_merkle_tree_aux(blocks_write_type).unwrap() } } @@ -370,7 +370,7 @@ mod tests { None, ); - let num_keys = 3; + let num_keys = 5; let blocks_write_type = blocks_write_type.into_iter().enumerate().map( |(index, write_type)| { // try to update some keys at each height @@ -394,6 +394,7 @@ mod tests { let hash = BlockHash::default(); storage.begin_block(hash, BlockHeight(1))?; for (height, key, write_type) in blocks_write_type.clone() { + let mut batch = PersistentStorage::batch(); if height != storage.block.height { // to check the root later roots.insert(storage.block.height, storage.merkle_root()); @@ -417,13 +418,26 @@ mod tests { 1 => { storage.delete(&key)?; } - _ => { + 2 => { let value_bytes = types::encode(&storage.block.height); storage.write(&key, value_bytes)?; } + 3 => { + storage.batch_delete_subspace_val(&mut batch, &key)?; + } + _ => { + let value_bytes = types::encode(&storage.block.height); + storage.batch_write_subspace_val( + &mut batch, + &key, + value_bytes, + )?; + } } + storage.exec_batch(batch)?; } roots.insert(storage.block.height, storage.merkle_root()); + storage.commit()?; let mut current_state = HashMap::new(); for i in 0..num_keys { @@ -442,7 +456,7 @@ mod tests { assert!(!tree.has_key(&key)?); } } - 1 => { + 1 | 3 => { assert!(!tree.has_key(&key)?); current_state.insert(key, false); } diff --git a/apps/src/lib/node/ledger/storage/rocksdb.rs b/apps/src/lib/node/ledger/storage/rocksdb.rs index d73c8b9a6a..6528a7a1e4 100644 --- a/apps/src/lib/node/ledger/storage/rocksdb.rs +++ b/apps/src/lib/node/ledger/storage/rocksdb.rs @@ -884,7 +884,7 @@ impl DB for RocksDB { // Check the length of previous value, if any let prev_len = match self .0 - .get(key.to_string()) + .get(subspace_key.to_string()) .map_err(|e| Error::DBError(e.into_string()))? { Some(prev_value) => { diff --git a/core/src/ledger/storage/mod.rs b/core/src/ledger/storage/mod.rs index 28914cc4c3..91fbc6577f 100644 --- a/core/src/ledger/storage/mod.rs +++ b/core/src/ledger/storage/mod.rs @@ -628,12 +628,10 @@ where &self, height: BlockHeight, ) -> Result> { - let (stored_height, stores) = - self.db.read_merkle_tree_stores(height)?.unwrap_or(( - // restore from the first height - BlockHeight::default(), - MerkleTreeStoresRead::default(), - )); + let (stored_height, stores) = self + .db + .read_merkle_tree_stores(height)? + .ok_or(Error::NoMerkleTree { height })?; // Restore the tree state with diffs let mut tree = MerkleTree::::new(stores).expect("invalid stores"); let mut target_height = stored_height; From d185de8046980d67bf5b32320691afe4765ab785 Mon Sep 17 00:00:00 2001 From: yito88 Date: Wed, 8 Feb 2023 11:01:38 +0100 Subject: [PATCH 07/10] fix to rebuild merkle tree before read --- core/src/ledger/storage/mod.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/core/src/ledger/storage/mod.rs b/core/src/ledger/storage/mod.rs index 91fbc6577f..21a2c347ee 100644 --- a/core/src/ledger/storage/mod.rs +++ b/core/src/ledger/storage/mod.rs @@ -400,6 +400,9 @@ where self.next_epoch_min_start_height = next_epoch_min_start_height; self.next_epoch_min_start_time = next_epoch_min_start_time; self.address_gen = address_gen; + // Rebuild Merkle tree + self.block.tree = MerkleTree::new(merkle_tree_stores) + .or_else(|_| self.get_merkle_tree(height))?; if self.last_epoch.0 > 1 { // The derived conversions will be placed in MASP address space let masp_addr = masp(); @@ -417,8 +420,6 @@ where ) .expect("unable to decode conversion state") } - self.block.tree = MerkleTree::new(merkle_tree_stores) - .or_else(|_| self.get_merkle_tree(height))?; #[cfg(feature = "ferveo-tpke")] { self.tx_queue = tx_queue; From e0d79ad265bd84a28fd80b18b5a742658afa976b Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Wed, 8 Feb 2023 10:48:39 +0000 Subject: [PATCH 08/10] [ci] wasm checksums update --- wasm/checksums.json | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/wasm/checksums.json b/wasm/checksums.json index 7c2b824921..76176c80bf 100644 --- a/wasm/checksums.json +++ b/wasm/checksums.json @@ -1,20 +1,20 @@ { - "tx_bond.wasm": "tx_bond.f0094b887c57565472bede01d98fb77f6faac2f72597e2efb2ebfe9b1bf7c234.wasm", + "tx_bond.wasm": "tx_bond.8e1f2150c4cc45493003eab69ee30491a47e15bcd0817911f42690a780e2bad7.wasm", "tx_change_validator_commission.wasm": "tx_change_validator_commission.02dca468021b1ec811d0f35cc4b55a24f7c3f7b5e51f16399709257421f4a1f4.wasm", - "tx_ibc.wasm": "tx_ibc.a1735e3221f1ae055c74bb52327765dd37e8676e15fab496f9ab0ed4d0628f51.wasm", - "tx_init_account.wasm": "tx_init_account.7b6eafeceb81b679c382279a5d9c40dfd81fcf37e5a1940340355c9f55af1543.wasm", + "tx_ibc.wasm": "tx_ibc.d973b3e5aca575d1468a29cc516188956007c979ea31b89ec94b4d0ee090d307.wasm", + "tx_init_account.wasm": "tx_init_account.54ef8abc2493bb3de852f1cb6480b52278c579f25be7b30002c6e4252ab932d5.wasm", "tx_init_proposal.wasm": "tx_init_proposal.f2ed71fe70fc564e1d67e4e7d2ea25466327b62ba2eee18ece0021abff9e2c82.wasm", - "tx_init_validator.wasm": "tx_init_validator.fedcfaecaf37e3e7d050c76a4512baa399fc528710a27038573df53596613a2c.wasm", - "tx_reveal_pk.wasm": "tx_reveal_pk.3e5417561e8108d4045775bf6d095cbaad22c73ff17a5ba2ad11a1821665a58a.wasm", + "tx_init_validator.wasm": "tx_init_validator.f11b163a34df813ae68910abad81cf9726f5b796ffcf6535aad691b5bdfe8fd5.wasm", + "tx_reveal_pk.wasm": "tx_reveal_pk.695dc1e16633f7ad3dec82d7df0cbc3f099f8bf3d31aa475872078ae61501198.wasm", "tx_transfer.wasm": "tx_transfer.833a3849ca2c417f4e907c95c6eb15e6b52827458cf603e1c4f5511ab3e4fe76.wasm", "tx_unbond.wasm": "tx_unbond.d4fd6c94abb947533a2728940b43fb021a008ad593c7df7a3886c4260cac39b5.wasm", "tx_update_vp.wasm": "tx_update_vp.6d1eabab15dc6d04eec5b25ad687f026a4d6c3f118a1d7aca9232394496bd845.wasm", "tx_vote_proposal.wasm": "tx_vote_proposal.54b594f686a72869b0d7f15492591854f26e287a1cf3b6e543b0246d5ac61003.wasm", - "tx_withdraw.wasm": "tx_withdraw.342c222d0707eb5b5a44b89fc1245f527be3fdf841af64152a9ab35a4766e1b5.wasm", - "vp_implicit.wasm": "vp_implicit.73678ac01aa009ac4e0d4a49eecaa19b49cdd3d95f6862a9558c9b175ae68260.wasm", + "tx_withdraw.wasm": "tx_withdraw.a5167cc5beadd4b6a3be04ad90eb40a8de3b0175564fe1aec4ebcb70d620a2de.wasm", + "vp_implicit.wasm": "vp_implicit.4eda2a9946b18309e1b00c12dbc2ec0794d8dc7a61ef0d695d56f6e1a7b8eb9f.wasm", "vp_masp.wasm": "vp_masp.85446251f8e1defed81549dab37edfe8e640339c7230e678b65340cf71ce1369.wasm", - "vp_testnet_faucet.wasm": "vp_testnet_faucet.573b882a806266d6cdfa635fe803e46d6ce89c99321196c231c61d05193a086d.wasm", + "vp_testnet_faucet.wasm": "vp_testnet_faucet.8915e0e5d28cf12d49971c65503887f9071edd02fcb8e74ccc0a4b5ce208d864.wasm", "vp_token.wasm": "vp_token.8c6e5a86f047e7b1f1004f0d8a4e91fad1b1c0226a6e42d7fe350f98dc84359b.wasm", - "vp_user.wasm": "vp_user.75c68f018f163d18d398cb4082b261323d115aae43ec021c868d1128e4b0ee29.wasm", - "vp_validator.wasm": "vp_validator.2dc9f1c8f106deeef5ee988955733955444d16b400ebb16a25e7d71e4b1be874.wasm" + "vp_user.wasm": "vp_user.5ca1c4598f85b87d1920924e08c51601bb8ba87f93fa20043696c80173a9072a.wasm", + "vp_validator.wasm": "vp_validator.25eba1b9ce7789981e8691473d27c55a393056921bc01d1fe467594c5be9f932.wasm" } \ No newline at end of file From 294f86f4f645f01516ab368f83012e2d913bbe35 Mon Sep 17 00:00:00 2001 From: yito88 Date: Mon, 13 Feb 2023 21:02:07 +0100 Subject: [PATCH 09/10] retry CI From a674174a770617040883a71e8f3d313e3f9147f3 Mon Sep 17 00:00:00 2001 From: yito88 Date: Mon, 20 Feb 2023 17:08:37 +0100 Subject: [PATCH 10/10] fix comments --- core/src/ledger/storage/mod.rs | 3 ++- core/src/types/storage.rs | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/core/src/ledger/storage/mod.rs b/core/src/ledger/storage/mod.rs index 2883a2c0b3..c86f46d27a 100644 --- a/core/src/ledger/storage/mod.rs +++ b/core/src/ledger/storage/mod.rs @@ -209,7 +209,8 @@ pub trait DB: std::fmt::Debug { /// Read the last committed block's metadata fn read_last_block(&mut self) -> Result>; - /// Write block's metadata + /// Write block's metadata. Merkle tree sub-stores are committed only when + /// `is_full_commit` is `true` (typically on a beginning of a new epoch). fn write_block( &mut self, state: BlockStateWrite, diff --git a/core/src/types/storage.rs b/core/src/types/storage.rs index 201cf16af8..527d9cd777 100644 --- a/core/src/types/storage.rs +++ b/core/src/types/storage.rs @@ -1107,7 +1107,8 @@ impl Epochs { None } - /// Look-up the starting block height of an epoch before a given height. + /// Look-up the starting block height of an epoch at or before a given + /// height. pub fn get_epoch_start_height( &self, height: BlockHeight,