From 527e0ace3977d19fcc56b92484381974fdc2fa54 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michael=20M=C3=BCller?= <michi@parity.io>
Date: Thu, 26 Nov 2020 14:01:49 +0100
Subject: [PATCH] Migrate magic number to global const (#595)

* 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 <robin.freyler@gmail.com>

* Fix binding

* Add binding

Co-authored-by: Hero Bird <robin.freyler@gmail.com>
---
 crates/storage/src/lazy/lazy_cell.rs | 8 +++++++-
 crates/storage/src/traits/mod.rs     | 5 ++++-
 crates/storage/src/traits/optspec.rs | 5 +++--
 crates/storage/src/traits/spread.rs  | 7 +++++++
 examples/erc721/lib.rs               | 5 ++---
 5 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/crates/storage/src/lazy/lazy_cell.rs b/crates/storage/src/lazy/lazy_cell.rs
index e86df080db5..f6c9acbee99 100644
--- a/crates/storage/src/lazy/lazy_cell.rs
+++ b/crates/storage/src/lazy/lazy_cell.rs
@@ -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));
diff --git a/crates/storage/src/traits/mod.rs b/crates/storage/src/traits/mod.rs
index 7108940ee28..d507d7894a1 100644
--- a/crates/storage/src/traits/mod.rs
+++ b/crates/storage/src/traits/mod.rs
@@ -55,7 +55,10 @@ pub use self::{
         KeyPtr,
     },
     packed::PackedLayout,
-    spread::SpreadLayout,
+    spread::{
+        SpreadLayout,
+        FOOTPRINT_CLEANUP_THRESHOLD,
+    },
 };
 pub use ::ink_storage_derive::{
     PackedLayout,
diff --git a/crates/storage/src/traits/optspec.rs b/crates/storage/src/traits/optspec.rs
index e18f845633b..abc9ce247ab 100644
--- a/crates/storage/src/traits/optspec.rs
+++ b/crates/storage/src/traits/optspec.rs
@@ -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`.
diff --git a/crates/storage/src/traits/spread.rs b/crates/storage/src/traits/spread.rs
index 057dfc68099..0b3f326bb9a 100644
--- a/crates/storage/src/traits/spread.rs
+++ b/crates/storage/src/traits/spread.rs
@@ -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.
diff --git a/examples/erc721/lib.rs b/examples/erc721/lib.rs
index 42adbafa77a..b4075d5be06 100644
--- a/examples/erc721/lib.rs
+++ b/examples/erc721/lib.rs
@@ -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(())
         }
@@ -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