contracts: Collect rent for the first block during deployment (#7847)

* Pay first rent during instantiation

* Fix and add new tests

* Do not increment trie id counter on failure

Author: athei

frame/contracts/src/benchmarking/mod.rs

@@ -336,7 +336,7 @@ benchmarks! {
 		let s in 0 .. code::max_pages::<T>() * 64;
 		let data = vec![42u8; (n * 1024) as usize];
 		let salt = vec![42u8; (s * 1024) as usize];
-		let endowment = ConfigCache::<T>::subsistence_threshold_uncached();
+		let endowment = caller_funding::<T>() / 3u32.into();
 		let caller = whitelisted_caller();
 		T::Currency::make_free_balance_be(&caller, caller_funding::<T>());
 		let WasmModule { code, hash, .. } = WasmModule::<T>::dummy_with_mem();
@@ -1373,7 +1373,7 @@ benchmarks! {
 		let hash_len = hashes.get(0).map(|x| x.encode().len()).unwrap_or(0);
 		let hashes_bytes = hashes.iter().flat_map(|x| x.encode()).collect::<Vec<_>>();
 		let hashes_len = hashes_bytes.len();
-		let value = ConfigCache::<T>::subsistence_threshold_uncached();
+		let value = Endow::max::<T>() / (r * API_BENCHMARK_BATCH_SIZE + 2).into();
 		assert!(value > 0u32.into());
 		let value_bytes = value.encode();
 		let value_len = value_bytes.len();
@@ -1457,7 +1457,8 @@ benchmarks! {
 	}: call(origin, callee, 0u32.into(), Weight::max_value(), vec![])
 	verify {
 		for addr in &addresses {
-			instance.alive_info()?;
+			ContractInfoOf::<T>::get(&addr).and_then(|c| c.get_alive())
+				.ok_or_else(|| "Contract should have been instantiated")?;
 		}
 	}
 
@@ -1493,7 +1494,7 @@ benchmarks! {
 		let input_len = inputs.get(0).map(|x| x.len()).unwrap_or(0);
 		let input_bytes = inputs.iter().cloned().flatten().collect::<Vec<_>>();
 		let inputs_len = input_bytes.len();
-		let value = ConfigCache::<T>::subsistence_threshold_uncached();
+		let value = Endow::max::<T>() / (API_BENCHMARK_BATCH_SIZE + 2).into();
 		assert!(value > 0u32.into());
 		let value_bytes = value.encode();
 		let value_len = value_bytes.len();

frame/contracts/src/exec.rs

@@ -322,53 +322,62 @@ where
 		let caller = self.self_account.clone();
 		let dest = Contracts::<T>::contract_address(&caller, code_hash, salt);
 
-		// TrieId has not been generated yet and storage is empty since contract is new.
-		//
-		// Generate it now.
-		let dest_trie_id = Storage::<T>::generate_trie_id(&dest);
+		let output = frame_support::storage::with_transaction(|| {
+			// Generate the trie id in a new transaction to only increment the counter on success.
+			let dest_trie_id = Storage::<T>::generate_trie_id(&dest);
 
-		let output = self.with_nested_context(dest.clone(), dest_trie_id, |nested| {
-			Storage::<T>::place_contract(
-				&dest,
-				nested
-					.self_trie_id
-					.clone()
-					.expect("the nested context always has to have self_trie_id"),
-				code_hash.clone()
-			)?;
-
-			// Send funds unconditionally here. If the `endowment` is below existential_deposit
-			// then error will be returned here.
-			transfer(
-				TransferCause::Instantiate,
-				transactor_kind,
-				&caller,
-				&dest,
-				endowment,
-				nested,
-			)?;
-
-			let executable = nested.loader.load_init(&code_hash)
-				.map_err(|_| Error::<T>::CodeNotFound)?;
-			let output = nested.vm
-				.execute(
-					&executable,
-					nested.new_call_context(caller.clone(), endowment),
-					input_data,
-					gas_meter,
-				).map_err(|e| ExecError { error: e.error, origin: ErrorOrigin::Callee })?;
-
-			// We need each contract that exists to be above the subsistence threshold
-			// in order to keep up the guarantuee that we always leave a tombstone behind
-			// with the exception of a contract that called `seal_terminate`.
-			if T::Currency::total_balance(&dest) < nested.config.subsistence_threshold() {
-				Err(Error::<T>::NewContractNotFunded)?
-			}
+			let output = self.with_nested_context(dest.clone(), dest_trie_id, |nested| {
+				Storage::<T>::place_contract(
+					&dest,
+					nested
+						.self_trie_id
+						.clone()
+						.expect("the nested context always has to have self_trie_id"),
+					code_hash.clone()
+				)?;
+
+				// Send funds unconditionally here. If the `endowment` is below existential_deposit
+				// then error will be returned here.
+				transfer(
+					TransferCause::Instantiate,
+					transactor_kind,
+					&caller,
+					&dest,
+					endowment,
+					nested,
+				)?;
+
+				let executable = nested.loader.load_init(&code_hash)
+					.map_err(|_| Error::<T>::CodeNotFound)?;
+				let output = nested.vm
+					.execute(
+						&executable,
+						nested.new_call_context(caller.clone(), endowment),
+						input_data,
+						gas_meter,
+					).map_err(|e| ExecError { error: e.error, origin: ErrorOrigin::Callee })?;
+
+
+				// Collect the rent for the first block to prevent the creation of very large
+				// contracts that never intended to pay for even one block.
+				// This also makes sure that it is above the subsistence threshold
+				// in order to keep up the guarantuee that we always leave a tombstone behind
+				// with the exception of a contract that called `seal_terminate`.
+				Rent::<T>::charge(&dest)?
+					.and_then(|c| c.get_alive())
+					.ok_or_else(|| Error::<T>::NewContractNotFunded)?;
+
+				// Deposit an instantiation event.
+				deposit_event::<T>(vec![], RawEvent::Instantiated(caller.clone(), dest.clone()));
 
-			// Deposit an instantiation event.
-			deposit_event::<T>(vec![], RawEvent::Instantiated(caller.clone(), dest.clone()));
+				Ok(output)
+			});
 
-			Ok(output)
+			use frame_support::storage::TransactionOutcome::*;
+			match output {
+				Ok(_) => Commit(output),
+				Err(_) => Rollback(output),
+			}
 		})?;
 
 		Ok((dest, output))
@@ -908,7 +917,7 @@ mod tests {
 			let mut ctx = ExecutionContext::top_level(origin.clone(), &cfg, &vm, &loader);
 			place_contract(&BOB, return_ch);
 			set_balance(&origin, 100);
-			set_balance(&dest, 0);
+			let balance = get_balance(&dest);
 
 			let output = ctx.call(
 				dest.clone(),
@@ -919,7 +928,9 @@ mod tests {
 
 			assert!(!output.is_success());
 			assert_eq!(get_balance(&origin), 100);
-			assert_eq!(get_balance(&dest), 0);
+
+			// the rent is still charged
+			assert!(get_balance(&dest) < balance);
 		});
 	}
 
@@ -1057,10 +1068,10 @@ mod tests {
 			let cfg = ConfigCache::preload();
 			let mut ctx = ExecutionContext::top_level(ALICE, &cfg, &vm, &loader);
 
-			set_balance(&ALICE, 100);
+			set_balance(&ALICE, cfg.subsistence_threshold() * 10);
 
 			let result = ctx.instantiate(
-				cfg.subsistence_threshold(),
+				cfg.subsistence_threshold() * 3,
 				&mut GasMeter::<Test>::new(GAS_LIMIT),
 				&input_data_ch,
 				vec![1, 2, 3, 4],
@@ -1307,7 +1318,7 @@ mod tests {
 				// Instantiate a contract and save it's address in `instantiated_contract_address`.
 				let (address, output) = ctx.ext.instantiate(
 					&dummy_ch,
-					ConfigCache::<Test>::subsistence_threshold_uncached(),
+					ConfigCache::<Test>::subsistence_threshold_uncached() * 3,
 					ctx.gas_meter,
 					vec![],
 					&[48, 49, 50],
@@ -1321,8 +1332,7 @@ mod tests {
 		ExtBuilder::default().existential_deposit(15).build().execute_with(|| {
 			let cfg = ConfigCache::preload();
 			let mut ctx = ExecutionContext::top_level(ALICE, &cfg, &vm, &loader);
-			set_balance(&ALICE, 1000);
-			set_balance(&BOB, 100);
+			set_balance(&ALICE, cfg.subsistence_threshold() * 100);
 			place_contract(&BOB, instantiator_ch);
 
 			assert_matches!(
@@ -1431,19 +1441,20 @@ mod tests {
 		let vm = MockVm::new();
 		let mut loader = MockLoader::empty();
 		let rent_allowance_ch = loader.insert(|ctx| {
+			let allowance = ConfigCache::<Test>::subsistence_threshold_uncached() * 3;
 			assert_eq!(ctx.ext.rent_allowance(), <BalanceOf<Test>>::max_value());
-			ctx.ext.set_rent_allowance(10);
-			assert_eq!(ctx.ext.rent_allowance(), 10);
+			ctx.ext.set_rent_allowance(allowance);
+			assert_eq!(ctx.ext.rent_allowance(), allowance);
 			exec_success()
 		});
 
 		ExtBuilder::default().build().execute_with(|| {
 			let cfg = ConfigCache::preload();
 			let mut ctx = ExecutionContext::top_level(ALICE, &cfg, &vm, &loader);
-			set_balance(&ALICE, 100);
+			set_balance(&ALICE, cfg.subsistence_threshold() * 10);
 
 			let result = ctx.instantiate(
-				cfg.subsistence_threshold(),
+				cfg.subsistence_threshold() * 5,
 				&mut GasMeter::<Test>::new(GAS_LIMIT),
 				&rent_allowance_ch,
 				vec![],

frame/contracts/src/storage.rs

@@ -27,7 +27,7 @@ use codec::{Encode, Decode};
 use sp_std::prelude::*;
 use sp_std::marker::PhantomData;
 use sp_io::hashing::blake2_256;
-use sp_runtime::traits::Bounded;
+use sp_runtime::traits::{Bounded, Saturating};
 use sp_core::crypto::UncheckedFrom;
 use frame_support::{
 	dispatch::DispatchResult,
@@ -182,7 +182,11 @@ where
 					code_hash: ch,
 					storage_size: 0,
 					trie_id,
-					deduct_block: <frame_system::Module<T>>::block_number(),
+					deduct_block:
+						// We want to charge rent for the first block in advance. Therefore we
+						// treat the contract as if it was created in the last block and then
+						// charge rent for it during instantation.
+						<frame_system::Module<T>>::block_number().saturating_sub(1u32.into()),
 					rent_allowance: <BalanceOf<T>>::max_value(),
 					pair_count: 0,
 					last_write: None,