Skip to content

Commit

Permalink
Make Lazy::Drop always clear up all its storage (#597)
Browse files Browse the repository at this point in the history 
* Make `Lazy::Drop` always clear up all its storage

* Fix typo: yet ➜ get

* Fix macro import

* Make tests off-chain since they interact with storage now due to Drop

* Make use of `REQUIRES_DEEP_CLEAN_UP`

* Clear up all footprint cells

* Fix typo: yet ➜ get

* Fix case

* Macro ➜ fn
  • Loading branch information
Michael Müller authored Nov 27, 2020
1 parent 527e0ac commit 2100fa7
Show file tree
Hide file tree
Showing 3 changed files with 136 additions and 33 deletions.
2 changes: 1 addition & 1 deletion crates/storage/src/collections/stash/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -770,7 +770,7 @@ fn storage_is_cleared_completely_after_pull_lazy() {
let contract_id = ink_env::test::get_current_contract_account_id::<
ink_env::DefaultEnvironment,
>()
.expect("Cannot yet contract id");
.expect("Cannot get contract id");
let storage_used = ink_env::test::count_used_storage_cells::<
ink_env::DefaultEnvironment,
>(&contract_id)
Expand Down
2 changes: 1 addition & 1 deletion crates/storage/src/collections/vec/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ fn drop_works() {
let contract_id = ink_env::test::get_current_contract_account_id::<
ink_env::DefaultEnvironment,
>()
.expect("Cannot yet contract id");
.expect("Cannot get contract id");
let used_cells = ink_env::test::count_used_storage_cells::<
ink_env::DefaultEnvironment,
>(&contract_id)
Expand Down
165 changes: 134 additions & 31 deletions crates/storage/src/lazy/lazy_cell.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,21 +77,22 @@ where
}

#[test]
fn debug_impl_works() {
let c1 = <LazyCell<i32>>::new(None);
assert_eq!(
format!("{:?}", &c1),
"LazyCell { key: None, cache: Some(Entry { value: None, state: Mutated }) }",
);
let c2 = <LazyCell<i32>>::new(Some(42));
assert_eq!(
format!("{:?}", &c2),
"LazyCell { key: None, cache: Some(Entry { value: Some(42), state: Mutated }) }",
);
let c3 = <LazyCell<i32>>::lazy(Key::from([0x00; 32]));
assert_eq!(
format!("{:?}", &c3),
"LazyCell { \
fn debug_impl_works() -> ink_env::Result<()> {
ink_env::test::run_test::<ink_env::DefaultEnvironment, _>(|_| {
let c1 = <LazyCell<i32>>::new(None);
assert_eq!(
format!("{:?}", &c1),
"LazyCell { key: None, cache: Some(Entry { value: None, state: Mutated }) }",
);
let c2 = <LazyCell<i32>>::new(Some(42));
assert_eq!(
format!("{:?}", &c2),
"LazyCell { key: None, cache: Some(Entry { value: Some(42), state: Mutated }) }",
);
let c3 = <LazyCell<i32>>::lazy(Key::from([0x00; 32]));
assert_eq!(
format!("{:?}", &c3),
"LazyCell { \
key: Some(Key(0x_\
0000000000000000_\
0000000000000000_\
Expand All @@ -100,17 +101,42 @@ fn debug_impl_works() {
), \
cache: None \
}",
);
);
Ok(())
})
}

impl<T> Drop for LazyCell<T>
where
T: SpreadLayout,
{
fn drop(&mut self) {
if let Some(key) = self.key() {
if let Some(entry) = self.entry() {
clear_spread_root_opt::<T, _>(key, || entry.value().into())
if let Some(root_key) = self.key() {
match self.entry() {
Some(entry) => {
// The inner cell needs to be cleared, no matter if it has
// been loaded or not. Otherwise there might be leftovers.
// Load from storage and then clear:
clear_spread_root_opt::<T, _>(root_key, || entry.value().into())
}
None => {
// The value is not yet in the cache. we need it in there
// though in order to properly clean up.
if <T as SpreadLayout>::REQUIRES_DEEP_CLEAN_UP {
// The inner cell needs to be cleared, no matter if it has
// been loaded or not. Otherwise there might be leftovers.
// Load from storage and then clear:
clear_spread_root_opt::<T, _>(root_key, || self.get())
} else {
// Clear without loading from storage:
let footprint = <T as SpreadLayout>::FOOTPRINT;
assert_footprint_threshold(footprint);
let mut key_ptr = KeyPtr::from(*root_key);
for _ in 0..footprint {
ink_env::clear_contract_storage(key_ptr.advance_by(1));
}
}
}
}
}
}
Expand Down Expand Up @@ -161,13 +187,7 @@ where
false => {
// Clear without loading from storage:
let footprint = <T as SpreadLayout>::FOOTPRINT;
let footprint_threshold = crate::traits::FOOTPRINT_CLEANUP_THRESHOLD;
assert!(
footprint <= footprint_threshold,
"cannot clean-up a storage entity with a footprint of {}. maximum threshold for clean-up is {}.",
footprint,
footprint_threshold,
);
assert_footprint_threshold(footprint);
let mut key_ptr = KeyPtr::from(*root_key);
for _ in 0..footprint {
ink_env::clear_contract_storage(key_ptr.advance_by(1));
Expand Down Expand Up @@ -361,6 +381,17 @@ where
}
}

/// Asserts that the given `footprint` is below `FOOTPRINT_CLEANUP_THRESHOLD`.
fn assert_footprint_threshold(footprint: u64) {
let footprint_threshold = crate::traits::FOOTPRINT_CLEANUP_THRESHOLD;
assert!(
footprint <= footprint_threshold,
"cannot clean-up a storage entity with a footprint of {}. maximum threshold for clean-up is {}.",
footprint,
footprint_threshold,
);
}

#[cfg(test)]
mod tests {
use super::{
Expand Down Expand Up @@ -408,10 +439,13 @@ mod tests {
}

#[test]
fn lazy_works() {
let root_key = Key::from([0x42; 32]);
let cell = <LazyCell<u8>>::lazy(root_key);
assert_eq!(cell.key(), Some(&root_key));
fn lazy_works() -> ink_env::Result<()> {
run_test::<ink_env::DefaultEnvironment, _>(|_| {
let root_key = Key::from([0x42; 32]);
let cell = <LazyCell<u8>>::lazy(root_key);
assert_eq!(cell.key(), Some(&root_key));
Ok(())
})
}

#[test]
Expand Down Expand Up @@ -693,7 +727,7 @@ mod tests {
let contract_id = ink_env::test::get_current_contract_account_id::<
ink_env::DefaultEnvironment,
>()
.expect("Cannot yet contract id");
.expect("Cannot get contract id");
let used_cells = ink_env::test::count_used_storage_cells::<
ink_env::DefaultEnvironment,
>(&contract_id)
Expand All @@ -706,4 +740,73 @@ mod tests {
})
.unwrap()
}

#[test]
#[should_panic(expected = "encountered empty storage cell")]
fn lazy_drop_works() {
ink_env::test::run_test::<ink_env::DefaultEnvironment, _>(|_| {
// given
let root_key = Key::from([0x42; 32]);

// when
let setup_result = std::panic::catch_unwind(|| {
let lazy: Lazy<u32> = Lazy::new(13u32);
SpreadLayout::push_spread(&lazy, &mut KeyPtr::from(root_key));
let _pulled_lazy =
<Lazy<u32> as SpreadLayout>::pull_spread(&mut KeyPtr::from(root_key));
// lazy is dropped which should clear the cells
});
assert!(setup_result.is_ok(), "setup should not panic");

// then
let contract_id = ink_env::test::get_current_contract_account_id::<
ink_env::DefaultEnvironment,
>()
.expect("Cannot get contract id");
let used_cells = ink_env::test::count_used_storage_cells::<
ink_env::DefaultEnvironment,
>(&contract_id)
.expect("used cells must be returned");
assert_eq!(used_cells, 0);
let _ =
*<Lazy<u32> as SpreadLayout>::pull_spread(&mut KeyPtr::from(root_key));
Ok(())
})
.unwrap()
}

#[test]
#[should_panic(expected = "encountered empty storage cell")]
fn lazy_drop_works_with_greater_footprint() {
ink_env::test::run_test::<ink_env::DefaultEnvironment, _>(|_| {
// given
let root_key = Key::from([0x42; 32]);

// when
let setup_result = std::panic::catch_unwind(|| {
let lazy: Lazy<[u32; 5]> = Lazy::new([13, 14, 15, 16, 17]);
SpreadLayout::push_spread(&lazy, &mut KeyPtr::from(root_key));
let _pulled_lazy = <Lazy<[u32; 5]> as SpreadLayout>::pull_spread(
&mut KeyPtr::from(root_key),
);
// lazy is dropped which should clear the cells
});
assert!(setup_result.is_ok(), "setup should not panic");

// then
let contract_id = ink_env::test::get_current_contract_account_id::<
ink_env::DefaultEnvironment,
>()
.expect("Cannot get contract id");
let used_cells = ink_env::test::count_used_storage_cells::<
ink_env::DefaultEnvironment,
>(&contract_id)
.expect("used cells must be returned");
assert_eq!(used_cells, 0);
let _ =
*<Lazy<u32> as SpreadLayout>::pull_spread(&mut KeyPtr::from(root_key));
Ok(())
})
.unwrap()
}
}

0 comments on commit 2100fa7

Please sign in to comment.