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
Show all changes
48 commits
Select commit Hold shift + click to select a range
31ab537
[Contracts review] Overflowing bounded `DeletionQueue` allows DoS aga…
pgherveou Mar 24, 2023
8a0b92f
wip
pgherveou Mar 24, 2023
10cccfd
wip
pgherveou Mar 24, 2023
329fe92
wip
pgherveou Mar 27, 2023
b549582
wip
pgherveou Mar 27, 2023
71f01f3
wip
pgherveou Mar 27, 2023
aed8821
fix doc
pgherveou Mar 27, 2023
85cca3a
wip
pgherveou Mar 27, 2023
1d27eeb
PR review
pgherveou Mar 27, 2023
f6bcda1
unbreak tests
pgherveou Mar 27, 2023
b4592cc
fixes
pgherveou Mar 28, 2023
2e722bd
update budget computation
pgherveou Mar 28, 2023
2867a3c
PR comment: use BlockWeights::get().max_block
pgherveou Mar 28, 2023
3a17f2b
PR comment: Update queue_trie_for_deletion signature
pgherveou Mar 28, 2023
f7f1f42
PR comment: update deletion budget docstring
pgherveou Mar 28, 2023
20a18c3
PR comment: impl Default with derive(DefaultNoBound)
pgherveou Mar 28, 2023
bd99f93
PR comment: Remove DeletedContract
pgherveou Mar 28, 2023
737a48a
PR comment Add ring_buffer test
pgherveou Mar 28, 2023
2d44b3d
remove missed comment
pgherveou Mar 28, 2023
b0bbbcf
misc comments
pgherveou Mar 28, 2023
d4600e0
contracts: add sr25519_recover
pgherveou Mar 27, 2023
7002c46
Merge branch 'master' into pg/contracts-review-overflowing-bounded-de…
pgherveou Mar 28, 2023
c30bf2b
Revert "contracts: add sr25519_recover"
pgherveou Mar 28, 2023
c17e0e6
".git/.scripts/commands/bench/bench.sh" pallet dev pallet_contracts
Mar 28, 2023
1880b62
PR comments update print_schedule
pgherveou Mar 29, 2023
9fc9db6
Update frame/contracts/src/benchmarking/mod.rs
pgherveou Mar 29, 2023
c0a7523
Update frame/contracts/src/storage.rs
pgherveou Mar 29, 2023
efafd7d
Update frame/contracts/src/storage.rs
pgherveou Mar 29, 2023
717151e
rm temporary fixes
pgherveou Mar 29, 2023
2da3648
fix extra ;
pgherveou Mar 29, 2023
5751764
Update frame/contracts/src/storage.rs
pgherveou Mar 30, 2023
b31603e
Update frame/contracts/src/storage.rs
pgherveou Mar 30, 2023
e269ad4
Update frame/contracts/src/lib.rs
pgherveou Mar 30, 2023
2f1d4fa
Update frame/contracts/src/lib.rs
pgherveou Mar 30, 2023
0b985aa
Support stable rust for compiling the runtime (#13580)
bkchr Mar 29, 2023
dedef69
github PR commit fixes
pgherveou Mar 30, 2023
6b31f97
Revert "Support stable rust for compiling the runtime (#13580)"
pgherveou Mar 30, 2023
4cec441
Restore DeletionQueueMap
pgherveou Mar 30, 2023
092d51c
fix namings
pgherveou Mar 30, 2023
6df076f
PR comment
pgherveou Mar 30, 2023
9d277cd
move comments
pgherveou Mar 30, 2023
5da9750
Update frame/contracts/src/storage.rs
pgherveou Mar 30, 2023
1e5eaaf
Update frame/contracts/src/storage.rs
pgherveou Mar 30, 2023
cec7e08
fixes
pgherveou Mar 30, 2023
d028fdc
Merge remote-tracking branch 'origin/master' into pg/contracts-review…
Mar 30, 2023
a7e45c9
Merge remote-tracking branch 'origin/master' into pg/contracts-review…
Mar 30, 2023
ef324ec
Merge remote-tracking branch 'origin/master' into pg/contracts-review…
Mar 30, 2023
7d39202
Merge remote-tracking branch 'origin/master' into pg/contracts-review…
Mar 31, 2023
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
update budget computation
  • Loading branch information
pgherveou committed Mar 28, 2023
commit 2e722bd34deb4471cdfa90d8445f795f3a0326bf
17 changes: 3 additions & 14 deletions frame/contracts/src/benchmarking/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,18 +219,6 @@ benchmarks! {
ContractInfo::<T>::process_deletion_queue_batch(Weight::MAX)
}

#[pov_mode = Measured]
on_initialize_per_queue_item {
let q in 0..1024;
for i in 0 .. q {
let instance = Contract::<T>::with_index(i, WasmModule::dummy(), vec![])?;
instance.info()?.queue_trie_for_deletion()?;
ContractInfoOf::<T>::remove(instance.account_id);
}
}: {
ContractInfo::<T>::process_deletion_queue_batch(Weight::MAX)
}

// This benchmarks the additional weight that is charged when a contract is executed the
// first time after a new schedule was deployed: For every new schedule a contract needs
// to re-run the instrumentation once.
Expand All @@ -239,6 +227,7 @@ benchmarks! {
let c in 0 .. Perbill::from_percent(49).mul_ceil(T::MaxCodeLen::get());
let WasmModule { code, hash, .. } = WasmModule::<T>::sized(c, Location::Call);
Contracts::<T>::store_code_raw(code, whitelisted_caller())?;

let schedule = T::Schedule::get();
let mut gas_meter = GasMeter::new(Weight::MAX);
let mut module = PrefabWasmModule::from_storage(hash, &schedule, &mut gas_meter)?;
Expand Down Expand Up @@ -3020,8 +3009,8 @@ benchmarks! {
print_schedule {
#[cfg(feature = "std")]
{
let empty_queue_throughput = ContractInfo::<T>::deletion_budget(0, Weight::MAX);
let full_queue_throughput = ContractInfo::<T>::deletion_budget(1024, Weight::MAX);
let empty_queue_throughput = ContractInfo::<T>::deletion_budget(Weight::MAX);
let full_queue_throughput = ContractInfo::<T>::deletion_budget(Weight::MAX);
println!("{:#?}", Schedule::<T>::default());
println!("###############################################");
println!("Lazy deletion weight per key: {}", empty_queue_throughput.0);
Expand Down
7 changes: 3 additions & 4 deletions frame/contracts/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -904,10 +904,9 @@ pub mod pallet {
/// A pair of monotonic counters used to track the latest contract marked for deletion
/// and the latest deleted contract in DeletionQueueMap.
///
/// These two nonces let us keep track of the length of the queue, so we can calculate the
/// weight of the deletion budget, additionally, when we iterate the map to remove contracts, we
/// simply use the `delete_nonce` counter and don't pay the cost of an extra call to
/// `sp_io::storage::next_key` to lookup the next entry in the map
/// When we iterate the map to remove contracts, we simply use the `delete_nonce` counter and
/// don't pay the cost of an extra call to `sp_io::storage::next_key` to lookup the next entry
/// in the map
#[pallet::storage]
pub(crate) type DeletionQueueNonces<T: Config> = StorageValue<_, DeletionQueue<T>, ValueQuery>;
}
Expand Down
18 changes: 6 additions & 12 deletions frame/contracts/src/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,19 +212,15 @@ impl<T: Config> ContractInfo<T> {
/// Calculates the weight that is necessary to remove one key from the trie and how many
/// of those keys can be deleted from the deletion queue given the supplied queue length
/// and weight limit.
pub fn deletion_budget(queue_len: u32, weight_limit: Weight) -> (Weight, u32) {
pub fn deletion_budget(weight_limit: Weight) -> (Weight, u32) {
let base_weight = T::WeightInfo::on_process_deletion_queue_batch();
let weight_per_queue_item = T::WeightInfo::on_initialize_per_queue_item(1) -
T::WeightInfo::on_initialize_per_queue_item(0);
let weight_per_key = T::WeightInfo::on_initialize_per_trie_key(1) -
T::WeightInfo::on_initialize_per_trie_key(0);
let decoding_weight = weight_per_queue_item.saturating_mul(queue_len as u64);

// `weight_per_key` being zero makes no sense and would constitute a failure to
// benchmark properly. We opt for not removing any keys at all in this case.
let key_budget = weight_limit
.saturating_sub(base_weight)
.saturating_sub(decoding_weight)
.checked_div_per_component(&weight_per_key)
.unwrap_or(0) as u32;

Expand All @@ -236,14 +232,12 @@ impl<T: Config> ContractInfo<T> {
/// It returns the amount of weight used for that task.
pub fn process_deletion_queue_batch(weight_limit: Weight) -> Weight {
let mut queue = <DeletionQueue<T>>::load();
let queue_len = queue.len();

if queue_len == 0 {
if queue.is_empty() {
return Weight::zero()
}

let (weight_per_key, mut remaining_key_budget) =
Self::deletion_budget(queue_len, weight_limit);
let (weight_per_key, mut remaining_key_budget) = Self::deletion_budget(weight_limit);

// We want to check whether we have enough weight to decode the queue before
// proceeding. Too little weight for decoding might happen during runtime upgrades
Expand Down Expand Up @@ -396,8 +390,8 @@ impl<T: Config> DeletionQueue<T> {
}

/// The number of contracts marked for deletion.
fn len(&self) -> u32 {
self.insert_nonce.wrapping_sub(self.delete_nonce)
fn is_empty(&self) -> bool {
self.insert_nonce.wrapping_sub(self.delete_nonce) == 0
}

/// Insert a contract in the deletion queue.
Expand All @@ -409,7 +403,7 @@ impl<T: Config> DeletionQueue<T> {

/// Fetch the next contract to be deleted.
fn next(&mut self) -> Option<DeletionQueueEntry<T>> {
if self.len() == 0 {
if self.is_empty() {
return None
}

Expand Down
4 changes: 2 additions & 2 deletions frame/contracts/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2032,7 +2032,7 @@ fn lazy_removal_partial_remove_works() {
// We create a contract with some extra keys above the weight limit
let extra_keys = 7u32;
let weight_limit = Weight::from_parts(5_000_000_000, 0);
let (_, max_keys) = ContractInfo::<Test>::deletion_budget(1, weight_limit);
let (_, max_keys) = ContractInfo::<Test>::deletion_budget(weight_limit);
let vals: Vec<_> = (0..max_keys + extra_keys)
.map(|i| (blake2_256(&i.encode()), (i as u32), (i as u32).encode()))
.collect();
Expand Down Expand Up @@ -2210,7 +2210,7 @@ fn lazy_removal_does_not_use_all_weight() {
.account_id;

let info = get_contract(&addr);
let (weight_per_key, max_keys) = ContractInfo::<Test>::deletion_budget(1, weight_limit);
let (weight_per_key, max_keys) = ContractInfo::<Test>::deletion_budget(weight_limit);

// We create a contract with one less storage item than we can remove within the limit
let vals: Vec<_> = (0..max_keys - 1)
Expand Down
31 changes: 0 additions & 31 deletions frame/contracts/src/weights.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.