diff --git a/.changelog/unreleased/features/3293-merklized-not-diffed.md b/.changelog/unreleased/features/3293-merklized-not-diffed.md new file mode 100644 index 0000000000..b77ebc0d1c --- /dev/null +++ b/.changelog/unreleased/features/3293-merklized-not-diffed.md @@ -0,0 +1,2 @@ +- Enable to write data to be updated in the subspace and Merkle tree, but not to + be updated in diffs ([\#3293](https://github.com/anoma/namada/issues/3293)) \ No newline at end of file diff --git a/crates/merkle_tree/Cargo.toml b/crates/merkle_tree/Cargo.toml index 56657bd76f..a53ad24b06 100644 --- a/crates/merkle_tree/Cargo.toml +++ b/crates/merkle_tree/Cargo.toml @@ -17,6 +17,7 @@ migrations = [ "namada_migrations", "linkme", ] +testing = ["namada_core/testing"] [dependencies] namada_core = { path = "../core" } diff --git a/crates/merkle_tree/src/lib.rs b/crates/merkle_tree/src/lib.rs index 3465dd15b0..cab4ae92c4 100644 --- a/crates/merkle_tree/src/lib.rs +++ b/crates/merkle_tree/src/lib.rs @@ -48,6 +48,9 @@ use namada_macros::BorshDeserializer; use namada_migrations::*; use thiserror::Error; +/// Key prefix for the data not stored to diffs +pub const NO_DIFF_KEY_PREFIX: &str = "no_diff"; + /// Trait for reading from a merkle tree that is a sub-tree /// of the global merkle tree. pub trait SubTreeRead { @@ -177,6 +180,8 @@ pub enum StoreType { PoS, /// For the Ethereum bridge Pool transfers BridgePool, + /// For data not stored to diffs + NoDiff, /// For the commit only data CommitData, } @@ -193,6 +198,8 @@ pub enum Store { PoS(SmtStore), /// For the Ethereum bridge Pool transfers BridgePool(BridgePoolStore), + /// For data not stored to diffs + NoDiff(SmtStore), /// For the commit only data CommitData, } @@ -206,6 +213,7 @@ impl Store { Self::Ibc(store) => StoreRef::Ibc(store), Self::PoS(store) => StoreRef::PoS(store), Self::BridgePool(store) => StoreRef::BridgePool(store), + Self::NoDiff(store) => StoreRef::NoDiff(store), Self::CommitData => StoreRef::CommitData, } } @@ -223,6 +231,8 @@ pub enum StoreRef<'a> { PoS(&'a SmtStore), /// For the Ethereum bridge Pool transfers BridgePool(&'a BridgePoolStore), + /// For data not stored to diffs + NoDiff(&'a SmtStore), /// For commit only data CommitData, } @@ -236,6 +246,7 @@ impl<'a> StoreRef<'a> { Self::Ibc(store) => Store::Ibc(store.to_owned()), Self::PoS(store) => Store::PoS(store.to_owned()), Self::BridgePool(store) => Store::BridgePool(store.to_owned()), + Self::NoDiff(store) => Store::NoDiff(store.to_owned()), Self::CommitData => Store::CommitData, } } @@ -248,6 +259,7 @@ impl<'a> StoreRef<'a> { Self::Ibc(store) => store.serialize_to_vec(), Self::PoS(store) => store.serialize_to_vec(), Self::BridgePool(store) => store.serialize_to_vec(), + Self::NoDiff(store) => store.serialize_to_vec(), Self::CommitData => vec![], } } @@ -256,12 +268,13 @@ impl<'a> StoreRef<'a> { impl StoreType { /// Get an iterator for the base tree and subtrees pub fn iter() -> std::slice::Iter<'static, Self> { - static SUB_TREE_TYPES: [StoreType; 6] = [ + static SUB_TREE_TYPES: [StoreType; 7] = [ StoreType::Base, StoreType::Account, StoreType::PoS, StoreType::Ibc, StoreType::BridgePool, + StoreType::NoDiff, StoreType::CommitData, ]; SUB_TREE_TYPES.iter() @@ -269,11 +282,12 @@ impl StoreType { /// Get an iterator for subtrees pub fn iter_subtrees() -> std::slice::Iter<'static, Self> { - static SUB_TREE_TYPES: [StoreType; 5] = [ + static SUB_TREE_TYPES: [StoreType; 6] = [ StoreType::Account, StoreType::PoS, StoreType::Ibc, StoreType::BridgePool, + StoreType::NoDiff, StoreType::CommitData, ]; SUB_TREE_TYPES.iter() @@ -293,6 +307,14 @@ impl StoreType { SUB_TREE_TYPES.iter() } + /// Return true if the subtree should be saved in every block + pub fn is_stored_every_block(&self) -> bool { + matches!( + self, + StoreType::Base | StoreType::NoDiff | StoreType::CommitData + ) + } + /// Get the store type and the sub key pub fn sub_key(key: &Key) -> Result<(Self, Key)> { if key.is_empty() { @@ -320,6 +342,9 @@ impl StoreType { _ => Ok((StoreType::Account, key.clone())), } } + Some(DbKeySeg::StringSeg(data)) if data.eq(NO_DIFF_KEY_PREFIX) => { + Ok((StoreType::NoDiff, key.clone())) + } Some(DbKeySeg::StringSeg(data)) if data.eq("commit_data") => { Ok((StoreType::CommitData, key.clone())) } @@ -352,6 +377,7 @@ impl StoreType { Self::Ibc => Ok(Store::Ibc(decode(bytes)?)), Self::PoS => Ok(Store::PoS(decode(bytes)?)), Self::BridgePool => Ok(Store::BridgePool(decode(bytes)?)), + Self::NoDiff => Ok(Store::NoDiff(decode(bytes)?)), Self::CommitData => Ok(Store::CommitData), } } @@ -367,6 +393,7 @@ impl FromStr for StoreType { "ibc" => Ok(StoreType::Ibc), "pos" => Ok(StoreType::PoS), "eth_bridge_pool" => Ok(StoreType::BridgePool), + "no_diff" => Ok(StoreType::NoDiff), "commit_data" => Ok(StoreType::CommitData), _ => Err(Error::StoreType(s.to_string())), } @@ -381,6 +408,7 @@ impl fmt::Display for StoreType { StoreType::Ibc => write!(f, "ibc"), StoreType::PoS => write!(f, "pos"), StoreType::BridgePool => write!(f, "eth_bridge_pool"), + StoreType::NoDiff => write!(f, "no_diff"), StoreType::CommitData => write!(f, "commit_data"), } } @@ -427,6 +455,7 @@ pub struct MerkleTree { ibc: Amt, pos: Smt, bridge_pool: BridgePoolTree, + no_diff: Smt, commit_data: CommitDataRoot, } @@ -448,6 +477,7 @@ impl MerkleTree { let pos = Smt::new(stores.pos.0.into(), stores.pos.1); let bridge_pool = BridgePoolTree::new(stores.bridge_pool.0, stores.bridge_pool.1); + let no_diff = Smt::new(stores.no_diff.0.into(), stores.no_diff.1); let commit_data = stores.commit.into(); let tree = Self { @@ -456,6 +486,7 @@ impl MerkleTree { ibc, pos, bridge_pool, + no_diff, commit_data, }; @@ -468,6 +499,8 @@ impl MerkleTree { let pos_root = tree.base.get(&pos_key.into())?; let bp_key = H::hash(StoreType::BridgePool.to_string()); let bp_root = tree.base.get(&bp_key.into())?; + let no_diff_key = H::hash(StoreType::NoDiff.to_string()); + let no_diff_root = tree.base.get(&no_diff_key.into())?; let commit_data_key = H::hash(StoreType::CommitData.to_string()); let commit_data_root = tree.base.get(&commit_data_key.into())?; if tree.base.root().is_zero() @@ -475,11 +508,13 @@ impl MerkleTree { && tree.ibc.root().is_zero() && tree.pos.root().is_zero() && tree.bridge_pool.root().is_zero() + && tree.no_diff.root().is_zero() && tree.commit_data.0.is_zero() || (account_root == tree.account.root().into() && ibc_root == tree.ibc.root().into() && pos_root == tree.pos.root().into() && bp_root == tree.bridge_pool.root().into() + && no_diff_root == tree.no_diff.root().into() && commit_data_root == tree.commit_data.0) { Ok(tree) @@ -498,6 +533,7 @@ impl MerkleTree { let pos = Smt::new(stores.pos.0.into(), stores.pos.1); let bridge_pool = BridgePoolTree::new(stores.bridge_pool.0, stores.bridge_pool.1); + let no_diff = Smt::new(stores.no_diff.0.into(), stores.no_diff.1); let commit_data = stores.commit.into(); Self { @@ -506,6 +542,7 @@ impl MerkleTree { ibc, pos, bridge_pool, + no_diff, commit_data, } } @@ -517,6 +554,7 @@ impl MerkleTree { StoreType::Ibc => Box::new(&self.ibc), StoreType::PoS => Box::new(&self.pos), StoreType::BridgePool => Box::new(&self.bridge_pool), + StoreType::NoDiff => Box::new(&self.no_diff), StoreType::CommitData => Box::new(&self.commit_data), } } @@ -531,6 +569,7 @@ impl MerkleTree { StoreType::Ibc => Box::new(&mut self.ibc), StoreType::PoS => Box::new(&mut self.pos), StoreType::BridgePool => Box::new(&mut self.bridge_pool), + StoreType::NoDiff => Box::new(&mut self.no_diff), StoreType::CommitData => Box::new(&mut self.commit_data), } } @@ -553,6 +592,7 @@ impl MerkleTree { } /// Check if the key exists in the tree + #[cfg(any(test, feature = "testing"))] pub fn has_key(&self, key: &Key) -> Result { let (store_type, sub_key) = StoreType::sub_key(key)?; self.tree(&store_type).subtree_has_key(&sub_key) @@ -601,6 +641,7 @@ impl MerkleTree { && self.ibc.validate() && self.pos.validate() && self.bridge_pool.validate() + && self.no_diff.validate() { let mut reconstructed = Smt::::default(); reconstructed.update( @@ -619,6 +660,10 @@ impl MerkleTree { H::hash(StoreType::BridgePool.to_string()).into(), self.bridge_pool.root().into(), )?; + reconstructed.update( + H::hash(StoreType::NoDiff.to_string()).into(), + self.no_diff.root().into(), + )?; reconstructed.update( H::hash(StoreType::CommitData.to_string()).into(), self.commit_data.0, @@ -649,6 +694,7 @@ impl MerkleTree { self.bridge_pool.root().into(), self.bridge_pool.store(), ), + no_diff: (self.no_diff.root().into(), self.no_diff.store()), commit: self.commit_data.0, } } @@ -803,6 +849,7 @@ pub struct MerkleTreeStoresRead { ibc: (Hash, AmtStore), pos: (Hash, SmtStore), bridge_pool: (KeccakHash, BridgePoolStore), + no_diff: (Hash, SmtStore), commit: Hash, } @@ -815,6 +862,7 @@ impl MerkleTreeStoresRead { StoreType::Ibc => self.ibc.0 = root, StoreType::PoS => self.pos.0 = root, StoreType::BridgePool => self.bridge_pool.0 = root.into(), + StoreType::NoDiff => self.no_diff.0 = root, StoreType::CommitData => self.commit = root, } } @@ -827,6 +875,7 @@ impl MerkleTreeStoresRead { Store::Ibc(store) => self.ibc.1 = store, Store::PoS(store) => self.pos.1 = store, Store::BridgePool(store) => self.bridge_pool.1 = store, + Store::NoDiff(store) => self.no_diff.1 = store, Store::CommitData => (), } } @@ -839,6 +888,7 @@ impl MerkleTreeStoresRead { StoreType::Ibc => StoreRef::Ibc(&self.ibc.1), StoreType::PoS => StoreRef::PoS(&self.pos.1), StoreType::BridgePool => StoreRef::BridgePool(&self.bridge_pool.1), + StoreType::NoDiff => StoreRef::NoDiff(&self.no_diff.1), StoreType::CommitData => StoreRef::CommitData, } } @@ -851,6 +901,7 @@ impl MerkleTreeStoresRead { StoreType::Ibc => self.ibc.0, StoreType::PoS => self.pos.0, StoreType::BridgePool => Hash(self.bridge_pool.0.0), + StoreType::NoDiff => self.no_diff.0, StoreType::CommitData => Hash(self.commit.0), } } @@ -863,6 +914,7 @@ pub struct MerkleTreeStoresWrite<'a> { ibc: (Hash, &'a AmtStore), pos: (Hash, &'a SmtStore), bridge_pool: (Hash, &'a BridgePoolStore), + no_diff: (Hash, &'a SmtStore), commit: Hash, } @@ -875,6 +927,7 @@ impl<'a> MerkleTreeStoresWrite<'a> { StoreType::Ibc => &self.ibc.0, StoreType::PoS => &self.pos.0, StoreType::BridgePool => &self.bridge_pool.0, + StoreType::NoDiff => &self.no_diff.0, StoreType::CommitData => &self.commit, } } @@ -887,6 +940,7 @@ impl<'a> MerkleTreeStoresWrite<'a> { StoreType::Ibc => StoreRef::Ibc(self.ibc.1), StoreType::PoS => StoreRef::PoS(self.pos.1), StoreType::BridgePool => StoreRef::BridgePool(self.bridge_pool.1), + StoreType::NoDiff => StoreRef::NoDiff(self.no_diff.1), StoreType::CommitData => StoreRef::CommitData, } } diff --git a/crates/node/src/shell/mod.rs b/crates/node/src/shell/mod.rs index b119c47bcd..610f43d0c6 100644 --- a/crates/node/src/shell/mod.rs +++ b/crates/node/src/shell/mod.rs @@ -353,9 +353,9 @@ where event_log: EventLog, } -/// Merkle tree storage key filter. Return `false` for keys that shouldn't be -/// merklized. -pub fn is_merklized_storage_key(key: &namada_sdk::storage::Key) -> bool { +/// Storage key filter to store the diffs into the storage. Return `false` for +/// keys whose diffs shouldn't be stored. +pub fn is_key_diff_storable(key: &namada_sdk::storage::Key) -> bool { !(token::storage_key::is_masp_key(key) && *key != token::storage_key::masp_convert_anchor_key() && *key != token::storage_key::masp_token_map_key() @@ -441,7 +441,7 @@ where chain_id.clone(), native_token, config.shell.storage_read_past_height_limit, - is_merklized_storage_key, + is_key_diff_storable, ); let vp_wasm_cache_dir = base_dir.join(chain_id.as_str()).join("vp_wasm_cache"); diff --git a/crates/node/src/storage/mod.rs b/crates/node/src/storage/mod.rs index 610e87b57b..5e3c030cf8 100644 --- a/crates/node/src/storage/mod.rs +++ b/crates/node/src/storage/mod.rs @@ -61,17 +61,19 @@ mod tests { use namada::core::ethereum_events::Uint; use namada::core::hash::Hash; use namada::core::keccak::KeccakHash; - use namada::core::storage::{BlockHeight, Key}; + use namada::core::storage::{BlockHeight, Key, KeySeg}; use namada::core::time::DurationSecs; use namada::core::{address, storage}; use namada::eth_bridge::storage::proof::BridgePoolRootProof; + use namada::ibc::storage::is_ibc_key; use namada::ledger::eth_bridge::storage::bridge_pool; use namada::ledger::gas::STORAGE_ACCESS_GAS_PER_BYTE; - use namada::ledger::ibc::storage::ibc_key; + use namada::ledger::ibc::storage::{client_counter_key, ibc_key}; use namada::ledger::parameters::{EpochDuration, Parameters}; use namada::state::{self, StorageRead, StorageWrite, StoreType, DB}; use namada::token::conversion::update_allowed_conversions; use namada::{decode, encode, parameters}; + use namada_sdk::state::merkle_tree::NO_DIFF_KEY_PREFIX; use namada_sdk::state::StateRead; use proptest::collection::vec; use proptest::prelude::*; @@ -79,7 +81,7 @@ mod tests { use tempfile::TempDir; use super::*; - use crate::shell::is_merklized_storage_key; + use crate::shell::is_key_diff_storable; #[test] fn test_crud_value() { @@ -91,7 +93,7 @@ mod tests { ChainId::default(), address::testing::nam(), None, - is_merklized_storage_key, + is_key_diff_storable, ); let key = Key::parse("key").expect("cannot parse the key string"); let value: u64 = 1; @@ -143,7 +145,7 @@ mod tests { ChainId::default(), address::testing::nam(), None, - is_merklized_storage_key, + is_key_diff_storable, ); state .in_mem_mut() @@ -205,7 +207,7 @@ mod tests { ChainId::default(), address::testing::nam(), None, - is_merklized_storage_key, + is_key_diff_storable, ); let (loaded_root, height) = state.in_mem().get_state().expect("no block exists"); @@ -226,7 +228,7 @@ mod tests { ChainId::default(), address::testing::nam(), None, - is_merklized_storage_key, + is_key_diff_storable, ); let mut expected = Vec::new(); @@ -270,7 +272,7 @@ mod tests { ChainId::default(), address::testing::nam(), None, - is_merklized_storage_key, + is_key_diff_storable, ); state .in_mem_mut() @@ -342,7 +344,7 @@ mod tests { ChainId::default(), address::testing::nam(), None, - is_merklized_storage_key, + is_key_diff_storable, ); // 1. For each `blocks_write_value`, write the current block height if @@ -436,16 +438,28 @@ mod tests { ChainId::default(), address::testing::nam(), None, - is_merklized_storage_key, + is_key_diff_storable, ); + // Prepare written keys for non-provable data, provable data (IBC), and + // no diffed data + let make_key = |suffix: u64| { + // For three type keys + match suffix % 3u64 { + // for non-provable data + 0 => Key::parse(format!("key{suffix}")).unwrap(), + // for provable data + 1 => ibc_key(format!("key{suffix}")).unwrap(), + // for no diff + _ => client_counter_key(), + } + }; 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 let height = BlockHeight::from(index as u64 / num_keys + 1); - let key = - ibc_key(format!("key{}", index as u64 % num_keys)).unwrap(); + let key = make_key(index as u64 % num_keys); (height, key, write_type) }, ); @@ -454,7 +468,7 @@ mod tests { // write values at Height 0 like init_storage for i in 0..num_keys { - let key = ibc_key(format!("key{}", i)).unwrap(); + let key = make_key(i); let value_bytes = encode(&state.in_mem().block.height); state.db_write(&key, value_bytes)?; } @@ -513,20 +527,26 @@ mod tests { } } } + // save the last root roots.insert(state.in_mem().block.height, state.in_mem().merkle_root()); state.commit_block_from_batch(batch)?; let mut current_state = HashMap::new(); for i in 0..num_keys { - let key = ibc_key(format!("key{}", i)).unwrap(); + let key = make_key(i); current_state.insert(key, true); } - // Check a Merkle tree - for (height, key, write_type) in blocks_write_type { + // Check IBC subtree + for (height, key, write_type) in blocks_write_type.clone() { + if !is_ibc_key(&key) || key == client_counter_key() { + continue; + } let tree = state.get_merkle_tree(height, Some(StoreType::Ibc))?; + // Check if the rebuilt tree's root is the same as the saved one assert_eq!(tree.root().0, roots.get(&height).unwrap().0); match write_type { 0 => { + // data was not updated if *current_state.get(&key).unwrap() { assert!(tree.has_key(&key)?); } else { @@ -534,16 +554,57 @@ mod tests { } } 1 | 3 => { + // data was deleted assert!(!tree.has_key(&key)?); current_state.insert(key, false); } _ => { + // data was updated assert!(tree.has_key(&key)?); current_state.insert(key, true); } } } + // Check NoDiff subtree + let mut current_state = HashMap::new(); + for i in 0..num_keys { + let key = make_key(i); + current_state.insert(key, true); + } + for (height, key, write_type) in blocks_write_type { + if key != client_counter_key() { + continue; + } + let merkle_key = + Key::from(NO_DIFF_KEY_PREFIX.to_string().to_db_key()) + .join(&key); + let tree = + state.get_merkle_tree(height, Some(StoreType::NoDiff))?; + // Check if the rebuilt tree's root is the same as the saved one + assert_eq!(tree.root().0, roots.get(&height).unwrap().0); + match write_type { + 0 => { + // data was not updated + if *current_state.get(&key).unwrap() { + assert!(tree.has_key(&merkle_key)?); + } else { + assert!(!tree.has_key(&merkle_key)?); + } + } + 1 | 3 => { + // data was deleted + assert!(!tree.has_key(&merkle_key)?); + current_state.insert(key, false); + } + _ => { + // data was updated + assert!(tree.has_key(&merkle_key)?); + current_state.insert(key, true); + } + } + } + Ok(()) } @@ -558,7 +619,7 @@ mod tests { ChainId::default(), address::testing::nam(), Some(5), - is_merklized_storage_key, + is_key_diff_storable, ); let new_epoch_start = BlockHeight(1); let signed_root_key = bridge_pool::get_signed_root_key(); @@ -680,7 +741,7 @@ mod tests { ChainId::default(), address::testing::nam(), None, - is_merklized_storage_key, + is_key_diff_storable, ); let prefix = storage::Key::parse("prefix").unwrap(); diff --git a/crates/node/src/storage/rocksdb.rs b/crates/node/src/storage/rocksdb.rs index f178f7149f..c5b91e95c8 100644 --- a/crates/node/src/storage/rocksdb.rs +++ b/crates/node/src/storage/rocksdb.rs @@ -903,15 +903,11 @@ impl DB for RocksDB { // Merkle tree for st in StoreType::iter() { - if *st == StoreType::Base - || *st == StoreType::CommitData - || is_full_commit - { - let key_prefix = match st { - StoreType::Base | StoreType::CommitData => { - tree_key_prefix_with_height(st, height) - } - _ => tree_key_prefix_with_epoch(st, epoch), + if st.is_stored_every_block() || is_full_commit { + let key_prefix = if st.is_stored_every_block() { + tree_key_prefix_with_height(st, height) + } else { + tree_key_prefix_with_epoch(st, epoch) }; let root_key = format!("{key_prefix}/{MERKLE_TREE_ROOT_KEY_SEGMENT}"); @@ -979,11 +975,10 @@ impl DB for RocksDB { .map(|st| Either::Left(std::iter::once(st))) .unwrap_or_else(|| Either::Right(StoreType::iter())); for st in store_types { - let key_prefix = match st { - StoreType::Base | StoreType::CommitData => { - tree_key_prefix_with_height(st, base_height) - } - _ => tree_key_prefix_with_epoch(st, epoch), + let key_prefix = if st.is_stored_every_block() { + tree_key_prefix_with_height(st, base_height) + } else { + tree_key_prefix_with_epoch(st, epoch) }; let root_key = format!("{key_prefix}/{MERKLE_TREE_ROOT_KEY_SEGMENT}"); diff --git a/crates/state/Cargo.toml b/crates/state/Cargo.toml index 12d2c4ff2f..69c34303af 100644 --- a/crates/state/Cargo.toml +++ b/crates/state/Cargo.toml @@ -16,7 +16,11 @@ version.workspace = true default = [] # for integration tests and test utilities -testing = ["proptest", "namada_core/testing"] +testing = [ + "proptest", + "namada_core/testing", + "namada_merkle_tree/testing" +] migrations = [ "namada_migrations", "linkme", @@ -50,6 +54,7 @@ proptest = { workspace = true, optional = true } [dev-dependencies] namada_core = { path = "../core", features = ["testing"] } +namada_merkle_tree = { path = "../merkle_tree", features = ["testing"] } assert_matches.workspace = true chrono.workspace = true diff --git a/crates/state/src/lib.rs b/crates/state/src/lib.rs index 2f136ca9d2..b7ff50f837 100644 --- a/crates/state/src/lib.rs +++ b/crates/state/src/lib.rs @@ -612,12 +612,12 @@ pub mod testing { write_log: Default::default(), db: MockDB::default(), in_mem: Default::default(), - merkle_tree_key_filter: merklize_all_keys, + diff_key_filter: diff_all_keys, }) } } - fn merklize_all_keys(_key: &storage::Key) -> bool { + fn diff_all_keys(_key: &storage::Key) -> bool { true } @@ -671,6 +671,7 @@ mod tests { use std::collections::BTreeMap; use chrono::{TimeZone, Utc}; + use merkle_tree::NO_DIFF_KEY_PREFIX; use namada_core::address::InternalAddress; use namada_core::borsh::{BorshDeserialize, BorshSerializeExt}; use namada_core::storage::DbKeySeg; @@ -895,16 +896,16 @@ mod tests { Key::parse("testing2").unwrap() } - fn merkle_tree_key_filter(key: &Key) -> bool { + fn diff_key_filter(key: &Key) -> bool { key == &test_key_1() } #[test] - fn test_writing_without_merklizing_or_diffs() { + fn test_writing_without_diffs() { let mut state = TestState::default(); assert_eq!(state.in_mem().block.height.0, 0); - (state.0.merkle_tree_key_filter) = merkle_tree_key_filter; + (state.0.diff_key_filter) = diff_key_filter; let key1 = test_key_1(); let val1 = 1u64; @@ -952,9 +953,14 @@ mod tests { let res2 = u64::try_from_slice(&res2).unwrap(); assert_eq!(res2, val2); - // Check explicitly that key-val-2 is not in merkle tree + // Check merkle tree inclusion of key-val-2 explicitly let is_merklized2 = state.in_mem().block.tree.has_key(&key2).unwrap(); assert!(!is_merklized2); + let no_diff_key2 = + Key::from(NO_DIFF_KEY_PREFIX.to_string().to_db_key()).join(&key2); + let is_merklized2 = + state.in_mem().block.tree.has_key(&no_diff_key2).unwrap(); + assert!(is_merklized2); // Check that the proper diffs exist for key-val-1 let res1 = state @@ -1005,7 +1011,8 @@ mod tests { // Check that the key-vals don't exist in the merkle tree anymore let is_merklized1 = state.in_mem().block.tree.has_key(&key1).unwrap(); - let is_merklized2 = state.in_mem().block.tree.has_key(&key2).unwrap(); + let is_merklized2 = + state.in_mem().block.tree.has_key(&no_diff_key2).unwrap(); assert!(!is_merklized1 && !is_merklized2); // Check that key-val-1 diffs are properly updated for blocks 0 and 1 diff --git a/crates/state/src/wl_state.rs b/crates/state/src/wl_state.rs index 64229e1fd3..dbf6fac944 100644 --- a/crates/state/src/wl_state.rs +++ b/crates/state/src/wl_state.rs @@ -8,6 +8,7 @@ use namada_core::chain::ChainId; use namada_core::storage; use namada_core::time::DateTimeUtc; use namada_events::{EmitEvents, EventToEmit}; +use namada_merkle_tree::NO_DIFF_KEY_PREFIX; use namada_parameters::EpochDuration; use namada_replay_protection as replay_protection; use namada_storage::conversion_state::{ConversionState, WithConversionState}; @@ -18,9 +19,9 @@ use namada_storage::{ use crate::in_memory::InMemory; use crate::write_log::{StorageModification, WriteLog}; use crate::{ - is_pending_transfer_key, DBIter, Epoch, Error, Hash, Key, LastBlock, - MembershipProof, MerkleTree, MerkleTreeError, ProofOps, Result, State, - StateRead, StorageHasher, StorageResult, StoreType, DB, + is_pending_transfer_key, DBIter, Epoch, Error, Hash, Key, KeySeg, + LastBlock, MembershipProof, MerkleTree, MerkleTreeError, ProofOps, Result, + State, StateRead, StorageHasher, StorageResult, StoreType, DB, EPOCH_SWITCH_BLOCKS_DELAY, STORAGE_ACCESS_GAS_PER_BYTE, }; @@ -46,8 +47,8 @@ where pub(crate) db: D, /// State in memory pub(crate) in_mem: InMemory, - /// Static merkle tree storage key filter - pub merkle_tree_key_filter: fn(&storage::Key) -> bool, + /// Static diff storage key filter + pub diff_key_filter: fn(&storage::Key) -> bool, } /// State with a temporary write log. This is used for dry-running txs and ABCI @@ -103,7 +104,7 @@ where chain_id: ChainId, native_token: Address, storage_read_past_height_limit: Option, - merkle_tree_key_filter: fn(&storage::Key) -> bool, + diff_key_filter: fn(&storage::Key) -> bool, ) -> Self { let write_log = WriteLog::default(); let db = D::open(db_path, cache); @@ -116,7 +117,7 @@ where write_log, db, in_mem, - merkle_tree_key_filter, + diff_key_filter, }); state.load_last_state(); state @@ -275,7 +276,7 @@ where value: impl AsRef<[u8]>, ) -> Result { let value = value.as_ref(); - let is_key_merklized = (self.merkle_tree_key_filter)(key); + let persist_diffs = (self.diff_key_filter)(key); if is_pending_transfer_key(key) { // The tree of the bridge pool stores the current height for the @@ -284,16 +285,20 @@ where self.in_mem.block.tree.update(key, height)?; } else { // Update the merkle tree - if is_key_merklized { + if !persist_diffs { + let prefix = + Key::from(NO_DIFF_KEY_PREFIX.to_string().to_db_key()); + self.in_mem.block.tree.update(&prefix.join(key), value)?; + } else { self.in_mem.block.tree.update(key, value)?; - } + }; } Ok(self.db.batch_write_subspace_val( batch, self.in_mem.block.height, key, value, - is_key_merklized, + persist_diffs, )?) } @@ -305,16 +310,19 @@ where batch: &mut D::WriteBatch, key: &Key, ) -> Result { - let is_key_merklized = (self.merkle_tree_key_filter)(key); + let persist_diffs = (self.diff_key_filter)(key); // Update the merkle tree - if is_key_merklized { + if !persist_diffs { + let prefix = Key::from(NO_DIFF_KEY_PREFIX.to_string().to_db_key()); + self.in_mem.block.tree.delete(&prefix.join(key))?; + } else { self.in_mem.block.tree.delete(key)?; } Ok(self.db.batch_delete_subspace_val( batch, self.in_mem.block.height, key, - is_key_merklized, + persist_diffs, )?) } @@ -680,7 +688,7 @@ where { self.db_read(key) } else { - if !(self.merkle_tree_key_filter)(key) { + if !(self.diff_key_filter)(key) { return Ok((None, 0)); } @@ -714,7 +722,7 @@ where // but with gas and storage bytes len diff accounting tracing::debug!("storage write key {}", key,); let value = value.as_ref(); - let is_key_merklized = (self.merkle_tree_key_filter)(key); + let persist_diffs = (self.diff_key_filter)(key); if is_pending_transfer_key(key) { // The tree of the bright pool stores the current height for the @@ -723,7 +731,11 @@ where self.in_mem.block.tree.update(key, height)?; } else { // Update the merkle tree - if is_key_merklized { + if !persist_diffs { + let prefix = + Key::from(NO_DIFF_KEY_PREFIX.to_string().to_db_key()); + self.in_mem.block.tree.update(&prefix.join(key), value)?; + } else { self.in_mem.block.tree.update(key, value)?; } } @@ -735,7 +747,7 @@ where self.in_mem.block.height, key, value, - is_key_merklized, + persist_diffs, )?; Ok((gas, size_diff)) } @@ -753,14 +765,18 @@ where // but with gas and storage bytes len diff accounting let mut deleted_bytes_len = 0; if self.db_has_key(key)?.0 { - let is_key_merklized = (self.merkle_tree_key_filter)(key); - if is_key_merklized { + let persist_diffs = (self.diff_key_filter)(key); + if !persist_diffs { + let prefix = + Key::from(NO_DIFF_KEY_PREFIX.to_string().to_db_key()); + self.in_mem.block.tree.delete(&prefix.join(key))?; + } else { self.in_mem.block.tree.delete(key)?; } deleted_bytes_len = self.db.delete_subspace_val( self.in_mem.block.height, key, - is_key_merklized, + persist_diffs, )?; } let gas = (key.len() + deleted_bytes_len as usize) as u64 @@ -872,12 +888,11 @@ where .pred_epochs .get_epoch(height) .unwrap_or_default(); - let start_height = if store_type == Some(StoreType::CommitData) { - // CommitData is stored every height - height - } else { + let start_height = match store_type { + // subtree is stored every height + Some(st) if st.is_stored_every_block() => height, // others are stored at the first height of each epoch - match self + _ => match self .in_mem .block .pred_epochs @@ -886,7 +901,7 @@ where Some(BlockHeight(0)) => BlockHeight(1), Some(height) => height, None => BlockHeight(1), - } + }, }; let stores = self .db @@ -920,38 +935,32 @@ where match old.0.cmp(&new.0) { Ordering::Equal => { // the value was updated - if (self.merkle_tree_key_filter)(&new_key) { - tree.update( - &new_key, - if is_pending_transfer_key(&new_key) { - target_height.serialize_to_vec() - } else { - new.1.clone() - }, - )?; - } + tree.update( + &new_key, + if is_pending_transfer_key(&new_key) { + target_height.serialize_to_vec() + } else { + new.1.clone() + }, + )?; old_diff = old_diff_iter.next(); new_diff = new_diff_iter.next(); } Ordering::Less => { // the value was deleted - if (self.merkle_tree_key_filter)(&old_key) { - tree.delete(&old_key)?; - } + tree.delete(&old_key)?; old_diff = old_diff_iter.next(); } Ordering::Greater => { // the value was inserted - if (self.merkle_tree_key_filter)(&new_key) { - tree.update( - &new_key, - if is_pending_transfer_key(&new_key) { - target_height.serialize_to_vec() - } else { - new.1.clone() - }, - )?; - } + tree.update( + &new_key, + if is_pending_transfer_key(&new_key) { + target_height.serialize_to_vec() + } else { + new.1.clone() + }, + )?; new_diff = new_diff_iter.next(); } } @@ -960,10 +969,7 @@ where // the value was deleted let key = Key::parse(old.0.clone()) .expect("the key should be parsable"); - - if (self.merkle_tree_key_filter)(&key) { - tree.delete(&key)?; - } + tree.delete(&key)?; old_diff = old_diff_iter.next(); } @@ -972,16 +978,14 @@ where let key = Key::parse(new.0.clone()) .expect("the key should be parsable"); - if (self.merkle_tree_key_filter)(&key) { - tree.update( - &key, - if is_pending_transfer_key(&key) { - target_height.serialize_to_vec() - } else { - new.1.clone() - }, - )?; - } + tree.update( + &key, + if is_pending_transfer_key(&key) { + target_height.serialize_to_vec() + } else { + new.1.clone() + }, + )?; new_diff = new_diff_iter.next(); } @@ -990,7 +994,7 @@ where } } - // Restore the base tree and CommitData tree + // Restore the base tree and subtrees match store_type { Some(st) => { // It is enough to get the base tree @@ -1008,15 +1012,16 @@ where tree = MerkleTree::::new_partial(stores); } None => { - // Get the base and CommitData trees + // Get the base and subtrees stored in every block let mut stores = self .db .read_merkle_tree_stores(epoch, height, None)? .ok_or(Error::NoMerkleTree { height })?; let restored_stores = tree.stores(); - // Set all rebuilt subtrees except for CommitData tree + // Set all rebuilt subtrees except for the subtrees stored in + // every block for st in StoreType::iter_subtrees() { - if *st != StoreType::CommitData { + if !st.is_stored_every_block() { stores.set_root(st, *restored_stores.root(st)); stores.set_store(restored_stores.store(st).to_owned()); } diff --git a/crates/storage/src/mockdb.rs b/crates/storage/src/mockdb.rs index 101829f434..20432d37ff 100644 --- a/crates/storage/src/mockdb.rs +++ b/crates/storage/src/mockdb.rs @@ -238,15 +238,11 @@ impl DB for MockDB { // Merkle tree for st in StoreType::iter() { - if *st == StoreType::Base - || *st == StoreType::CommitData - || is_full_commit - { - let key_prefix = match st { - StoreType::Base | StoreType::CommitData => { - tree_key_prefix_with_height(st, height) - } - _ => tree_key_prefix_with_epoch(st, epoch), + if st.is_stored_every_block() || is_full_commit { + let key_prefix = if st.is_stored_every_block() { + tree_key_prefix_with_height(st, height) + } else { + tree_key_prefix_with_epoch(st, epoch) }; let root_key = format!("{key_prefix}/{MERKLE_TREE_ROOT_KEY_SEGMENT}"); @@ -302,11 +298,10 @@ impl DB for MockDB { .map(|st| Either::Left(std::iter::once(st))) .unwrap_or_else(|| Either::Right(StoreType::iter())); for st in store_types { - let key_prefix = match st { - StoreType::Base | StoreType::CommitData => { - tree_key_prefix_with_height(st, base_height) - } - _ => tree_key_prefix_with_epoch(st, epoch), + let key_prefix = if st.is_stored_every_block() { + tree_key_prefix_with_height(st, base_height) + } else { + tree_key_prefix_with_epoch(st, epoch) }; let root_key = format!("{key_prefix}/{MERKLE_TREE_ROOT_KEY_SEGMENT}");