Contracts remove deposit accounts

frame/contracts/src/address.rs

@@ -40,12 +40,6 @@ pub trait AddressGenerator<T: Config> {
 		input_data: &[u8],
 		salt: &[u8],
 	) -> T::AccountId;
-
-	/// The address of the deposit account of `contract_addr`.
-	///
-	/// The address is generated once on instantiation and then stored in the contracts
-	/// metadata. Hence changes do only affect newly created contracts.
-	fn deposit_address(contract_addr: &T::AccountId) -> T::AccountId;
 }
 
 /// Default address generator.
@@ -71,11 +65,4 @@ impl<T: Config> AddressGenerator<T> for DefaultAddressGenerator {
 		Decode::decode(&mut TrailingZeroInput::new(entropy.as_ref()))
 			.expect("infinite length input; no invalid inputs for type; qed")
 	}
-
-	/// Formula: `hash("contract_depo_v1" ++ contract_addr)`
-	fn deposit_address(contract_addr: &T::AccountId) -> T::AccountId {
-		let entropy = (b"contract_depo_v1", contract_addr).using_encoded(T::Hashing::hash);
-		Decode::decode(&mut TrailingZeroInput::new(entropy.as_ref()))
-			.expect("infinite length input; no invalid inputs for type; qed")
-	}
 }

frame/contracts/src/benchmarking/mod.rs

@@ -30,7 +30,9 @@ use self::{
 };
 use crate::{
 	exec::{AccountIdOf, Key},
-	migration::{codegen::LATEST_MIGRATION_VERSION, v09, v10, v11, v12, v13, v14, MigrationStep},
+	migration::{
+		codegen::LATEST_MIGRATION_VERSION, v09, v10, v11, v12, v13, v14, v15, MigrationStep,
+	},
 	wasm::CallFlags,
 	Pallet as Contracts, *,
 };
@@ -296,6 +298,19 @@ benchmarks! {
 		m.step();
 	}
 
+	// This benchmarks the v15 migration step (remove deposit account).
+	#[pov_mode = Measured]
+	v15_migration_step {
+		let contract = <Contract<T>>::with_caller(
+			whitelisted_caller(), WasmModule::dummy(), vec![],
+		)?;
+
+		v15::store_old_contract_info::<T>(contract.account_id.clone(), contract.info()?);
+		let mut m = v15::Migration::<T>::default();
+	}: {
+		m.step();
+	}
+
 	// This benchmarks the weight of executing Migration::migrate to execute a noop migration.
 	#[pov_mode = Measured]
 	migration_noop {
@@ -397,10 +412,9 @@ benchmarks! {
 		let addr = Contracts::<T>::contract_address(&caller, &hash, &input, &salt);
 	}: _(origin, value, Weight::MAX, None, code, input, salt)
 	verify {
-		let deposit_account = Contract::<T>::address_info(&addr)?.deposit_account().clone();
-		let deposit = T::Currency::balance(&deposit_account);
+		let deposit = T::Currency::balance_on_hold(&HoldReason::StorageDepositReserve.into(), &addr);
 		// uploading the code reserves some balance in the callers account
-		let code_deposit = T::Currency::total_balance_on_hold(&caller);
+		let code_deposit = T::Currency::balance_on_hold(&HoldReason::CodeUploadDepositReserve.into(), &caller);
 		assert_eq!(
 			T::Currency::balance(&caller),
 			caller_funding::<T>() - value - deposit - code_deposit - Pallet::<T>::min_balance(),
@@ -427,8 +441,7 @@ benchmarks! {
 		Contracts::<T>::store_code_raw(code, caller.clone())?;
 	}: _(origin, value, Weight::MAX, None, hash, input, salt)
 	verify {
-		let deposit_account = Contract::<T>::address_info(&addr)?.deposit_account().clone();
-		let deposit = T::Currency::balance(&deposit_account);
+		let deposit = T::Currency::balance_on_hold(&HoldReason::StorageDepositReserve.into(), &addr);
 		// value was removed from the caller
 		assert_eq!(
 			T::Currency::balance(&caller),
@@ -451,15 +464,13 @@ benchmarks! {
 		let instance = Contract::<T>::with_caller(
 			whitelisted_caller(), WasmModule::dummy(), vec![],
 		)?;
-		let deposit_account = instance.info()?.deposit_account().clone();
 		let value = Pallet::<T>::min_balance();
 		let origin = RawOrigin::Signed(instance.caller.clone());
 		let callee = instance.addr.clone();
 		let before = T::Currency::balance(&instance.account_id);
-		let before_deposit = T::Currency::balance(&deposit_account);
 	}: _(origin, callee, value, Weight::MAX, None, data)
 	verify {
-		let deposit = T::Currency::balance(&deposit_account);
+		let deposit = T::Currency::balance_on_hold(&HoldReason::StorageDepositReserve.into(), &instance.account_id);
 		// value and value transferred via call should be removed from the caller
 		assert_eq!(
 			T::Currency::balance(&instance.caller),
@@ -930,15 +941,14 @@ benchmarks! {
 		});
 		let instance = Contract::<T>::new(code, vec![])?;
 		let origin = RawOrigin::Signed(instance.caller.clone());
-		let deposit_account = instance.info()?.deposit_account().clone();
 		assert_eq!(T::Currency::total_balance(&beneficiary), 0u32.into());
 		assert_eq!(T::Currency::balance(&instance.account_id), Pallet::<T>::min_balance() * 2u32.into());
-		assert_ne!(T::Currency::balance(&deposit_account), 0u32.into());
+		assert_ne!(T::Currency::balance_on_hold(&HoldReason::StorageDepositReserve.into(), &instance.account_id), 0u32.into());
 	}: call(origin, instance.addr.clone(), 0u32.into(), Weight::MAX, None, vec![])
 	verify {
 		if r > 0 {
 			assert_eq!(T::Currency::total_balance(&instance.account_id), 0u32.into());
-			assert_eq!(T::Currency::total_balance(&deposit_account), 0u32.into());
+			assert_eq!(T::Currency::balance_on_hold(&HoldReason::StorageDepositReserve.into(), &instance.account_id), 0u32.into());
 			assert_eq!(T::Currency::total_balance(&beneficiary), Pallet::<T>::min_balance() * 2u32.into());
 		}
 	}

frame/contracts/src/exec.rs

@@ -19,10 +19,10 @@
 use crate::unsafe_debug::ExecutionObserver;
 use crate::{
 	gas::GasMeter,
-	storage::{self, meter::Diff, DepositAccount, WriteOutcome},
+	storage::{self, meter::Diff, WriteOutcome},
 	BalanceOf, CodeHash, CodeInfo, CodeInfoOf, Config, ContractInfo, ContractInfoOf,
 	DebugBufferVec, Determinism, Error, Event, Nonce, Origin, Pallet as Contracts, Schedule,
-	System, WasmBlob, LOG_TARGET,
+	WasmBlob, LOG_TARGET,
 };
 use frame_support::{
 	crypto::ecdsa::ECDSAExt,
@@ -33,7 +33,7 @@ use frame_support::{
 	storage::{with_transaction, TransactionOutcome},
 	traits::{
 		fungible::{Inspect, Mutate},
-		tokens::{Fortitude::Polite, Preservation},
+		tokens::Preservation,
 		Contains, OriginTrait, Randomness, Time,
 	},
 	weights::Weight,
@@ -547,7 +547,7 @@ enum CachedContract<T: Config> {
 	///
 	/// In this case a reload is neither allowed nor possible. Please note that recursive
 	/// calls cannot remove a contract as this is checked and denied.
-	Terminated(DepositAccount<T>),
+	Terminated,
 }
 
 impl<T: Config> CachedContract<T> {
@@ -568,15 +568,6 @@ impl<T: Config> CachedContract<T> {
 			None
 		}
 	}
-
-	/// Returns `Some` iff the contract is not `Cached::Invalidated`.
-	fn deposit_account(&self) -> Option<&DepositAccount<T>> {
-		match self {
-			CachedContract::Cached(contract) => Some(contract.deposit_account()),
-			CachedContract::Terminated(deposit_account) => Some(&deposit_account),
-			CachedContract::Invalidated => None,
-		}
-	}
 }
 
 impl<T: Config> Frame<T> {
@@ -655,9 +646,7 @@ impl<T: Config> CachedContract<T> {
 	/// Terminate and return the contract info.
 	fn terminate(&mut self, account_id: &T::AccountId) -> ContractInfo<T> {
 		self.load(account_id);
-		let contract = get_cached_or_panic_after_load!(self);
-		let deposit_account = contract.deposit_account().clone();
-		get_cached_or_panic_after_load!(mem::replace(self, Self::Terminated(deposit_account)))
+		get_cached_or_panic_after_load!(mem::replace(self, Self::Terminated))
 	}
 }
 
@@ -955,7 +944,7 @@ where
 			match (entry_point, delegated_code_hash) {
 				(ExportedFunction::Constructor, _) => {
 					// It is not allowed to terminate a contract inside its constructor.
-					if matches!(frame.contract_info, CachedContract::Terminated(_)) {
+					if matches!(frame.contract_info, CachedContract::Terminated) {
 						return Err(Error::<T>::TerminatedInConstructor.into())
 					}
 
@@ -1059,18 +1048,8 @@ where
 			// in its contract info. The load is necessary to pull it from storage in case
 			// it was invalidated.
 			frame.contract_info.load(account_id);
-			let deposit_account = frame
-				.contract_info
-				.deposit_account()
-				.expect(
-					"Is only `None` when the info is invalidated.
-				We just re-loaded from storage which either makes the state `Cached` or `Terminated`.
-				qed",
-				)
-				.clone();
 			let mut contract = frame.contract_info.into_contract();
-			prev.nested_storage
-				.absorb(frame.nested_storage, deposit_account, contract.as_mut());
+			prev.nested_storage.absorb(frame.nested_storage, account_id, contract.as_mut());
 
 			// In case the contract wasn't terminated we need to persist changes made to it.
 			if let Some(contract) = contract {
@@ -1105,14 +1084,10 @@ where
 			if !persist {
 				return
 			}
-			let deposit_account = self.first_frame.contract_info.deposit_account().expect(
-				"Is only `None` when the info is invalidated. The first frame can't be invalidated.
-				qed",
-			).clone();
 			let mut contract = self.first_frame.contract_info.as_contract();
 			self.storage_meter.absorb(
 				mem::take(&mut self.first_frame.nested_storage),
-				deposit_account,
+				&self.first_frame.account_id,
 				contract.as_deref_mut(),
 			);
 			if let Some(contract) = contract {
@@ -1311,14 +1286,8 @@ where
 		}
 		let frame = self.top_frame_mut();
 		let info = frame.terminate();
-		frame.nested_storage.terminate(&info);
-		System::<T>::dec_consumers(&frame.account_id);
-		Self::transfer(
-			Preservation::Expendable,
-			&frame.account_id,
-			beneficiary,
-			T::Currency::reducible_balance(&frame.account_id, Preservation::Expendable, Polite),
-		)?;
+		frame.nested_storage.terminate(&info, beneficiary.clone());
+
 		info.queue_trie_for_deletion();
 		ContractInfoOf::<T>::remove(&frame.account_id);
 		E::decrement_refcount(info.code_hash);
@@ -1327,7 +1296,7 @@ where
 			E::decrement_refcount(*code_hash);
 			frame
 				.nested_storage
-				.charge_deposit(info.deposit_account().clone(), StorageDeposit::Refund(*deposit));
+				.charge_deposit(frame.account_id.clone(), StorageDeposit::Refund(*deposit));
 		}
 
 		Contracts::<T>::deposit_event(
@@ -1521,8 +1490,7 @@ where
 		let deposit = StorageDeposit::Charge(new_base_deposit)
 			.saturating_sub(&StorageDeposit::Charge(old_base_deposit));
 
-		let deposit_account = info.deposit_account().clone();
-		frame.nested_storage.charge_deposit(deposit_account, deposit);
+		frame.nested_storage.charge_deposit(frame.account_id.clone(), deposit);
 
 		E::increment_refcount(hash)?;
 		E::decrement_refcount(prev_hash);
@@ -1573,7 +1541,7 @@ where
 		<WasmBlob<T>>::increment_refcount(code_hash)?;
 		frame
 			.nested_storage
-			.charge_deposit(info.deposit_account().clone(), StorageDeposit::Charge(deposit));
+			.charge_deposit(frame.account_id.clone(), StorageDeposit::Charge(deposit));
 		Ok(())
 	}
 
@@ -1589,7 +1557,7 @@ where
 
 		frame
 			.nested_storage
-			.charge_deposit(info.deposit_account().clone(), StorageDeposit::Refund(deposit));
+			.charge_deposit(frame.account_id.clone(), StorageDeposit::Refund(deposit));
 		Ok(())
 	}
 }

frame/contracts/src/lib.rs

@@ -187,7 +187,7 @@ pub mod pallet {
 	use sp_runtime::Perbill;
 
 	/// The current storage version.
-	pub(crate) const STORAGE_VERSION: StorageVersion = StorageVersion::new(14);
+	pub(crate) const STORAGE_VERSION: StorageVersion = StorageVersion::new(15);
 
 	#[pallet::pallet]
 	#[pallet::storage_version(STORAGE_VERSION)]
@@ -899,6 +899,20 @@ pub mod pallet {
 			/// The code hash that was delegate called.
 			code_hash: CodeHash<T>,
 		},
+
+		/// Some funds have been transferred and held as storage deposit.
+		StorageDepositTransferredAndHeld {
+			from: T::AccountId,
+			to: T::AccountId,
+			amount: BalanceOf<T>,
+		},
+
+		/// Some storage deposit funds have been transferred and released.
+		StorageDepositTransferredAndReleased {
+			from: T::AccountId,
+			to: T::AccountId,
+			amount: BalanceOf<T>,
+		},
 	}
 
 	#[pallet::error]
@@ -999,6 +1013,8 @@ pub mod pallet {
 	pub enum HoldReason {
 		/// The Pallet has reserved it for storing code on-chain.
 		CodeUploadDepositReserve,
+		/// The Pallet has reserved it for storage deposit.
+		StorageDepositReserve,
 	}
 
 	/// A mapping from a contract's code hash to its code.

frame/contracts/src/migration.rs

@@ -63,6 +63,7 @@ pub mod v11;
 pub mod v12;
 pub mod v13;
 pub mod v14;
+pub mod v15;
 include!(concat!(env!("OUT_DIR"), "/migration_codegen.rs"));
 
 use crate::{weights::WeightInfo, Config, Error, MigrationInProgress, Pallet, Weight, LOG_TARGET};

frame/contracts/src/migration/v10.rs

@@ -19,7 +19,6 @@
 //! See <https://github.com/paritytech/substrate/pull/13369>.
 
 use crate::{
-	address::AddressGenerator,
 	exec::AccountIdOf,
 	migration::{IsFinished, MigrationStep},
 	weights::WeightInfo,
@@ -40,7 +39,10 @@ use frame_support::{
 use sp_core::hexdisplay::HexDisplay;
 #[cfg(feature = "try-runtime")]
 use sp_runtime::TryRuntimeError;
-use sp_runtime::{traits::Zero, Perbill, Saturating};
+use sp_runtime::{
+	traits::{Hash, TrailingZeroInput, Zero},
+	Perbill, Saturating,
+};
 use sp_std::{ops::Deref, prelude::*};
 
 mod old {
@@ -135,6 +137,16 @@ type ContractInfoOf<T: Config, OldCurrency> = StorageMap<
 	ContractInfo<T, OldCurrency>,
 >;
 
+/// Formula: `hash("contract_depo_v1" ++ contract_addr)`
+fn deposit_address<T: Config>(
+	contract_addr: &<T as frame_system::Config>::AccountId,
+) -> <T as frame_system::Config>::AccountId {
+	let entropy = (b"contract_depo_v1", contract_addr)
+		.using_encoded(<T as frame_system::Config>::Hashing::hash);
+	Decode::decode(&mut TrailingZeroInput::new(entropy.as_ref()))
+		.expect("infinite length input; no invalid inputs for type; qed")
+}
+
 impl<T: Config, OldCurrency: 'static> MigrationStep for Migration<T, OldCurrency>
 where
 	OldCurrency: ReservableCurrency<<T as frame_system::Config>::AccountId>
@@ -162,8 +174,7 @@ where
 			log::debug!(target: LOG_TARGET, "Account: 0x{} ", HexDisplay::from(&account.encode()));
 
 			// Get the new deposit account address
-			let deposit_account: DepositAccount<T> =
-				DepositAccount(T::AddressGenerator::deposit_address(&account));
+			let deposit_account: DepositAccount<T> = DepositAccount(deposit_address::<T>(&account));
 
 			// Calculate the existing deposit, that should be reserved on the contract account
 			let old_deposit = contract