Return unified gas for gas_left syscalls and opcodes

prdoc/pr_9968.prdoc

@@ -0,0 +1,22 @@
+title: Return unified gas for `gas_left` syscalls and opcodes
+doc:
+- audience: Runtime Dev
+  description: |-
+    In https://github.com/paritytech/polkadot-sdk/pull/9803 we introduced the new gas mapping. However, when contracts are querying the remaining gas we still returned the `ref_time`. This PR changes that.
+
+    ## Changes
+    - Added a new `Stack::gas_left` function that calculates the remaining gas as eth gas that matches the gas passed in the transaction. It supports both the `eth_` and non `eth_` flavors of dispatchables.
+    - Changed the PVM syscall `ref_time_left` to return the new unified gas.
+    - Changes the EVM `GAS` opcode to return the new unified gas
+    - When calculating the consumed storage we now take into account what was charged during the current frame
+    - Removed `storage_deposit_limit` from `eth_*` dispatchables. It is always uncapped in this case and the overall limit is conveyed using the tx credit.
+
+    ## Follow ups
+    Now that we can return the proper remaining gas that also includes the storage deposit we can change the EVM `call` instruction next to take the passed `gas` into account. Since the unified gas takes both the txfee and the deposit into account it will be able to limit both effectively.
+crates:
+- name: pallet-revive-fixtures
+  bump: major
+- name: pallet-revive
+  bump: major
+- name: pallet-revive-uapi
+  bump: major

substrate/frame/revive/fixtures/contracts/create_storage_and_instantiate.rs

@@ -64,6 +64,10 @@ pub extern "C" fn call() {
 		api::return_value(uapi::ReturnFlags::REVERT, &(code as u32).to_le_bytes());
 	};
 
+	// fail in the caller
+	key[1] = 1;
+	api::set_storage(StorageFlags::empty(), &key, data);
+
 	// Return the deployed contract address.
 	api::return_value(uapi::ReturnFlags::empty(), &address);
 }

substrate/frame/revive/fixtures/contracts/ref_time_left.rs → substrate/frame/revive/fixtures/contracts/gas_left.rs

@@ -23,12 +23,10 @@ use uapi::{HostFn, HostFnImpl as api, ReturnFlags};
 
 #[no_mangle]
 #[polkavm_derive::polkavm_export]
-pub extern "C" fn deploy() {
-	assert!(api::ref_time_left() > api::ref_time_left());
-}
+pub extern "C" fn deploy() {}
 
 #[no_mangle]
 #[polkavm_derive::polkavm_export]
 pub extern "C" fn call() {
-	api::return_value(ReturnFlags::empty(), &api::ref_time_left().to_le_bytes());
+	api::return_value(ReturnFlags::empty(), &api::gas_left().to_le_bytes());
 }

substrate/frame/revive/src/benchmarking.rs

@@ -297,7 +297,6 @@ mod benchmarks {
 		let nonce = System::<T>::account_nonce(&caller).try_into().unwrap_or_default();
 		let addr = crate::address::create1(&deployer, nonce);
 		let account_id = T::AddressMapper::to_fallback_account_id(&addr);
-		let storage_deposit = default_deposit_limit::<T>();
 
 		assert!(AccountInfoOf::<T>::get(&deployer).is_none());
 
@@ -306,7 +305,7 @@ mod benchmarks {
 		);
 
 		#[extrinsic_call]
-		_(origin, evm_value, Weight::MAX, storage_deposit, code, input, 0u32.into(), 0);
+		_(origin, evm_value, Weight::MAX, code, input, 0u32.into(), 0);
 
 		let deposit =
 			T::Currency::balance_on_hold(&HoldReason::StorageDepositReserve.into(), &account_id);
@@ -444,9 +443,10 @@ mod benchmarks {
 		let caller_addr = T::AddressMapper::to_address(&instance.caller);
 		let origin = Origin::EthTransaction(instance.caller.clone());
 		let before = Pallet::<T>::evm_balance(&instance.address);
-		let storage_deposit = default_deposit_limit::<T>();
+
 		#[extrinsic_call]
-		_(origin, instance.address, evm_value, Weight::MAX, storage_deposit, data, 0u32.into(), 0);
+		_(origin, instance.address, evm_value, Weight::MAX, data, 0u32.into(), 0);
+
 		let deposit = T::Currency::balance_on_hold(
 			&HoldReason::StorageDepositReserve.into(),
 			&instance.account_id,
@@ -807,7 +807,7 @@ mod benchmarks {
 		{
 			result = runtime.bench_ref_time_left(memory.as_mut_slice());
 		}
-		assert_eq!(result.unwrap(), runtime.ext().gas_meter().gas_left().ref_time());
+		assert_eq!(result.unwrap(), runtime.ext().gas_left());
 	}
 
 	#[benchmark(pov_mode = Measured)]

substrate/frame/revive/src/evm/call.rs

@@ -24,11 +24,8 @@ use crate::{
 };
 use codec::DecodeLimit;
 use frame_support::MAX_EXTRINSIC_DEPTH;
-use num_traits::Bounded;
 use sp_core::Get;
-use sp_runtime::{
-	transaction_validity::InvalidTransaction, FixedPointNumber, SaturatedConversion, Saturating,
-};
+use sp_runtime::{transaction_validity::InvalidTransaction, FixedPointNumber, SaturatedConversion};
 
 /// Result of decoding an eth transaction into a dispatchable call.
 pub struct CallInfo<T: Config> {
@@ -117,7 +114,6 @@ where
 				dest,
 				value,
 				gas_limit: Zero::zero(),
-				storage_deposit_limit: BalanceOf::<T>::max_value(),
 				data,
 				effective_gas_price,
 				encoded_len,
@@ -139,7 +135,6 @@ where
 		let call = crate::Call::eth_instantiate_with_code::<T> {
 			value,
 			gas_limit: Zero::zero(),
-			storage_deposit_limit: BalanceOf::<T>::max_value(),
 			code,
 			data,
 			effective_gas_price,
@@ -154,11 +149,8 @@ where
 	let eth_fee = effective_gas_price.saturating_mul(gas) / <T as Config>::NativeToEthRatio::get();
 
 	let weight_limit = {
+		let fixed_fee = <T as Config>::FeeInfo::fixed_fee(encoded_len as u32);
 		let info = <T as Config>::FeeInfo::dispatch_info(&call);
-		let fixed_fee = <T as Config>::FeeInfo::weight_to_fee(
-			<T as frame_system::Config>::BlockWeights::get().get(info.class).base_extrinsic,
-		)
-		.saturating_add(<T as Config>::FeeInfo::length_to_fee(encoded_len as u32));
 
 		let remaining_fee = {
 			let adjusted = eth_fee.checked_sub(fixed_fee.into()).ok_or_else(|| {

substrate/frame/revive/src/evm/fees.rs

@@ -18,14 +18,17 @@
 //! Contains the fee types that need to be configured for `pallet-transaction-payment`.
 
 use crate::{
-	evm::{runtime::EthExtra, OnChargeTransactionBalanceOf},
+	evm::{
+		runtime::{EthExtra, SetWeightLimit},
+		OnChargeTransactionBalanceOf,
+	},
 	BalanceOf, CallOf, Config, DispatchErrorWithPostInfo, DispatchResultWithPostInfo, Error,
 	PostDispatchInfo, LOG_TARGET,
 };
 use codec::Encode;
 use core::marker::PhantomData;
 use frame_support::{
-	dispatch::{DispatchInfo, GetDispatchInfo},
+	dispatch::{DispatchClass, DispatchInfo, GetDispatchInfo},
 	pallet_prelude::Weight,
 	traits::{fungible::Credit, Get, SuppressedDrop},
 	weights::WeightToFee,
@@ -62,24 +65,6 @@ pub struct BlockRatioFee<const P: u128, const Q: u128, T: Config>(PhantomData<T>
 /// pallet_transaction_payment. This way we bundle all the trait bounds in once place.
 pub struct Info<Address, Signature, Extra>(PhantomData<(Address, Signature, Extra)>);
 
-/// A trait that signals that [`BlockRatioFee`] is used by the runtime.
-///
-/// This trait is sealed. Use [`BlockRatioFee`].
-pub trait BlockRatioWeightToFee: seal::Sealed {
-	/// The runtime.
-	type T: Config;
-	/// The ref_time to fee coefficient.
-	const REF_TIME_TO_FEE: FixedU128;
-
-	/// The proof_size to fee coefficient.
-	fn proof_size_to_fee() -> FixedU128 {
-		let max_weight = <Self::T as frame_system::Config>::BlockWeights::get().max_block;
-		let ratio =
-			FixedU128::from_rational(max_weight.ref_time().into(), max_weight.proof_size().into());
-		Self::REF_TIME_TO_FEE.saturating_mul(ratio)
-	}
-}
-
 /// A trait that exposes all the transaction payment details to `pallet_revive`.
 ///
 /// This trait is sealed. Use [`Info`].
@@ -110,6 +95,16 @@ pub trait InfoT<T: Config>: seal::Sealed {
 		Zero::zero()
 	}
 
+	/// Calculate the fee using the weight instead of a dispatch info.
+	fn tx_fee_from_weight(_encoded_len: u32, _weight: &Weight) -> BalanceOf<T> {
+		Zero::zero()
+	}
+
+	/// The base extrinsic and len fee.
+	fn fixed_fee(_encoded_len: u32) -> BalanceOf<T> {
+		Zero::zero()
+	}
+
 	/// Makes sure that not too much storage deposit was withdrawn.
 	fn ensure_not_overdrawn(
 		_encoded_len: u32,
@@ -124,13 +119,18 @@ pub trait InfoT<T: Config>: seal::Sealed {
 		Default::default()
 	}
 
+	/// The dispatch info with the weight argument set to `0`.
+	fn base_dispatch_info(_call: &mut CallOf<T>) -> DispatchInfo {
+		Default::default()
+	}
+
 	/// Calculate the encoded length of a call.
 	fn encoded_len(_eth_transact_call: CallOf<T>) -> u32 {
 		0
 	}
 
 	/// Convert a weight to an unadjusted fee.
-	fn weight_to_fee(_weight: Weight) -> BalanceOf<T> {
+	fn weight_to_fee(_weight: &Weight, _combinator: Combinator) -> BalanceOf<T> {
 		Zero::zero()
 	}
 
@@ -158,34 +158,60 @@ pub trait InfoT<T: Config>: seal::Sealed {
 	}
 }
 
-impl<const P: u128, const Q: u128, T: Config> BlockRatioWeightToFee for BlockRatioFee<P, Q, T> {
-	type T = T;
+/// Which function to use in order to combine `ref_time` and `proof_size` to a fee.
+pub enum Combinator {
+	/// Minimum function.
+	Min,
+	/// Maximum function.
+	Max,
+}
+
+impl<const P: u128, const Q: u128, T: Config> BlockRatioFee<P, Q, T> {
 	const REF_TIME_TO_FEE: FixedU128 = {
 		assert!(P > 0 && Q > 0);
 		FixedU128::from_rational(P, Q)
 	};
+
+	/// The proof_size to fee coefficient.
+	fn proof_size_to_fee() -> FixedU128 {
+		let max_weight = <T as frame_system::Config>::BlockWeights::get().max_block;
+		let ratio =
+			FixedU128::from_rational(max_weight.ref_time().into(), max_weight.proof_size().into());
+		Self::REF_TIME_TO_FEE.saturating_mul(ratio)
+	}
+
+	/// Calculate the fee for a weight.
+	fn weight_to_fee(weight: &Weight, combinator: Combinator) -> BalanceOf<T> {
+		let ref_time_fee = Self::REF_TIME_TO_FEE
+			.saturating_mul_int(BalanceOf::<T>::saturated_from(weight.ref_time()));
+		let proof_size_fee = Self::proof_size_to_fee()
+			.saturating_mul_int(BalanceOf::<T>::saturated_from(weight.proof_size()));
+
+		match combinator {
+			Combinator::Max => ref_time_fee.max(proof_size_fee),
+			Combinator::Min => ref_time_fee.min(proof_size_fee),
+		}
+	}
 }
 
 impl<const P: u128, const Q: u128, T: Config> WeightToFee for BlockRatioFee<P, Q, T> {
 	type Balance = BalanceOf<T>;
 
 	fn weight_to_fee(weight: &Weight) -> Self::Balance {
-		let ref_time_fee = Self::REF_TIME_TO_FEE
-			.saturating_mul_int(Self::Balance::saturated_from(weight.ref_time()));
-		let proof_size_fee = Self::proof_size_to_fee()
-			.saturating_mul_int(Self::Balance::saturated_from(weight.proof_size()));
-		ref_time_fee.max(proof_size_fee)
+		Self::weight_to_fee(weight, Combinator::Max)
 	}
 }
 
-impl<Address, Signature, E: EthExtra> InfoT<E::Config> for Info<Address, Signature, E>
+impl<const P: u128, const Q: u128, Address, Signature, E: EthExtra> InfoT<E::Config>
+	for Info<Address, Signature, E>
 where
+	E::Config: TxConfig<WeightToFee = BlockRatioFee<P, Q, E::Config>>,
 	BalanceOf<E::Config>: From<OnChargeTransactionBalanceOf<E::Config>>,
 	<E::Config as frame_system::Config>::RuntimeCall:
 		Dispatchable<Info = DispatchInfo, PostInfo = PostDispatchInfo>,
+	CallOf<E::Config>: SetWeightLimit,
 	<<E::Config as SysConfig>::Block as BlockT>::Extrinsic:
 		From<UncheckedExtrinsic<Address, CallOf<E::Config>, Signature, E::Extension>>,
-	<E::Config as TxConfig>::WeightToFee: BlockRatioWeightToFee<T = E::Config>,
 	<<E::Config as TxConfig>::OnChargeTransaction as TxCreditHold<E::Config>>::Credit:
 		SuppressedDrop<Inner = CreditOf<E::Config>>,
 {
@@ -215,6 +241,24 @@ where
 		TxPallet::<E::Config>::compute_fee(len, &dispatch_info, 0u32.into()).into()
 	}
 
+	/// Calculate the fee using the weight instead of a dispatch info.
+	fn tx_fee_from_weight(encoded_len: u32, weight: &Weight) -> BalanceOf<E::Config> {
+		let fixed_fee = Self::fixed_fee(encoded_len);
+		let weight_fee = Self::next_fee_multiplier()
+			.saturating_mul_int(Self::weight_to_fee(weight, Combinator::Max));
+		fixed_fee.saturating_add(weight_fee)
+	}
+
+	fn fixed_fee(encoded_len: u32) -> BalanceOf<E::Config> {
+		Self::weight_to_fee(
+			&<E::Config as frame_system::Config>::BlockWeights::get()
+				.get(DispatchClass::Normal)
+				.base_extrinsic,
+			Combinator::Max,
+		)
+		.saturating_add(Self::length_to_fee(encoded_len))
+	}
+
 	fn ensure_not_overdrawn(
 		encoded_len: u32,
 		info: &DispatchInfo,
@@ -259,14 +303,21 @@ where
 		dispatch_info
 	}
 
+	fn base_dispatch_info(call: &mut CallOf<E::Config>) -> DispatchInfo {
+		let pre_weight = call.set_weight_limit(Zero::zero());
+		let info = Self::dispatch_info(call);
+		call.set_weight_limit(pre_weight);
+		info
+	}
+
 	fn encoded_len(eth_transact_call: CallOf<E::Config>) -> u32 {
 		let uxt: <<E::Config as SysConfig>::Block as BlockT>::Extrinsic =
 			UncheckedExtrinsic::new_bare(eth_transact_call).into();
 		uxt.encoded_size() as u32
 	}
 
-	fn weight_to_fee(weight: Weight) -> BalanceOf<E::Config> {
-		TxPallet::<E::Config>::weight_to_fee(weight).into()
+	fn weight_to_fee(weight: &Weight, combinator: Combinator) -> BalanceOf<E::Config> {
+		<E::Config as TxConfig>::WeightToFee::weight_to_fee(&weight, combinator)
 	}
 
 	/// Convert an unadjusted fee back to a weight.
@@ -303,7 +354,6 @@ impl<T: Config> InfoT<T> for () {}
 
 mod seal {
 	pub trait Sealed {}
-	impl<const P: u128, const Q: u128, T: super::Config> Sealed for super::BlockRatioFee<P, Q, T> {}
 	impl<Address, Signature, E: super::EthExtra> Sealed for super::Info<Address, Signature, E> {}
 	impl Sealed for () {}
 }

substrate/frame/revive/src/evm/runtime.rs

@@ -46,7 +46,9 @@ use sp_runtime::{
 /// Used to set the weight limit argument of a `eth_call` or `eth_instantiate_with_code` call.
 pub trait SetWeightLimit {
 	/// Set the weight limit of this call.
-	fn set_weight_limit(&mut self, weight_limit: Weight);
+	///
+	/// Returns the replaced weight.
+	fn set_weight_limit(&mut self, weight_limit: Weight) -> Weight;
 }
 
 /// Wraps [`generic::UncheckedExtrinsic`] to support checking unsigned
@@ -510,13 +512,11 @@ mod test {
 				value,
 				data,
 				gas_limit,
-				storage_deposit_limit,
 				effective_gas_price,
 				encoded_len,
 			}) if dest == tx.to.unwrap() &&
 				value == tx.value.unwrap_or_default().as_u64().into() &&
 				data == tx.input.to_vec() &&
-				storage_deposit_limit == <BalanceOf<Test>>::max_value() &&
 				effective_gas_price == expected_effective_gas_price.into() =>
 			{
 				assert_eq!(encoded_len, expected_encoded_len);
@@ -547,13 +547,11 @@ mod test {
 				code,
 				data,
 				gas_limit,
-				storage_deposit_limit,
 				effective_gas_price,
 				encoded_len,
 			}) if value == expected_value &&
 				code == expected_code &&
 				data == expected_data &&
-				storage_deposit_limit == <BalanceOf<Test>>::max_value() &&
 				effective_gas_price == expected_effective_gas_price.into() =>
 			{
 				assert_eq!(encoded_len, expected_encoded_len);