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
review: Use a new version of kill_storage instead of a new interface
  • Loading branch information
athei committed Dec 7, 2020
commit f02f68066eb95c5aeac94afefd48a29eb92f1c46
2 changes: 2 additions & 0 deletions frame/contracts/src/rent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ where
<ContractInfoOf<T>>::remove(account);
child::kill_storage(
&alive_contract_info.child_trie_info(),
None,
);
<Module<T>>::deposit_event(RawEvent::Evicted(account.clone(), false));
None
Expand All @@ -263,6 +264,7 @@ where

child::kill_storage(
&alive_contract_info.child_trie_info(),
None,
);

<Module<T>>::deposit_event(RawEvent::Evicted(account.clone(), true));
Expand Down
2 changes: 1 addition & 1 deletion frame/contracts/src/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ where
/// This function doesn't affect the account.
pub fn destroy_contract(address: &AccountIdOf<T>, trie_id: &TrieId) {
<ContractInfoOf<T>>::remove(address);
child::kill_storage(&crate::child_trie_info(&trie_id));
child::kill_storage(&crate::child_trie_info(&trie_id), None);
}

/// This generator uses inner counter for account id and applies the hash over `AccountId +
Expand Down
36 changes: 34 additions & 2 deletions frame/support/src/storage/child.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,14 @@ use crate::sp_std::prelude::*;
use codec::{Codec, Encode, Decode};
pub use sp_core::storage::{ChildInfo, ChildType};

/// The outcome of calling [`kill_storage`].
pub enum KillOutcome {
/// No key remains in the child trie.
AllRemoved,
/// At least one key still resides in the child trie due to the supplied limit.
SomeRemaining,
}

/// Return the value of the item in storage under `key`, or `None` if there is no explicit entry.
pub fn get<T: Decode + Sized>(
child_info: &ChildInfo,
Expand Down Expand Up @@ -148,13 +156,37 @@ pub fn exists(
}

/// Remove all `storage_key` key/values
///
/// Deletes all keys from the overlay and up to `limit` keys from the backend if
/// it is set to `Some`. No limit is applied when `limit` is set to `None`.
///
/// The limit can be used to partially delete a child trie in case it is too large
/// to delete in one go (block).
///
/// # Note
///
/// Please note that keys that are residing in the overlay for that child trie when
/// issuing this call are all deleted without counting towards the `limit`. Only keys
/// written during the current block are part of the overlay. Deleting with a `limit`
/// mostly makes sense with an empty overlay for that child trie.
///
/// Calling this function multiple times per block for the same `storage_key` does
/// not make much sense because it is not cumulative when called inside the same block.
/// Use this function to distribute the deletion of a single child trie across multiple
/// blocks.
pub fn kill_storage(
child_info: &ChildInfo,
) {
match child_info.child_type() {
limit: Option<u32>,
) -> KillOutcome {
let all_removed = match child_info.child_type() {
ChildType::ParentKeyId => sp_io::default_child_storage::storage_kill(
child_info.storage_key(),
limit
),
};
match all_removed {
true => KillOutcome::AllRemoved,
false => KillOutcome::SomeRemaining,
}
}

Expand Down
30 changes: 16 additions & 14 deletions primitives/io/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,30 +282,32 @@ pub trait DefaultChildStorage {
self.kill_child_storage(&child_info, None);
}

/// Clear at most `limit` keys from the specified child storage.
/// Clear a child storage key.
///
/// Deletes all keys from the overlay and up to `limit` keys from the backend.
/// This can be used to partially delete a child trie in case it is too large
/// to delete in one go (block). It returns false iff some keys are remaining in
/// the child trie after the functions returns.
/// Deletes all keys from the overlay and up to `limit` keys from the backend if
/// it is set to `Some`. No limit is applied when `limit` is set to `None`.
///
/// Please note that keys that are residing in the overlay for that child tie when
/// issuing this call are all deleted without counting towards the `limit`. Only keys
/// written during the current block are part of the overlay.
/// The limit can be used to partially delete a child trie in case it is too large
/// to delete in one go (block).
///
/// It returns false iff some keys are remaining in
/// the child trie after the functions returns.
///
/// # Note
///
/// Please note that keys that are residing in the overlay for that child trie when
/// issuing this call are all deleted without counting towards the `limit`. Only keys
/// written during the current block are part of the overlay. Deleting with a `limit`
/// mostly makes sense with an empty overlay for that child trie.
///
/// Calling this function multiple times per block for the same `storage_key` does
/// not make much sense because it is not cumulative when called inside the same block.
/// Use this function to distribute the deletion of a single child trie across multiple
/// blocks.
fn storage_kill_limited(
&mut self,
storage_key: &[u8],
limit: u32,
) -> bool {
#[version(2)]
fn storage_kill(&mut self, storage_key: &[u8], limit: Option<u32>) -> bool {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for being late. The only change that I would have proposed would be to use your KillOutcome enum here directly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Decided against it because everyone else there seems to use primitive types. I think it is not too bad to leave it this way now?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is fine :)

let child_info = ChildInfo::new_default(storage_key);
self.kill_child_storage(&child_info, Some(limit))
self.kill_child_storage(&child_info, limit)
}

/// Check a child storage key.
Expand Down