Skip to content

Commit

Permalink
remove is_merklized_storage_key
Browse files Browse the repository at this point in the history
  • Loading branch information
@yito88
yito88 committed May 22, 2024
1 parent d7660f3 commit cd441a2
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 163 deletions.
8 changes: 0 additions & 8 deletions crates/node/src/shell/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -353,13 +353,6 @@ 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 {
// We need to merklize all keys for now
true
}

/// 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 {
Expand Down Expand Up @@ -448,7 +441,6 @@ 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 =
Expand Down
17 changes: 6 additions & 11 deletions crates/node/src/storage/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ mod tests {
use tempfile::TempDir;

use super::*;
use crate::shell::{is_key_diff_storable, is_merklized_storage_key};
use crate::shell::is_key_diff_storable;

#[test]
fn test_crud_value() {
Expand All @@ -93,7 +93,6 @@ 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");
Expand Down Expand Up @@ -146,7 +145,6 @@ mod tests {
ChainId::default(),
address::testing::nam(),
None,
is_merklized_storage_key,
is_key_diff_storable,
);
state
Expand Down Expand Up @@ -209,7 +207,6 @@ mod tests {
ChainId::default(),
address::testing::nam(),
None,
is_merklized_storage_key,
is_key_diff_storable,
);
let (loaded_root, height) =
Expand All @@ -231,7 +228,6 @@ mod tests {
ChainId::default(),
address::testing::nam(),
None,
is_merklized_storage_key,
is_key_diff_storable,
);

Expand Down Expand Up @@ -276,7 +272,6 @@ mod tests {
ChainId::default(),
address::testing::nam(),
None,
is_merklized_storage_key,
is_key_diff_storable,
);
state
Expand Down Expand Up @@ -349,7 +344,6 @@ mod tests {
ChainId::default(),
address::testing::nam(),
None,
is_merklized_storage_key,
is_key_diff_storable,
);

Expand Down Expand Up @@ -444,9 +438,10 @@ mod tests {
ChainId::default(),
address::testing::nam(),
None,
is_merklized_storage_key,
is_key_diff_storable,
);
// Prepare written keys for non-probable data, probable data (IBC), and
// no diffed data
let make_key = |suffix: u64| {
match suffix % 3u64 {
0 => Key::parse(format!("key{suffix}")).unwrap(),
Expand Down Expand Up @@ -529,6 +524,7 @@ mod tests {
}
}
}
// save the last root
roots.insert(state.in_mem().block.height, state.in_mem().merkle_root());
state.commit_block_from_batch(batch)?;

Expand All @@ -543,6 +539,7 @@ mod tests {
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 => {
Expand Down Expand Up @@ -578,6 +575,7 @@ mod tests {
.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 => {
Expand Down Expand Up @@ -612,7 +610,6 @@ mod tests {
ChainId::default(),
address::testing::nam(),
Some(5),
is_merklized_storage_key,
is_key_diff_storable,
);
let new_epoch_start = BlockHeight(1);
Expand Down Expand Up @@ -735,7 +732,6 @@ mod tests {
ChainId::default(),
address::testing::nam(),
None,
is_merklized_storage_key,
is_key_diff_storable,
);

Expand Down Expand Up @@ -845,7 +841,6 @@ mod tests {
None,
// Only merkelize and persist diffs for `test_key_1`
|key: &Key| -> bool { key == &test_key_1() },
|key: &Key| -> bool { key == &test_key_1() },
);
// Start the first block
let first_height = BlockHeight::first();
Expand Down
95 changes: 4 additions & 91 deletions crates/state/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -612,16 +612,11 @@ 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 {
true
}

fn diff_all_keys(_key: &storage::Key) -> bool {
true
}
Expand Down Expand Up @@ -901,32 +896,21 @@ mod tests {
Key::parse("testing2").unwrap()
}

fn test_key_3() -> Key {
Key::parse("testing3").unwrap()
}

fn merkle_tree_key_filter(key: &Key) -> bool {
key == &test_key_1() || key == &test_key_2()
}

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;
let key2 = test_key_2();
let val2 = 2u64;
let key3 = test_key_3();
let val3 = 3u64;

// Standard write of key-val-1
state.write(&key1, val1).unwrap();
Expand All @@ -947,13 +931,6 @@ mod tests {
let res = state.read::<u64>(&key2).unwrap().unwrap();
assert_eq!(res, val2);

// Write key-val-3 without merklizing or diffs
state.write(&key3, val3).unwrap();

// Read from state should return val3
let res = state.read::<u64>(&key3).unwrap().unwrap();
assert_eq!(res, val3);

// Commit block and storage changes
state.commit_block().unwrap();
state.in_mem_mut().block.height =
Expand Down Expand Up @@ -985,23 +962,6 @@ mod tests {
state.in_mem().block.tree.has_key(&no_diff_key2).unwrap();
assert!(is_merklized2);

// Key3 should be in storage. Confirm by reading from
// state and also by reading DB subspace directly
let res3 = state.read::<u64>(&key3).unwrap().unwrap();
assert_eq!(res3, val3);
let res3 = state.db().read_subspace_val(&key3).unwrap().unwrap();
let res3 = u64::try_from_slice(&res3).unwrap();
assert_eq!(res3, val3);

// Check explicitly that key-val-3 is not in merkle tree
let is_merklized3 = state.in_mem().block.tree.has_key(&key3).unwrap();
assert!(!is_merklized3);
let no_diff_key3 =
Key::from(NO_DIFF_KEY_PREFIX.to_string().to_db_key()).join(&key3);
let is_merklized3 =
state.in_mem().block.tree.has_key(&no_diff_key3).unwrap();
assert!(!is_merklized3);

// Check that the proper diffs exist for key-val-1
let res1 = state
.db()
Expand Down Expand Up @@ -1032,25 +992,9 @@ mod tests {
let res2 = u64::try_from_slice(&res2).unwrap();
assert_eq!(res2, val2);

// Check that there are diffs for key-val-3 in block 0, since all keys
// need to have diffs for at least 1 block for rollback purposes
let res3 = state
.db()
.read_diffs_val(&key3, BlockHeight(0), true)
.unwrap();
assert!(res3.is_none());
let res3 = state
.db()
.read_diffs_val(&key3, BlockHeight(0), false)
.unwrap()
.unwrap();
let res3 = u64::try_from_slice(&res3).unwrap();
assert_eq!(res3, val3);

// Now delete the keys properly
state.delete(&key1).unwrap();
state.delete(&key2).unwrap();
state.delete(&key3).unwrap();

// Commit the block again
state.commit_block().unwrap();
Expand All @@ -1060,20 +1004,16 @@ mod tests {
// Check the key-vals are removed from the storage subspace
let res1 = state.read::<u64>(&key1).unwrap();
let res2 = state.read::<u64>(&key2).unwrap();
let res3 = state.read::<u64>(&key3).unwrap();
assert!(res1.is_none() && res2.is_none() && res3.is_none());
assert!(res1.is_none() && res2.is_none());
let res1 = state.db().read_subspace_val(&key1).unwrap();
let res2 = state.db().read_subspace_val(&key2).unwrap();
let res3 = state.db().read_subspace_val(&key3).unwrap();
assert!(res1.is_none() && res2.is_none() && res3.is_none());
assert!(res1.is_none() && res2.is_none());

// 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(&no_diff_key2).unwrap();
let is_merklized3 =
state.in_mem().block.tree.has_key(&no_diff_key3).unwrap();
assert!(!is_merklized1 && !is_merklized2 && !is_merklized3);
assert!(!is_merklized1 && !is_merklized2);

// Check that key-val-1 diffs are properly updated for blocks 0 and 1
let res1 = state
Expand Down Expand Up @@ -1116,18 +1056,6 @@ mod tests {
.unwrap();
assert!(res2.is_none());

// Check that key-val-3 diffs don't exist for block 0 anymore
let res3 = state
.db()
.read_diffs_val(&key3, BlockHeight(0), true)
.unwrap();
assert!(res3.is_none());
let res3 = state
.db()
.read_diffs_val(&key3, BlockHeight(0), false)
.unwrap();
assert!(res3.is_none());

// Check that the block 1 diffs for key-val-2 include an "old" value of
// val2 and no "new" value
let res2 = state
Expand All @@ -1142,21 +1070,6 @@ mod tests {
.read_diffs_val(&key2, BlockHeight(1), false)
.unwrap();
assert!(res2.is_none());

// Check that the block 1 diffs for key-val-2 include an "old" value of
// val2 and no "new" value
let res3 = state
.db()
.read_diffs_val(&key3, BlockHeight(1), true)
.unwrap()
.unwrap();
let res3 = u64::try_from_slice(&res3).unwrap();
assert_eq!(res3, val3);
let res3 = state
.db()
.read_diffs_val(&key3, BlockHeight(1), false)
.unwrap();
assert!(res3.is_none());
}

proptest! {
Expand Down
Loading

0 comments on commit cd441a2

Please sign in to comment.