contracts: Add configurable per-storage item cost (#7819)

* Rework rent parameters

* No need for empty_pair_count any longer

* Parameterize runtime

Author: athei

src/benchmarking/mod.rs

@@ -116,12 +116,13 @@ where
 				// the subsistence threshold does not pay rent given a large enough subsistence
 				// threshold. But we need rent payments to occur in order to benchmark for worst cases.
 				let storage_size = ConfigCache::<T>::subsistence_threshold_uncached()
-					.checked_div(&T::RentDepositOffset::get())
+					.checked_div(&T::DepositPerStorageByte::get())
 					.unwrap_or_else(Zero::zero);
 
 				// Endowment should be large but not as large to inhibit rent payments.
-				let endowment = T::RentDepositOffset::get()
-					.saturating_mul(storage_size + T::StorageSizeOffset::get().into())
+				let endowment = T::DepositPerStorageByte::get()
+					.saturating_mul(storage_size)
+					.saturating_add(T::DepositPerContract::get())
 					.saturating_sub(1u32.into());
 
 				(storage_size, endowment)

src/lib.rs

@@ -115,7 +115,7 @@ use sp_runtime::{
 	traits::{
 		Hash, StaticLookup, Zero, MaybeSerializeDeserialize, Member, Convert, Saturating,
 	},
-	RuntimeDebug,
+	RuntimeDebug, Perbill,
 };
 use frame_support::{
 	decl_module, decl_event, decl_storage, decl_error, ensure,
@@ -205,11 +205,8 @@ pub struct RawAliveContractInfo<CodeHash, Balance, BlockNumber> {
 	///
 	/// It is a sum of each key-value pair stored by this contract.
 	pub storage_size: u32,
-	/// The number of key-value pairs that have values of zero length.
-	/// The condition `empty_pair_count ≤ total_pair_count` always holds.
-	pub empty_pair_count: u32,
 	/// The total number of key-value pairs in storage of this contract.
-	pub total_pair_count: u32,
+	pub pair_count: u32,
 	/// The code associated with a given account.
 	pub code_hash: CodeHash,
 	/// Pay rent at most up to this value.
@@ -286,24 +283,35 @@ pub trait Config: frame_system::Config {
 	/// The minimum amount required to generate a tombstone.
 	type TombstoneDeposit: Get<BalanceOf<Self>>;
 
-	/// A size offset for an contract. A just created account with untouched storage will have that
-	/// much of storage from the perspective of the state rent.
+	/// The balance every contract needs to deposit to stay alive indefinitely.
+	///
+	/// This is different from the [`Self::TombstoneDeposit`] because this only needs to be
+	/// deposited while the contract is alive. Costs for additional storage are added to
+	/// this base cost.
 	///
 	/// This is a simple way to ensure that contracts with empty storage eventually get deleted by
 	/// making them pay rent. This creates an incentive to remove them early in order to save rent.
-	type StorageSizeOffset: Get<u32>;
-
-	/// Price of a byte of storage per one block interval. Should be greater than 0.
-	type RentByteFee: Get<BalanceOf<Self>>;
+	type DepositPerContract: Get<BalanceOf<Self>>;
 
-	/// The amount of funds a contract should deposit in order to offset
-	/// the cost of one byte.
+	/// The balance a contract needs to deposit per storage byte to stay alive indefinitely.
 	///
 	/// Let's suppose the deposit is 1,000 BU (balance units)/byte and the rent is 1 BU/byte/day,
 	/// then a contract with 1,000,000 BU that uses 1,000 bytes of storage would pay no rent.
 	/// But if the balance reduced to 500,000 BU and the storage stayed the same at 1,000,
 	/// then it would pay 500 BU/day.
-	type RentDepositOffset: Get<BalanceOf<Self>>;
+	type DepositPerStorageByte: Get<BalanceOf<Self>>;
+
+	/// The balance a contract needs to deposit per storage item to stay alive indefinitely.
+	///
+	/// It works the same as [`Self::DepositPerStorageByte`] but for storage items.
+	type DepositPerStorageItem: Get<BalanceOf<Self>>;
+
+	/// The fraction of the deposit that should be used as rent per block.
+	///
+	/// When a contract hasn't enough balance deposited to stay alive indefinitely it needs
+	/// to pay per block for the storage it consumes that is not covered by the deposit.
+	/// This determines how high this rent payment is per block as a fraction of the deposit.
+	type RentFraction: Get<Perbill>;
 
 	/// Reward that is received by the party whose touch has led
 	/// to removal of a contract.
@@ -435,25 +443,35 @@ decl_module! {
 		/// The minimum amount required to generate a tombstone.
 		const TombstoneDeposit: BalanceOf<T> = T::TombstoneDeposit::get();
 
-		/// A size offset for an contract. A just created account with untouched storage will have that
-		/// much of storage from the perspective of the state rent.
+		/// The balance every contract needs to deposit to stay alive indefinitely.
 		///
-		/// This is a simple way to ensure that contracts with empty storage eventually get deleted
-		/// by making them pay rent. This creates an incentive to remove them early in order to save
-		/// rent.
-		const StorageSizeOffset: u32 = T::StorageSizeOffset::get();
-
-		/// Price of a byte of storage per one block interval. Should be greater than 0.
-		const RentByteFee: BalanceOf<T> = T::RentByteFee::get();
+		/// This is different from the [`Self::TombstoneDeposit`] because this only needs to be
+		/// deposited while the contract is alive. Costs for additional storage are added to
+		/// this base cost.
+		///
+		/// This is a simple way to ensure that contracts with empty storage eventually get deleted by
+		/// making them pay rent. This creates an incentive to remove them early in order to save rent.
+		const DepositPerContract: BalanceOf<T> = T::DepositPerContract::get();
 
-		/// The amount of funds a contract should deposit in order to offset
-		/// the cost of one byte.
+		/// The balance a contract needs to deposit per storage byte to stay alive indefinitely.
 		///
 		/// Let's suppose the deposit is 1,000 BU (balance units)/byte and the rent is 1 BU/byte/day,
 		/// then a contract with 1,000,000 BU that uses 1,000 bytes of storage would pay no rent.
 		/// But if the balance reduced to 500,000 BU and the storage stayed the same at 1,000,
 		/// then it would pay 500 BU/day.
-		const RentDepositOffset: BalanceOf<T> = T::RentDepositOffset::get();
+		const DepositPerStorageByte: BalanceOf<T> = T::DepositPerStorageByte::get();
+
+		/// The balance a contract needs to deposit per storage item to stay alive indefinitely.
+		///
+		/// It works the same as [`Self::DepositPerStorageByte`] but for storage items.
+		const DepositPerStorageItem: BalanceOf<T> = T::DepositPerStorageItem::get();
+
+		/// The fraction of the deposit that should be used as rent per block.
+		///
+		/// When a contract hasn't enough balance deposited to stay alive indefinitely it needs
+		/// to pay per block for the storage it consumes that is not covered by the deposit.
+		/// This determines how high this rent payment is per block as a fraction of the deposit.
+		const RentFraction: Perbill = T::RentFraction::get();
 
 		/// Reward that is received by the party whose touch has led
 		/// to removal of a contract.

src/rent.rs

@@ -101,21 +101,15 @@ where
 		free_balance: &BalanceOf<T>,
 		contract: &AliveContractInfo<T>
 	) -> BalanceOf<T> {
-		let free_storage = free_balance
-			.checked_div(&T::RentDepositOffset::get())
-			.unwrap_or_else(Zero::zero);
-
-		// For now, we treat every empty KV pair as if it was one byte long.
-		let empty_pairs_equivalent = contract.empty_pair_count;
-
-		let effective_storage_size = <BalanceOf<T>>::from(
-			contract.storage_size + T::StorageSizeOffset::get() + empty_pairs_equivalent,
-		)
-		.saturating_sub(free_storage);
-
-		effective_storage_size
-			.checked_mul(&T::RentByteFee::get())
-			.unwrap_or_else(|| <BalanceOf<T>>::max_value())
+		let uncovered_by_balance = T::DepositPerStorageByte::get()
+			.saturating_mul(contract.storage_size.into())
+			.saturating_add(
+				T::DepositPerStorageItem::get()
+					.saturating_mul(contract.pair_count.into())
+			)
+			.saturating_add(T::DepositPerContract::get())
+			.saturating_sub(*free_balance);
+		T::RentFraction::get().mul_ceil(uncovered_by_balance)
 	}
 
 	/// Returns amount of funds available to consume by rent mechanism.
@@ -484,8 +478,7 @@ where
 		<ContractInfoOf<T>>::insert(&dest, ContractInfo::Alive(AliveContractInfo::<T> {
 			trie_id: origin_contract.trie_id,
 			storage_size: origin_contract.storage_size,
-			empty_pair_count: origin_contract.empty_pair_count,
-			total_pair_count: origin_contract.total_pair_count,
+			pair_count: origin_contract.pair_count,
 			code_hash,
 			rent_allowance,
 			deduct_block: current_block,

src/storage.rs

@@ -102,27 +102,14 @@ where
 
 		// Update the total number of KV pairs and the number of empty pairs.
 		match (&opt_prev_value, &opt_new_value) {
-			(Some(prev_value), None) => {
-				new_info.total_pair_count -= 1;
-				if prev_value.is_empty() {
-					new_info.empty_pair_count -= 1;
-				}
+			(Some(_), None) => {
+				new_info.pair_count -= 1;
 			},
-			(None, Some(new_value)) => {
-				new_info.total_pair_count += 1;
-				if new_value.is_empty() {
-					new_info.empty_pair_count += 1;
-				}
+			(None, Some(_)) => {
+				new_info.pair_count += 1;
 			},
-			(Some(prev_value), Some(new_value)) => {
-				if prev_value.is_empty() {
-					new_info.empty_pair_count -= 1;
-				}
-				if new_value.is_empty() {
-					new_info.empty_pair_count += 1;
-				}
-			}
-			(None, None) => {}
+			(Some(_), Some(_)) => {},
+			(None, None) => {},
 		}
 
 		// Update the total storage size.
@@ -197,8 +184,7 @@ where
 					trie_id,
 					deduct_block: <frame_system::Module<T>>::block_number(),
 					rent_allowance: <BalanceOf<T>>::max_value(),
-					empty_pair_count: 0,
-					total_pair_count: 0,
+					pair_count: 0,
 					last_write: None,
 				}
 				.into(),
@@ -217,7 +203,7 @@ where
 			Err(Error::<T>::DeletionQueueFull.into())
 		} else {
 			DeletionQueue::append(DeletedContract {
-				pair_count: contract.total_pair_count,
+				pair_count: contract.pair_count,
 				trie_id: contract.trie_id.clone(),
 			});
 			Ok(())

src/tests.rs

@@ -30,7 +30,7 @@ use codec::Encode;
 use sp_runtime::{
 	traits::{BlakeTwo256, Hash, IdentityLookup, Convert},
 	testing::{Header, H256},
-	AccountId32,
+	AccountId32, Perbill,
 };
 use sp_io::hashing::blake2_256;
 use frame_support::{
@@ -239,9 +239,10 @@ impl pallet_timestamp::Config for Test {
 parameter_types! {
 	pub const SignedClaimHandicap: u64 = 2;
 	pub const TombstoneDeposit: u64 = 16;
-	pub const StorageSizeOffset: u32 = 8;
-	pub const RentByteFee: u64 = 4;
-	pub const RentDepositOffset: u64 = 10_000;
+	pub const DepositPerContract: u64 = 8 * DepositPerStorageByte::get();
+	pub const DepositPerStorageByte: u64 = 10_000;
+	pub const DepositPerStorageItem: u64 = 10_000;
+	pub RentFraction: Perbill = Perbill::from_rational_approximation(4u32, 10_000u32);
 	pub const SurchargeReward: u64 = 150;
 	pub const MaxDepth: u32 = 100;
 	pub const MaxValueSize: u32 = 16_384;
@@ -267,9 +268,10 @@ impl Config for Test {
 	type RentPayment = ();
 	type SignedClaimHandicap = SignedClaimHandicap;
 	type TombstoneDeposit = TombstoneDeposit;
-	type StorageSizeOffset = StorageSizeOffset;
-	type RentByteFee = RentByteFee;
-	type RentDepositOffset = RentDepositOffset;
+	type DepositPerContract = DepositPerContract;
+	type DepositPerStorageByte = DepositPerStorageByte;
+	type DepositPerStorageItem = DepositPerStorageItem;
+	type RentFraction = RentFraction;
 	type SurchargeReward = SurchargeReward;
 	type MaxDepth = MaxDepth;
 	type MaxValueSize = MaxValueSize;
@@ -384,8 +386,7 @@ fn account_removal_does_not_remove_storage() {
 			let alice_contract_info = ContractInfo::Alive(RawAliveContractInfo {
 				trie_id: trie_id1.clone(),
 				storage_size: 0,
-				empty_pair_count: 0,
-				total_pair_count: 0,
+				pair_count: 0,
 				deduct_block: System::block_number(),
 				code_hash: H256::repeat_byte(1),
 				rent_allowance: 40,
@@ -399,8 +400,7 @@ fn account_removal_does_not_remove_storage() {
 			let bob_contract_info = ContractInfo::Alive(RawAliveContractInfo {
 				trie_id: trie_id2.clone(),
 				storage_size: 0,
-				empty_pair_count: 0,
-				total_pair_count: 0,
+				pair_count: 0,
 				deduct_block: System::block_number(),
 				code_hash: H256::repeat_byte(2),
 				rent_allowance: 40,
@@ -690,13 +690,9 @@ fn storage_size() {
 				4
 			);
 			assert_eq!(
-				bob_contract.total_pair_count,
+				bob_contract.pair_count,
 				1,
 			);
-			assert_eq!(
-				bob_contract.empty_pair_count,
-				0,
-			);
 
 			assert_ok!(Contracts::call(
 				Origin::signed(ALICE),
@@ -714,13 +710,9 @@ fn storage_size() {
 				4 + 4
 			);
 			assert_eq!(
-				bob_contract.total_pair_count,
+				bob_contract.pair_count,
 				2,
 			);
-			assert_eq!(
-				bob_contract.empty_pair_count,
-				0,
-			);
 
 			assert_ok!(Contracts::call(
 				Origin::signed(ALICE),
@@ -738,13 +730,9 @@ fn storage_size() {
 				4
 			);
 			assert_eq!(
-				bob_contract.total_pair_count,
+				bob_contract.pair_count,
 				1,
 			);
-			assert_eq!(
-				bob_contract.empty_pair_count,
-				0,
-			);
 		});
 }
 
@@ -776,11 +764,7 @@ fn empty_kv_pairs() {
 				0,
 			);
 			assert_eq!(
-				bob_contract.total_pair_count,
-				1,
-			);
-			assert_eq!(
-				bob_contract.empty_pair_count,
+				bob_contract.pair_count,
 				1,
 			);
 		});
@@ -828,9 +812,11 @@ fn deduct_blocks() {
 			);
 
 			// Check result
-			let rent = (8 + 4 - 3) // storage size = size_offset + deploy_set_storage - deposit_offset
-				* 4 // rent byte price
-				* 4; // blocks to rent
+			let rent = <Test as Config>::RentFraction::get()
+				// base_deposit + deploy_set_storage (4 bytes in 1 item) - free_balance
+				.mul_ceil(80_000 + 40_000 + 10_000 - 30_000)
+				// blocks to rent
+				* 4;
 			let bob_contract = ContractInfoOf::<Test>::get(&addr).unwrap().get_alive().unwrap();
 			assert_eq!(bob_contract.rent_allowance, 1_000 - rent);
 			assert_eq!(bob_contract.deduct_block, 5);
@@ -845,9 +831,11 @@ fn deduct_blocks() {
 			);
 
 			// Check result
-			let rent_2 = (8 + 4 - 2) // storage size = size_offset + deploy_set_storage - deposit_offset
-				* 4 // rent byte price
-				* 7; // blocks to rent
+			let rent_2 = <Test as Config>::RentFraction::get()
+				// base_deposit + deploy_set_storage (4 bytes in 1 item) - free_balance
+				.mul_ceil(80_000 + 40_000 + 10_000 - (30_000 - rent))
+				// blocks to rent
+				* 7;
 			let bob_contract = ContractInfoOf::<Test>::get(&addr).unwrap().get_alive().unwrap();
 			assert_eq!(bob_contract.rent_allowance, 1_000 - rent - rent_2);
 			assert_eq!(bob_contract.deduct_block, 12);