Fix lazy batch contract removal (paritytech#10728)

* Fix lazy batch contract removal

* Apply suggestions

* Qualify ChildInfo

* Negligible change to restart pipeline

* Revert "Negligible change to restart pipeline"

This reverts commit b38abb6.

* cargo run --quiet --profile=production  --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --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

Co-authored-by: Parity Bot <[email protected]>

frame/contracts/src/storage.rs

@@ -268,15 +268,17 @@ where
 
 		let mut queue = <DeletionQueue<T>>::get();
 
-		if let (Some(trie), true) = (queue.get(0), remaining_key_budget > 0) {
+		while !queue.is_empty() && remaining_key_budget > 0 {
+			// Cannot panic due to loop condition
+			let trie = &mut queue[0];
 			let outcome =
 				child::kill_storage(&child_trie_info(&trie.trie_id), Some(remaining_key_budget));
 			let keys_removed = match outcome {
 				// This happens when our budget wasn't large enough to remove all keys.
 				KillStorageResult::SomeRemaining(count) => count,
 				KillStorageResult::AllRemoved(count) => {
 					// We do not care to preserve order. The contract is deleted already and
-					// noone waits for the trie to be deleted.
+					// no one waits for the trie to be deleted.
 					queue.swap_remove(0);
 					count
 				},

frame/contracts/src/tests.rs

@@ -1513,6 +1513,59 @@ fn lazy_removal_works() {
 	});
 }
 
+#[test]
+fn lazy_batch_removal_works() {
+	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();
+		let _ = Balances::deposit_creating(&ALICE, 1000 * min_balance);
+		let mut tries: Vec<child::ChildInfo> = vec![];
+
+		for i in 0..3u8 {
+			assert_ok!(Contracts::instantiate_with_code(
+				Origin::signed(ALICE),
+				min_balance * 100,
+				GAS_LIMIT,
+				None,
+				code.clone(),
+				vec![],
+				vec![i],
+			),);
+
+			let addr = Contracts::contract_address(&ALICE, &hash, &[i]);
+			let info = <ContractInfoOf<Test>>::get(&addr).unwrap();
+			let trie = &info.child_trie_info();
+
+			// Put value into the contracts child trie
+			child::put(trie, &[99], &42);
+
+			// Terminate the contract. Contract info should be gone, but value should be still there
+			// as the lazy removal did not run, yet.
+			assert_ok!(Contracts::call(
+				Origin::signed(ALICE),
+				addr.clone(),
+				0,
+				GAS_LIMIT,
+				None,
+				vec![]
+			));
+
+			assert!(!<ContractInfoOf::<Test>>::contains_key(&addr));
+			assert_matches!(child::get(trie, &[99]), Some(42));
+
+			tries.push(trie.clone())
+		}
+
+		// Run single lazy removal
+		Contracts::on_initialize(Weight::max_value());
+
+		// The single lazy removal should have removed all queued tries
+		for trie in tries.iter() {
+			assert_matches!(child::get::<i32>(trie, &[99]), None);
+		}
+	});
+}
+
 #[test]
 fn lazy_removal_partial_remove_works() {
 	let (code, hash) = compile_module::<Test>("self_destruct").unwrap();