From 9d679322227e8a7442f0353c08433f365ec0b3dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Fri, 6 May 2022 08:00:46 +0200 Subject: [PATCH 1/2] Add ContractAccessWeight --- bin/node/runtime/src/lib.rs | 8 +++-- frame/contracts/src/lib.rs | 50 ++++++++++++++++++++++++-- frame/contracts/src/tests.rs | 5 +-- frame/contracts/src/wasm/code_cache.rs | 10 ++++-- 4 files changed, 63 insertions(+), 10 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 660ba7ab86229..b6e01e688976b 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -1059,8 +1059,11 @@ parameter_types! { pub const DepositPerByte: Balance = deposit(0, 1); pub const MaxValueSize: u32 = 16 * 1024; // The lazy deletion runs inside on_initialize. - pub DeletionWeightLimit: Weight = AVERAGE_ON_INITIALIZE_RATIO * - RuntimeBlockWeights::get().max_block; + pub DeletionWeightLimit: Weight = RuntimeBlockWeights::get() + .per_class + .get(DispatchClass::Normal) + .max_total + .unwrap_or(RuntimeBlockWeights::get().max_block); // The weight needed for decoding the queue should be less or equal than a fifth // of the overall weight dedicated to the lazy deletion. pub DeletionQueueDepth: u32 = ((DeletionWeightLimit::get() / ( @@ -1093,6 +1096,7 @@ impl pallet_contracts::Config for Runtime { type DeletionWeightLimit = DeletionWeightLimit; type Schedule = Schedule; type AddressGenerator = pallet_contracts::DefaultAddressGenerator; + type ContractAccessWeight = pallet_contracts::DefaultContractAccessWeight; } impl pallet_sudo::Config for Runtime { diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 4edf43a672152..b5d283e2f0419 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -115,9 +115,9 @@ use frame_support::{ dispatch::Dispatchable, ensure, traits::{Contains, Currency, Get, Randomness, ReservableCurrency, StorageVersion, Time}, - weights::{GetDispatchInfo, Pays, PostDispatchInfo, Weight}, + weights::{DispatchClass, GetDispatchInfo, Pays, PostDispatchInfo, Weight}, }; -use frame_system::Pallet as System; +use frame_system::{limits::BlockWeights, Pallet as System}; use pallet_contracts_primitives::{ Code, CodeUploadResult, CodeUploadReturnValue, ContractAccessError, ContractExecResult, ContractInstantiateResult, ExecReturnValue, GetStorageResult, InstantiateReturnValue, @@ -126,7 +126,7 @@ use pallet_contracts_primitives::{ use scale_info::TypeInfo; use sp_core::{crypto::UncheckedFrom, Bytes}; use sp_runtime::traits::{Convert, Hash, Saturating, StaticLookup}; -use sp_std::{fmt::Debug, prelude::*}; +use sp_std::{fmt::Debug, marker::PhantomData, prelude::*}; type CodeHash = ::Hash; type TrieId = Vec; @@ -193,6 +193,29 @@ where } } +/// A conservative implementation to be used for [`pallet::Config::ContractAccessWeight`]. +/// +/// This derives the weight from the [`BlockWeights`] passed as `B` and the `maxPovSize` passed +/// as `P`. The default value for `P` is the `maxPovSize` used by polkadot and kusama. +/// +/// It simply charges from the weight meter pro rata: If loading the contract code would consume +/// 50% of the max storage proof then this charges 50% of the max block weight. +pub struct DefaultContractAccessWeight, const P: u32 = 5_242_880>( + PhantomData, +); + +impl, const P: u32> Get for DefaultContractAccessWeight { + fn get() -> Weight { + let block_weights = B::get(); + block_weights + .per_class + .get(DispatchClass::Normal) + .max_total + .unwrap_or(block_weights.max_block) / + Weight::from(P) + } +} + #[frame_support::pallet] pub mod pallet { use super::*; @@ -297,6 +320,27 @@ pub mod pallet { #[pallet::constant] type DepositPerByte: Get>; + /// The weight per byte of code that is charged when loading a contract from storage. + /// + /// Currently, FRAME only charged fees for computation incurred but not for PoV + /// consumption caused for storage access. This is usually not exploitable because + /// accessing storage carries some substantial weight costs, too. However in case + /// of contract code very much PoV consumption can be caused while consuming very little + /// computation. This could be used to keep the chain busy without paying the + /// proper fee for it. Until this is resolved we charge from the weight meter for + /// contract access. + /// + /// For more information check out: + /// + /// [`DefaultContractAccessWeight`] is a safe default to be used for polkadot or kusama + /// parachains. + /// + /// # Note + /// + /// This is only relevant for parachains. Set to zero in case of a standalone chain. + #[pallet::constant] + type ContractAccessWeight: Get; + /// The amount of balance a caller has to pay for each storage item. /// /// # Note diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index eaec4df698e5a..407e1e999dde1 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -24,8 +24,8 @@ use crate::{ storage::Storage, wasm::{PrefabWasmModule, ReturnCode as RuntimeReturnCode}, weights::WeightInfo, - BalanceOf, Code, CodeStorage, Config, ContractInfoOf, DefaultAddressGenerator, Error, Pallet, - Schedule, + BalanceOf, Code, CodeStorage, Config, ContractInfoOf, DefaultAddressGenerator, + DefaultContractAccessWeight, Error, Pallet, Schedule, }; use assert_matches::assert_matches; use codec::Encode; @@ -287,6 +287,7 @@ impl Config for Test { type DepositPerByte = DepositPerByte; type DepositPerItem = DepositPerItem; type AddressGenerator = DefaultAddressGenerator; + type ContractAccessWeight = DefaultContractAccessWeight; } pub const ALICE: AccountId32 = AccountId32::new([1u8; 32]); diff --git a/frame/contracts/src/wasm/code_cache.rs b/frame/contracts/src/wasm/code_cache.rs index ca91fa3131474..b194c283e90cd 100644 --- a/frame/contracts/src/wasm/code_cache.rs +++ b/frame/contracts/src/wasm/code_cache.rs @@ -38,7 +38,7 @@ use crate::{ use frame_support::{ dispatch::{DispatchError, DispatchResult}, ensure, - traits::ReservableCurrency, + traits::{Get, ReservableCurrency}, }; use sp_core::crypto::UncheckedFrom; use sp_runtime::traits::BadOrigin; @@ -216,8 +216,12 @@ impl Token for CodeToken { // size of the contract. match *self { Reinstrument(len) => T::WeightInfo::reinstrument(len), - Load(len) => T::WeightInfo::call_with_code_per_byte(len) - .saturating_sub(T::WeightInfo::call_with_code_per_byte(0)), + Load(len) => { + let computation = T::WeightInfo::call_with_code_per_byte(len) + .saturating_sub(T::WeightInfo::call_with_code_per_byte(0)); + let bandwith = T::ContractAccessWeight::get().saturating_mul(len.into()); + computation.max(bandwith) + }, } } } From e2cf8077c2ee8d983b2701e5858a546405092bad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Fri, 6 May 2022 17:28:16 +0200 Subject: [PATCH 2/2] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Michael Müller --- frame/contracts/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index b5d283e2f0419..0ae326f6c1e8f 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -196,7 +196,7 @@ where /// A conservative implementation to be used for [`pallet::Config::ContractAccessWeight`]. /// /// This derives the weight from the [`BlockWeights`] passed as `B` and the `maxPovSize` passed -/// as `P`. The default value for `P` is the `maxPovSize` used by polkadot and kusama. +/// as `P`. The default value for `P` is the `maxPovSize` used by Polkadot and Kusama. /// /// It simply charges from the weight meter pro rata: If loading the contract code would consume /// 50% of the max storage proof then this charges 50% of the max block weight. @@ -322,7 +322,7 @@ pub mod pallet { /// The weight per byte of code that is charged when loading a contract from storage. /// - /// Currently, FRAME only charged fees for computation incurred but not for PoV + /// Currently, FRAME only charges fees for computation incurred but not for PoV /// consumption caused for storage access. This is usually not exploitable because /// accessing storage carries some substantial weight costs, too. However in case /// of contract code very much PoV consumption can be caused while consuming very little