-
Notifications
You must be signed in to change notification settings - Fork 443
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make nested Lazy's clear storage properly #583
Conversation
Codecov Report
@@ Coverage Diff @@
## master #583 +/- ##
==========================================
- Coverage 67.86% 67.78% -0.08%
==========================================
Files 155 155
Lines 6895 6914 +19
==========================================
+ Hits 4679 4687 +8
- Misses 2216 2227 +11
Continue to review full report at Codecov.
|
Good catch! The fixed fn clear_spread(&self, ptr: &mut KeyPtr) {
let root_key = ExtKeyPtr::next_for::<Self>(ptr);
match <T as SpreadLayout>::REQUIRES_DEEP_CLEAN_UP {
true => {
// Load from storage and then clear:
clear_spread_root_opt::<T, _>(root_key, || self.get())
}
false => {
// Clear without loading from storage:
let footprint = <T as SpreadLayout>::FOOTPRINT;
assert!(footprint <= 16); // magic number
let mut key_ptr = KeyPtr::from(*root_key);
for _ in 0..footprint {
ink_env::clear_contract_storage(key_ptr.advance_by(1));
}
}
}
} |
Co-authored-by: Hero Bird <robin.freyler@gmail.com>
Closes #580.
The bug is that a
Lazy
doesn't clear up it's inner cell forclear_spread
, if the inner cell has not been loaded. This can easily happen with nestedLazy
's ‒ likeLazy<Lazy<…>>
.But also we often use
Lazy
in storage structs likeStorageHashMap<…>
(withvalues: LazyHashMap<…>
‒ if these structs are then wrapped in aLazy
there is already a nestedLazy
.SmallVec
,Vec
,Stash
or other storage structs which have this bug if they are wrapped in aLazy
.The way I fixed this for now will probably lead to some discussion and I'm open for other approaches.