Skip to content

Commit

Permalink
Migrate magic number to global const (#595)
Browse files Browse the repository at this point in the history
* Migrate magic number to global const

* Replace format!

* Make nightly clippy happy

* Remove unnecessary return param

* Apply comments

* Update crates/storage/src/lazy/lazy_cell.rs

Co-authored-by: Hero Bird <[email protected]>

* Fix binding

* Add binding

Co-authored-by: Hero Bird <[email protected]>
  • Loading branch information
Michael Müller and Robbepop authored Nov 26, 2020
1 parent dd890b8 commit 527e0ac
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 7 deletions.
8 changes: 7 additions & 1 deletion crates/storage/src/lazy/lazy_cell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,13 @@ where
false => {
// Clear without loading from storage:
let footprint = <T as SpreadLayout>::FOOTPRINT;
assert!(footprint <= 16); // magic number
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,
);
let mut key_ptr = KeyPtr::from(*root_key);
for _ in 0..footprint {
ink_env::clear_contract_storage(key_ptr.advance_by(1));
Expand Down
5 changes: 4 additions & 1 deletion crates/storage/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,10 @@ pub use self::{
KeyPtr,
},
packed::PackedLayout,
spread::SpreadLayout,
spread::{
SpreadLayout,
FOOTPRINT_CLEANUP_THRESHOLD,
},
};
pub use ::ink_storage_derive::{
PackedLayout,
Expand Down
5 changes: 3 additions & 2 deletions crates/storage/src/traits/optspec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,9 @@ where
// we will still load it first in order to not clean-up too many unneeded
// storage cells.
let footprint = <T as SpreadLayout>::FOOTPRINT;
let threshold = 16; // Arbitrarily chosen. Might need adjustments later.
if footprint >= threshold || <T as SpreadLayout>::REQUIRES_DEEP_CLEAN_UP {
if footprint >= super::FOOTPRINT_CLEANUP_THRESHOLD
|| <T as SpreadLayout>::REQUIRES_DEEP_CLEAN_UP
{
// We need to load the entity before we remove its associated contract storage
// because it requires a deep clean-up which propagates clearing to its fields,
// for example in the case of `T` being a `storage::Box`.
Expand Down
7 changes: 7 additions & 0 deletions crates/storage/src/traits/spread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,13 @@

use super::KeyPtr;

/// This constant is used by some types to make sure that cleaning up
/// behind them won't become way too expensive. Since we are missing
/// Substrate's storage bulk removal feature we cannot do better than
/// this at the moment.
/// The number is arbitrarily chosen. Might need adjustments later.
pub const FOOTPRINT_CLEANUP_THRESHOLD: u64 = 32;

/// Types that can be stored to and loaded from the contract storage.
pub trait SpreadLayout {
/// The footprint of the type.
Expand Down
5 changes: 2 additions & 3 deletions examples/erc721/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ mod erc721 {
return Err(Error::NotAllowed)
};
let entry = owned_tokens_count.entry(*to);
increase_counter_of(entry)?;
increase_counter_of(entry);
vacant_token_owner.insert(*to);
Ok(())
}
Expand Down Expand Up @@ -431,9 +431,8 @@ mod erc721 {
}

/// Increase token counter from the `of` AccountId.
fn increase_counter_of(entry: Entry<AccountId, u32>) -> Result<(), Error> {
fn increase_counter_of(entry: Entry<AccountId, u32>) {
entry.and_modify(|v| *v += 1).or_insert(1);
Ok(())
}

/// Unit tests
Expand Down

0 comments on commit 527e0ac

Please sign in to comment.