From 762f4b0242bb9d14fa0df5b6a1a3a64b005268e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Wed, 6 Jan 2021 16:47:22 +0100 Subject: [PATCH] contracts: Add configurable per-storage item cost (#7819) * Rework rent parameters * No need for empty_pair_count any longer * Parameterize runtime --- bin/node/runtime/src/lib.rs | 19 ++++--- frame/contracts/src/benchmarking/mod.rs | 7 +-- frame/contracts/src/lib.rs | 70 ++++++++++++++++--------- frame/contracts/src/rent.rs | 27 ++++------ frame/contracts/src/storage.rs | 30 +++-------- frame/contracts/src/tests.rs | 62 +++++++++------------- 6 files changed, 103 insertions(+), 112 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index fa07cf1bd8e9e..3e6452465831f 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -709,13 +709,17 @@ impl pallet_tips::Config for Runtime { } parameter_types! { - pub const TombstoneDeposit: Balance = 16 * MILLICENTS; - pub const RentByteFee: Balance = 4 * MILLICENTS; - pub const RentDepositOffset: Balance = 1000 * MILLICENTS; + pub const TombstoneDeposit: Balance = deposit( + 1, + sp_std::mem::size_of::>() as u32 + ); + pub const DepositPerContract: Balance = TombstoneDeposit::get(); + pub const DepositPerStorageByte: Balance = deposit(0, 1); + pub const DepositPerStorageItem: Balance = deposit(1, 0); + pub RentFraction: Perbill = Perbill::from_rational_approximation(1u32, 30 * DAYS); pub const SurchargeReward: Balance = 150 * MILLICENTS; pub const SignedClaimHandicap: u32 = 2; pub const MaxDepth: u32 = 32; - pub const StorageSizeOffset: u32 = 8; pub const MaxValueSize: u32 = 16 * 1024; // The lazy deletion runs inside on_initialize. pub DeletionWeightLimit: Weight = AVERAGE_ON_INITIALIZE_RATIO * @@ -736,9 +740,10 @@ impl pallet_contracts::Config for Runtime { 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; diff --git a/frame/contracts/src/benchmarking/mod.rs b/frame/contracts/src/benchmarking/mod.rs index 9fa365116c7a0..393b0f60875bc 100644 --- a/frame/contracts/src/benchmarking/mod.rs +++ b/frame/contracts/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::::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) diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 7b919fe2172e7..2e9b934e4dc1c 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/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 { /// /// 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>; - /// 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; - - /// Price of a byte of storage per one block interval. Should be greater than 0. - type RentByteFee: Get>; + type 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. - type RentDepositOffset: Get>; + type 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. + type 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. + type RentFraction: Get; /// 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::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::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::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::RentDepositOffset::get(); + const DepositPerStorageByte: BalanceOf = 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::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. diff --git a/frame/contracts/src/rent.rs b/frame/contracts/src/rent.rs index f30b25b447a25..c67776c9e1098 100644 --- a/frame/contracts/src/rent.rs +++ b/frame/contracts/src/rent.rs @@ -101,21 +101,15 @@ where free_balance: &BalanceOf, contract: &AliveContractInfo ) -> BalanceOf { - 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 = >::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(|| >::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 >::insert(&dest, ContractInfo::Alive(AliveContractInfo:: { 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, diff --git a/frame/contracts/src/storage.rs b/frame/contracts/src/storage.rs index 520a114986f46..282c1acc0709a 100644 --- a/frame/contracts/src/storage.rs +++ b/frame/contracts/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: >::block_number(), rent_allowance: >::max_value(), - empty_pair_count: 0, - total_pair_count: 0, + pair_count: 0, last_write: None, } .into(), @@ -217,7 +203,7 @@ where Err(Error::::DeletionQueueFull.into()) } else { DeletionQueue::append(DeletedContract { - pair_count: contract.total_pair_count, + pair_count: contract.pair_count, trie_id: contract.trie_id.clone(), }); Ok(()) diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index d1a9521924f75..9021e9677d76c 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/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 = ::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::::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 = ::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::::get(&addr).unwrap().get_alive().unwrap(); assert_eq!(bob_contract.rent_allowance, 1_000 - rent - rent_2); assert_eq!(bob_contract.deduct_block, 12);