From d47fe23c15e5e9d01672f19a048da644b8a15b19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Tue, 23 Jun 2020 09:46:38 +0200 Subject: [PATCH 1/3] seal: Refactor ext_gas_price * Remove seals dependency on pallet_transaction_payment * Add weight as an argument to ext_gas_price --- Cargo.lock | 1 - bin/node/runtime/src/lib.rs | 2 ++ frame/contracts/Cargo.toml | 2 -- frame/contracts/src/exec.rs | 12 +++++----- frame/contracts/src/lib.rs | 17 ++++++++++---- frame/contracts/src/tests.rs | 12 +++------- frame/contracts/src/wasm/mod.rs | 13 ++++++----- frame/contracts/src/wasm/runtime.rs | 8 ++++--- frame/transaction-payment/src/lib.rs | 33 +++++++++++++--------------- 9 files changed, 50 insertions(+), 50 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 75976823954a1..aa7ccbc5b321a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3997,7 +3997,6 @@ dependencies = [ "pallet-contracts-primitives", "pallet-randomness-collective-flip", "pallet-timestamp", - "pallet-transaction-payment", "parity-scale-codec", "parity-wasm 0.41.0", "pwasm-utils", diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index cf1b0de8f7922..5acaafcab424f 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -581,6 +581,7 @@ parameter_types! { impl pallet_contracts::Trait for Runtime { type Time = Timestamp; type Randomness = RandomnessCollectiveFlip; + type Currency = Balances; type Call = Call; type Event = Event; type DetermineContractAddress = pallet_contracts::SimpleAddressDeterminer; @@ -594,6 +595,7 @@ impl pallet_contracts::Trait for Runtime { type SurchargeReward = SurchargeReward; type MaxDepth = pallet_contracts::DefaultMaxDepth; type MaxValueSize = pallet_contracts::DefaultMaxValueSize; + type WeightPrice = pallet_transaction_payment::Module; } impl pallet_sudo::Trait for Runtime { diff --git a/frame/contracts/Cargo.toml b/frame/contracts/Cargo.toml index b2ba8d014ae78..e2ab36f6dab72 100644 --- a/frame/contracts/Cargo.toml +++ b/frame/contracts/Cargo.toml @@ -25,7 +25,6 @@ sp-sandbox = { version = "0.8.0-rc3", default-features = false, path = "../../pr frame-support = { version = "2.0.0-rc3", default-features = false, path = "../support" } frame-system = { version = "2.0.0-rc3", default-features = false, path = "../system" } pallet-contracts-primitives = { version = "2.0.0-rc3", default-features = false, path = "common" } -pallet-transaction-payment = { version = "2.0.0-rc3", default-features = false, path = "../transaction-payment" } [dev-dependencies] wabt = "0.9.2" @@ -51,5 +50,4 @@ std = [ "pwasm-utils/std", "wasmi-validation/std", "pallet-contracts-primitives/std", - "pallet-transaction-payment/std", ] diff --git a/frame/contracts/src/exec.rs b/frame/contracts/src/exec.rs index 9cc1c50260db9..3c524a001a30d 100644 --- a/frame/contracts/src/exec.rs +++ b/frame/contracts/src/exec.rs @@ -21,10 +21,11 @@ use crate::gas::{Gas, GasMeter, Token}; use crate::rent; use sp_std::prelude::*; -use sp_runtime::traits::{Bounded, CheckedAdd, CheckedSub, Zero}; +use sp_runtime::traits::{Bounded, CheckedAdd, CheckedSub, Zero, Convert}; use frame_support::{ storage::unhashed, dispatch::DispatchError, traits::{WithdrawReason, Currency, Time, Randomness}, + weights::Weight, }; pub type AccountIdOf = ::AccountId; @@ -205,7 +206,7 @@ pub trait Ext { fn get_runtime_storage(&self, key: &[u8]) -> Option>; /// Returns the price of one weight unit. - fn get_weight_price(&self) -> BalanceOf; + fn get_weight_price(&self, weight: Weight) -> BalanceOf; } /// Loader is a companion of the `Vm` trait. It loads an appropriate abstract @@ -869,11 +870,8 @@ where unhashed::get_raw(&key) } - fn get_weight_price(&self) -> BalanceOf { - use pallet_transaction_payment::Module as Payment; - use sp_runtime::SaturatedConversion; - let price = Payment::::weight_to_fee_with_adjustment::(1); - price.saturated_into() + fn get_weight_price(&self, weight: Weight) -> BalanceOf { + T::WeightPrice::convert(weight) } } diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 245c95a4fa4e9..ab8b97df1ac67 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -105,7 +105,7 @@ use codec::{Codec, Encode, Decode}; use sp_io::hashing::blake2_256; use sp_runtime::{ traits::{ - Hash, StaticLookup, Zero, MaybeSerializeDeserialize, Member, + Hash, StaticLookup, Zero, MaybeSerializeDeserialize, Member, Convert, }, RuntimeDebug, }; @@ -120,6 +120,7 @@ use frame_support::traits::{OnUnbalanced, Currency, Get, Time, Randomness}; use frame_support::weights::GetDispatchInfo; use frame_system::{self as system, ensure_signed, RawOrigin, ensure_root}; use pallet_contracts_primitives::{RentProjection, ContractAccessError}; +use frame_support::weights::Weight; pub type CodeHash = ::Hash; pub type TrieId = Vec; @@ -291,9 +292,10 @@ where } } -pub type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; +pub type BalanceOf = + <::Currency as Currency<::AccountId>>::Balance; pub type NegativeImbalanceOf = - <::Currency as Currency<::AccountId>>::NegativeImbalance; + <::Currency as Currency<::AccountId>>::NegativeImbalance; parameter_types! { /// A reasonable default value for [`Trait::SignedClaimedHandicap`]. @@ -314,10 +316,13 @@ parameter_types! { pub const DefaultMaxValueSize: u32 = 16_384; } -pub trait Trait: frame_system::Trait + pallet_transaction_payment::Trait { +pub trait Trait: frame_system::Trait { type Time: Time; type Randomness: Randomness; + /// The currency in which fees are paid and contract balances are held. + type Currency: Currency; + /// The outer call dispatch type. type Call: Parameter + @@ -373,6 +378,10 @@ pub trait Trait: frame_system::Trait + pallet_transaction_payment::Trait { /// The maximum size of a storage value in bytes. type MaxValueSize: Get; + + /// Used to supply contracts with the current gas price. This is **not** used to + /// calculate the actual fee and is only for informational purposes. + type WeightPrice: Convert>; } /// Simple contract address determiner. diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index a98fdf2d25804..c11ae4c044b4a 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -32,7 +32,7 @@ use frame_support::{ assert_ok, assert_err_ignore_postinfo, impl_outer_dispatch, impl_outer_event, impl_outer_origin, parameter_types, StorageMap, StorageValue, traits::{Currency, Get}, - weights::{Weight, PostDispatchInfo, IdentityFee}, + weights::{Weight, PostDispatchInfo}, }; use std::cell::RefCell; use frame_system::{self as system, EventRecord, Phase}; @@ -143,17 +143,10 @@ impl Convert> for Test { } } -impl pallet_transaction_payment::Trait for Test { - type Currency = Balances; - type OnTransactionPayment = (); - type TransactionByteFee = TransactionByteFee; - type WeightToFee = IdentityFee>; - type FeeMultiplierUpdate = (); -} - impl Trait for Test { type Time = Timestamp; type Randomness = Randomness; + type Currency = Balances; type Call = Call; type DetermineContractAddress = DummyContractAddressFor; type Event = MetaEvent; @@ -167,6 +160,7 @@ impl Trait for Test { type SurchargeReward = SurchargeReward; type MaxDepth = MaxDepth; type MaxValueSize = MaxValueSize; + type WeightPrice = Self; } type Balances = pallet_balances::Module; diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index cb69cd689b265..7854e318d3e77 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -162,6 +162,7 @@ mod tests { use hex_literal::hex; use assert_matches::assert_matches; use sp_runtime::DispatchError; + use frame_support::weights::Weight; const GAS_LIMIT: Gas = 10_000_000_000; @@ -375,8 +376,8 @@ mod tests { ) ) } - fn get_weight_price(&self) -> BalanceOf { - 1312_u32.into() + fn get_weight_price(&self, weight: Weight) -> BalanceOf { + (weight * 1312).into() } } @@ -483,8 +484,8 @@ mod tests { fn get_runtime_storage(&self, key: &[u8]) -> Option> { (**self).get_runtime_storage(key) } - fn get_weight_price(&self) -> BalanceOf { - (**self).get_weight_price() + fn get_weight_price(&self, weight: Weight) -> BalanceOf { + (**self).get_weight_price(weight) } } @@ -1060,7 +1061,7 @@ mod tests { const CODE_GAS_PRICE: &str = r#" (module - (import "env" "ext_gas_price" (func $ext_gas_price)) + (import "env" "ext_gas_price" (func $ext_gas_price (param i64))) (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) (import "env" "memory" (memory 1 1)) @@ -1076,7 +1077,7 @@ mod tests { (func (export "call") ;; This stores the gas price in the scratch buffer - (call $ext_gas_price) + (call $ext_gas_price (i64.const 1)) ;; assert $ext_scratch_size == 8 (call $assert diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index f87f5d1ef53cc..117a4b3534619 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -688,12 +688,14 @@ define_env!(Env, , Ok(()) }, - // Stores the gas price for the current transaction into the scratch buffer. + // Stores the price for the specified amount of gas in scratch buffer. // // The data is encoded as T::Balance. The current contents of the scratch buffer are overwritten. - ext_gas_price(ctx) => { + // It is recommended to avoid specifying very small values for `gas` as the prices for a single + // gas can be smaller than one. + ext_gas_price(ctx, gas: u64) => { ctx.scratch_buf.clear(); - ctx.ext.get_weight_price().encode_to(&mut ctx.scratch_buf); + ctx.ext.get_weight_price(gas).encode_to(&mut ctx.scratch_buf); Ok(()) }, diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index 31d0cfb20ded0..6570d265510ec 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -51,7 +51,7 @@ use sp_runtime::{ }, traits::{ Zero, Saturating, SignedExtension, SaturatedConversion, Convert, Dispatchable, - DispatchInfoOf, PostDispatchInfoOf, UniqueSaturatedFrom, UniqueSaturatedInto, + DispatchInfoOf, PostDispatchInfoOf, }, }; use pallet_transaction_payment_rpc_runtime_api::RuntimeDispatchInfo; @@ -372,23 +372,6 @@ impl Module where tip } } -} - -impl Module { - /// Compute the fee for the specified weight. - /// - /// This fee is already adjusted by the per block fee adjustment factor and is therefore the - /// share that the weight contributes to the overall fee of a transaction. - /// - /// This function is generic in order to supply the contracts module with a way to calculate the - /// gas price. The contracts module is not able to put the necessary `BalanceOf` constraints - /// on its trait. This function is not to be used by this module. - pub fn weight_to_fee_with_adjustment(weight: Weight) -> Balance where - Balance: UniqueSaturatedFrom - { - let fee: u128 = Self::weight_to_fee(weight).unique_saturated_into(); - Balance::unique_saturated_from(NextFeeMultiplier::get().saturating_mul_acc_int(fee)) - } fn weight_to_fee(weight: Weight) -> BalanceOf { // cap the weight to the maximum defined in runtime, otherwise it will be the @@ -398,6 +381,20 @@ impl Module { } } +impl Convert> for Module where + T: Trait, + BalanceOf: FixedPointOperand, +{ + /// Compute the fee for the specified weight. + /// + /// This fee is already adjusted by the per block fee adjustment factor and is therefore the + /// share that the weight contributes to the overall fee of a transaction. It is mainly + /// for informational purposes and not used in the actual fee calculation. + fn convert(weight: Weight) -> BalanceOf { + NextFeeMultiplier::get().saturating_mul_int(Self::weight_to_fee(weight)) + } +} + /// Require the transactor pay for themselves and maybe include a tip to gain additional priority /// in the queue. #[derive(Encode, Decode, Clone, Eq, PartialEq)] From 5821d87cfd98b0eba8744999f523570d3389476c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Tue, 23 Jun 2020 14:57:12 +0200 Subject: [PATCH 2/3] Fixed documentation nits from review --- frame/contracts/src/exec.rs | 2 +- frame/contracts/src/lib.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/contracts/src/exec.rs b/frame/contracts/src/exec.rs index 3c524a001a30d..5c8d1456d5572 100644 --- a/frame/contracts/src/exec.rs +++ b/frame/contracts/src/exec.rs @@ -205,7 +205,7 @@ pub trait Ext { /// Returns `None` if the value doesn't exist. fn get_runtime_storage(&self, key: &[u8]) -> Option>; - /// Returns the price of one weight unit. + /// Returns the price for the specified amount of weight. fn get_weight_price(&self, weight: Weight) -> BalanceOf; } diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index ab8b97df1ac67..82eb96a5972b9 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -379,8 +379,8 @@ pub trait Trait: frame_system::Trait { /// The maximum size of a storage value in bytes. type MaxValueSize: Get; - /// Used to supply contracts with the current gas price. This is **not** used to - /// calculate the actual fee and is only for informational purposes. + /// Used to answer contracts's queries regarding the current weight price. This is **not** + /// used to calculate the actual fee and is only for informational purposes. type WeightPrice: Convert>; } From f877bf9f376314dc2ad7153ddb0d8f0e94bcc9c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Wed, 24 Jun 2020 10:45:44 +0200 Subject: [PATCH 3/3] Do not use unchecked math even in test code --- frame/contracts/src/wasm/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index dce7715065729..a4814a1b22f43 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -375,7 +375,7 @@ mod tests { ) } fn get_weight_price(&self, weight: Weight) -> BalanceOf { - (weight * 1312).into() + BalanceOf::::from(1312_u32).saturating_mul(weight.into()) } }