Contracts pallet: removal on idle (#11202)

* on_initialize -> on_idle

* use remaining_weight info

* no weight_limit for on_idle

* call on_idle in tests

* attempt to fix tests

* run on_initiaize when queue full

* add on_idle to weight info

* add on_idle weight info to on_idle hook

* add basic test for on_initialize with full queue

* disbale check for all keys gone in full queue, full block test

* queue_deth as usize, add comment

* comment was removed by accident

* Update frame/contracts/src/lib.rs

Co-authored-by: Alexander Theißen <alex.theissen@me.com>

* cargo +nightly fmt

* update lazy_removal_does_no_run_on_full_queue_and_full_block

* remove changes in weights.rs

* weights on_idle -> on_process_deletion_queue_batch

* use block number for on_idle

* use BlockNumber for on_initialize

* cargo run --quiet --profile=production  --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark pallet --chain=dev --steps=50 --repeat=20 --pallet=pallet_contracts --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/contracts/src/weights.rs --template=./.maintain/frame-weight-template.hbs

* Update frame/contracts/src/lib.rs

Co-authored-by: Alexander Theißen <alex.theissen@me.com>

* remove outcommented code

* add check that queue still full for test

* cargo fmt

* cargo +nightly fmt

* Update frame/contracts/src/benchmarking/mod.rs

Co-authored-by: Alexander Gryaznov <hi@agryaznov.com>

* fix weights.rs

* add lazy_removal_does_no_run_on_low_remaining_weight test

* Apply suggestions from code review

Co-authored-by: Alexander Gryaznov <hi@agryaznov.com>

Co-authored-by: Alexander Theißen <alex.theissen@me.com>
Co-authored-by: Parity Bot <admin@parity.io>
Co-authored-by: Alexander Gryaznov <hi@agryaznov.com>
This commit is contained in:
Achim Schneider
2022-05-24 16:17:23 +02:00
committed by GitHub
parent 6198a5fb06
commit f744a1a01b
5 changed files with 200 additions and 74 deletions
+72 -41
View File
@@ -25,7 +25,7 @@ use crate::{
wasm::{PrefabWasmModule, ReturnCode as RuntimeReturnCode},
weights::WeightInfo,
BalanceOf, Code, CodeStorage, Config, ContractInfoOf, DefaultAddressGenerator,
DefaultContractAccessWeight, Error, Pallet, Schedule,
DefaultContractAccessWeight, DeletionQueue, Error, Pallet, Schedule,
};
use assert_matches::assert_matches;
use codec::Encode;
@@ -35,7 +35,8 @@ use frame_support::{
parameter_types,
storage::child,
traits::{
BalanceStatus, ConstU32, ConstU64, Contains, Currency, OnInitialize, ReservableCurrency,
BalanceStatus, ConstU32, ConstU64, Contains, Currency, Get, OnIdle, OnInitialize,
ReservableCurrency,
},
weights::{constants::WEIGHT_PER_SECOND, DispatchClass, PostDispatchInfo, Weight},
};
@@ -1610,13 +1611,32 @@ fn lazy_removal_works() {
assert_matches!(child::get(trie, &[99]), Some(42));
// Run the lazy removal
Contracts::on_initialize(Weight::max_value());
Contracts::on_idle(System::block_number(), Weight::max_value());
// Value should be gone now
assert_matches!(child::get::<i32>(trie, &[99]), None);
});
}
#[test]
fn lazy_removal_on_full_queue_works_on_initialize() {
ExtBuilder::default().existential_deposit(50).build().execute_with(|| {
// Fill the deletion queue with dummy values, so that on_initialize attempts
// to clear the queue
Storage::<Test>::fill_queue_with_dummies();
let queue_len_initial = <DeletionQueue<Test>>::decode_len().unwrap_or(0);
// Run the lazy removal
Contracts::on_initialize(System::block_number());
let queue_len_after_on_initialize = <DeletionQueue<Test>>::decode_len().unwrap_or(0);
// Queue length should be decreased after call of on_initialize()
assert!(queue_len_initial - queue_len_after_on_initialize > 0);
});
}
#[test]
fn lazy_batch_removal_works() {
let (code, hash) = compile_module::<Test>("self_destruct").unwrap();
@@ -1661,7 +1681,7 @@ fn lazy_batch_removal_works() {
}
// Run single lazy removal
Contracts::on_initialize(Weight::max_value());
Contracts::on_idle(System::block_number(), Weight::max_value());
// The single lazy removal should have removed all queued tries
for trie in tries.iter() {
@@ -1761,7 +1781,34 @@ fn lazy_removal_partial_remove_works() {
}
#[test]
fn lazy_removal_does_no_run_on_full_block() {
fn lazy_removal_does_no_run_on_full_queue_and_full_block() {
ExtBuilder::default().existential_deposit(50).build().execute_with(|| {
// Fill up the block which should prevent the lazy storage removal from running.
System::register_extra_weight_unchecked(
<Test as system::Config>::BlockWeights::get().max_block,
DispatchClass::Mandatory,
);
// Fill the deletion queue with dummy values, so that on_initialize attempts
// to clear the queue
Storage::<Test>::fill_queue_with_dummies();
// Check that on_initialize() tries to perform lazy removal but removes nothing
// as no more weight is left for that.
let weight_used = Contracts::on_initialize(System::block_number());
let base = <<Test as Config>::WeightInfo as WeightInfo>::on_process_deletion_queue_batch();
assert_eq!(weight_used, base);
// Check that the deletion queue is still full after execution of the
// on_initialize() hook.
let max_len: u32 = <Test as Config>::DeletionQueueDepth::get();
let queue_len: u32 = <DeletionQueue<Test>>::decode_len().unwrap_or(0).try_into().unwrap();
assert_eq!(max_len, queue_len);
});
}
#[test]
fn lazy_removal_does_no_run_on_low_remaining_weight() {
let (code, hash) = compile_module::<Test>("self_destruct").unwrap();
ExtBuilder::default().existential_deposit(50).build().execute_with(|| {
let min_balance = <Test as Config>::Currency::minimum_balance();
@@ -1779,19 +1826,10 @@ fn lazy_removal_does_no_run_on_full_block() {
let addr = Contracts::contract_address(&ALICE, &hash, &[]);
let info = <ContractInfoOf<Test>>::get(&addr).unwrap();
let max_keys = 30;
// Create some storage items for the contract.
let vals: Vec<_> = (0..max_keys)
.map(|i| (blake2_256(&i.encode()), (i as u32), (i as u32).encode()))
.collect();
let trie = &info.child_trie_info();
// Put value into the contracts child trie
for val in &vals {
Storage::<Test>::write(&info.trie_id, &val.0, Some(val.2.clone()), None, false)
.unwrap();
}
<ContractInfoOf<Test>>::insert(&addr, info.clone());
child::put(trie, &[99], &42);
// Terminate the contract
assert_ok!(Contracts::call(
@@ -1806,37 +1844,30 @@ fn lazy_removal_does_no_run_on_full_block() {
// Contract info should be gone
assert!(!<ContractInfoOf::<Test>>::contains_key(&addr));
let trie = info.child_trie_info();
// But value should be still there as the lazy removal did not run, yet.
for val in &vals {
assert_eq!(child::get::<u32>(&trie, &blake2_256(&val.0)), Some(val.1));
}
assert_matches!(child::get(trie, &[99]), Some(42));
// Fill up the block which should prevent the lazy storage removal from running.
System::register_extra_weight_unchecked(
<Test as system::Config>::BlockWeights::get().max_block,
DispatchClass::Mandatory,
);
// Assign a remaining weight which is too low for a successfull deletion of the contract
let low_remaining_weight =
<<Test as Config>::WeightInfo as WeightInfo>::on_process_deletion_queue_batch();
// Run the lazy removal without any limit so that all keys would be removed if there
// had been some weight left in the block.
let weight_used = Contracts::on_initialize(Weight::max_value());
let base = <<Test as Config>::WeightInfo as WeightInfo>::on_initialize();
assert_eq!(weight_used, base);
// Run the lazy removal
Contracts::on_idle(System::block_number(), low_remaining_weight);
// All the keys are still in place
for val in &vals {
assert_eq!(child::get::<u32>(&trie, &blake2_256(&val.0)), Some(val.1));
}
// Value should still be there, since remaining weight was too low for removal
assert_matches!(child::get::<i32>(trie, &[99]), Some(42));
// Run the lazy removal directly which disregards the block limits
Storage::<Test>::process_deletion_queue_batch(Weight::max_value());
// Run the lazy removal while deletion_queue is not full
Contracts::on_initialize(System::block_number());
// Now the keys should be gone
for val in &vals {
assert_eq!(child::get::<u32>(&trie, &blake2_256(&val.0)), None);
}
// Value should still be there, since deletion_queue was not full
assert_matches!(child::get::<i32>(trie, &[99]), Some(42));
// Run on_idle with max remaining weight, this should remove the value
Contracts::on_idle(System::block_number(), Weight::max_value());
// Value should be gone
assert_matches!(child::get::<i32>(trie, &[99]), None);
});
}