contracts: Charge rent for code storage [WIP]

Author: athei

frame/contracts/src/exec.rs

@@ -269,16 +269,17 @@ where
 			Err(Error::<T>::MaxCallDepthReached)?
 		}
 
+		let contract = <ContractInfoOf<T>>::get(&dest)
+			.and_then(|contract| contract.get_alive())
+			.ok_or(Error::<T>::NotCallable)?;
+
 		// This charges the rent and denies access to a contract that is in need of
 		// eviction by returning `None`. We cannot evict eagerly here because those
 		// changes would be rolled back in case this contract is called by another
 		// contract.
 		// See: https://github.com/paritytech/substrate/issues/6439#issuecomment-648754324
-		let contract = if let Ok(Some(ContractInfo::Alive(info))) = Rent::<T>::charge(&dest) {
-			info
-		} else {
-			Err(Error::<T>::NotCallable)?
-		};
+		let contract = Rent::<T>::charge(&dest, contract)?
+			.ok_or(Error::<T>::NotCallable)?;
 
 		let transactor_kind = self.transactor_kind();
 		let caller = self.self_account.clone();
@@ -360,9 +361,12 @@ where
 				// 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)?;
+				<ContractInfoOf<T>>::get(&dest)
+					.and_then(|contract| contract.get_alive())
+					.map(|contract| Rent::<T>::charge(&dest, contract))
+					.transpose()?
+					.flatten()
+					.ok_or(Error::<T>::NewContractNotFunded)?;
 
 				// Deposit an instantiation event.
 				deposit_event::<T>(vec![], RawEvent::Instantiated(caller.clone(), dest.clone()));

frame/contracts/src/lib.rs

@@ -432,6 +432,8 @@ decl_error! {
 		/// This can either happen when the accumulated storage in bytes is too large or
 		/// when number of storage items is too large.
 		StorageExhausted,
+		/// A contract with the same AccountId already exists.
+		DuplicateContract,
 	}
 }
 

frame/contracts/src/rent.rs

@@ -234,9 +234,9 @@ where
 		current_block_number: T::BlockNumber,
 		verdict: Verdict<T>,
 		allow_eviction: bool,
-	) -> Result<Option<ContractInfo<T>>, DispatchError> {
+	) -> Result<Option<AliveContractInfo<T>>, DispatchError> {
 		match verdict {
-			Verdict::Exempt => return Ok(Some(ContractInfo::Alive(alive_contract_info))),
+			Verdict::Exempt => return Ok(Some(alive_contract_info)),
 			Verdict::Evict { amount: _ } if !allow_eviction => {
 				Ok(None)
 			}
@@ -263,18 +263,18 @@ where
 				<ContractInfoOf<T>>::insert(account, &tombstone_info);
 				crate::wasm::remove_code_user::<T>(alive_contract_info.code_hash);
 				<Module<T>>::deposit_event(RawEvent::Evicted(account.clone(), true));
-				Ok(Some(tombstone_info))
+				Ok(None)
 			}
 			Verdict::Charge { amount } => {
-				let contract_info = ContractInfo::Alive(AliveContractInfo::<T> {
+				let contract = ContractInfo::Alive(AliveContractInfo::<T> {
 					rent_allowance: alive_contract_info.rent_allowance - amount.peek(),
 					deduct_block: current_block_number,
 					rent_payed: alive_contract_info.rent_payed.saturating_add(amount.peek()),
 					..alive_contract_info
 				});
-				<ContractInfoOf<T>>::insert(account, &contract_info);
+				<ContractInfoOf<T>>::insert(account, &contract);
 				amount.withdraw(account);
-				Ok(Some(contract_info))
+				Ok(Some(contract.get_alive().expect("We just constructed it as alive. qed")))
 			}
 		}
 	}
@@ -284,21 +284,15 @@ where
 	/// This functions does **not** evict the contract. It returns `None` in case the
 	/// contract is in need of eviction. [`try_eviction`] must
 	/// be called to perform the eviction.
-	pub fn charge(account: &T::AccountId) -> Result<Option<ContractInfo<T>>, DispatchError> {
-		let contract_info = <ContractInfoOf<T>>::get(account);
-		let alive_contract_info = match contract_info {
-			None | Some(ContractInfo::Tombstone(_)) => return Ok(contract_info),
-			Some(ContractInfo::Alive(contract)) => contract,
-		};
-
+	pub fn charge(account: &T::AccountId, contract: AliveContractInfo<T>) -> Result<Option<AliveContractInfo<T>>, DispatchError> {
 		let current_block_number = <frame_system::Module<T>>::block_number();
 		let verdict = Self::consider_case(
 			account,
 			current_block_number,
 			Zero::zero(),
-			&alive_contract_info,
+			&contract,
 		);
-		Self::enact_verdict(account, alive_contract_info, current_block_number, verdict, false)
+		Self::enact_verdict(account, contract, current_block_number, verdict, false)
 	}
 
 	/// Process a report that a contract under the given address should be evicted.
@@ -380,8 +374,8 @@ where
 
 		// Check what happened after enaction of the verdict.
 		let alive_contract_info = match new_contract_info.map_err(|_| IsTombstone)? {
-			None | Some(ContractInfo::Tombstone(_)) => return Err(IsTombstone),
-			Some(ContractInfo::Alive(contract)) => contract,
+			None => return Err(IsTombstone),
+			Some(contract) => contract,
 		};
 
 		// Compute how much would the fee per block be with the *updated* balance.

frame/contracts/src/storage.rs

@@ -164,31 +164,30 @@ where
 		account: &AccountIdOf<T>,
 		trie_id: TrieId,
 		ch: CodeHash<T>,
-	) -> Result<(), &'static str> {
-		<ContractInfoOf<T>>::mutate(account, |maybe_contract_info| {
-			if maybe_contract_info.is_some() {
-				return Err("Alive contract or tombstone already exists");
+	) -> Result<AliveContractInfo<T>, &'static str> {
+		<ContractInfoOf<T>>::try_mutate(account, |existing| {
+			if existing.is_some() {
+				return Err(Error::<T>::DuplicateContract.into());
 			}
 
-			*maybe_contract_info = Some(
-				AliveContractInfo::<T> {
-					code_hash: ch,
-					storage_size: 0,
-					trie_id,
-					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(),
-					rent_payed: <BalanceOf<T>>::zero(),
-					pair_count: 0,
-					last_write: None,
-				}
-				.into(),
-			);
-
-			Ok(())
+			let contract = AliveContractInfo::<T> {
+				code_hash: ch,
+				storage_size: 0,
+				trie_id,
+				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(),
+				rent_payed: <BalanceOf<T>>::zero(),
+				pair_count: 0,
+				last_write: None,
+			};
+
+			*existing = Some(contract.clone().into());
+
+			Ok(contract)
 		})
 	}
 

frame/contracts/src/tests.rs

@@ -93,7 +93,7 @@ pub mod test_utils {
 	pub fn place_contract(address: &AccountIdOf<Test>, code_hash: CodeHash<Test>) {
 		let trie_id = Storage::<Test>::generate_trie_id(address);
 		set_balance(address, ConfigCache::<Test>::subsistence_threshold_uncached() * 10);
-		Storage::<Test>::place_contract(&address, trie_id, code_hash).unwrap()
+		Storage::<Test>::place_contract(&address, trie_id, code_hash).unwrap();
 	}
 	pub fn set_balance(who: &AccountIdOf<Test>, amount: u64) {
 		let imbalance = Balances::deposit_creating(who, amount);