Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Ext::kill_child_storage now takes an upper limit for backend deletion
  • Loading branch information
athei committed Dec 4, 2020
commit 2eb4d60e856fe72786593e39685d3350f79a6d6a
12 changes: 11 additions & 1 deletion primitives/externalities/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,17 @@ pub trait Externalities: ExtensionStore {
) -> Option<Vec<u8>>;

/// Clear an entire child storage.
fn kill_child_storage(&mut self, child_info: &ChildInfo);
///
/// Deletes all keys from the overlay and up to `limit` keys from the backend. No
/// limit is applied if `limit` is `None`. Returns `true` if the child trie was
/// removed completely and `false` if there are remaining keys after the function
/// returns.
///
/// # Note
///
/// An implementation is free to delete more keys than the specified limit as long as
/// it is able to do that in constant time.
fn kill_child_storage(&mut self, child_info: &ChildInfo, limit: Option<u32>) -> bool;

/// Clear storage entries which keys are start with the given prefix.
fn clear_prefix(&mut self, prefix: &[u8]);
Expand Down
2 changes: 1 addition & 1 deletion primitives/io/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ pub trait DefaultChildStorage {
storage_key: &[u8],
) {
let child_info = ChildInfo::new_default(storage_key);
self.kill_child_storage(&child_info);
self.kill_child_storage(&child_info, u32::max_value());
}

/// Check a child storage key.
Expand Down
6 changes: 4 additions & 2 deletions primitives/state-machine/src/basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,10 @@ impl Externalities for BasicExternalities {
fn kill_child_storage(
&mut self,
child_info: &ChildInfo,
) {
_limit: Option<u32>,
) -> bool {
self.inner.children_default.remove(child_info.storage_key());
true
}

fn clear_prefix(&mut self, prefix: &[u8]) {
Expand Down Expand Up @@ -407,7 +409,7 @@ mod tests {
ext.clear_child_storage(child_info, b"dog");
assert_eq!(ext.child_storage(child_info, b"dog"), None);

ext.kill_child_storage(child_info);
ext.kill_child_storage(child_info, None);
assert_eq!(ext.child_storage(child_info, b"doe"), None);
}

Expand Down
31 changes: 26 additions & 5 deletions primitives/state-machine/src/ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -411,19 +411,40 @@ where
fn kill_child_storage(
&mut self,
child_info: &ChildInfo,
) {
limit: Option<u32>,
) -> bool {
trace!(target: "state", "{:04x}: KillChild({})",
self.id,
HexDisplay::from(&child_info.storage_key()),
);
let _guard = guard();

self.mark_dirty();
self.overlay.clear_child_storage(child_info);
self.backend.for_keys_in_child_storage(child_info, |key| {
self.overlay.set_child_storage(child_info, key.to_vec(), None);

if let Some(limit) = limit {
let mut num_deleted: u32 = 0;
let mut all_deleted = true;
self.backend.for_keys_in_child_storage(child_info, |key| {
if num_deleted == limit {
all_deleted = false;
return false;
}
if let Some(num) = num_deleted.checked_add(1) {
num_deleted = num;
} else {
return false;
}
self.overlay.set_child_storage(child_info, key.to_vec(), None);
num_deleted <= limit
});
all_deleted
} else {
self.backend.for_keys_in_child_storage(child_info, |key| {
self.overlay.set_child_storage(child_info, key.to_vec(), None);
true
});
true
});
}
}

fn clear_prefix(&mut self, prefix: &[u8]) {
Expand Down
81 changes: 81 additions & 0 deletions primitives/state-machine/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1147,6 +1147,86 @@ mod tests {
);
}

#[test]
fn limited_child_kill_works() {
let child_info = ChildInfo::new_default(b"sub1");
let initial: HashMap<_, BTreeMap<_, _>> = map![
Some(child_info.clone()) => map![
b"a".to_vec() => b"0".to_vec(),
b"b".to_vec() => b"1".to_vec(),
b"c".to_vec() => b"2".to_vec(),
b"d".to_vec() => b"3".to_vec()
],
];
let backend = InMemoryBackend::<BlakeTwo256>::from(initial);

let mut overlay = OverlayedChanges::default();
overlay.set_child_storage(&child_info, b"1".to_vec(), Some(b"1312".to_vec()));
overlay.set_child_storage(&child_info, b"2".to_vec(), Some(b"1312".to_vec()));
overlay.set_child_storage(&child_info, b"3".to_vec(), Some(b"1312".to_vec()));
overlay.set_child_storage(&child_info, b"4".to_vec(), Some(b"1312".to_vec()));

{
let mut offchain_overlay = Default::default();
let mut cache = StorageTransactionCache::default();
let mut ext = Ext::new(
&mut overlay,
&mut offchain_overlay,
&mut cache,
&backend,
changes_trie::disabled_state::<_, u64>(),
None,
);
assert_eq!(ext.kill_child_storage(&child_info, Some(2)), false);
}

assert_eq!(
overlay.children()
.flat_map(|(iter, _child_info)| iter)
.map(|(k, v)| (k.clone(), v.value().clone()))
.collect::<BTreeMap<_, _>>(),
map![
b"1".to_vec() => None.into(),
b"2".to_vec() => None.into(),
b"3".to_vec() => None.into(),
b"4".to_vec() => None.into(),
b"a".to_vec() => None.into(),
b"b".to_vec() => None.into(),
],
);
}

#[test]
fn limited_child_kill_off_by_one_works() {
let child_info = ChildInfo::new_default(b"sub1");
let initial: HashMap<_, BTreeMap<_, _>> = map![
Some(child_info.clone()) => map![
b"a".to_vec() => b"0".to_vec(),
b"b".to_vec() => b"1".to_vec(),
b"c".to_vec() => b"2".to_vec(),
b"d".to_vec() => b"3".to_vec()
],
];
let backend = InMemoryBackend::<BlakeTwo256>::from(initial);
let mut overlay = OverlayedChanges::default();
let mut offchain_overlay = Default::default();
let mut cache = StorageTransactionCache::default();
let mut ext = Ext::new(
&mut overlay,
&mut offchain_overlay,
&mut cache,
&backend,
changes_trie::disabled_state::<_, u64>(),
None,
);
assert_eq!(ext.kill_child_storage(&child_info, Some(0)), false);
assert_eq!(ext.kill_child_storage(&child_info, Some(1)), false);
assert_eq!(ext.kill_child_storage(&child_info, Some(2)), false);
assert_eq!(ext.kill_child_storage(&child_info, Some(3)), false);
assert_eq!(ext.kill_child_storage(&child_info, Some(4)), true);
assert_eq!(ext.kill_child_storage(&child_info, Some(5)), true);
}

#[test]
fn set_child_storage_works() {
let child_info = ChildInfo::new_default(b"sub1");
Expand Down Expand Up @@ -1179,6 +1259,7 @@ mod tests {
);
ext.kill_child_storage(
child_info,
None,
);
assert_eq!(
ext.child_storage(
Expand Down
3 changes: 2 additions & 1 deletion primitives/state-machine/src/read_only.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,8 @@ impl<'a, H: Hasher, B: 'a + Backend<H>> Externalities for ReadOnlyExternalities<
fn kill_child_storage(
&mut self,
_child_info: &ChildInfo,
) {
_limit: Option<u32>,
) -> bool {
unimplemented!("kill_child_storage is not supported in ReadOnlyExternalities")
}

Expand Down
3 changes: 2 additions & 1 deletion primitives/tasks/src/async_externalities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,8 @@ impl Externalities for AsyncExternalities {
fn kill_child_storage(
&mut self,
_child_info: &ChildInfo,
) {
_limit: Option<u32>,
) -> bool {
panic!("`kill_child_storage`: should not be used in async externalities!")
}

Expand Down