Skip to content

Commit

Permalink
Resolution of bugs for the hashing in MapView (#1439)
Browse files Browse the repository at this point in the history
* Addition of the missing hash=None and corresponding test code.
With the test code inserted but without the hash=None, the test fails.
  • Loading branch information
MathieuDutSik authored Jan 5, 2024
1 parent a097292 commit 7852bde
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 4 deletions.
3 changes: 1 addition & 2 deletions linera-views/src/collection_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,6 @@ where
/// # })
/// ```
pub async fn load_entry_mut(&mut self, short_key: Vec<u8>) -> Result<&mut W, ViewError> {
*self.hash.get_mut() = None;
self.do_load_entry_mut(short_key).await
}

Expand Down Expand Up @@ -277,7 +276,6 @@ where
/// # })
/// ```
pub async fn reset_entry_to_default(&mut self, short_key: Vec<u8>) -> Result<(), ViewError> {
*self.hash.get_mut() = None;
let view = self.load_entry_mut(short_key).await?;
view.clear();
Ok(())
Expand Down Expand Up @@ -316,6 +314,7 @@ where
}

async fn do_load_entry_mut(&mut self, short_key: Vec<u8>) -> Result<&mut W, ViewError> {
*self.hash.get_mut() = None;
match self.updates.get_mut().entry(short_key.clone()) {
btree_map::Entry::Occupied(entry) => {
let entry = entry.into_mut();
Expand Down
2 changes: 2 additions & 0 deletions linera-views/src/map_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ where
/// # })
/// ```
pub async fn get_mut(&mut self, short_key: Vec<u8>) -> Result<Option<&mut V>, ViewError> {
*self.hash.get_mut() = None;
self.load_value(&short_key).await?;
if let Some(update) = self.updates.get_mut(&short_key) {
let value = match update {
Expand Down Expand Up @@ -698,6 +699,7 @@ where
pub async fn get_mut_or_default(&mut self, short_key: Vec<u8>) -> Result<&mut V, ViewError> {
use std::collections::btree_map::Entry;

*self.hash.get_mut() = None;
let update = match self.updates.entry(short_key.clone()) {
Entry::Vacant(e) if self.delete_storage_first => e.insert(Update::Set(V::default())),
Entry::Vacant(e) => {
Expand Down
43 changes: 41 additions & 2 deletions linera-views/tests/map_view_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
use linera_views::{
map_view::ByteMapView,
memory::create_memory_context,
views::{CryptoHashRootView, RootView, View},
views::{CryptoHashRootView, CryptoHashView, RootView, View},
};
use rand::{distributions::Uniform, Rng, RngCore, SeedableRng};
use std::collections::{BTreeMap, BTreeSet};
Expand All @@ -27,14 +27,15 @@ async fn run_map_view_mutability<R: RngCore + Clone>(rng: &mut R) {
let mut view = StateView::load(context.clone()).await.unwrap();
let save = rng.gen::<bool>();
let read_state = view.map.key_values().await.unwrap();
let read_hash = view.crypto_hash().await.unwrap();
let state_vec = state_map.clone().into_iter().collect::<Vec<_>>();
assert_eq!(state_vec, read_state);
//
let count_oper = rng.gen_range(0..25);
let mut new_state_map = state_map.clone();
let mut new_state_vec = state_vec.clone();
for _ in 0..count_oper {
let choice = rng.gen_range(0..5);
let choice = rng.gen_range(0..7);
let count = view.map.count().await.unwrap();
if choice == 0 {
// inserting random stuff
Expand Down Expand Up @@ -79,7 +80,45 @@ async fn run_map_view_mutability<R: RngCore + Clone>(rng: &mut R) {
view.rollback();
new_state_map = state_map.clone();
}
if choice == 5 && count > 0 {
let pos = rng.gen_range(0..count);
let vec = new_state_vec[pos].clone();
let key = vec.0;
let result = view.map.get_mut(key.clone()).await.unwrap().unwrap();
let new_value = rng.gen::<u8>();
*result = new_value;
new_state_map.insert(key, new_value);
}
if choice == 6 && count > 0 {
let choice = rng.gen_range(0..count);
let key = match choice {
0 => {
// Scenario 1 of using existing key
let pos = rng.gen_range(0..count);
let vec = new_state_vec[pos].clone();
vec.0
}
_ => {
let len = rng.gen_range(1..6);
rng.clone()
.sample_iter(Uniform::from(0..4))
.take(len)
.collect::<Vec<_>>()
}
};
let result = view.map.get_mut_or_default(key.clone()).await.unwrap();
let new_value = rng.gen::<u8>();
*result = new_value;
new_state_map.insert(key, new_value);
}
new_state_vec = new_state_map.clone().into_iter().collect();
let new_hash = view.crypto_hash().await.unwrap();
if state_vec == new_state_vec {
assert_eq!(new_hash, read_hash);
} else {
// Hash equality is a bug or a hash collision (unlikely)
assert_ne!(new_hash, read_hash);
}
let new_key_values = view.map.key_values().await.unwrap();
assert_eq!(new_state_vec, new_key_values);
for u in 0..4 {
Expand Down

0 comments on commit 7852bde

Please sign in to comment.