Adapt pallet-contracts to WeightV2 (#12421)

* Replace contract access weight by proper PoV component

* Return the whole weight struct from dry-runs

* Fixup `seal_call` and `seal_instantiate`

* Fix duplicate extrinsics

* Remove ContractAccessWeight from runtime

* Fix doc link

* Remove leftover debugging output

substrate/Cargo.lock

@@ -5531,6 +5531,7 @@ dependencies = [
  "parity-scale-codec",
  "sp-runtime",
  "sp-std",
+ "sp-weights",
 ]
 
 [[package]]

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

@@ -1175,7 +1175,6 @@ impl pallet_contracts::Config for Runtime {
 	type DeletionWeightLimit = DeletionWeightLimit;
 	type Schedule = Schedule;
 	type AddressGenerator = pallet_contracts::DefaultAddressGenerator;
-	type ContractAccessWeight = pallet_contracts::DefaultContractAccessWeight<RuntimeBlockWeights>;
 	type MaxCodeLen = ConstU32<{ 128 * 1024 }>;
 	type MaxStorageKeyLen = ConstU32<128>;
 }

substrate/frame/contracts/primitives/Cargo.toml

@@ -19,6 +19,7 @@ codec = { package = "parity-scale-codec", version = "3.0.0", default-features =
 # Substrate Dependencies (This crate should not rely on frame)
 sp-std = { version = "4.0.0", default-features = false, path = "../../../primitives/std" }
 sp-runtime = { version = "6.0.0", default-features = false, path = "../../../primitives/runtime" }
+sp-weights = { version = "4.0.0", default-features = false, path = "../../../primitives/weights" }
 
 [features]
 default = ["std"]

substrate/frame/contracts/primitives/src/lib.rs

@@ -26,17 +26,18 @@ use sp_runtime::{
 	DispatchError, RuntimeDebug,
 };
 use sp_std::prelude::*;
+use sp_weights::Weight;
 
 /// Result type of a `bare_call` or `bare_instantiate` call.
 ///
 /// It contains the execution result together with some auxiliary information.
 #[derive(Eq, PartialEq, Encode, Decode, RuntimeDebug)]
 pub struct ContractResult<R, Balance> {
-	/// How much gas was consumed during execution.
-	pub gas_consumed: u64,
-	/// How much gas is required as gas limit in order to execute this call.
+	/// How much weight was consumed during execution.
+	pub gas_consumed: Weight,
+	/// How much weight is required as gas limit in order to execute this call.
 	///
-	/// This value should be used to determine the gas limit for on-chain execution.
+	/// This value should be used to determine the weight limit for on-chain execution.
 	///
 	/// # Note
 	///
@@ -44,7 +45,7 @@ pub struct ContractResult<R, Balance> {
 	/// is used. Currently, only `seal_call_runtime` makes use of pre charging.
 	/// Additionally, any `seal_call` or `seal_instantiate` makes use of pre-charging
 	/// when a non-zero `gas_limit` argument is supplied.
-	pub gas_required: u64,
+	pub gas_required: Weight,
 	/// How much balance was deposited and reserved during execution in order to pay for storage.
 	///
 	/// The storage deposit is never actually charged from the caller in case of [`Self::result`]

substrate/frame/contracts/src/gas.rs

@@ -107,32 +107,45 @@ where
 	///
 	/// Passing `0` as amount is interpreted as "all remaining gas".
 	pub fn nested(&mut self, amount: Weight) -> Result<Self, DispatchError> {
-		let amount = if amount == Weight::zero() { self.gas_left } else { amount };
-
 		// NOTE that it is ok to allocate all available gas since it still ensured
 		// by `charge` that it doesn't reach zero.
-		if self.gas_left.any_lt(amount) {
-			Err(<Error<T>>::OutOfGas.into())
-		} else {
-			self.gas_left -= amount;
-			Ok(GasMeter::new(amount))
-		}
+		let amount = Weight::from_components(
+			if amount.ref_time().is_zero() {
+				self.gas_left().ref_time()
+			} else {
+				amount.ref_time()
+			},
+			if amount.proof_size().is_zero() {
+				self.gas_left().proof_size()
+			} else {
+				amount.proof_size()
+			},
+		);
+		self.gas_left = self.gas_left.checked_sub(&amount).ok_or_else(|| <Error<T>>::OutOfGas)?;
+		Ok(GasMeter::new(amount))
 	}
 
 	/// Absorb the remaining gas of a nested meter after we are done using it.
 	pub fn absorb_nested(&mut self, nested: Self) {
-		if self.gas_left == Weight::zero() {
+		if self.gas_left.ref_time().is_zero() {
 			// All of the remaining gas was inherited by the nested gas meter. When absorbing
 			// we can therefore safely inherit the lowest gas that the nested gas meter experienced
 			// as long as it is lower than the lowest gas that was experienced by the parent.
 			// We cannot call `self.gas_left_lowest()` here because in the state that this
 			// code is run the parent gas meter has `0` gas left.
-			self.gas_left_lowest = nested.gas_left_lowest().min(self.gas_left_lowest);
+			*self.gas_left_lowest.ref_time_mut() =
+				nested.gas_left_lowest().ref_time().min(self.gas_left_lowest.ref_time());
 		} else {
 			// The nested gas meter was created with a fixed amount that did not consume all of the
 			// parents (self) gas. The lowest gas that self will experience is when the nested
 			// gas was pre charged with the fixed amount.
-			self.gas_left_lowest = self.gas_left_lowest();
+			*self.gas_left_lowest.ref_time_mut() = self.gas_left_lowest().ref_time();
+		}
+		if self.gas_left.proof_size().is_zero() {
+			*self.gas_left_lowest.proof_size_mut() =
+				nested.gas_left_lowest().proof_size().min(self.gas_left_lowest.proof_size());
+		} else {
+			*self.gas_left_lowest.proof_size_mut() = self.gas_left_lowest().proof_size();
 		}
 		self.gas_left += nested.gas_left;
 	}
@@ -155,17 +168,11 @@ where
 				ErasedToken { description: format!("{:?}", token), token: Box::new(token) };
 			self.tokens.push(erased_tok);
 		}
-
 		let amount = token.weight();
-		let new_value = self.gas_left.checked_sub(&amount);
-
-		// We always consume the gas even if there is not enough gas.
-		self.gas_left = new_value.unwrap_or_else(Zero::zero);
-
-		match new_value {
-			Some(_) => Ok(ChargedAmount(amount)),
-			None => Err(Error::<T>::OutOfGas.into()),
-		}
+		// It is OK to not charge anything on failure because we always charge _before_ we perform
+		// any action
+		self.gas_left = self.gas_left.checked_sub(&amount).ok_or_else(|| Error::<T>::OutOfGas)?;
+		Ok(ChargedAmount(amount))
 	}
 
 	/// Adjust a previously charged amount down to its actual amount.
@@ -298,20 +305,16 @@ mod tests {
 		assert!(gas_meter.charge(SimpleToken(1)).is_err());
 	}
 
-	// Make sure that if the gas meter is charged by exceeding amount then not only an error
-	// returned for that charge, but also for all consequent charges.
-	//
-	// This is not strictly necessary, because the execution should be interrupted immediately
-	// if the gas meter runs out of gas. However, this is just a nice property to have.
+	// Make sure that the gas meter does not charge in case of overcharger
 	#[test]
-	fn overcharge_is_unrecoverable() {
+	fn overcharge_does_not_charge() {
 		let mut gas_meter = GasMeter::<Test>::new(Weight::from_ref_time(200));
 
 		// The first charge is should lead to OOG.
 		assert!(gas_meter.charge(SimpleToken(300)).is_err());
 
-		// The gas meter is emptied at this moment, so this should also fail.
-		assert!(gas_meter.charge(SimpleToken(1)).is_err());
+		// The gas meter should still contain the full 200.
+		assert!(gas_meter.charge(SimpleToken(200)).is_ok());
 	}
 
 	// Charging the exact amount that the user paid for should be

substrate/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
@@ -413,23 +369,8 @@ pub mod pallet {
 		T::AccountId: AsRef<[u8]>,
 		<BalanceOf<T> as HasCompact>::Type: Clone + Eq + PartialEq + Debug + TypeInfo + Encode,
 	{
-		/// Makes a call to an account, optionally transferring some balance.
-		///
-		/// # Parameters
-		///
-		/// * `dest`: Address of the contract to call.
-		/// * `value`: The balance to transfer from the `origin` to `dest`.
-		/// * `gas_limit`: The gas limit enforced when executing the constructor.
-		/// * `storage_deposit_limit`: The maximum amount of balance that can be charged from the
-		///   caller to pay for the storage consumed.
-		/// * `data`: The input data to pass to the contract.
-		///
-		/// * If the account is a smart-contract account, the associated code will be
-		/// executed and any value will be transferred.
-		/// * If the account is a regular account, any value will be transferred.
-		/// * If no account exists and the call value is not less than `existential_deposit`,
-		/// a regular account will be created and any value will be transferred.
-		#[pallet::weight(T::WeightInfo::call().saturating_add((*gas_limit).into()))]
+		/// Deprecated version if [`Self::call`] for use in an in-storage `Call`.
+		#[pallet::weight(T::WeightInfo::call().saturating_add(<Pallet<T>>::compat_weight(*gas_limit)))]
 		#[allow(deprecated)]
 		#[deprecated(note = "1D weight is used in this extrinsic, please migrate to `call`")]
 		pub fn call_old_weight(
@@ -440,55 +381,20 @@ pub mod pallet {
 			storage_deposit_limit: Option<<BalanceOf<T> as codec::HasCompact>::Type>,
 			data: Vec<u8>,
 		) -> DispatchResultWithPostInfo {
-			let gas_limit: Weight = gas_limit.into();
-			let origin = ensure_signed(origin)?;
-			let dest = T::Lookup::lookup(dest)?;
-			let mut output = Self::internal_call(
+			Self::call(
 				origin,
 				dest,
 				value,
-				gas_limit,
-				storage_deposit_limit.map(Into::into),
+				<Pallet<T>>::compat_weight(gas_limit),
+				storage_deposit_limit,
 				data,
-				None,
-			);
-			if let Ok(retval) = &output.result {
-				if retval.did_revert() {
-					output.result = Err(<Error<T>>::ContractReverted.into());
-				}
-			}
-			output.gas_meter.into_dispatch_result(output.result, T::WeightInfo::call())
+			)
 		}
 
-		/// Instantiates a new contract from the supplied `code` optionally transferring
-		/// some balance.
-		///
-		/// This dispatchable has the same effect as calling [`Self::upload_code`] +
-		/// [`Self::instantiate`]. Bundling them together provides efficiency gains. Please
-		/// also check the documentation of [`Self::upload_code`].
-		///
-		/// # Parameters
-		///
-		/// * `value`: The balance to transfer from the `origin` to the newly created contract.
-		/// * `gas_limit`: The gas limit enforced when executing the constructor.
-		/// * `storage_deposit_limit`: The maximum amount of balance that can be charged/reserved
-		///   from the caller to pay for the storage consumed.
-		/// * `code`: The contract code to deploy in raw bytes.
-		/// * `data`: The input data to pass to the contract constructor.
-		/// * `salt`: Used for the address derivation. See [`Pallet::contract_address`].
-		///
-		/// Instantiation is executed as follows:
-		///
-		/// - The supplied `code` is instrumented, deployed, and a `code_hash` is created for that
-		///   code.
-		/// - If the `code_hash` already exists on the chain the underlying `code` will be shared.
-		/// - The destination address is computed based on the sender, code_hash and the salt.
-		/// - The smart-contract account is created at the computed address.
-		/// - The `value` is transferred to the new account.
-		/// - The `deploy` function is executed in the context of the newly-created account.
+		/// Deprecated version if [`Self::instantiate_with_code`] for use in an in-storage `Call`.
 		#[pallet::weight(
 			T::WeightInfo::instantiate_with_code(code.len() as u32, salt.len() as u32)
-			.saturating_add((*gas_limit).into())
+			.saturating_add(<Pallet<T>>::compat_weight(*gas_limit))
 		)]
 		#[allow(deprecated)]
 		#[deprecated(
@@ -503,38 +409,20 @@ pub mod pallet {
 			data: Vec<u8>,
 			salt: Vec<u8>,
 		) -> DispatchResultWithPostInfo {
-			let gas_limit: Weight = gas_limit.into();
-			let origin = ensure_signed(origin)?;
-			let code_len = code.len() as u32;
-			let salt_len = salt.len() as u32;
-			let mut output = Self::internal_instantiate(
+			Self::instantiate_with_code(
 				origin,
 				value,
-				gas_limit,
-				storage_deposit_limit.map(Into::into),
-				Code::Upload(code),
+				<Pallet<T>>::compat_weight(gas_limit),
+				storage_deposit_limit,
+				code,
 				data,
 				salt,
-				None,
-			);
-			if let Ok(retval) = &output.result {
-				if retval.1.did_revert() {
-					output.result = Err(<Error<T>>::ContractReverted.into());
-				}
-			}
-			output.gas_meter.into_dispatch_result(
-				output.result.map(|(_address, result)| result),
-				T::WeightInfo::instantiate_with_code(code_len, salt_len),
 			)
 		}
 
-		/// Instantiates a contract from a previously deployed wasm binary.
-		///
-		/// This function is identical to [`Self::instantiate_with_code`] but without the
-		/// code deployment step. Instead, the `code_hash` of an on-chain deployed wasm binary
-		/// must be supplied.
+		/// Deprecated version if [`Self::instantiate`] for use in an in-storage `Call`.
 		#[pallet::weight(
-			T::WeightInfo::instantiate(salt.len() as u32).saturating_add((*gas_limit).into())
+			T::WeightInfo::instantiate(salt.len() as u32).saturating_add(<Pallet<T>>::compat_weight(*gas_limit))
 		)]
 		#[allow(deprecated)]
 		#[deprecated(note = "1D weight is used in this extrinsic, please migrate to `instantiate`")]
@@ -547,27 +435,14 @@ pub mod pallet {
 			data: Vec<u8>,
 			salt: Vec<u8>,
 		) -> DispatchResultWithPostInfo {
-			let gas_limit: Weight = gas_limit.into();
-			let origin = ensure_signed(origin)?;
-			let salt_len = salt.len() as u32;
-			let mut output = Self::internal_instantiate(
+			Self::instantiate(
 				origin,
 				value,
-				gas_limit,
-				storage_deposit_limit.map(Into::into),
-				Code::Existing(code_hash),
+				<Pallet<T>>::compat_weight(gas_limit),
+				storage_deposit_limit,
+				code_hash,
 				data,
 				salt,
-				None,
-			);
-			if let Ok(retval) = &output.result {
-				if retval.1.did_revert() {
-					output.result = Err(<Error<T>>::ContractReverted.into());
-				}
-			}
-			output.gas_meter.into_dispatch_result(
-				output.result.map(|(_address, output)| output),
-				T::WeightInfo::instantiate(salt_len),
 			)
 		}
 
@@ -1059,8 +934,8 @@ where
 		);
 		ContractExecResult {
 			result: output.result.map_err(|r| r.error),
-			gas_consumed: output.gas_meter.gas_consumed().ref_time(),
-			gas_required: output.gas_meter.gas_required().ref_time(),
+			gas_consumed: output.gas_meter.gas_consumed(),
+			gas_required: output.gas_meter.gas_required(),
 			storage_deposit: output.storage_deposit,
 			debug_message: debug_message.unwrap_or_default(),
 		}
@@ -1104,8 +979,8 @@ where
 				.result
 				.map(|(account_id, result)| InstantiateReturnValue { result, account_id })
 				.map_err(|e| e.error),
-			gas_consumed: output.gas_meter.gas_consumed().ref_time(),
-			gas_required: output.gas_meter.gas_required().ref_time(),
+			gas_consumed: output.gas_meter.gas_consumed(),
+			gas_required: output.gas_meter.gas_required(),
 			storage_deposit: output.storage_deposit,
 			debug_message: debug_message.unwrap_or_default(),
 		}
@@ -1287,4 +1162,12 @@ where
 	fn min_balance() -> BalanceOf<T> {
 		<T::Currency as Inspect<AccountIdOf<T>>>::minimum_balance()
 	}
+
+	/// Convert a 1D Weight to a 2D weight.
+	///
+	/// Used by backwards compatible extrinsics. We cannot just set the proof to zero
+	/// or an old `Call` will just fail.
+	fn compat_weight(gas_limit: OldWeight) -> Weight {
+		Weight::from(gas_limit).set_proof_size(u64::from(T::MaxCodeLen::get()) * 2)
+	}
 }

substrate/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![],
 			),
@@ -1760,7 +1759,7 @@ fn chain_extension_works() {
 			false,
 		);
 		assert_ok!(result.result);
-		assert_eq!(result.gas_consumed, gas_consumed + 42);
+		assert_eq!(result.gas_consumed.ref_time(), gas_consumed.ref_time() + 42);
 		let result = Contracts::bare_call(
 			ALICE,
 			addr.clone(),
@@ -1771,7 +1770,7 @@ fn chain_extension_works() {
 			false,
 		);
 		assert_ok!(result.result);
-		assert_eq!(result.gas_consumed, gas_consumed + 95);
+		assert_eq!(result.gas_consumed.ref_time(), gas_consumed.ref_time() + 95);
 
 		// 3 = diverging chain extension call that sets flags to 0x1 and returns a fixed buffer
 		let result = Contracts::bare_call(
@@ -2409,10 +2408,11 @@ fn reinstrument_does_charge() {
 		let result2 =
 			Contracts::bare_call(ALICE, addr.clone(), 0, GAS_LIMIT, None, zero.clone(), false);
 		assert!(!result2.result.unwrap().did_revert());
-		assert!(result2.gas_consumed > result1.gas_consumed);
+		assert!(result2.gas_consumed.ref_time() > result1.gas_consumed.ref_time());
 		assert_eq!(
-			result2.gas_consumed,
-			result1.gas_consumed + <Test as Config>::WeightInfo::reinstrument(code_len).ref_time(),
+			result2.gas_consumed.ref_time(),
+			result1.gas_consumed.ref_time() +
+				<Test as Config>::WeightInfo::reinstrument(code_len).ref_time(),
 		);
 	});
 }
@@ -2536,7 +2536,7 @@ fn gas_estimation_nested_call_fixed_limit() {
 		assert_ok!(&result.result);
 
 		// We have a subcall with a fixed gas limit. This constitutes precharging.
-		assert!(result.gas_required > result.gas_consumed);
+		assert!(result.gas_required.ref_time() > result.gas_consumed.ref_time());
 
 		// Make the same call using the estimated gas. Should succeed.
 		assert_ok!(
@@ -2544,7 +2544,7 @@ fn gas_estimation_nested_call_fixed_limit() {
 				ALICE,
 				addr_caller,
 				0,
-				Weight::from_ref_time(result.gas_required).set_proof_size(u64::MAX),
+				result.gas_required,
 				Some(result.storage_deposit.charge_or_zero()),
 				input,
 				false,
@@ -2557,6 +2557,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 +2592,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,9 +2605,10 @@ fn gas_estimation_call_runtime() {
 			call.encode(),
 			false,
 		);
-		assert_ok!(&result.result);
-
-		assert!(result.gas_required > result.gas_consumed);
+		// 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.ref_time() > result.gas_consumed.ref_time());
 
 		// Make the same call using the required gas. Should succeed.
 		assert_ok!(
@@ -2614,7 +2616,7 @@ fn gas_estimation_call_runtime() {
 				ALICE,
 				addr_caller,
 				0,
-				Weight::from_ref_time(result.gas_required).set_proof_size(u64::MAX),
+				result.gas_required,
 				None,
 				call.encode(),
 				false,

substrate/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()),
+		}
 	}
 }