Replace contract access weight by proper PoV component

frame/contracts/src/lib.rs

@@ -107,7 +107,7 @@ use crate::{
 };
 use codec::{Encode, HasCompact};
 use frame_support::{
-	dispatch::{DispatchClass, Dispatchable, GetDispatchInfo, Pays, PostDispatchInfo},
+	dispatch::{Dispatchable, GetDispatchInfo, Pays, PostDispatchInfo},
 	ensure,
 	traits::{
 		tokens::fungible::Inspect, ConstU32, Contains, Currency, Get, Randomness,
@@ -116,7 +116,7 @@ use frame_support::{
 	weights::{OldWeight, Weight},
 	BoundedVec, WeakBoundedVec,
 };
-use frame_system::{limits::BlockWeights, Pallet as System};
+use frame_system::Pallet as System;
 use pallet_contracts_primitives::{
 	Code, CodeUploadResult, CodeUploadReturnValue, ContractAccessError, ContractExecResult,
 	ContractInstantiateResult, ExecReturnValue, GetStorageResult, InstantiateReturnValue,
@@ -199,29 +199,6 @@ 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<B: Get<BlockWeights>, const P: u32 = 5_242_880>(
-	PhantomData<B>,
-);
-
-impl<B: Get<BlockWeights>, const P: u32> Get<Weight> for DefaultContractAccessWeight<B, P> {
-	fn get() -> Weight {
-		let block_weights = B::get();
-		block_weights
-			.per_class
-			.get(DispatchClass::Normal)
-			.max_total
-			.unwrap_or(block_weights.max_block) /
-			u64::from(P)
-	}
-}
-
 #[frame_support::pallet]
 pub mod pallet {
 	use super::*;
@@ -334,27 +311,6 @@ pub mod pallet {
 		#[pallet::constant]
 		type DepositPerByte: Get<BalanceOf<Self>>;
 
-		/// The weight per byte of code that is charged when loading a contract from storage.
-		///
-		/// 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
-		/// 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: <https://github.com/paritytech/substrate/issues/10301>
-		///
-		/// [`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<Weight>;
-
 		/// The amount of balance a caller has to pay for each storage item.
 		///
 		/// # Note

frame/contracts/src/tests.rs

@@ -26,8 +26,8 @@ use crate::{
 	tests::test_utils::{get_contract, get_contract_checked},
 	wasm::{PrefabWasmModule, ReturnCode as RuntimeReturnCode},
 	weights::WeightInfo,
-	BalanceOf, Code, CodeStorage, Config, ContractInfoOf, DefaultAddressGenerator,
-	DefaultContractAccessWeight, DeletionQueue, Error, Pallet, Schedule,
+	BalanceOf, Code, CodeStorage, Config, ContractInfoOf, DefaultAddressGenerator, DeletionQueue,
+	Error, Pallet, Schedule,
 };
 use assert_matches::assert_matches;
 use codec::Encode;
@@ -404,7 +404,6 @@ impl Config for Test {
 	type DepositPerByte = DepositPerByte;
 	type DepositPerItem = DepositPerItem;
 	type AddressGenerator = DefaultAddressGenerator;
-	type ContractAccessWeight = DefaultContractAccessWeight<BlockWeights>;
 	type MaxCodeLen = ConstU32<{ 128 * 1024 }>;
 	type MaxStorageKeyLen = ConstU32<128>;
 }
@@ -414,7 +413,7 @@ pub const BOB: AccountId32 = AccountId32::new([2u8; 32]);
 pub const CHARLIE: AccountId32 = AccountId32::new([3u8; 32]);
 pub const DJANGO: AccountId32 = AccountId32::new([4u8; 32]);
 
-pub const GAS_LIMIT: Weight = Weight::from_ref_time(100_000_000_000).set_proof_size(u64::MAX);
+pub const GAS_LIMIT: Weight = Weight::from_ref_time(100_000_000_000).set_proof_size(256 * 1024);
 
 pub struct ExtBuilder {
 	existential_deposit: u64,
@@ -674,7 +673,7 @@ fn run_out_of_gas() {
 				RuntimeOrigin::signed(ALICE),
 				addr, // newly created account
 				0,
-				Weight::from_ref_time(1_000_000_000_000),
+				Weight::from_ref_time(1_000_000_000_000).set_proof_size(u64::MAX),
 				None,
 				vec![],
 			),
@@ -2557,6 +2556,7 @@ fn gas_estimation_nested_call_fixed_limit() {
 #[test]
 #[cfg(feature = "unstable-interface")]
 fn gas_estimation_call_runtime() {
+	use codec::Decode;
 	let (caller_code, caller_hash) = compile_module::<Test>("call_runtime").unwrap();
 	let (callee_code, callee_hash) = compile_module::<Test>("dummy").unwrap();
 	ExtBuilder::default().existential_deposit(50).build().execute_with(|| {
@@ -2591,7 +2591,7 @@ fn gas_estimation_call_runtime() {
 		let call = RuntimeCall::Contracts(crate::Call::call {
 			dest: addr_callee,
 			value: 0,
-			gas_limit: GAS_LIMIT / 3,
+			gas_limit: GAS_LIMIT.set_ref_time(GAS_LIMIT.ref_time() / 3),
 			storage_deposit_limit: None,
 			data: vec![],
 		});
@@ -2604,8 +2604,9 @@ fn gas_estimation_call_runtime() {
 			call.encode(),
 			false,
 		);
-		assert_ok!(&result.result);
-
+		// contract encodes the result of the dispatch runtime
+		let outcome = u32::decode(&mut result.result.unwrap().data.as_ref()).unwrap();
+		assert_eq!(outcome, 0);
 		assert!(result.gas_required > result.gas_consumed);
 
 		// Make the same call using the required gas. Should succeed.

frame/contracts/src/wasm/code_cache.rs

@@ -228,16 +228,11 @@ impl<T: Config> Token<T> for CodeToken {
 		// contract code. This is why we subtract `T::*::(0)`. We need to do this at this
 		// point because when charging the general weight for calling the contract we not know the
 		// size of the contract.
-		let ref_time_weight = match *self {
+		match *self {
 			Reinstrument(len) => T::WeightInfo::reinstrument(len),
-			Load(len) => {
-				let computation = T::WeightInfo::call_with_code_per_byte(len)
-					.saturating_sub(T::WeightInfo::call_with_code_per_byte(0));
-				let bandwidth = T::ContractAccessWeight::get().saturating_mul(len as u64);
-				computation.max(bandwidth)
-			},
-		};
-
-		ref_time_weight
+			Load(len) => T::WeightInfo::call_with_code_per_byte(len)
+				.saturating_sub(T::WeightInfo::call_with_code_per_byte(0))
+				.set_proof_size(len.into()),
+		}
 	}
 }