Make automatic storage deposits resistant against changing deposit prices (#12083)

* Require `FixedPointOperand` for Balances

* Delay deposit calculation

* Make refunds pro rata of consumed storage

* Add storage migration

* Fix clippy

* Add liquidity checks

* Fixe delayed deposit limit enforcement

* Defer charges

* Import Vec

* Add try-runtime hooks for migration

* Fix warning

* Adapt to new OnRuntimeUpgrade trait

* Apply suggestions from code review

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

* fmt

* Apply suggestions from code review

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

* More suggestions from code review

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

substrate/bin/node/runtime/src/lib.rs

@@ -1706,6 +1706,7 @@ pub type Executive = frame_executive::Executive<
 type Migrations = (
 	pallet_nomination_pools::migration::v2::MigrateToV2<Runtime>,
 	pallet_alliance::migration::Migration<Runtime>,
+	pallet_contracts::Migration<Runtime>,
 );
 
 /// MMR helper types.

substrate/frame/balances/src/lib.rs

@@ -185,7 +185,7 @@ use sp_runtime::{
 		AtLeast32BitUnsigned, Bounded, CheckedAdd, CheckedSub, MaybeSerializeDeserialize,
 		Saturating, StaticLookup, Zero,
 	},
-	ArithmeticError, DispatchError, RuntimeDebug,
+	ArithmeticError, DispatchError, FixedPointOperand, RuntimeDebug,
 };
 use sp_std::{cmp, fmt::Debug, mem, ops::BitOr, prelude::*, result};
 pub use weights::WeightInfo;
@@ -212,7 +212,8 @@ pub mod pallet {
 			+ MaybeSerializeDeserialize
 			+ Debug
 			+ MaxEncodedLen
-			+ TypeInfo;
+			+ TypeInfo
+			+ FixedPointOperand;
 
 		/// Handler for the unbalanced reduction when removing a dust account.
 		type DustRemoval: OnUnbalanced<NegativeImbalance<Self, I>>;

substrate/frame/contracts/fixtures/create_storage_and_call.wat

@@ -0,0 +1,55 @@
+;; This calls another contract as passed as its account id. It also creates some storage.
+(module
+	(import "seal0" "seal_input" (func $seal_input (param i32 i32)))
+	(import "seal0" "seal_set_storage" (func $seal_set_storage (param i32 i32 i32)))
+	(import "seal1" "seal_call" (func $seal_call (param i32 i32 i64 i32 i32 i32 i32 i32) (result i32)))
+	(import "env" "memory" (memory 1 1))
+
+	(func $assert (param i32)
+		(block $ok
+			(br_if $ok
+				(get_local 0)
+			)
+			(unreachable)
+		)
+	)
+
+    (func (export "deploy"))
+
+    (func (export "call")
+    	;; store length of input buffer
+        (i32.store (i32.const 0) (i32.const 512))
+
+        ;; copy input at address 4
+        (call $seal_input (i32.const 4) (i32.const 0))
+
+       	;; create 4 byte of storage before calling
+		(call $seal_set_storage
+			(i32.const 0)		;; Pointer to storage key
+			(i32.const 0)		;; Pointer to value
+			(i32.const 4)		;; Size of value
+		)
+
+		;; call passed contract
+		(call $assert (i32.eqz
+			(call $seal_call
+				(i32.const 0) ;; No flags
+				(i32.const 8)  ;; Pointer to "callee" address.
+				(i64.const 0)  ;; How much gas to devote for the execution. 0 = all.
+				(i32.const 512) ;; Pointer to the buffer with value to transfer
+				(i32.const 4) ;; Pointer to input data buffer address
+				(i32.const 4)  ;; Length of input data buffer
+				(i32.const 4294967295) ;; u32 max value is the sentinel value: do not copy output
+				(i32.const 0) ;; Length is ignored in this case
+			)
+		))
+
+		;; create 8 byte of storage after calling
+		;; item of 12 bytes because we override 4 bytes
+		(call $seal_set_storage
+			(i32.const 0)		;; Pointer to storage key
+			(i32.const 0)		;; Pointer to value
+			(i32.const 12)		;; Size of value
+		)
+	)
+)

substrate/frame/contracts/src/benchmarking/mod.rs

@@ -87,7 +87,7 @@ where
 		module: WasmModule<T>,
 		data: Vec<u8>,
 	) -> Result<Contract<T>, &'static str> {
-		let value = T::Currency::minimum_balance();
+		let value = Pallet::<T>::min_balance();
 		T::Currency::make_free_balance_be(&caller, caller_funding::<T>());
 		let salt = vec![0xff];
 		let addr = Contracts::<T>::contract_address(&caller, &module.hash, &salt);
@@ -257,7 +257,7 @@ benchmarks! {
 		let instance = Contract::<T>::with_caller(
 			whitelisted_caller(), WasmModule::sized(c, Location::Deploy), vec![],
 		)?;
-		let value = T::Currency::minimum_balance();
+		let value = Pallet::<T>::min_balance();
 		let origin = RawOrigin::Signed(instance.caller.clone());
 		let callee = instance.addr;
 	}: call(origin, callee, value, Weight::MAX, None, vec![])
@@ -280,7 +280,7 @@ benchmarks! {
 		let c in 0 .. Perbill::from_percent(49).mul_ceil(T::MaxCodeLen::get());
 		let s in 0 .. code::max_pages::<T>() * 64 * 1024;
 		let salt = vec![42u8; s as usize];
-		let value = T::Currency::minimum_balance();
+		let value = Pallet::<T>::min_balance();
 		let caller = whitelisted_caller();
 		T::Currency::make_free_balance_be(&caller, caller_funding::<T>());
 		let WasmModule { code, hash, .. } = WasmModule::<T>::sized(c, Location::Call);
@@ -307,7 +307,7 @@ benchmarks! {
 	instantiate {
 		let s in 0 .. code::max_pages::<T>() * 64 * 1024;
 		let salt = vec![42u8; s as usize];
-		let value = T::Currency::minimum_balance();
+		let value = Pallet::<T>::min_balance();
 		let caller = whitelisted_caller();
 		T::Currency::make_free_balance_be(&caller, caller_funding::<T>());
 		let WasmModule { code, hash, .. } = WasmModule::<T>::dummy();
@@ -338,7 +338,7 @@ benchmarks! {
 		let instance = Contract::<T>::with_caller(
 			whitelisted_caller(), WasmModule::dummy(), vec![],
 		)?;
-		let value = T::Currency::minimum_balance();
+		let value = Pallet::<T>::min_balance();
 		let origin = RawOrigin::Signed(instance.caller.clone());
 		let callee = instance.addr.clone();
 		let before = T::Currency::free_balance(&instance.account_id);
@@ -767,13 +767,13 @@ benchmarks! {
 		let instance = Contract::<T>::new(code, vec![])?;
 		let origin = RawOrigin::Signed(instance.caller.clone());
 		assert_eq!(T::Currency::total_balance(&beneficiary), 0u32.into());
-		assert_eq!(T::Currency::free_balance(&instance.account_id), T::Currency::minimum_balance());
+		assert_eq!(T::Currency::free_balance(&instance.account_id), Pallet::<T>::min_balance());
 		assert_ne!(T::Currency::reserved_balance(&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(&beneficiary), T::Currency::minimum_balance());
+			assert_eq!(T::Currency::total_balance(&beneficiary), Pallet::<T>::min_balance());
 		}
 	}
 
@@ -1469,7 +1469,7 @@ benchmarks! {
 			.collect::<Vec<_>>();
 		let account_len = accounts.get(0).map(|i| i.encode().len()).unwrap_or(0);
 		let account_bytes = accounts.iter().flat_map(|x| x.encode()).collect();
-		let value = T::Currency::minimum_balance();
+		let value = Pallet::<T>::min_balance();
 		assert!(value > 0u32.into());
 		let value_bytes = value.encode();
 		let value_len = value_bytes.len();
@@ -1705,7 +1705,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 = T::Currency::minimum_balance();
+		let value = Pallet::<T>::min_balance();
 		assert!(value > 0u32.into());
 		let value_bytes = value.encode();
 		let value_len = value_bytes.len();

substrate/frame/contracts/src/exec.rs

@@ -813,6 +813,15 @@ where
 				return Ok(output)
 			}
 
+			// Storage limit is enforced as late as possible (when the last frame returns) so that
+			// the ordering of storage accesses does not matter.
+			if self.frames.is_empty() {
+				let frame = &mut self.first_frame;
+				frame.contract_info.load(&frame.account_id);
+				let contract = frame.contract_info.as_contract();
+				frame.nested_storage.enforce_limit(contract)?;
+			}
+
 			let frame = self.top_frame();
 			let account_id = &frame.account_id;
 			match (entry_point, delegated_code_hash) {
@@ -911,12 +920,7 @@ where
 			// it was invalidated.
 			frame.contract_info.load(account_id);
 			let mut contract = frame.contract_info.into_contract();
-			prev.nested_storage.absorb(
-				frame.nested_storage,
-				&self.origin,
-				account_id,
-				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 {
@@ -954,7 +958,6 @@ where
 			let mut contract = self.first_frame.contract_info.as_contract();
 			self.storage_meter.absorb(
 				mem::take(&mut self.first_frame.nested_storage),
-				&self.origin,
 				&self.first_frame.account_id,
 				contract.as_deref_mut(),
 			);
@@ -2354,10 +2357,10 @@ mod tests {
 		let code_bob = MockLoader::insert(Call, |ctx, _| {
 			if ctx.input_data[0] == 0 {
 				let info = ctx.ext.contract_info();
-				assert_eq!(info.storage_deposit, 0);
-				info.storage_deposit = 42;
+				assert_eq!(info.storage_byte_deposit, 0);
+				info.storage_byte_deposit = 42;
 				assert_eq!(ctx.ext.call(Weight::zero(), CHARLIE, 0, vec![], true), exec_trapped());
-				assert_eq!(ctx.ext.contract_info().storage_deposit, 42);
+				assert_eq!(ctx.ext.contract_info().storage_byte_deposit, 42);
 			}
 			exec_success()
 		});

substrate/frame/contracts/src/lib.rs

@@ -87,12 +87,12 @@
 mod gas;
 mod benchmarking;
 mod exec;
+mod migration;
 mod schedule;
 mod storage;
 mod wasm;
 
 pub mod chain_extension;
-pub mod migration;
 pub mod weights;
 
 #[cfg(test)]
@@ -109,7 +109,10 @@ use codec::{Encode, HasCompact};
 use frame_support::{
 	dispatch::{DispatchClass, Dispatchable, GetDispatchInfo, Pays, PostDispatchInfo},
 	ensure,
-	traits::{ConstU32, Contains, Currency, Get, Randomness, ReservableCurrency, Time},
+	traits::{
+		tokens::fungible::Inspect, ConstU32, Contains, Currency, Get, Randomness,
+		ReservableCurrency, Time,
+	},
 	weights::Weight,
 	BoundedVec, WeakBoundedVec,
 };
@@ -126,6 +129,7 @@ use sp_std::{fmt::Debug, marker::PhantomData, prelude::*};
 
 pub use crate::{
 	exec::{Frame, VarSizedKey as StorageKey},
+	migration::Migration,
 	pallet::*,
 	schedule::{HostFnWeights, InstructionWeights, Limits, Schedule},
 };
@@ -225,7 +229,7 @@ pub mod pallet {
 	use frame_system::pallet_prelude::*;
 
 	/// The current storage version.
-	const STORAGE_VERSION: StorageVersion = StorageVersion::new(7);
+	const STORAGE_VERSION: StorageVersion = StorageVersion::new(8);
 
 	#[pallet::pallet]
 	#[pallet::storage_version(STORAGE_VERSION)]
@@ -236,11 +240,12 @@ pub mod pallet {
 		/// The time implementation used to supply timestamps to contracts through `seal_now`.
 		type Time: Time;
 
-		/// The generator used to supply randomness to contracts through `seal_random`.
+		/// The generator used to supply randomness to contracts through `seal_random`
 		type Randomness: Randomness<Self::Hash, Self::BlockNumber>;
 
 		/// The currency in which fees are paid and contract balances are held.
-		type Currency: ReservableCurrency<Self::AccountId>;
+		type Currency: ReservableCurrency<Self::AccountId>
+			+ Inspect<Self::AccountId, Balance = BalanceOf<Self>>;
 
 		/// The overarching event type.
 		type RuntimeEvent: From<Event<Self>> + IsType<<Self as frame_system::Config>::RuntimeEvent>;
@@ -1036,7 +1041,7 @@ where
 		};
 		let schedule = T::Schedule::get();
 		let result = ExecStack::<T, PrefabWasmModule<T>>::run_call(
-			origin,
+			origin.clone(),
 			dest,
 			&mut gas_meter,
 			&mut storage_meter,
@@ -1045,7 +1050,11 @@ where
 			data,
 			debug_message,
 		);
-		InternalCallOutput { result, gas_meter, storage_deposit: storage_meter.into_deposit() }
+		InternalCallOutput {
+			result,
+			gas_meter,
+			storage_deposit: storage_meter.into_deposit(&origin),
+		}
 	}
 
 	/// Internal function that does the actual instantiation.
@@ -1089,7 +1098,7 @@ where
 				value.saturating_add(extra_deposit),
 			)?;
 			let result = ExecStack::<T, PrefabWasmModule<T>>::run_instantiate(
-				origin,
+				origin.clone(),
 				executable,
 				&mut gas_meter,
 				&mut storage_meter,
@@ -1100,17 +1109,23 @@ where
 				debug_message,
 			);
 			storage_deposit = storage_meter
-				.into_deposit()
+				.into_deposit(&origin)
 				.saturating_add(&StorageDeposit::Charge(extra_deposit));
 			result
 		};
 		InternalInstantiateOutput { result: try_exec(), gas_meter, storage_deposit }
 	}
 
+	/// Deposit a pallet contracts event. Handles the conversion to the overarching event type.
 	fn deposit_event(topics: Vec<T::Hash>, event: Event<T>) {
 		<frame_system::Pallet<T>>::deposit_event_indexed(
 			&topics,
 			<T as Config>::RuntimeEvent::from(event).into(),
 		)
 	}
+
+	/// Return the existential deposit of [`Config::Currency`].
+	fn min_balance() -> BalanceOf<T> {
+		<T::Currency as Inspect<AccountIdOf<T>>>::minimum_balance()
+	}
 }

substrate/frame/contracts/src/migration.rs

@@ -18,37 +18,80 @@
 use crate::{BalanceOf, CodeHash, Config, Pallet, TrieId, Weight};
 use codec::{Decode, Encode};
 use frame_support::{
-	codec, pallet_prelude::*, storage::migration, storage_alias, traits::Get, Identity,
-	Twox64Concat,
+	codec,
+	pallet_prelude::*,
+	storage::migration,
+	storage_alias,
+	traits::{Get, OnRuntimeUpgrade},
+	Identity, Twox64Concat,
 };
+use sp_runtime::traits::Saturating;
 use sp_std::{marker::PhantomData, prelude::*};
 
-/// Wrapper for all migrations of this pallet, based on `StorageVersion`.
-pub fn migrate<T: Config>() -> Weight {
-	let version = StorageVersion::get::<Pallet<T>>();
-	let mut weight = Weight::zero();
+/// Performs all necessary migrations based on `StorageVersion`.
+pub struct Migration<T: Config>(PhantomData<T>);
+impl<T: Config> OnRuntimeUpgrade for Migration<T> {
+	fn on_runtime_upgrade() -> Weight {
+		let version = StorageVersion::get::<Pallet<T>>();
+		let mut weight = Weight::zero();
 
-	if version < 4 {
-		weight = weight.saturating_add(v4::migrate::<T>());
-		StorageVersion::new(4).put::<Pallet<T>>();
+		if version < 4 {
+			weight = weight.saturating_add(v4::migrate::<T>());
+			StorageVersion::new(4).put::<Pallet<T>>();
+		}
+
+		if version < 5 {
+			weight = weight.saturating_add(v5::migrate::<T>());
+			StorageVersion::new(5).put::<Pallet<T>>();
+		}
+
+		if version < 6 {
+			weight = weight.saturating_add(v6::migrate::<T>());
+			StorageVersion::new(6).put::<Pallet<T>>();
+		}
+
+		if version < 7 {
+			weight = weight.saturating_add(v7::migrate::<T>());
+			StorageVersion::new(7).put::<Pallet<T>>();
+		}
+
+		if version < 8 {
+			weight = weight.saturating_add(v8::migrate::<T>());
+			StorageVersion::new(8).put::<Pallet<T>>();
+		}
+
+		weight
 	}
 
-	if version < 5 {
-		weight = weight.saturating_add(v5::migrate::<T>());
-		StorageVersion::new(5).put::<Pallet<T>>();
+	#[cfg(feature = "try-runtime")]
+	fn pre_upgrade() -> Result<Vec<u8>, &'static str> {
+		let version = StorageVersion::get::<Pallet<T>>();
+
+		if version < 7 {
+			return Ok(vec![])
+		}
+
+		if version < 8 {
+			v8::pre_upgrade::<T>()?;
+		}
+
+		Ok(vec![])
 	}
 
-	if version < 6 {
-		weight = weight.saturating_add(v6::migrate::<T>());
-		StorageVersion::new(6).put::<Pallet<T>>();
-	}
+	#[cfg(feature = "try-runtime")]
+	fn post_upgrade(_state: Vec<u8>) -> Result<(), &'static str> {
+		let version = StorageVersion::get::<Pallet<T>>();
 
-	if version < 7 {
-		weight = weight.saturating_add(v7::migrate::<T>());
-		StorageVersion::new(7).put::<Pallet<T>>();
-	}
+		if version < 7 {
+			return Ok(())
+		}
 
-	weight
+		if version < 8 {
+			v8::post_upgrade::<T>()?;
+		}
+
+		Ok(())
+	}
 }
 
 /// V4: `Schedule` is changed to be a config item rather than an in-storage value.
@@ -185,9 +228,9 @@ mod v6 {
 
 	#[derive(Encode, Decode)]
 	pub struct RawContractInfo<CodeHash, Balance> {
-		trie_id: TrieId,
-		code_hash: CodeHash,
-		storage_deposit: Balance,
+		pub trie_id: TrieId,
+		pub code_hash: CodeHash,
+		pub storage_deposit: Balance,
 	}
 
 	#[derive(Encode, Decode)]
@@ -199,7 +242,7 @@ mod v6 {
 		refcount: u64,
 	}
 
-	type ContractInfo<T> = RawContractInfo<CodeHash<T>, BalanceOf<T>>;
+	pub type ContractInfo<T> = RawContractInfo<CodeHash<T>, BalanceOf<T>>;
 
 	#[storage_alias]
 	type ContractInfoOf<T: Config> = StorageMap<
@@ -266,3 +309,108 @@ mod v7 {
 		T::DbWeight::get().reads_writes(1, 2)
 	}
 }
+
+/// Update `ContractInfo` with new fields that track storage deposits.
+mod v8 {
+	use super::*;
+	use sp_io::default_child_storage as child;
+	use v6::ContractInfo as OldContractInfo;
+
+	#[derive(Encode, Decode)]
+	struct ContractInfo<T: Config> {
+		trie_id: TrieId,
+		code_hash: CodeHash<T>,
+		storage_bytes: u32,
+		storage_items: u32,
+		storage_byte_deposit: BalanceOf<T>,
+		storage_item_deposit: BalanceOf<T>,
+		storage_base_deposit: BalanceOf<T>,
+	}
+
+	#[storage_alias]
+	type ContractInfoOf<T: Config, V> =
+		StorageMap<Pallet<T>, Twox64Concat, <T as frame_system::Config>::AccountId, V>;
+
+	pub fn migrate<T: Config>() -> Weight {
+		let mut weight = Weight::zero();
+
+		<ContractInfoOf<T, ContractInfo<T>>>::translate_values(|old: OldContractInfo<T>| {
+			// Count storage items of this contract
+			let mut storage_bytes = 0u32;
+			let mut storage_items = 0u32;
+			let mut key = Vec::new();
+			while let Some(next) = child::next_key(&old.trie_id, &key) {
+				key = next;
+				let mut val_out = [];
+				let len = child::read(&old.trie_id, &key, &mut val_out, 0)
+					.expect("The loop conditions checks for existence of the key; qed");
+				storage_bytes.saturating_accrue(len);
+				storage_items.saturating_accrue(1);
+			}
+
+			let storage_byte_deposit =
+				T::DepositPerByte::get().saturating_mul(storage_bytes.into());
+			let storage_item_deposit =
+				T::DepositPerItem::get().saturating_mul(storage_items.into());
+			let storage_base_deposit = old
+				.storage_deposit
+				.saturating_sub(storage_byte_deposit)
+				.saturating_sub(storage_item_deposit);
+
+			// Reads: One read for each storage item plus the contract info itself.
+			// Writes: Only the new contract info.
+			weight = weight
+				.saturating_add(T::DbWeight::get().reads_writes(u64::from(storage_items) + 1, 1));
+
+			Some(ContractInfo {
+				trie_id: old.trie_id,
+				code_hash: old.code_hash,
+				storage_bytes,
+				storage_items,
+				storage_byte_deposit,
+				storage_item_deposit,
+				storage_base_deposit,
+			})
+		});
+
+		weight
+	}
+
+	#[cfg(feature = "try-runtime")]
+	pub fn pre_upgrade<T: Config>() -> Result<(), &'static str> {
+		use frame_support::traits::ReservableCurrency;
+		for (key, value) in ContractInfoOf::<T, OldContractInfo<T>>::iter() {
+			let reserved = T::Currency::reserved_balance(&key);
+			ensure!(reserved >= value.storage_deposit, "Reserved balance out of sync.");
+		}
+		Ok(())
+	}
+
+	#[cfg(feature = "try-runtime")]
+	pub fn post_upgrade<T: Config>() -> Result<(), &'static str> {
+		use frame_support::traits::ReservableCurrency;
+		for (key, value) in ContractInfoOf::<T, ContractInfo<T>>::iter() {
+			let reserved = T::Currency::reserved_balance(&key);
+			let stored = value
+				.storage_base_deposit
+				.saturating_add(value.storage_byte_deposit)
+				.saturating_add(value.storage_item_deposit);
+			ensure!(reserved >= stored, "Reserved balance out of sync.");
+
+			let mut storage_bytes = 0u32;
+			let mut storage_items = 0u32;
+			let mut key = Vec::new();
+			while let Some(next) = child::next_key(&value.trie_id, &key) {
+				key = next;
+				let mut val_out = [];
+				let len = child::read(&value.trie_id, &key, &mut val_out, 0)
+					.expect("The loop conditions checks for existence of the key; qed");
+				storage_bytes.saturating_accrue(len);
+				storage_items.saturating_accrue(1);
+			}
+			ensure!(storage_bytes == value.storage_bytes, "Storage bytes do not match.",);
+			ensure!(storage_items == value.storage_items, "Storage items do not match.",);
+		}
+		Ok(())
+	}
+}

substrate/frame/contracts/src/storage.rs

@@ -34,31 +34,51 @@ use scale_info::TypeInfo;
 use sp_core::crypto::UncheckedFrom;
 use sp_io::KillStorageResult;
 use sp_runtime::{
-	traits::{Hash, Zero},
+	traits::{Hash, Saturating, Zero},
 	RuntimeDebug,
 };
 use sp_std::{marker::PhantomData, prelude::*};
 
-pub type ContractInfo<T> = RawContractInfo<CodeHash<T>, BalanceOf<T>>;
-
 /// Information for managing an account and its sub trie abstraction.
 /// This is the required info to cache for an account.
 #[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug, TypeInfo, MaxEncodedLen)]
-pub struct RawContractInfo<CodeHash, Balance> {
+#[scale_info(skip_type_params(T))]
+pub struct ContractInfo<T: Config> {
 	/// Unique ID for the subtree encoded as a bytes vector.
 	pub trie_id: TrieId,
 	/// The code associated with a given account.
-	pub code_hash: CodeHash,
-	/// The amount of balance that is currently deposited to pay for consumed storage.
-	pub storage_deposit: Balance,
+	pub code_hash: CodeHash<T>,
+	/// How many bytes of storage are accumulated in this contract's child trie.
+	pub storage_bytes: u32,
+	/// How many items of storage are accumulated in this contract's child trie.
+	pub storage_items: u32,
+	/// This records to how much deposit the accumulated `storage_bytes` amount to.
+	pub storage_byte_deposit: BalanceOf<T>,
+	/// This records to how much deposit the accumulated `storage_items` amount to.
+	pub storage_item_deposit: BalanceOf<T>,
+	/// This records how much deposit is put down in order to pay for the contract itself.
+	///
+	/// We need to store this information separately so it is not used when calculating any refunds
+	/// since the base deposit can only ever be refunded on contract termination.
+	pub storage_base_deposit: BalanceOf<T>,
 }
 
-impl<CodeHash, Balance> RawContractInfo<CodeHash, Balance> {
+impl<T: Config> ContractInfo<T> {
 	/// Associated child trie unique id is built from the hash part of the trie id.
 	#[cfg(test)]
 	pub fn child_trie_info(&self) -> ChildInfo {
 		child_trie_info(&self.trie_id[..])
 	}
+
+	/// The deposit paying for the accumulated storage generated within the contract's child trie.
+	pub fn extra_deposit(&self) -> BalanceOf<T> {
+		self.storage_byte_deposit.saturating_add(self.storage_item_deposit)
+	}
+
+	/// Same as [`Self::extra_deposit`] but including the base deposit.
+	pub fn total_deposit(&self) -> BalanceOf<T> {
+		self.extra_deposit().saturating_add(self.storage_base_deposit)
+	}
 }
 
 /// Associated child trie unique id is built from the hash part of the trie id.
@@ -178,7 +198,7 @@ where
 				},
 				(None, None) => (),
 			}
-			storage_meter.charge(&diff)?;
+			storage_meter.charge(&diff);
 		}
 
 		match &new_value {
@@ -200,14 +220,21 @@ where
 	pub fn new_contract(
 		account: &AccountIdOf<T>,
 		trie_id: TrieId,
-		ch: CodeHash<T>,
+		code_hash: CodeHash<T>,
 	) -> Result<ContractInfo<T>, DispatchError> {
 		if <ContractInfoOf<T>>::contains_key(account) {
 			return Err(Error::<T>::DuplicateContract.into())
 		}
 
-		let contract =
-			ContractInfo::<T> { code_hash: ch, trie_id, storage_deposit: <BalanceOf<T>>::zero() };
+		let contract = ContractInfo::<T> {
+			code_hash,
+			trie_id,
+			storage_bytes: 0,
+			storage_items: 0,
+			storage_byte_deposit: Zero::zero(),
+			storage_item_deposit: Zero::zero(),
+			storage_base_deposit: Zero::zero(),
+		};
 
 		Ok(contract)
 	}

substrate/frame/contracts/src/storage/meter.rs

@@ -17,17 +17,24 @@
 
 //! This module contains functions to meter the storage deposit.
 
-use crate::{storage::ContractInfo, BalanceOf, Config, Error};
+use crate::{storage::ContractInfo, BalanceOf, Config, Error, Inspect, Pallet};
 use codec::Encode;
 use frame_support::{
 	dispatch::DispatchError,
-	traits::{tokens::BalanceStatus, Currency, ExistenceRequirement, Get, ReservableCurrency},
-	DefaultNoBound,
+	ensure,
+	traits::{
+		tokens::{BalanceStatus, WithdrawConsequence},
+		Currency, ExistenceRequirement, Get, ReservableCurrency,
+	},
+	DefaultNoBound, RuntimeDebugNoBound,
 };
 use pallet_contracts_primitives::StorageDeposit as Deposit;
 use sp_core::crypto::UncheckedFrom;
-use sp_runtime::traits::{Saturating, Zero};
-use sp_std::marker::PhantomData;
+use sp_runtime::{
+	traits::{Saturating, Zero},
+	FixedPointNumber, FixedU128,
+};
+use sp_std::{marker::PhantomData, vec::Vec};
 
 /// Deposit that uses the native currency's balance type.
 pub type DepositOf<T> = Deposit<BalanceOf<T>>;
@@ -99,22 +106,25 @@ impl State for Root {}
 impl State for Nested {}
 
 /// A type that allows the metering of consumed or freed storage of a single contract call stack.
-#[derive(DefaultNoBound)]
-pub struct RawMeter<T: Config, E: Ext<T>, S: State> {
+#[derive(DefaultNoBound, RuntimeDebugNoBound)]
+pub struct RawMeter<T: Config, E, S: State> {
 	/// The limit of how much balance this meter is allowed to consume.
 	limit: BalanceOf<T>,
 	/// The amount of balance that was used in this meter and all of its already absorbed children.
 	total_deposit: DepositOf<T>,
-	/// The amount of balance that was used in this meter alone.
-	own_deposit: DepositOf<T>,
-	/// Only when a contract was terminated we allow it to drop below the minimum balance.
-	terminated: bool,
+	/// The amount of storage changes that were recorded in this meter alone.
+	own_contribution: Contribution<T>,
+	/// List of charges that should be applied at the end of a contract stack execution.
+	///
+	/// We only have one charge per contract hence the size of this vector is
+	/// limited by the maximum call depth.
+	charges: Vec<Charge<T>>,
 	/// Type parameters are only used in impls.
 	_phantom: PhantomData<(E, S)>,
 }
 
 /// This type is used to describe a storage change when charging from the meter.
-#[derive(Default)]
+#[derive(Default, RuntimeDebugNoBound)]
 pub struct Diff {
 	/// How many bytes were added to storage.
 	pub bytes_added: u32,
@@ -124,43 +134,120 @@ pub struct Diff {
 	pub items_added: u32,
 	/// How many storage items were removed from storage.
 	pub items_removed: u32,
-	/// If set to true the derived deposit will always a `Charge` larger than the
-	/// the existential deposit.
-	pub require_ed: bool,
 }
 
 impl Diff {
-	/// Calculate how much of a charge or refund results from applying the diff.
-	pub fn to_deposit<T: Config>(&self) -> DepositOf<T> {
-		let mut deposit = Deposit::default();
+	/// Calculate how much of a charge or refund results from applying the diff and store it
+	/// in the passed `info` if any.
+	///
+	/// # Note
+	///
+	/// In case `None` is passed for `info` only charges are calculated. This is because refunds
+	/// are calculated pro rata of the existing storage within a contract and hence need extract
+	/// this information from the passed `info`.
+	pub fn update_contract<T: Config>(&self, info: Option<&mut ContractInfo<T>>) -> DepositOf<T> {
 		let per_byte = T::DepositPerByte::get();
 		let per_item = T::DepositPerItem::get();
+		let bytes_added = self.bytes_added.saturating_sub(self.bytes_removed);
+		let items_added = self.items_added.saturating_sub(self.items_removed);
+		let mut bytes_deposit = Deposit::Charge(per_byte.saturating_mul((bytes_added).into()));
+		let mut items_deposit = Deposit::Charge(per_item.saturating_mul((items_added).into()));
 
-		if self.bytes_added > self.bytes_removed {
-			deposit = deposit.saturating_add(&Deposit::Charge(
-				per_byte.saturating_mul((self.bytes_added - self.bytes_removed).into()),
-			));
-		} else if self.bytes_removed > self.bytes_added {
-			deposit = deposit.saturating_add(&Deposit::Refund(
-				per_byte.saturating_mul((self.bytes_removed - self.bytes_added).into()),
-			));
+		// Without any contract info we can only calculate diffs which add storage
+		let info = if let Some(info) = info {
+			info
+		} else {
+			debug_assert_eq!(self.bytes_removed, 0);
+			debug_assert_eq!(self.items_removed, 0);
+			return bytes_deposit.saturating_add(&items_deposit)
+		};
+
+		// Refunds are calculated pro rata based on the accumulated storage within the contract
+		let bytes_removed = self.bytes_removed.saturating_sub(self.bytes_added);
+		let items_removed = self.items_removed.saturating_sub(self.items_added);
+		let ratio = FixedU128::checked_from_rational(bytes_removed, info.storage_bytes)
+			.unwrap_or_default()
+			.min(FixedU128::from_u32(1));
+		bytes_deposit = bytes_deposit
+			.saturating_add(&Deposit::Refund(ratio.saturating_mul_int(info.storage_byte_deposit)));
+		let ratio = FixedU128::checked_from_rational(items_removed, info.storage_items)
+			.unwrap_or_default()
+			.min(FixedU128::from_u32(1));
+		items_deposit = items_deposit
+			.saturating_add(&Deposit::Refund(ratio.saturating_mul_int(info.storage_item_deposit)));
+
+		// We need to update the contract info structure with the new deposits
+		info.storage_bytes =
+			info.storage_bytes.saturating_add(bytes_added).saturating_sub(bytes_removed);
+		info.storage_items =
+			info.storage_items.saturating_add(items_added).saturating_sub(items_removed);
+		match &bytes_deposit {
+			Deposit::Charge(amount) =>
+				info.storage_byte_deposit = info.storage_byte_deposit.saturating_add(*amount),
+			Deposit::Refund(amount) =>
+				info.storage_byte_deposit = info.storage_byte_deposit.saturating_sub(*amount),
+		}
+		match &items_deposit {
+			Deposit::Charge(amount) =>
+				info.storage_item_deposit = info.storage_item_deposit.saturating_add(*amount),
+			Deposit::Refund(amount) =>
+				info.storage_item_deposit = info.storage_item_deposit.saturating_sub(*amount),
 		}
 
-		if self.items_added > self.items_removed {
-			deposit = deposit.saturating_add(&Deposit::Charge(
-				per_item.saturating_mul((self.items_added - self.items_removed).into()),
-			));
-		} else if self.items_removed > self.items_added {
-			deposit = deposit.saturating_add(&Deposit::Refund(
-				per_item.saturating_mul((self.items_removed - self.items_added).into()),
-			));
-		}
+		bytes_deposit.saturating_add(&items_deposit)
+	}
+}
 
-		if self.require_ed {
-			deposit = deposit.max(Deposit::Charge(T::Currency::minimum_balance()))
+impl Diff {
+	fn saturating_add(&self, rhs: &Self) -> Self {
+		Self {
+			bytes_added: self.bytes_added.saturating_add(rhs.bytes_added),
+			bytes_removed: self.bytes_removed.saturating_add(rhs.bytes_removed),
+			items_added: self.items_added.saturating_add(rhs.items_added),
+			items_removed: self.items_removed.saturating_add(rhs.items_removed),
 		}
+	}
+}
 
-		deposit
+/// Records information to charge or refund a plain account.
+///
+/// All the charges are deferred to the end of a whole call stack. Reason is that by doing
+/// this we can do all the refunds before doing any charge. This way a plain account can use
+/// more deposit than it has balance as along as it is covered by a refund. This
+/// essentially makes the order of storage changes irrelevant with regard to the deposit system.
+#[derive(RuntimeDebugNoBound, Clone)]
+struct Charge<T: Config> {
+	contract: T::AccountId,
+	amount: DepositOf<T>,
+	terminated: bool,
+}
+
+/// Records the storage changes of a storage meter.
+#[derive(RuntimeDebugNoBound)]
+enum Contribution<T: Config> {
+	/// The contract the meter belongs to is alive and accumulates changes using a [`Diff`].
+	Alive(Diff),
+	/// The meter was checked against its limit using [`RawMeter::enforce_limit`] at the end of
+	/// its execution. In this process the [`Diff`] was converted into a [`Deposit`].
+	Checked(DepositOf<T>),
+	/// The contract was terminated. In this process the [`Diff`] was converted into a [`Deposit`]
+	/// in order to calculate the refund.
+	Terminated(DepositOf<T>),
+}
+
+impl<T: Config> Contribution<T> {
+	/// See [`Diff::update_contract`].
+	fn update_contract(&self, info: Option<&mut ContractInfo<T>>) -> DepositOf<T> {
+		match self {
+			Self::Alive(diff) => diff.update_contract::<T>(info),
+			Self::Terminated(deposit) | Self::Checked(deposit) => deposit.clone(),
+		}
+	}
+}
+
+impl<T: Config> Default for Contribution<T> {
+	fn default() -> Self {
+		Self::Alive(Default::default())
 	}
 }
 
@@ -178,6 +265,7 @@ where
 	/// usage for this sub call separately. This is necessary because we want to exchange balance
 	/// with the current contract we are interacting with.
 	pub fn nested(&self) -> RawMeter<T, E, Nested> {
+		debug_assert!(self.is_alive());
 		RawMeter { limit: self.available(), ..Default::default() }
 	}
 
@@ -192,45 +280,28 @@ where
 	///
 	/// # Parameters
 	///
-	/// `absorbed`: The child storage meter that should be absorbed.
-	/// `origin`: The origin that spawned the original root meter.
-	/// `contract`: The contract that this sub call belongs to.
-	/// `info`: The info of the contract in question. `None` if the contract was terminated.
+	/// - `absorbed`: The child storage meter that should be absorbed.
+	/// - `origin`: The origin that spawned the original root meter.
+	/// - `contract`: The contract that this sub call belongs to.
+	/// - `info`: The info of the contract in question. `None` if the contract was terminated.
 	pub fn absorb(
 		&mut self,
-		mut absorbed: RawMeter<T, E, Nested>,
-		origin: &T::AccountId,
+		absorbed: RawMeter<T, E, Nested>,
 		contract: &T::AccountId,
 		info: Option<&mut ContractInfo<T>>,
 	) {
-		// Absorbing from an existing (non terminated) contract.
-		if let Some(info) = info {
-			match &mut absorbed.own_deposit {
-				Deposit::Charge(amount) =>
-					info.storage_deposit = info.storage_deposit.saturating_add(*amount),
-				Deposit::Refund(amount) => {
-					// We need to make sure to never refund more than what was deposited and
-					// still leave the existential deposit inside the contract's account.
-					// This case can happen when costs change due to a runtime upgrade where
-					// increased costs could remove an account due to refunds.
-					let amount = {
-						let corrected_amount = (*amount).min(
-							info.storage_deposit.saturating_sub(T::Currency::minimum_balance()),
-						);
-						let correction = (*amount).saturating_sub(corrected_amount);
-						absorbed.total_deposit =
-							absorbed.total_deposit.saturating_sub(&Deposit::Refund(correction));
-						*amount = corrected_amount;
-						corrected_amount
-					};
-					info.storage_deposit = info.storage_deposit.saturating_sub(amount);
-				},
-			}
-		}
-
-		self.total_deposit = self.total_deposit.saturating_add(&absorbed.total_deposit);
-		if !absorbed.own_deposit.is_zero() {
-			E::charge(origin, contract, &absorbed.own_deposit, absorbed.terminated);
+		let own_deposit = absorbed.own_contribution.update_contract(info);
+		self.total_deposit = self
+			.total_deposit
+			.saturating_add(&absorbed.total_deposit)
+			.saturating_add(&own_deposit);
+		if !own_deposit.is_zero() {
+			self.charges.extend_from_slice(&absorbed.charges);
+			self.charges.push(Charge {
+				contract: contract.clone(),
+				amount: own_deposit,
+				terminated: absorbed.is_terminated(),
+			});
 		}
 	}
 
@@ -238,6 +309,16 @@ where
 	fn available(&self) -> BalanceOf<T> {
 		self.total_deposit.available(&self.limit)
 	}
+
+	/// True if the contract is alive.
+	fn is_alive(&self) -> bool {
+		matches!(self.own_contribution, Contribution::Alive(_))
+	}
+
+	/// True if the contract is terminated.
+	fn is_terminated(&self) -> bool {
+		matches!(self.own_contribution, Contribution::Terminated(_))
+	}
 }
 
 /// Functions that only apply to the root state.
@@ -260,11 +341,18 @@ where
 	}
 
 	/// The total amount of deposit that should change hands as result of the execution
-	/// that this meter was passed into.
+	/// that this meter was passed into. This will also perform all the charges accumulated
+	/// in the whole contract stack.
 	///
 	/// This drops the root meter in order to make sure it is only called when the whole
 	/// execution did finish.
-	pub fn into_deposit(self) -> DepositOf<T> {
+	pub fn into_deposit(self, origin: &T::AccountId) -> DepositOf<T> {
+		for charge in self.charges.iter().filter(|c| matches!(c.amount, Deposit::Refund(_))) {
+			E::charge(origin, &charge.contract, &charge.amount, charge.terminated);
+		}
+		for charge in self.charges.iter().filter(|c| matches!(c.amount, Deposit::Charge(_))) {
+			E::charge(origin, &charge.contract, &charge.amount, charge.terminated);
+		}
 		self.total_deposit
 	}
 }
@@ -277,18 +365,12 @@ where
 	E: Ext<T>,
 {
 	/// Try to charge the `diff` from the meter. Fails if this would exceed the original limit.
-	pub fn charge(&mut self, diff: &Diff) -> Result<DepositOf<T>, DispatchError> {
-		debug_assert!(!self.terminated);
-		let deposit = diff.to_deposit::<T>();
-		let total_deposit = self.total_deposit.saturating_add(&deposit);
-		if let Deposit::Charge(amount) = total_deposit {
-			if amount > self.limit {
-				return Err(<Error<T>>::StorageDepositLimitExhausted.into())
-			}
-		}
-		self.total_deposit = total_deposit;
-		self.own_deposit = self.own_deposit.saturating_add(&deposit);
-		Ok(deposit)
+	pub fn charge(&mut self, diff: &Diff) {
+		debug_assert!(self.is_alive());
+		match &mut self.own_contribution {
+			Contribution::Alive(own) => *own = own.saturating_add(diff),
+			_ => panic!("Charge is never called after termination; qed"),
+		};
 	}
 
 	/// Charge from `origin` a storage deposit for contract instantiation.
@@ -300,25 +382,22 @@ where
 		contract: &T::AccountId,
 		info: &mut ContractInfo<T>,
 	) -> Result<DepositOf<T>, DispatchError> {
-		debug_assert!(!self.terminated);
-		let deposit = Diff {
-			bytes_added: info.encoded_size() as u32,
-			items_added: 1,
-			require_ed: true,
-			..Default::default()
+		debug_assert!(self.is_alive());
+		let mut deposit =
+			Diff { bytes_added: info.encoded_size() as u32, items_added: 1, ..Default::default() }
+				.update_contract::<T>(None);
+
+		// Instantiate needs to transfer the minimum balance at least in order to pull the
+		// contract's account into existence.
+		deposit = deposit.max(Deposit::Charge(Pallet::<T>::min_balance()));
+		if deposit.charge_or_zero() > self.limit {
+			return Err(<Error<T>>::StorageDepositLimitExhausted.into())
 		}
-		.to_deposit::<T>();
-		debug_assert!(matches!(deposit, Deposit::Charge(_)));
-		// We do not increase `own_deposit` because this will be charged later when the contract
-		// execution does conclude.
-		let total_deposit = self.total_deposit.saturating_add(&deposit);
-		if let Deposit::Charge(amount) = &total_deposit {
-			if amount > &self.limit {
-				return Err(<Error<T>>::StorageDepositLimitExhausted.into())
-			}
-		}
-		info.storage_deposit = info.storage_deposit.saturating_add(deposit.charge_or_zero());
-		self.total_deposit = total_deposit;
+
+		// We do not increase `own_contribution` because this will be charged later when the
+		// contract execution does conclude and hence would lead to a double charge.
+		self.total_deposit = deposit.clone();
+		info.storage_base_deposit = deposit.charge_or_zero();
 		if !deposit.is_zero() {
 			// We need to charge immediately so that the account is created before the `value`
 			// is transferred from the caller to the contract.
@@ -331,34 +410,55 @@ where
 	///
 	/// This will manipulate the meter so that all storage deposit accumulated in
 	/// `contract_info` will be refunded to the `origin` of the meter.
-	pub fn terminate(&mut self, contract_info: &ContractInfo<T>) {
-		debug_assert!(!self.terminated);
-		let refund = Deposit::Refund(contract_info.storage_deposit);
+	pub fn terminate(&mut self, info: &ContractInfo<T>) {
+		debug_assert!(self.is_alive());
+		self.own_contribution = Contribution::Terminated(Deposit::Refund(info.total_deposit()));
+	}
 
-		// The deposit for `own_deposit` isn't persisted into the contract info until the current
-		// frame is dropped. This means that whatever changes were introduced during the
-		// current frame are dicarded when terminating.
-		self.total_deposit =
-			self.total_deposit.saturating_add(&refund).saturating_sub(&self.own_deposit);
-		self.own_deposit = refund;
-		self.terminated = true;
+	/// [`Self::charge`] does not enforce the storage limit since we want to do this check as late
+	/// as possible to allow later refunds to offset earlier charges.
+	///
+	/// # Note
+	///
+	/// We only need to call this **once** for every call stack and not for every cross contract
+	/// call. Hence this is only called when the last call frame returns.
+	pub fn enforce_limit(
+		&mut self,
+		info: Option<&mut ContractInfo<T>>,
+	) -> Result<(), DispatchError> {
+		let deposit = self.own_contribution.update_contract(info);
+		let total_deposit = self.total_deposit.saturating_add(&deposit);
+		// We don't want to override a `Terminated` with a `Checked`.
+		if self.is_alive() {
+			self.own_contribution = Contribution::Checked(deposit);
+		}
+		if let Deposit::Charge(amount) = total_deposit {
+			if amount > self.limit {
+				return Err(<Error<T>>::StorageDepositLimitExhausted.into())
+			}
+		}
+		Ok(())
 	}
 }
 
-impl<T: Config> Ext<T> for ReservingExt {
+impl<T> Ext<T> for ReservingExt
+where
+	T: Config,
+	T::AccountId: UncheckedFrom<T::Hash> + AsRef<[u8]>,
+{
 	fn check_limit(
 		origin: &T::AccountId,
 		limit: Option<BalanceOf<T>>,
 		min_leftover: BalanceOf<T>,
 	) -> Result<BalanceOf<T>, DispatchError> {
-		let max = T::Currency::free_balance(origin)
-			.saturating_sub(T::Currency::minimum_balance())
-			.saturating_sub(min_leftover);
-		match limit {
-			Some(limit) if limit <= max => Ok(limit),
-			None => Ok(max),
-			_ => Err(<Error<T>>::StorageDepositNotEnoughFunds.into()),
-		}
+		let max = T::Currency::reducible_balance(origin, true).saturating_sub(min_leftover);
+		let limit = limit.unwrap_or(max);
+		ensure!(
+			limit <= max &&
+				matches!(T::Currency::can_withdraw(origin, limit), WithdrawConsequence::Success),
+			<Error<T>>::StorageDepositNotEnoughFunds,
+		);
+		Ok(limit)
 	}
 
 	fn charge(
@@ -393,6 +493,9 @@ impl<T: Config> Ext<T> for ReservingExt {
 						"Failed to transfer storage deposit {:?} from origin {:?} to contract {:?}: {:?}",
 						amount, origin, contract, err,
 					);
+					if cfg!(debug_assertions) {
+						panic!("Unable to collect storage deposit. This is a bug.");
+					}
 				}
 			},
 			// For `Refund(_)` no error happen because the initial value transfer from the
@@ -411,7 +514,7 @@ impl<T: Config> Ext<T> for ReservingExt {
 					// refund because we consider this unexpected behaviour.
 					*amount.min(
 						&T::Currency::reserved_balance(contract)
-							.saturating_sub(T::Currency::minimum_balance()),
+							.saturating_sub(Pallet::<T>::min_balance()),
 					)
 				};
 				let result =
@@ -422,6 +525,9 @@ impl<T: Config> Ext<T> for ReservingExt {
 						"Failed to repatriate storage deposit {:?} from contract {:?} to origin {:?}: {:?}",
 						amount, contract, origin, result,
 					);
+					if cfg!(debug_assertions) {
+						panic!("Unable to refund storage deposit. This is a bug.");
+					}
 				}
 			},
 		};
@@ -513,14 +619,26 @@ mod tests {
 		TestExtTestValue::mutate(|ext| ext.clear())
 	}
 
-	fn new_info(deposit: BalanceOf<Test>) -> ContractInfo<Test> {
+	#[derive(Default)]
+	struct StorageInfo {
+		bytes: u32,
+		items: u32,
+		bytes_deposit: BalanceOf<Test>,
+		items_deposit: BalanceOf<Test>,
+	}
+
+	fn new_info(info: StorageInfo) -> ContractInfo<Test> {
 		use crate::storage::Storage;
 		use sp_runtime::traits::Hash;
 
 		ContractInfo::<Test> {
 			trie_id: <Storage<Test>>::generate_trie_id(&ALICE, 42),
 			code_hash: <Test as frame_system::Config>::Hashing::hash(b"42"),
-			storage_deposit: deposit,
+			storage_bytes: info.bytes,
+			storage_items: info.items,
+			storage_byte_deposit: info.bytes_deposit,
+			storage_item_deposit: info.items_deposit,
+			storage_base_deposit: Default::default(),
 		}
 	}
 
@@ -548,8 +666,8 @@ mod tests {
 
 		// an empty charge does not create a `Charge` entry
 		let mut nested0 = meter.nested();
-		nested0.charge(&Default::default()).unwrap();
-		meter.absorb(nested0, &ALICE, &BOB, None);
+		nested0.charge(&Default::default());
+		meter.absorb(nested0, &BOB, None);
 
 		assert_eq!(
 			TestExtTestValue::get(),
@@ -560,79 +678,49 @@ mod tests {
 		)
 	}
 
-	#[test]
-	fn existential_deposit_works() {
-		clear_ext();
-
-		let mut meter = TestMeter::new(&ALICE, Some(1_000), 0).unwrap();
-		assert_eq!(meter.available(), 1_000);
-
-		// a `Refund` will be turned into a `Charge(ed)` which is intended behaviour
-		let mut nested0 = meter.nested();
-		nested0.charge(&Diff { require_ed: true, ..Default::default() }).unwrap();
-		nested0
-			.charge(&Diff { bytes_removed: 1, require_ed: true, ..Default::default() })
-			.unwrap();
-		meter.absorb(nested0, &ALICE, &BOB, None);
-
-		assert_eq!(
-			TestExtTestValue::get(),
-			TestExt {
-				limit_checks: vec![LimitCheck { origin: ALICE, limit: 1_000, min_leftover: 0 }],
-				charges: vec![Charge {
-					origin: ALICE,
-					contract: BOB,
-					amount: Deposit::Charge(<Test as Config>::Currency::minimum_balance() * 2),
-					terminated: false,
-				}]
-			}
-		)
-	}
-
 	#[test]
 	fn charging_works() {
 		clear_ext();
 
-		let min_balance = <Test as Config>::Currency::minimum_balance();
+		let mut meter = TestMeter::new(&ALICE, Some(100), 0).unwrap();
+		assert_eq!(meter.available(), 100);
 
-		let mut meter = TestMeter::new(&ALICE, Some(1_000), 0).unwrap();
-		assert_eq!(meter.available(), 1_000);
-
-		let mut nested0_info = new_info(100);
+		let mut nested0_info =
+			new_info(StorageInfo { bytes: 100, items: 5, bytes_deposit: 100, items_deposit: 10 });
 		let mut nested0 = meter.nested();
-		nested0
-			.charge(&Diff {
-				bytes_added: 10,
-				bytes_removed: 5,
-				items_added: 1,
-				items_removed: 2,
-				..Default::default()
-			})
-			.unwrap();
-		nested0.charge(&Diff { bytes_removed: 1, ..Default::default() }).unwrap();
+		nested0.charge(&Diff {
+			bytes_added: 108,
+			bytes_removed: 5,
+			items_added: 1,
+			items_removed: 2,
+		});
+		nested0.charge(&Diff { bytes_removed: 99, ..Default::default() });
 
-		let mut nested1_info = new_info(50);
+		let mut nested1_info =
+			new_info(StorageInfo { bytes: 100, items: 10, bytes_deposit: 100, items_deposit: 20 });
 		let mut nested1 = nested0.nested();
-		nested1.charge(&Diff { items_removed: 5, ..Default::default() }).unwrap();
-		nested0.absorb(nested1, &ALICE, &CHARLIE, Some(&mut nested1_info));
+		nested1.charge(&Diff { items_removed: 5, ..Default::default() });
+		nested0.absorb(nested1, &CHARLIE, Some(&mut nested1_info));
 
-		// Trying to refund more than is available in the contract will cap the charge
-		// to (deposit_in_contract - ed).
-		let mut nested2_info = new_info(5);
+		let mut nested2_info =
+			new_info(StorageInfo { bytes: 100, items: 7, bytes_deposit: 100, items_deposit: 20 });
 		let mut nested2 = nested0.nested();
-		nested2.charge(&Diff { bytes_removed: 7, ..Default::default() }).unwrap();
-		nested0.absorb(nested2, &ALICE, &CHARLIE, Some(&mut nested2_info));
+		nested2.charge(&Diff { items_removed: 7, ..Default::default() });
+		nested0.absorb(nested2, &CHARLIE, Some(&mut nested2_info));
 
-		meter.absorb(nested0, &ALICE, &BOB, Some(&mut nested0_info));
+		nested0.enforce_limit(Some(&mut nested0_info)).unwrap();
+		meter.absorb(nested0, &BOB, Some(&mut nested0_info));
 
-		assert_eq!(nested0_info.storage_deposit, 102);
-		assert_eq!(nested1_info.storage_deposit, 40);
-		assert_eq!(nested2_info.storage_deposit, min_balance);
+		meter.into_deposit(&ALICE);
+
+		assert_eq!(nested0_info.extra_deposit(), 112);
+		assert_eq!(nested1_info.extra_deposit(), 110);
+		assert_eq!(nested2_info.extra_deposit(), 100);
 
 		assert_eq!(
 			TestExtTestValue::get(),
 			TestExt {
-				limit_checks: vec![LimitCheck { origin: ALICE, limit: 1_000, min_leftover: 0 }],
+				limit_checks: vec![LimitCheck { origin: ALICE, limit: 100, min_leftover: 0 }],
 				charges: vec![
 					Charge {
 						origin: ALICE,
@@ -643,7 +731,7 @@ mod tests {
 					Charge {
 						origin: ALICE,
 						contract: CHARLIE,
-						amount: Deposit::Refund(4),
+						amount: Deposit::Refund(20),
 						terminated: false
 					},
 					Charge {
@@ -665,26 +753,25 @@ mod tests {
 		assert_eq!(meter.available(), 1_000);
 
 		let mut nested0 = meter.nested();
-		nested0
-			.charge(&Diff {
-				bytes_added: 5,
-				bytes_removed: 1,
-				items_added: 3,
-				items_removed: 1,
-				..Default::default()
-			})
-			.unwrap();
-		nested0.charge(&Diff { items_added: 2, ..Default::default() }).unwrap();
+		nested0.charge(&Diff {
+			bytes_added: 5,
+			bytes_removed: 1,
+			items_added: 3,
+			items_removed: 1,
+		});
+		nested0.charge(&Diff { items_added: 2, ..Default::default() });
 
-		let nested1_info = new_info(400);
+		let mut nested1_info =
+			new_info(StorageInfo { bytes: 100, items: 10, bytes_deposit: 100, items_deposit: 20 });
 		let mut nested1 = nested0.nested();
-		nested1.charge(&Diff { items_removed: 5, ..Default::default() }).unwrap();
-		nested1.charge(&Diff { bytes_added: 20, ..Default::default() }).unwrap();
+		nested1.charge(&Diff { items_removed: 5, ..Default::default() });
+		nested1.charge(&Diff { bytes_added: 20, ..Default::default() });
 		nested1.terminate(&nested1_info);
-		nested0.absorb(nested1, &ALICE, &CHARLIE, None);
+		nested0.enforce_limit(Some(&mut nested1_info)).unwrap();
+		nested0.absorb(nested1, &CHARLIE, None);
 
-		meter.absorb(nested0, &ALICE, &BOB, None);
-		drop(meter);
+		meter.absorb(nested0, &BOB, None);
+		meter.into_deposit(&ALICE);
 
 		assert_eq!(
 			TestExtTestValue::get(),
@@ -694,7 +781,7 @@ mod tests {
 					Charge {
 						origin: ALICE,
 						contract: CHARLIE,
-						amount: Deposit::Refund(400),
+						amount: Deposit::Refund(120),
 						terminated: true
 					},
 					Charge {

substrate/frame/contracts/src/tests.rs

@@ -23,6 +23,7 @@ use crate::{
 	},
 	exec::{FixSizedKey, Frame},
 	storage::Storage,
+	tests::test_utils::{get_contract, get_contract_checked},
 	wasm::{PrefabWasmModule, ReturnCode as RuntimeReturnCode},
 	weights::WeightInfo,
 	BalanceOf, Code, CodeStorage, Config, ContractInfoOf, DefaultAddressGenerator,
@@ -36,8 +37,8 @@ use frame_support::{
 	parameter_types,
 	storage::child,
 	traits::{
-		BalanceStatus, ConstU32, ConstU64, Contains, Currency, Get, OnIdle, OnInitialize,
-		ReservableCurrency,
+		BalanceStatus, ConstU32, ConstU64, Contains, Currency, Get, LockableCurrency, OnIdle,
+		OnInitialize, ReservableCurrency, WithdrawReasons,
 	},
 	weights::{constants::WEIGHT_PER_SECOND, Weight},
 };
@@ -76,7 +77,9 @@ frame_support::construct_runtime!(
 #[macro_use]
 pub mod test_utils {
 	use super::{Balances, Hash, SysConfig, Test};
-	use crate::{exec::AccountIdOf, storage::Storage, CodeHash, Config, ContractInfoOf, Nonce};
+	use crate::{
+		exec::AccountIdOf, storage::Storage, CodeHash, Config, ContractInfo, ContractInfoOf, Nonce,
+	};
 	use codec::Encode;
 	use frame_support::traits::Currency;
 
@@ -97,6 +100,12 @@ pub mod test_utils {
 	pub fn get_balance(who: &AccountIdOf<Test>) -> u64 {
 		Balances::free_balance(who)
 	}
+	pub fn get_contract(addr: &AccountIdOf<Test>) -> ContractInfo<Test> {
+		get_contract_checked(addr).unwrap()
+	}
+	pub fn get_contract_checked(addr: &AccountIdOf<Test>) -> Option<ContractInfo<Test>> {
+		ContractInfoOf::<Test>::get(addr)
+	}
 	pub fn hash<S: Encode>(s: &S) -> <<Test as SysConfig>::Hashing as Hash>::Output {
 		<<Test as SysConfig>::Hashing as Hash>::hash_of(s)
 	}
@@ -105,6 +114,7 @@ pub mod test_utils {
 			assert_eq!(u32::from_le_bytes($x.data[..].try_into().unwrap()), $y as u32);
 		}};
 	}
+
 	macro_rules! assert_refcount {
 		( $code_hash:expr , $should:expr $(,)? ) => {{
 			let is = crate::OwnerInfoOf::<Test>::get($code_hash).map(|m| m.refcount()).unwrap();
@@ -693,7 +703,7 @@ fn instantiate_unique_trie_id() {
 			vec![],
 			vec![],
 		));
-		let trie_id = ContractInfoOf::<Test>::get(&addr).unwrap().trie_id;
+		let trie_id = get_contract(&addr).trie_id;
 
 		// Try to instantiate it again without termination should yield an error.
 		assert_err_ignore_postinfo!(
@@ -731,7 +741,7 @@ fn instantiate_unique_trie_id() {
 		));
 
 		// Trie ids shouldn't match or we might have a collision
-		assert_ne!(trie_id, ContractInfoOf::<Test>::get(&addr).unwrap().trie_id);
+		assert_ne!(trie_id, get_contract(&addr).trie_id);
 	});
 }
 
@@ -752,7 +762,7 @@ fn storage_max_value_limit() {
 			vec![],
 		));
 		let addr = Contracts::contract_address(&ALICE, &code_hash, &[]);
-		ContractInfoOf::<Test>::get(&addr).unwrap();
+		get_contract(&addr);
 
 		// Call contract with allowed storage value.
 		assert_ok!(Contracts::call(
@@ -962,7 +972,7 @@ fn cannot_self_destruct_through_draning() {
 		let addr = Contracts::contract_address(&ALICE, &code_hash, &[]);
 
 		// Check that the BOB contract has been instantiated.
-		assert_matches!(ContractInfoOf::<Test>::get(&addr), Some(_));
+		get_contract(&addr);
 
 		// Call BOB which makes it send all funds to the zero address
 		// The contract code asserts that the transfer was successful
@@ -1003,11 +1013,11 @@ fn cannot_self_destruct_through_storage_refund_after_price_change() {
 		let addr = Contracts::contract_address(&ALICE, &code_hash, &[]);
 
 		// Check that the BOB contract has been instantiated and has the minimum balance
-		let info = ContractInfoOf::<Test>::get(&addr).unwrap();
-		assert_eq!(info.storage_deposit, min_balance);
+		assert_eq!(get_contract(&addr).total_deposit(), min_balance);
+		assert_eq!(get_contract(&addr).extra_deposit(), 0);
 		assert_eq!(<Test as Config>::Currency::total_balance(&addr), min_balance);
 
-		// Create 100 bytes of storage with a price of per byte
+		// Create 100 bytes of storage with a price of per byte and a single storage item of price 2
 		assert_ok!(Contracts::call(
 			RuntimeOrigin::signed(ALICE),
 			addr.clone(),
@@ -1016,9 +1026,10 @@ fn cannot_self_destruct_through_storage_refund_after_price_change() {
 			None,
 			100u32.to_le_bytes().to_vec()
 		));
+		assert_eq!(get_contract(&addr).total_deposit(), min_balance + 102);
 
-		// Increase the byte price and trigger a refund. This could potentially destroy the account
-		// because the refund removes the reserved existential deposit. This should not happen.
+		// Increase the byte price and trigger a refund. This should not have any influence because
+		// the removal is pro rata and exactly those 100 bytes should have been removed.
 		DEPOSIT_PER_BYTE.with(|c| *c.borrow_mut() = 500);
 		assert_ok!(Contracts::call(
 			RuntimeOrigin::signed(ALICE),
@@ -1032,8 +1043,9 @@ fn cannot_self_destruct_through_storage_refund_after_price_change() {
 		// Make sure the account wasn't removed by the refund
 		assert_eq!(
 			<Test as Config>::Currency::total_balance(&addr),
-			<Test as Config>::Currency::minimum_balance(),
+			get_contract(&addr).total_deposit(),
 		);
+		assert_eq!(get_contract(&addr).extra_deposit(), 2,);
 	});
 }
 
@@ -1086,9 +1098,6 @@ fn cannot_self_destruct_by_refund_after_slash() {
 		// Make sure the account kept the minimum balance and was not destroyed
 		assert_eq!(<Test as Config>::Currency::total_balance(&addr), min_balance);
 
-		// even though it was not charged it is still substracted from the storage deposit tracker
-		assert_eq!(ContractInfoOf::<Test>::get(&addr).unwrap().storage_deposit, 550);
-
 		assert_eq!(
 			System::events(),
 			vec![
@@ -1142,7 +1151,7 @@ fn cannot_self_destruct_while_live() {
 		let addr = Contracts::contract_address(&ALICE, &code_hash, &[]);
 
 		// Check that the BOB contract has been instantiated.
-		assert_matches!(ContractInfoOf::<Test>::get(&addr), Some(_));
+		get_contract(&addr);
 
 		// Call BOB with input data, forcing it make a recursive call to itself to
 		// self-destruct, resulting in a trap.
@@ -1159,7 +1168,7 @@ fn cannot_self_destruct_while_live() {
 		);
 
 		// Check that BOB is still there.
-		assert_matches!(ContractInfoOf::<Test>::get(&addr), Some(_));
+		get_contract(&addr);
 	});
 }
 
@@ -1183,7 +1192,7 @@ fn self_destruct_works() {
 		let addr = Contracts::contract_address(&ALICE, &code_hash, &[]);
 
 		// Check that the BOB contract has been instantiated.
-		assert_matches!(ContractInfoOf::<Test>::get(&addr), Some(_));
+		get_contract(&addr);
 
 		// Drop all previous events
 		initialize_block(2);
@@ -1198,7 +1207,7 @@ fn self_destruct_works() {
 		assert_refcount!(&code_hash, 0);
 
 		// Check that account is gone
-		assert!(ContractInfoOf::<Test>::get(&addr).is_none());
+		assert!(get_contract_checked(&addr).is_none());
 		assert_eq!(Balances::total_balance(&addr), 0);
 
 		// check that the beneficiary (django) got remaining balance
@@ -1289,7 +1298,7 @@ fn destroy_contract_and_transfer_funds() {
 		let addr_charlie = Contracts::contract_address(&addr_bob, &callee_code_hash, &[0x47, 0x11]);
 
 		// Check that the CHARLIE contract has been instantiated.
-		assert_matches!(ContractInfoOf::<Test>::get(&addr_charlie), Some(_));
+		get_contract(&addr_charlie);
 
 		// Call BOB, which calls CHARLIE, forcing CHARLIE to self-destruct.
 		assert_ok!(Contracts::call(
@@ -1302,7 +1311,7 @@ fn destroy_contract_and_transfer_funds() {
 		));
 
 		// Check that CHARLIE has moved on to the great beyond (ie. died).
-		assert!(ContractInfoOf::<Test>::get(&addr_charlie).is_none());
+		assert!(get_contract_checked(&addr_charlie).is_none());
 	});
 }
 
@@ -1862,7 +1871,7 @@ fn lazy_removal_works() {
 		),);
 
 		let addr = Contracts::contract_address(&ALICE, &hash, &[]);
-		let info = <ContractInfoOf<Test>>::get(&addr).unwrap();
+		let info = get_contract(&addr);
 		let trie = &info.child_trie_info();
 
 		// Put value into the contracts child trie
@@ -1931,7 +1940,7 @@ fn lazy_batch_removal_works() {
 			),);
 
 			let addr = Contracts::contract_address(&ALICE, &hash, &[i]);
-			let info = <ContractInfoOf<Test>>::get(&addr).unwrap();
+			let info = get_contract(&addr);
 			let trie = &info.child_trie_info();
 
 			// Put value into the contracts child trie
@@ -1993,7 +2002,7 @@ fn lazy_removal_partial_remove_works() {
 		),);
 
 		let addr = Contracts::contract_address(&ALICE, &hash, &[]);
-		let info = <ContractInfoOf<Test>>::get(&addr).unwrap();
+		let info = get_contract(&addr);
 
 		// Put value into the contracts child trie
 		for val in &vals {
@@ -2105,7 +2114,7 @@ fn lazy_removal_does_no_run_on_low_remaining_weight() {
 		),);
 
 		let addr = Contracts::contract_address(&ALICE, &hash, &[]);
-		let info = <ContractInfoOf<Test>>::get(&addr).unwrap();
+		let info = get_contract(&addr);
 		let trie = &info.child_trie_info();
 
 		// Put value into the contracts child trie
@@ -2173,7 +2182,7 @@ fn lazy_removal_does_not_use_all_weight() {
 		),);
 
 		let addr = Contracts::contract_address(&ALICE, &hash, &[]);
-		let info = <ContractInfoOf<Test>>::get(&addr).unwrap();
+		let info = get_contract(&addr);
 		let (weight_per_key, max_keys) = Storage::<Test>::deletion_budget(1, weight_limit);
 
 		// We create a contract with one less storage item than we can remove within the limit
@@ -2264,7 +2273,7 @@ fn deletion_queue_full() {
 		);
 
 		// Contract should exist because removal failed
-		<ContractInfoOf<Test>>::get(&addr).unwrap();
+		get_contract(&addr);
 	});
 }
 
@@ -2909,7 +2918,7 @@ fn instantiate_with_zero_balance_works() {
 		let addr = Contracts::contract_address(&ALICE, &code_hash, &[]);
 
 		// Check that the BOB contract has been instantiated.
-		assert_matches!(ContractInfoOf::<Test>::get(&addr), Some(_));
+		get_contract(&addr);
 
 		// Make sure the account exists even though no free balance was send
 		assert_eq!(<Test as Config>::Currency::free_balance(&addr), 0,);
@@ -3002,7 +3011,7 @@ fn instantiate_with_below_existential_deposit_works() {
 		let addr = Contracts::contract_address(&ALICE, &code_hash, &[]);
 
 		// Check that the BOB contract has been instantiated.
-		assert_matches!(ContractInfoOf::<Test>::get(&addr), Some(_));
+		get_contract(&addr);
 
 		// Make sure the account exists even though no free balance was send
 		assert_eq!(<Test as Config>::Currency::free_balance(&addr), 50,);
@@ -3114,7 +3123,7 @@ fn storage_deposit_works() {
 		// 4 is for creating 2 storage items
 		let charged0 = 4 + 1_000 + 5_000;
 		deposit += charged0;
-		assert_eq!(<ContractInfoOf<Test>>::get(&addr).unwrap().storage_deposit, deposit);
+		assert_eq!(get_contract(&addr).total_deposit(), deposit);
 
 		// Add more storage (but also remove some)
 		assert_ok!(Contracts::call(
@@ -3127,7 +3136,7 @@ fn storage_deposit_works() {
 		));
 		let charged1 = 1_000 - 100;
 		deposit += charged1;
-		assert_eq!(<ContractInfoOf<Test>>::get(&addr).unwrap().storage_deposit, deposit);
+		assert_eq!(get_contract(&addr).total_deposit(), deposit);
 
 		// Remove more storage (but also add some)
 		assert_ok!(Contracts::call(
@@ -3138,9 +3147,10 @@ fn storage_deposit_works() {
 			None,
 			(2_100u32, 900u32).encode(),
 		));
-		let refunded0 = 4_000 - 100;
+		// -1 for numeric instability
+		let refunded0 = 4_000 - 100 - 1;
 		deposit -= refunded0;
-		assert_eq!(<ContractInfoOf<Test>>::get(&addr).unwrap().storage_deposit, deposit);
+		assert_eq!(get_contract(&addr).total_deposit(), deposit);
 
 		assert_eq!(
 			System::events(),
@@ -3253,7 +3263,7 @@ fn set_code_extrinsic() {
 		// Drop previous events
 		initialize_block(2);
 
-		assert_eq!(<ContractInfoOf<Test>>::get(&addr).unwrap().code_hash, code_hash);
+		assert_eq!(get_contract(&addr).code_hash, code_hash);
 		assert_refcount!(&code_hash, 1);
 		assert_refcount!(&new_code_hash, 0);
 
@@ -3262,7 +3272,7 @@ fn set_code_extrinsic() {
 			Contracts::set_code(RuntimeOrigin::signed(ALICE), addr.clone(), new_code_hash),
 			sp_runtime::traits::BadOrigin,
 		);
-		assert_eq!(<ContractInfoOf<Test>>::get(&addr).unwrap().code_hash, code_hash);
+		assert_eq!(get_contract(&addr).code_hash, code_hash);
 		assert_refcount!(&code_hash, 1);
 		assert_refcount!(&new_code_hash, 0);
 		assert_eq!(System::events(), vec![],);
@@ -3272,7 +3282,7 @@ fn set_code_extrinsic() {
 			Contracts::set_code(RuntimeOrigin::root(), BOB, new_code_hash),
 			<Error<Test>>::ContractNotFound,
 		);
-		assert_eq!(<ContractInfoOf<Test>>::get(&addr).unwrap().code_hash, code_hash);
+		assert_eq!(get_contract(&addr).code_hash, code_hash);
 		assert_refcount!(&code_hash, 1);
 		assert_refcount!(&new_code_hash, 0);
 		assert_eq!(System::events(), vec![],);
@@ -3282,14 +3292,14 @@ fn set_code_extrinsic() {
 			Contracts::set_code(RuntimeOrigin::root(), addr.clone(), Default::default()),
 			<Error<Test>>::CodeNotFound,
 		);
-		assert_eq!(<ContractInfoOf<Test>>::get(&addr).unwrap().code_hash, code_hash);
+		assert_eq!(get_contract(&addr).code_hash, code_hash);
 		assert_refcount!(&code_hash, 1);
 		assert_refcount!(&new_code_hash, 0);
 		assert_eq!(System::events(), vec![],);
 
 		// successful call
 		assert_ok!(Contracts::set_code(RuntimeOrigin::root(), addr.clone(), new_code_hash));
-		assert_eq!(<ContractInfoOf<Test>>::get(&addr).unwrap().code_hash, new_code_hash);
+		assert_eq!(get_contract(&addr).code_hash, new_code_hash);
 		assert_refcount!(&code_hash, 0);
 		assert_refcount!(&new_code_hash, 1);
 		assert_eq!(
@@ -3639,3 +3649,301 @@ fn set_code_hash() {
 		);
 	});
 }
+
+#[test]
+fn storage_deposit_limit_is_enforced() {
+	let (wasm, code_hash) = compile_module::<Test>("store").unwrap();
+	ExtBuilder::default().existential_deposit(200).build().execute_with(|| {
+		let _ = Balances::deposit_creating(&ALICE, 1_000_000);
+		let min_balance = <Test as Config>::Currency::minimum_balance();
+
+		// Instantiate the BOB contract.
+		assert_ok!(Contracts::instantiate_with_code(
+			RuntimeOrigin::signed(ALICE),
+			0,
+			GAS_LIMIT,
+			None,
+			wasm,
+			vec![],
+			vec![],
+		));
+		let addr = Contracts::contract_address(&ALICE, &code_hash, &[]);
+
+		// Check that the BOB contract has been instantiated and has the minimum balance
+		assert_eq!(get_contract(&addr).total_deposit(), min_balance);
+		assert_eq!(<Test as Config>::Currency::total_balance(&addr), min_balance);
+
+		// Create 100 bytes of storage with a price of per byte
+		assert_err_ignore_postinfo!(
+			Contracts::call(
+				RuntimeOrigin::signed(ALICE),
+				addr.clone(),
+				0,
+				GAS_LIMIT,
+				Some(codec::Compact(1)),
+				100u32.to_le_bytes().to_vec()
+			),
+			<Error<Test>>::StorageDepositLimitExhausted,
+		);
+	});
+}
+
+#[test]
+fn storage_deposit_limit_is_enforced_late() {
+	let (wasm_caller, code_hash_caller) =
+		compile_module::<Test>("create_storage_and_call").unwrap();
+	let (wasm_callee, code_hash_callee) = compile_module::<Test>("store").unwrap();
+	ExtBuilder::default().existential_deposit(200).build().execute_with(|| {
+		let _ = Balances::deposit_creating(&ALICE, 1_000_000);
+
+		// Create both contracts: Constructors do nothing.
+		assert_ok!(Contracts::instantiate_with_code(
+			RuntimeOrigin::signed(ALICE),
+			0,
+			GAS_LIMIT,
+			None,
+			wasm_caller,
+			vec![],
+			vec![],
+		));
+		let addr_caller = Contracts::contract_address(&ALICE, &code_hash_caller, &[]);
+		assert_ok!(Contracts::instantiate_with_code(
+			RuntimeOrigin::signed(ALICE),
+			0,
+			GAS_LIMIT,
+			None,
+			wasm_callee,
+			vec![],
+			vec![],
+		));
+		let addr_callee = Contracts::contract_address(&ALICE, &code_hash_callee, &[]);
+
+		// Create 100 bytes of storage with a price of per byte
+		// This is 100 Balance + 2 Balance for the item
+		assert_ok!(Contracts::call(
+			RuntimeOrigin::signed(ALICE),
+			addr_callee.clone(),
+			0,
+			GAS_LIMIT,
+			Some(codec::Compact(102)),
+			100u32.to_le_bytes().to_vec()
+		));
+
+		// We do not remove any storage but require 14 bytes of storage for the new
+		// storage created in the immediate contract.
+		assert_err_ignore_postinfo!(
+			Contracts::call(
+				RuntimeOrigin::signed(ALICE),
+				addr_caller.clone(),
+				0,
+				GAS_LIMIT,
+				Some(codec::Compact(5)),
+				100u32
+					.to_le_bytes()
+					.as_ref()
+					.iter()
+					.chain(<_ as AsRef<[u8]>>::as_ref(&addr_callee))
+					.cloned()
+					.collect(),
+			),
+			<Error<Test>>::StorageDepositLimitExhausted,
+		);
+
+		// Allow for the additional 14 bytes but demand an additional byte in the callee contract.
+		assert_err_ignore_postinfo!(
+			Contracts::call(
+				RuntimeOrigin::signed(ALICE),
+				addr_caller.clone(),
+				0,
+				GAS_LIMIT,
+				Some(codec::Compact(14)),
+				101u32
+					.to_le_bytes()
+					.as_ref()
+					.iter()
+					.chain(<_ as AsRef<[u8]>>::as_ref(&addr_callee))
+					.cloned()
+					.collect(),
+			),
+			<Error<Test>>::StorageDepositLimitExhausted,
+		);
+
+		// Refund in the callee contract but not enough to cover the 14 balance required by the
+		// caller.
+		assert_err_ignore_postinfo!(
+			Contracts::call(
+				RuntimeOrigin::signed(ALICE),
+				addr_caller.clone(),
+				0,
+				GAS_LIMIT,
+				Some(codec::Compact(0)),
+				87u32
+					.to_le_bytes()
+					.as_ref()
+					.iter()
+					.chain(<_ as AsRef<[u8]>>::as_ref(&addr_callee))
+					.cloned()
+					.collect(),
+			),
+			<Error<Test>>::StorageDepositLimitExhausted,
+		);
+
+		let _ = Balances::make_free_balance_be(&ALICE, 1_000);
+
+		// Send more than the sender has balance.
+		assert_err_ignore_postinfo!(
+			Contracts::call(
+				RuntimeOrigin::signed(ALICE),
+				addr_caller.clone(),
+				0,
+				GAS_LIMIT,
+				Some(codec::Compact(50)),
+				1_200u32
+					.to_le_bytes()
+					.as_ref()
+					.iter()
+					.chain(<_ as AsRef<[u8]>>::as_ref(&addr_callee))
+					.cloned()
+					.collect(),
+			),
+			<Error<Test>>::StorageDepositLimitExhausted,
+		);
+
+		// Same as above but allow for the additional balance.
+		assert_ok!(Contracts::call(
+			RuntimeOrigin::signed(ALICE),
+			addr_caller.clone(),
+			0,
+			GAS_LIMIT,
+			Some(codec::Compact(1)),
+			87u32
+				.to_le_bytes()
+				.as_ref()
+				.iter()
+				.chain(<_ as AsRef<[u8]>>::as_ref(&addr_callee))
+				.cloned()
+				.collect(),
+		));
+	});
+}
+
+#[test]
+fn deposit_limit_honors_liquidity_restrictions() {
+	let (wasm, code_hash) = compile_module::<Test>("store").unwrap();
+	ExtBuilder::default().existential_deposit(200).build().execute_with(|| {
+		let _ = Balances::deposit_creating(&ALICE, 1_000_000);
+		let _ = Balances::deposit_creating(&BOB, 1_000);
+		let min_balance = <Test as Config>::Currency::minimum_balance();
+
+		// Instantiate the BOB contract.
+		assert_ok!(Contracts::instantiate_with_code(
+			RuntimeOrigin::signed(ALICE),
+			0,
+			GAS_LIMIT,
+			None,
+			wasm,
+			vec![],
+			vec![],
+		));
+		let addr = Contracts::contract_address(&ALICE, &code_hash, &[]);
+
+		// Check that the contract has been instantiated and has the minimum balance
+		assert_eq!(get_contract(&addr).total_deposit(), min_balance);
+		assert_eq!(<Test as Config>::Currency::total_balance(&addr), min_balance);
+
+		// check that the lock ins honored
+		Balances::set_lock([0; 8], &BOB, 1_000, WithdrawReasons::TRANSFER);
+		assert_err_ignore_postinfo!(
+			Contracts::call(
+				RuntimeOrigin::signed(BOB),
+				addr.clone(),
+				0,
+				GAS_LIMIT,
+				Some(codec::Compact(200)),
+				100u32.to_le_bytes().to_vec()
+			),
+			<Error<Test>>::StorageDepositNotEnoughFunds,
+		);
+		assert_eq!(Balances::free_balance(&BOB), 1_000);
+	});
+}
+
+#[test]
+fn deposit_limit_honors_existential_deposit() {
+	let (wasm, code_hash) = compile_module::<Test>("store").unwrap();
+	ExtBuilder::default().existential_deposit(200).build().execute_with(|| {
+		let _ = Balances::deposit_creating(&ALICE, 1_000_000);
+		let _ = Balances::deposit_creating(&BOB, 1_000);
+		let min_balance = <Test as Config>::Currency::minimum_balance();
+
+		// Instantiate the BOB contract.
+		assert_ok!(Contracts::instantiate_with_code(
+			RuntimeOrigin::signed(ALICE),
+			0,
+			GAS_LIMIT,
+			None,
+			wasm,
+			vec![],
+			vec![],
+		));
+		let addr = Contracts::contract_address(&ALICE, &code_hash, &[]);
+
+		// Check that the contract has been instantiated and has the minimum balance
+		assert_eq!(get_contract(&addr).total_deposit(), min_balance);
+		assert_eq!(<Test as Config>::Currency::total_balance(&addr), min_balance);
+
+		// check that the deposit can't bring the account below the existential deposit
+		assert_err_ignore_postinfo!(
+			Contracts::call(
+				RuntimeOrigin::signed(BOB),
+				addr.clone(),
+				0,
+				GAS_LIMIT,
+				Some(codec::Compact(900)),
+				100u32.to_le_bytes().to_vec()
+			),
+			<Error<Test>>::StorageDepositNotEnoughFunds,
+		);
+		assert_eq!(Balances::free_balance(&BOB), 1_000);
+	});
+}
+
+#[test]
+fn deposit_limit_honors_min_leftover() {
+	let (wasm, code_hash) = compile_module::<Test>("store").unwrap();
+	ExtBuilder::default().existential_deposit(200).build().execute_with(|| {
+		let _ = Balances::deposit_creating(&ALICE, 1_000_000);
+		let _ = Balances::deposit_creating(&BOB, 1_000);
+		let min_balance = <Test as Config>::Currency::minimum_balance();
+
+		// Instantiate the BOB contract.
+		assert_ok!(Contracts::instantiate_with_code(
+			RuntimeOrigin::signed(ALICE),
+			0,
+			GAS_LIMIT,
+			None,
+			wasm,
+			vec![],
+			vec![],
+		));
+		let addr = Contracts::contract_address(&ALICE, &code_hash, &[]);
+
+		// Check that the contract has been instantiated and has the minimum balance
+		assert_eq!(get_contract(&addr).total_deposit(), min_balance);
+		assert_eq!(<Test as Config>::Currency::total_balance(&addr), min_balance);
+
+		// check that the minumum leftover (value send) is considered
+		assert_err_ignore_postinfo!(
+			Contracts::call(
+				RuntimeOrigin::signed(BOB),
+				addr.clone(),
+				400,
+				GAS_LIMIT,
+				Some(codec::Compact(500)),
+				100u32.to_le_bytes().to_vec()
+			),
+			<Error<Test>>::StorageDepositNotEnoughFunds,
+		);
+		assert_eq!(Balances::free_balance(&BOB), 1_000);
+	});
+}

substrate/frame/contracts/src/wasm/prepare.rs

@@ -426,7 +426,7 @@ fn do_preparation<C: ImportSatisfyCheck, T: Config>(
 		.saturating_add(original_code_len)
 		.saturating_add(<OwnerInfo<T>>::max_encoded_len()) as u32;
 	let deposit = Diff { bytes_added, items_added: 3, ..Default::default() }
-		.to_deposit::<T>()
+		.update_contract::<T>(None)
 		.charge_or_zero();
 
 	module.owner_info = Some(OwnerInfo { owner, deposit, refcount: 0 });

substrate/frame/support/src/traits/tokens/currency.rs

@@ -26,7 +26,7 @@ use crate::{
 	traits::Get,
 };
 use codec::MaxEncodedLen;
-use sp_runtime::traits::MaybeSerializeDeserialize;
+use sp_runtime::{traits::MaybeSerializeDeserialize, FixedPointOperand};
 use sp_std::fmt::Debug;
 
 mod reservable;
@@ -37,7 +37,7 @@ pub use lockable::{LockIdentifier, LockableCurrency, VestingSchedule};
 /// Abstraction over a fungible assets system.
 pub trait Currency<AccountId> {
 	/// The balance of an account.
-	type Balance: Balance + MaybeSerializeDeserialize + Debug + MaxEncodedLen;
+	type Balance: Balance + MaybeSerializeDeserialize + Debug + MaxEncodedLen + FixedPointOperand;
 
 	/// The opaque token type for an imbalance. This is returned by unbalanced operations
 	/// and must be dealt with. It may be dropped but cannot be cloned.