From 4ce31630d7a84b8984544f71c15cb6a1e77bfe1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Tue, 14 Apr 2020 14:40:49 +0200 Subject: [PATCH 1/8] Integrate pallet_contracts gas with the weight system --- Cargo.lock | 1 + bin/node/cli/src/chain_spec.rs | 1 - bin/node/cli/src/factory_impl.rs | 3 +- bin/node/cli/src/service.rs | 3 +- bin/node/runtime/src/lib.rs | 17 +- bin/node/testing/src/genesis.rs | 1 - bin/node/testing/src/keyring.rs | 1 - bin/utils/subkey/src/main.rs | 2 - frame/contracts/Cargo.toml | 1 + frame/contracts/rpc/src/lib.rs | 7 +- frame/contracts/src/exec.rs | 76 ++++---- frame/contracts/src/gas.rs | 154 ++++----------- frame/contracts/src/lib.rs | 274 ++++++--------------------- frame/contracts/src/tests.rs | 167 +++++----------- frame/contracts/src/wasm/mod.rs | 62 +++--- frame/contracts/src/wasm/runtime.rs | 23 ++- frame/support/src/lib.rs | 16 +- frame/transaction-payment/src/lib.rs | 20 +- 18 files changed, 272 insertions(+), 557 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7b36a7d08a475..688c0ffb0cab8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4070,6 +4070,7 @@ 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/cli/src/chain_spec.rs b/bin/node/cli/src/chain_spec.rs index 6d67c61381270..461474038b8b8 100644 --- a/bin/node/cli/src/chain_spec.rs +++ b/bin/node/cli/src/chain_spec.rs @@ -292,7 +292,6 @@ pub fn testnet_genesis( enable_println, // this should only be enabled on development chains ..Default::default() }, - gas_price: 1 * MILLICENTS, }), pallet_sudo: Some(SudoConfig { key: root_key, diff --git a/bin/node/cli/src/factory_impl.rs b/bin/node/cli/src/factory_impl.rs index 1d1eabe29cbfb..bc7653538247f 100644 --- a/bin/node/cli/src/factory_impl.rs +++ b/bin/node/cli/src/factory_impl.rs @@ -57,7 +57,6 @@ impl FactoryState { frame_system::CheckNonce::from(index), frame_system::CheckWeight::new(), pallet_transaction_payment::ChargeTransactionPayment::from(0), - Default::default(), ) } } @@ -122,7 +121,7 @@ impl RuntimeAdapter for FactoryState { (*amount).into() ) ) - }, key, (version, genesis_hash.clone(), prior_block_hash.clone(), (), (), (), ())) + }, key, (version, genesis_hash.clone(), prior_block_hash.clone(), (), (), ())) } fn inherent_extrinsics(&self) -> InherentData { diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs index 0acd553ea0127..1ccc6e5ec4850 100644 --- a/bin/node/cli/src/service.rs +++ b/bin/node/cli/src/service.rs @@ -633,12 +633,11 @@ mod tests { check_nonce, check_weight, payment, - Default::default(), ); let raw_payload = SignedPayload::from_raw( function, extra, - (version, genesis_hash, genesis_hash, (), (), (), ()) + (version, genesis_hash, genesis_hash, (), (), ()) ); let signature = raw_payload.using_encoded(|payload| { signer.sign(payload) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index b1797fffb3321..69bdca6b15c4d 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -73,7 +73,6 @@ use constants::{time::*, currency::*}; #[cfg(feature = "std")] include!(concat!(env!("OUT_DIR"), "/wasm_binary.rs")); - /// Runtime version. pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("node"), @@ -456,9 +455,6 @@ impl pallet_treasury::Trait for Runtime { } parameter_types! { - pub const ContractTransactionBaseFee: Balance = 1 * CENTS; - pub const ContractTransactionByteFee: Balance = 10 * MILLICENTS; - pub const ContractFee: Balance = 1 * CENTS; pub const TombstoneDeposit: Balance = 1 * DOLLARS; pub const RentByteFee: Balance = 1 * DOLLARS; pub const RentDepositOffset: Balance = 1000 * DOLLARS; @@ -466,15 +462,12 @@ parameter_types! { } impl pallet_contracts::Trait for Runtime { - type Currency = Balances; type Time = Timestamp; type Randomness = RandomnessCollectiveFlip; type Call = Call; type Event = Event; type DetermineContractAddress = pallet_contracts::SimpleAddressDeterminer; - type ComputeDispatchFee = pallet_contracts::DefaultDispatchFeeComputor; type TrieIdGenerator = pallet_contracts::TrieIdFromParentCounter; - type GasPayment = (); type RentPayment = (); type SignedClaimHandicap = pallet_contracts::DefaultSignedClaimHandicap; type TombstoneDeposit = TombstoneDeposit; @@ -482,14 +475,8 @@ impl pallet_contracts::Trait for Runtime { type RentByteFee = RentByteFee; type RentDepositOffset = RentDepositOffset; type SurchargeReward = SurchargeReward; - type TransactionBaseFee = ContractTransactionBaseFee; - type TransactionByteFee = ContractTransactionByteFee; - type ContractFee = ContractFee; - type CallBaseFee = pallet_contracts::DefaultCallBaseFee; - type InstantiateBaseFee = pallet_contracts::DefaultInstantiateBaseFee; type MaxDepth = pallet_contracts::DefaultMaxDepth; type MaxValueSize = pallet_contracts::DefaultMaxValueSize; - type BlockGasLimit = pallet_contracts::DefaultBlockGasLimit; } impl pallet_sudo::Trait for Runtime { @@ -532,7 +519,6 @@ impl frame_system::offchain::CreateSignedTransaction for R frame_system::CheckNonce::::from(nonce), frame_system::CheckWeight::::new(), pallet_transaction_payment::ChargeTransactionPayment::::from(tip), - Default::default(), ); let raw_payload = SignedPayload::new(call, extra).map_err(|e| { debug::warn!("Unable to create signed payload: {:?}", e); @@ -688,7 +674,7 @@ construct_runtime!( FinalityTracker: pallet_finality_tracker::{Module, Call, Inherent}, Grandpa: pallet_grandpa::{Module, Call, Storage, Config, Event}, Treasury: pallet_treasury::{Module, Call, Storage, Config, Event}, - Contracts: pallet_contracts::{Module, Call, Config, Storage, Event}, + Contracts: pallet_contracts::{Module, Call, Config, Storage, Event}, Sudo: pallet_sudo::{Module, Call, Config, Storage, Event}, ImOnline: pallet_im_online::{Module, Call, Storage, Event, ValidateUnsigned, Config}, AuthorityDiscovery: pallet_authority_discovery::{Module, Call, Config}, @@ -720,7 +706,6 @@ pub type SignedExtra = ( frame_system::CheckNonce, frame_system::CheckWeight, pallet_transaction_payment::ChargeTransactionPayment, - pallet_contracts::CheckBlockGasLimit, ); /// Unchecked extrinsic type as expected by this runtime. pub type UncheckedExtrinsic = generic::UncheckedExtrinsic; diff --git a/bin/node/testing/src/genesis.rs b/bin/node/testing/src/genesis.rs index 8a57010770f3d..0f72d2c5471bd 100644 --- a/bin/node/testing/src/genesis.rs +++ b/bin/node/testing/src/genesis.rs @@ -97,7 +97,6 @@ pub fn config_endowed( }), pallet_contracts: Some(ContractsConfig { current_schedule: Default::default(), - gas_price: 1 * MILLICENTS, }), pallet_babe: Some(Default::default()), pallet_grandpa: Some(GrandpaConfig { diff --git a/bin/node/testing/src/keyring.rs b/bin/node/testing/src/keyring.rs index 6b0d06875d692..5eebc09f4b7af 100644 --- a/bin/node/testing/src/keyring.rs +++ b/bin/node/testing/src/keyring.rs @@ -74,7 +74,6 @@ pub fn signed_extra(nonce: Index, extra_fee: Balance) -> SignedExtra { frame_system::CheckNonce::from(nonce), frame_system::CheckWeight::new(), pallet_transaction_payment::ChargeTransactionPayment::from(extra_fee), - Default::default(), ) } diff --git a/bin/utils/subkey/src/main.rs b/bin/utils/subkey/src/main.rs index 2d9302bf8c72e..22706ebd822da 100644 --- a/bin/utils/subkey/src/main.rs +++ b/bin/utils/subkey/src/main.rs @@ -708,7 +708,6 @@ fn create_extrinsic( frame_system::CheckNonce::::from(i), frame_system::CheckWeight::::new(), pallet_transaction_payment::ChargeTransactionPayment::::from(f), - Default::default(), ) }; let raw_payload = SignedPayload::from_raw( @@ -721,7 +720,6 @@ fn create_extrinsic( (), (), (), - (), ), ); let signature = raw_payload.using_encoded(|payload| signer.sign(payload)).into_runtime(); diff --git a/frame/contracts/Cargo.toml b/frame/contracts/Cargo.toml index 42dba7299de0f..e26efe5b690f1 100644 --- a/frame/contracts/Cargo.toml +++ b/frame/contracts/Cargo.toml @@ -25,6 +25,7 @@ sp-sandbox = { version = "0.8.0-dev", default-features = false, path = "../../pr frame-support = { version = "2.0.0-dev", default-features = false, path = "../support" } frame-system = { version = "2.0.0-dev", default-features = false, path = "../system" } pallet-contracts-primitives = { version = "2.0.0-dev", default-features = false, path = "common" } +pallet-transaction-payment = { version = "2.0.0-dev", default-features = false, path = "../transaction-payment" } [dev-dependencies] wabt = "0.9.2" diff --git a/frame/contracts/rpc/src/lib.rs b/frame/contracts/rpc/src/lib.rs index 52dddb177bbc7..53e8d938703c8 100644 --- a/frame/contracts/rpc/src/lib.rs +++ b/frame/contracts/rpc/src/lib.rs @@ -46,9 +46,10 @@ const CONTRACT_IS_A_TOMBSTONE: i64 = 3; /// This value is used to set the upper bound for maximal contract calls to /// prevent blocking the RPC for too long. /// -/// Based on W3F research spreadsheet: -/// https://docs.google.com/spreadsheets/d/1h0RqncdqiWI4KgxO0z9JIpZEJESXjX_ZCK6LFX6veDo/view -const GAS_PER_SECOND: u64 = 1_000_000_000; +/// As 1 gas is equal to 1 weight we base this on the conducted benchmarks which +/// determined runtime weights: +/// https://github.com/paritytech/substrate/pull/5446 +const GAS_PER_SECOND: u64 = 1_000_000_000_000; /// A private newtype for converting `ContractAccessError` into an RPC error. struct ContractAccessError(pallet_contracts_primitives::ContractAccessError); diff --git a/frame/contracts/src/exec.rs b/frame/contracts/src/exec.rs index 402622331d0ec..027a10bb4de02 100644 --- a/frame/contracts/src/exec.rs +++ b/frame/contracts/src/exec.rs @@ -17,7 +17,7 @@ use super::{CodeHash, Config, ContractAddressFor, Event, RawEvent, Trait, TrieId, BalanceOf, ContractInfo}; use crate::account_db::{AccountDb, DirectAccountDb, OverlayAccountDb}; -use crate::gas::{Gas, GasMeter, Token, approx_gas_for_balance}; +use crate::gas::{Gas, GasMeter, Token}; use crate::rent; use sp_std::prelude::*; @@ -203,6 +203,9 @@ 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. + fn get_weight_price(&self) -> BalanceOf; } /// Loader is a companion of the `Vm` trait. It loads an appropriate abstract @@ -605,21 +608,19 @@ pub enum TransferFeeKind { #[cfg_attr(test, derive(Debug, PartialEq, Eq))] #[derive(Copy, Clone)] -pub struct TransferFeeToken { +pub struct TransferFeeToken { kind: TransferFeeKind, - gas_price: Balance, } -impl Token for TransferFeeToken> { +impl Token for TransferFeeToken { type Metadata = Config; #[inline] fn calculate_amount(&self, metadata: &Config) -> Gas { - let balance_fee = match self.kind { - TransferFeeKind::ContractInstantiate => metadata.contract_account_instantiate_fee, - TransferFeeKind::Transfer => return metadata.schedule.transfer_cost, - }; - approx_gas_for_balance(self.gas_price, balance_fee) + match self.kind { + TransferFeeKind::ContractInstantiate => metadata.schedule.instantiate_cost, + TransferFeeKind::Transfer => metadata.schedule.transfer_cost, + } } } @@ -668,7 +669,6 @@ fn transfer<'a, T: Trait, V: Vm, L: Loader>( }; TransferFeeToken { kind, - gas_price: gas_meter.gas_price(), } }; @@ -868,6 +868,13 @@ where fn get_runtime_storage(&self, key: &[u8]) -> Option> { 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() + } } /// These tests exercise the executive layer. @@ -1003,7 +1010,7 @@ mod tests { #[test] fn it_works() { let value = Default::default(); - let mut gas_meter = GasMeter::::with_limit(10000, 1); + let mut gas_meter = GasMeter::::new(10000); let data = vec![]; let vm = MockVm::new(); @@ -1044,7 +1051,7 @@ mod tests { ctx.overlay.set_balance(&origin, 100); ctx.overlay.set_balance(&dest, 0); - let mut gas_meter = GasMeter::::with_limit(1000, 1); + let mut gas_meter = GasMeter::::new(1000); let result = ctx.call(dest, 0, &mut gas_meter, vec![]); assert_matches!(result, Ok(_)); @@ -1064,7 +1071,7 @@ mod tests { ctx.overlay.set_balance(&origin, 100); - let mut gas_meter = GasMeter::::with_limit(1000, 1); + let mut gas_meter = GasMeter::::new(1000); let result = ctx.instantiate(1, &mut gas_meter, &code, vec![]); assert_matches!(result, Ok(_)); @@ -1093,7 +1100,7 @@ mod tests { let output = ctx.call( dest, 55, - &mut GasMeter::::with_limit(1000, 1), + &mut GasMeter::::new(1000), vec![], ).unwrap(); @@ -1126,7 +1133,7 @@ mod tests { let output = ctx.call( dest, 55, - &mut GasMeter::::with_limit(1000, 1), + &mut GasMeter::::new(1000), vec![], ).unwrap(); @@ -1152,7 +1159,7 @@ mod tests { ctx.overlay.set_balance(&origin, 100); ctx.overlay.set_balance(&dest, 0); - let mut gas_meter = GasMeter::::with_limit(1000, 1); + let mut gas_meter = GasMeter::::new(1000); let result = ctx.call(dest, 50, &mut gas_meter, vec![]); assert_matches!(result, Ok(_)); @@ -1163,7 +1170,6 @@ mod tests { ExecFeeToken::Call, TransferFeeToken { kind: TransferFeeKind::Transfer, - gas_price: 1u64 }, ); }); @@ -1178,7 +1184,7 @@ mod tests { ctx.overlay.set_balance(&origin, 100); ctx.overlay.set_balance(&dest, 15); - let mut gas_meter = GasMeter::::with_limit(1000, 1); + let mut gas_meter = GasMeter::::new(1000); let result = ctx.call(dest, 50, &mut gas_meter, vec![]); assert_matches!(result, Ok(_)); @@ -1189,7 +1195,6 @@ mod tests { ExecFeeToken::Call, TransferFeeToken { kind: TransferFeeKind::Transfer, - gas_price: 1u64 }, ); }); @@ -1207,7 +1212,7 @@ mod tests { ctx.overlay.set_balance(&origin, 100); ctx.overlay.set_balance(&dest, 15); - let mut gas_meter = GasMeter::::with_limit(1000, 1); + let mut gas_meter = GasMeter::::new(1000); let result = ctx.instantiate(50, &mut gas_meter, &code, vec![]); assert_matches!(result, Ok(_)); @@ -1218,7 +1223,6 @@ mod tests { ExecFeeToken::Instantiate, TransferFeeToken { kind: TransferFeeKind::ContractInstantiate, - gas_price: 1u64 }, ); }); @@ -1242,7 +1246,7 @@ mod tests { let result = ctx.call( dest, 100, - &mut GasMeter::::with_limit(1000, 1), + &mut GasMeter::::new(1000), vec![], ); @@ -1279,7 +1283,7 @@ mod tests { let result = ctx.call( dest, 0, - &mut GasMeter::::with_limit(1000, 1), + &mut GasMeter::::new(1000), vec![], ); @@ -1310,7 +1314,7 @@ mod tests { let result = ctx.call( dest, 0, - &mut GasMeter::::with_limit(1000, 1), + &mut GasMeter::::new(1000), vec![], ); @@ -1338,7 +1342,7 @@ mod tests { let result = ctx.call( BOB, 0, - &mut GasMeter::::with_limit(10000, 1), + &mut GasMeter::::new(10000), vec![1, 2, 3, 4], ); assert_matches!(result, Ok(_)); @@ -1363,7 +1367,7 @@ mod tests { let result = ctx.instantiate( 1, - &mut GasMeter::::with_limit(10000, 1), + &mut GasMeter::::new(10000), &input_data_ch, vec![1, 2, 3, 4], ); @@ -1413,7 +1417,7 @@ mod tests { let result = ctx.call( BOB, value, - &mut GasMeter::::with_limit(100000, 1), + &mut GasMeter::::new(100000), vec![], ); @@ -1459,7 +1463,7 @@ mod tests { let result = ctx.call( dest, 0, - &mut GasMeter::::with_limit(10000, 1), + &mut GasMeter::::new(10000), vec![], ); @@ -1500,7 +1504,7 @@ mod tests { let result = ctx.call( BOB, 0, - &mut GasMeter::::with_limit(10000, 1), + &mut GasMeter::::new(10000), vec![], ); @@ -1522,7 +1526,7 @@ mod tests { assert_matches!( ctx.instantiate( 0, // <- zero endowment - &mut GasMeter::::with_limit(10000, 1), + &mut GasMeter::::new(10000), &dummy_ch, vec![], ), @@ -1548,7 +1552,7 @@ mod tests { let instantiated_contract_address = assert_matches!( ctx.instantiate( 100, - &mut GasMeter::::with_limit(10000, 1), + &mut GasMeter::::new(10000), &dummy_ch, vec![], ), @@ -1588,7 +1592,7 @@ mod tests { let instantiated_contract_address = assert_matches!( ctx.instantiate( 100, - &mut GasMeter::::with_limit(10000, 1), + &mut GasMeter::::new(10000), &dummy_ch, vec![], ), @@ -1633,7 +1637,7 @@ mod tests { ctx.overlay.instantiate_contract(&BOB, instantiator_ch).unwrap(); assert_matches!( - ctx.call(BOB, 20, &mut GasMeter::::with_limit(1000, 1), vec![]), + ctx.call(BOB, 20, &mut GasMeter::::new(1000), vec![]), Ok(_) ); @@ -1693,7 +1697,7 @@ mod tests { ctx.overlay.instantiate_contract(&BOB, instantiator_ch).unwrap(); assert_matches!( - ctx.call(BOB, 20, &mut GasMeter::::with_limit(1000, 1), vec![]), + ctx.call(BOB, 20, &mut GasMeter::::new(1000), vec![]), Ok(_) ); @@ -1730,7 +1734,7 @@ mod tests { assert_matches!( ctx.instantiate( 100, - &mut GasMeter::::with_limit(10000, 1), + &mut GasMeter::::new(10000), &terminate_ch, vec![], ), @@ -1766,7 +1770,7 @@ mod tests { let result = ctx.instantiate( 1, - &mut GasMeter::::with_limit(10000, 1), + &mut GasMeter::::new(10000), &rent_allowance_ch, vec![], ); diff --git a/frame/contracts/src/gas.rs b/frame/contracts/src/gas.rs index 362f15f3aae79..0881394880ce8 100644 --- a/frame/contracts/src/gas.rs +++ b/frame/contracts/src/gas.rs @@ -14,22 +14,18 @@ // You should have received a copy of the GNU General Public License // along with Substrate. If not, see . -use crate::{GasSpent, Module, Trait, BalanceOf, NegativeImbalanceOf}; -use sp_std::convert::TryFrom; -use sp_runtime::traits::{ - CheckedMul, Zero, SaturatedConversion, AtLeast32Bit, UniqueSaturatedInto, -}; -use frame_support::{ - traits::{Currency, ExistenceRequirement, Imbalance, OnUnbalanced, WithdrawReason}, StorageValue, - dispatch::DispatchError, +use crate::Trait; +use sp_std::marker::PhantomData; +use sp_runtime::traits::Zero; +use frame_support::dispatch::{ + DispatchError, DispatchResultWithPostInfo, PostDispatchInfo, DispatchErrorWithPostInfo, }; #[cfg(test)] use std::{any::Any, fmt::Debug}; -// Gas units are chosen to be represented by u64 so that gas metering instructions can operate on -// them efficiently. -pub type Gas = u64; +// Gas is essentially the same as weight. It is a 1 to 1 correspondence. +pub type Gas = frame_support::weights::Weight; #[must_use] #[derive(Debug, PartialEq, Eq)] @@ -88,20 +84,19 @@ pub struct ErasedToken { } pub struct GasMeter { - limit: Gas, + gas_limit: Gas, /// Amount of gas left from initial gas limit. Can reach zero. gas_left: Gas, - gas_price: BalanceOf, - + _phantom: PhantomData, #[cfg(test)] tokens: Vec, } impl GasMeter { - pub fn with_limit(gas_limit: Gas, gas_price: BalanceOf) -> GasMeter { + pub fn new(gas_limit: Gas) -> Self { GasMeter { - limit: gas_limit, + gas_limit, gas_left: gas_limit, - gas_price, + _phantom: PhantomData, #[cfg(test)] tokens: Vec::new(), } @@ -147,6 +142,14 @@ impl GasMeter { } } + // Account for not fully used gas. + // + // This can be used after dispatching a runtime call to refund gas that was not + // used by the dispatchable. + pub fn refund(&mut self, gas: Gas) { + self.gas_left = (self.gas_left + gas).max(self.gas_limit); + } + /// Allocate some amount of gas and perform some work with /// a newly created nested gas meter. /// @@ -165,7 +168,7 @@ impl GasMeter { f(None) } else { self.gas_left = self.gas_left - amount; - let mut nested = GasMeter::with_limit(amount, self.gas_price); + let mut nested = GasMeter::new(amount); let r = f(Some(&mut nested)); @@ -175,8 +178,9 @@ impl GasMeter { } } - pub fn gas_price(&self) -> BalanceOf { - self.gas_price + /// Returns how much gas left from the initial budget. + fn gas_spent(&self) -> Gas { + self.gas_limit - self.gas_left } /// Returns how much gas left from the initial budget. @@ -184,9 +188,17 @@ impl GasMeter { self.gas_left } - /// Returns how much gas was spent. - fn spent(&self) -> Gas { - self.limit - self.gas_left + // Turn this GasMeter into a DispatchResult that contains the actually used gas + pub fn into_dispatch_result(self, result: Result) -> DispatchResultWithPostInfo where + E: Into + { + let post_info = PostDispatchInfo { + actual_weight: Some(self.gas_spent()), + }; + + result + .map(|_| post_info) + .map_err(|e| DispatchErrorWithPostInfo { post_info, error: e.into() }) } #[cfg(test)] @@ -195,68 +207,6 @@ impl GasMeter { } } -/// Buy the given amount of gas. -/// -/// Cost is calculated by multiplying the gas cost (taken from the storage) by the `gas_limit`. -/// The funds are deducted from `transactor`. -pub fn buy_gas( - transactor: &T::AccountId, - gas_limit: Gas, -) -> Result<(GasMeter, NegativeImbalanceOf), DispatchError> { - // Buy the specified amount of gas. - let gas_price = >::gas_price(); - let cost = if gas_price.is_zero() { - >::zero() - } else { - as TryFrom>::try_from(gas_limit).ok() - .and_then(|gas_limit| gas_price.checked_mul(&gas_limit)) - .ok_or("overflow multiplying gas limit by price")? - }; - - let imbalance = T::Currency::withdraw( - transactor, - cost, - WithdrawReason::Fee.into(), - ExistenceRequirement::KeepAlive - )?; - - Ok((GasMeter::with_limit(gas_limit, gas_price), imbalance)) -} - -/// Refund the unused gas. -pub fn refund_unused_gas( - transactor: &T::AccountId, - gas_meter: GasMeter, - imbalance: NegativeImbalanceOf, -) { - let gas_spent = gas_meter.spent(); - let gas_left = gas_meter.gas_left(); - - // Increase total spent gas. - // This cannot overflow, since `gas_spent` is never greater than `block_gas_limit`, which - // also has Gas type. - GasSpent::mutate(|block_gas_spent| *block_gas_spent += gas_spent); - - // Refund gas left by the price it was bought at. - let refund = gas_meter.gas_price * gas_left.unique_saturated_into(); - let refund_imbalance = T::Currency::deposit_creating(transactor, refund); - if let Ok(imbalance) = imbalance.offset(refund_imbalance) { - T::GasPayment::on_unbalanced(imbalance); - } -} - -/// A little handy utility for converting a value in balance units into approximate value in gas units -/// at the given gas price. -pub fn approx_gas_for_balance(gas_price: Balance, balance: Balance) -> Gas - where Balance: AtLeast32Bit -{ - if gas_price.is_zero() { - Zero::zero() - } else { - (balance / gas_price).saturated_into::() - } -} - /// A simple utility macro that helps to match against a /// list of tokens. #[macro_export] @@ -298,7 +248,7 @@ macro_rules! match_tokens { #[cfg(test)] mod tests { use super::{GasMeter, Token}; - use crate::{tests::Test, gas::approx_gas_for_balance}; + use crate::tests::Test; /// A trivial token that charges the specified number of gas units. #[derive(Copy, Clone, PartialEq, Eq, Debug)] @@ -326,26 +276,24 @@ mod tests { #[test] fn it_works() { - let gas_meter = GasMeter::::with_limit(50000, 10); + let gas_meter = GasMeter::::new(50000); assert_eq!(gas_meter.gas_left(), 50000); } #[test] fn simple() { - let mut gas_meter = GasMeter::::with_limit(50000, 10); + let mut gas_meter = GasMeter::::new(50000); let result = gas_meter .charge(&MultiplierTokenMetadata { multiplier: 3 }, MultiplierToken(10)); assert!(!result.is_out_of_gas()); assert_eq!(gas_meter.gas_left(), 49_970); - assert_eq!(gas_meter.spent(), 30); - assert_eq!(gas_meter.gas_price(), 10); } #[test] fn tracing() { - let mut gas_meter = GasMeter::::with_limit(50000, 10); + let mut gas_meter = GasMeter::::new(50000); assert!(!gas_meter.charge(&(), SimpleToken(1)).is_out_of_gas()); assert!(!gas_meter .charge(&MultiplierTokenMetadata { multiplier: 3 }, MultiplierToken(10)) @@ -358,7 +306,7 @@ mod tests { // This test makes sure that nothing can be executed if there is no gas. #[test] fn refuse_to_execute_anything_if_zero() { - let mut gas_meter = GasMeter::::with_limit(0, 10); + let mut gas_meter = GasMeter::::new(0); assert!(gas_meter.charge(&(), SimpleToken(1)).is_out_of_gas()); } @@ -369,7 +317,7 @@ mod tests { // if the gas meter runs out of gas. However, this is just a nice property to have. #[test] fn overcharge_is_unrecoverable() { - let mut gas_meter = GasMeter::::with_limit(200, 10); + let mut gas_meter = GasMeter::::new(200); // The first charge is should lead to OOG. assert!(gas_meter.charge(&(), SimpleToken(300)).is_out_of_gas()); @@ -383,25 +331,7 @@ mod tests { // possible. #[test] fn charge_exact_amount() { - let mut gas_meter = GasMeter::::with_limit(25, 10); + let mut gas_meter = GasMeter::::new(25); assert!(!gas_meter.charge(&(), SimpleToken(25)).is_out_of_gas()); } - - // A unit test for `fn approx_gas_for_balance()`, and makes - // sure setting gas_price 0 does not cause `div by zero` error. - #[test] - fn approx_gas_for_balance_works() { - let tests = vec![ - (approx_gas_for_balance(0_u64, 123), 0), - (approx_gas_for_balance(0_u64, 456), 0), - (approx_gas_for_balance(1_u64, 123), 123), - (approx_gas_for_balance(1_u64, 456), 456), - (approx_gas_for_balance(100_u64, 900), 9), - (approx_gas_for_balance(123_u64, 900), 7), - ]; - - for (lhs, rhs) in tests { - assert_eq!(lhs, rhs); - } - } } diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 2513f2fb618e2..ce0b93002b9bb 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -64,15 +64,6 @@ //! initialize the contract. //! * `call` - Makes a call to an account, optionally transferring some balance. //! -//! ### Signed Extensions -//! -//! The contracts module defines the following extension: -//! -//! - [`CheckBlockGasLimit`]: Ensures that the transaction does not exceeds the block gas limit. -//! -//! The signed extension needs to be added as signed extra to the transaction type to be used in the -//! runtime. -//! //! ## Usage //! //! The Contract module is a work in progress. The following examples show how this Contract module @@ -114,21 +105,20 @@ use codec::{Codec, Encode, Decode}; use sp_io::hashing::blake2_256; use sp_runtime::{ traits::{ - Hash, StaticLookup, Zero, MaybeSerializeDeserialize, Member, SignedExtension, - DispatchInfoOf, - }, - transaction_validity::{ - ValidTransaction, InvalidTransaction, TransactionValidity, TransactionValidityError, + Hash, StaticLookup, Zero, MaybeSerializeDeserialize, Member, }, RuntimeDebug, }; -use frame_support::dispatch::{DispatchResult, Dispatchable}; +use frame_support::dispatch::{ + PostDispatchInfo, DispatchResult, Dispatchable, DispatchResultWithPostInfo +}; use frame_support::weights::MINIMUM_WEIGHT; use frame_support::{ Parameter, decl_module, decl_event, decl_storage, decl_error, parameter_types, IsSubType, storage::child::{self, ChildInfo}, }; use frame_support::traits::{OnUnbalanced, Currency, Get, Time, Randomness}; +use frame_support::weights::{FunctionOf, DispatchClass, Weight, GetDispatchInfo, Pays}; use frame_system::{self as system, ensure_signed, RawOrigin, ensure_root}; use pallet_contracts_primitives::{RentProjection, ContractAccessError}; @@ -295,9 +285,9 @@ 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`]. @@ -312,35 +302,21 @@ parameter_types! { pub const DefaultRentDepositOffset: u32 = 1000; /// A reasonable default value for [`Trait::SurchargeReward`]. pub const DefaultSurchargeReward: u32 = 150; - /// A reasonable default value for [`Trait::TransferFee`]. - pub const DefaultTransferFee: u32 = 0; - /// A reasonable default value for [`Trait::InstantiationFee`]. - pub const DefaultInstantiationFee: u32 = 0; - /// A reasonable default value for [`Trait::TransactionBaseFee`]. - pub const DefaultTransactionBaseFee: u32 = 0; - /// A reasonable default value for [`Trait::TransactionByteFee`]. - pub const DefaultTransactionByteFee: u32 = 0; - /// A reasonable default value for [`Trait::ContractFee`]. - pub const DefaultContractFee: u32 = 21; - /// A reasonable default value for [`Trait::CallBaseFee`]. - pub const DefaultCallBaseFee: u32 = 1000; - /// A reasonable default value for [`Trait::InstantiateBaseFee`]. - pub const DefaultInstantiateBaseFee: u32 = 1000; /// A reasonable default value for [`Trait::MaxDepth`]. pub const DefaultMaxDepth: u32 = 32; /// A reasonable default value for [`Trait::MaxValueSize`]. pub const DefaultMaxValueSize: u32 = 16_384; - /// A reasonable default value for [`Trait::BlockGasLimit`]. - pub const DefaultBlockGasLimit: u32 = 10_000_000; } -pub trait Trait: frame_system::Trait { - type Currency: Currency; +pub trait Trait: frame_system::Trait + pallet_transaction_payment::Trait { type Time: Time; type Randomness: Randomness; /// The outer call dispatch type. - type Call: Parameter + Dispatchable::Origin> + IsSubType, Self>; + type Call: + Parameter + + Dispatchable::Origin> + + IsSubType, Self> + GetDispatchInfo; /// The overarching event type. type Event: From> + Into<::Event>; @@ -348,18 +324,9 @@ pub trait Trait: frame_system::Trait { /// A function type to get the contract address given the instantiator. type DetermineContractAddress: ContractAddressFor, Self::AccountId>; - /// A function type that computes the fee for dispatching the given `Call`. - /// - /// It is recommended (though not required) for this function to return a fee that would be - /// taken by the Executive module for regular dispatch. - type ComputeDispatchFee: ComputeDispatchFee<::Call, BalanceOf>; - /// trie id generator type TrieIdGenerator: TrieIdGenerator; - /// Handler for the unbalanced reduction when making a gas payment. - type GasPayment: OnUnbalanced>; - /// Handler for rent payments. type RentPayment: OnUnbalanced>; @@ -392,29 +359,11 @@ pub trait Trait: frame_system::Trait { /// to removal of a contract. type SurchargeReward: Get>; - /// The fee to be paid for making a transaction; the base. - type TransactionBaseFee: Get>; - - /// The fee to be paid for making a transaction; the per-byte portion. - type TransactionByteFee: Get>; - - /// The fee required to instantiate a contract instance. - type ContractFee: Get>; - - /// The base fee charged for calling into a contract. - type CallBaseFee: Get; - - /// The base fee charged for instantiating a contract. - type InstantiateBaseFee: Get; - /// The maximum nesting level of a call/instantiate stack. type MaxDepth: Get; /// The maximum size of a storage value in bytes. type MaxValueSize: Get; - - /// The maximum amount of gas that could be expended per block. - type BlockGasLimit: Get; } /// Simple contract address determiner. @@ -440,19 +389,6 @@ where } } -/// The default dispatch fee computor computes the fee in the same way that -/// the implementation of `ChargeTransactionPayment` for the Balances module does. Note that this only takes a fixed -/// fee based on size. Unlike the balances module, weight-fee is applied. -pub struct DefaultDispatchFeeComputor(PhantomData); -impl ComputeDispatchFee<::Call, BalanceOf> for DefaultDispatchFeeComputor { - fn compute_dispatch_fee(call: &::Call) -> BalanceOf { - let encoded_len = call.using_encoded(|encoded| encoded.len() as u32); - let base_fee = T::TransactionBaseFee::get(); - let byte_fee = T::TransactionByteFee::get(); - base_fee + byte_fee * encoded_len.into() - } -} - decl_error! { /// Error for the contracts module. pub enum Error for Module { @@ -505,24 +441,6 @@ decl_module! { /// to removal of a contract. const SurchargeReward: BalanceOf = T::SurchargeReward::get(); - /// The fee to be paid for making a transaction; the base. - const TransactionBaseFee: BalanceOf = T::TransactionBaseFee::get(); - - /// The fee to be paid for making a transaction; the per-byte portion. - const TransactionByteFee: BalanceOf = T::TransactionByteFee::get(); - - /// The fee required to instantiate a contract instance. A reasonable default value - /// is 21. - const ContractFee: BalanceOf = T::ContractFee::get(); - - /// The base fee charged for calling into a contract. A reasonable default - /// value is 135. - const CallBaseFee: Gas = T::CallBaseFee::get(); - - /// The base fee charged for instantiating a contract. A reasonable default value - /// is 175. - const InstantiateBaseFee: Gas = T::InstantiateBaseFee::get(); - /// The maximum nesting level of a call/instantiate stack. A reasonable default /// value is 100. const MaxDepth: u32 = T::MaxDepth::get(); @@ -530,10 +448,6 @@ decl_module! { /// The maximum size of a storage value in bytes. A reasonable default is 16 KiB. const MaxValueSize: u32 = T::MaxValueSize::get(); - /// The maximum amount of gas that could be expended per block. A reasonable - /// default value is 10_000_000. - const BlockGasLimit: Gas = T::BlockGasLimit::get(); - fn deposit_event() = default; /// Updates the schedule for metering contracts. @@ -554,25 +468,24 @@ decl_module! { /// Stores the given binary Wasm code into the chain's storage and returns its `codehash`. /// You can instantiate contracts only with stored code. - #[weight = MINIMUM_WEIGHT] + #[weight = FunctionOf( + |args: (&Weight, &Vec)| args.0 + MINIMUM_WEIGHT, + DispatchClass::Normal, + Pays::Yes + )] pub fn put_code( origin, #[compact] gas_limit: Gas, code: Vec - ) -> DispatchResult { - let origin = ensure_signed(origin)?; - - let (mut gas_meter, imbalance) = gas::buy_gas::(&origin, gas_limit)?; - + ) -> DispatchResultWithPostInfo { + ensure_signed(origin)?; + let mut gas_meter = GasMeter::new(gas_limit); let schedule = >::current_schedule(); let result = wasm::save_code::(code, &mut gas_meter, &schedule); if let Ok(code_hash) = result { Self::deposit_event(RawEvent::CodeStored(code_hash)); } - - gas::refund_unused_gas::(&origin, gas_meter, imbalance); - - result.map(|_| ()).map_err(Into::into) + gas_meter.into_dispatch_result(result) } /// Makes a call to an account, optionally transferring some balance. @@ -582,20 +495,27 @@ decl_module! { /// * 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. - #[weight = MINIMUM_WEIGHT] + #[weight = FunctionOf( + |args: (&::Source, &BalanceOf, &Weight, &Vec)| + args.2 + MINIMUM_WEIGHT, + DispatchClass::Normal, + Pays::Yes + )] pub fn call( origin, dest: ::Source, #[compact] value: BalanceOf, #[compact] gas_limit: Gas, data: Vec - ) -> DispatchResult { + ) -> DispatchResultWithPostInfo { let origin = ensure_signed(origin)?; let dest = T::Lookup::lookup(dest)?; + let mut gas_meter = GasMeter::new(gas_limit); - Self::bare_call(origin, dest, value, gas_limit, data) - .map(|_| ()) - .map_err(|e| e.reason.into()) + let result = Self::execute_wasm(origin, &mut gas_meter, |ctx, gas_meter| { + ctx.call(dest, value, gas_meter, data) + }); + gas_meter.into_dispatch_result(result.map_err(|e| e.reason)) } /// Instantiates a new contract from the `codehash` generated by `put_code`, optionally transferring some balance. @@ -608,22 +528,26 @@ decl_module! { /// after the execution is saved as the `code` of the account. That code will be invoked /// upon any call received by this account. /// - The contract is initialized. - #[weight = MINIMUM_WEIGHT] + #[weight = FunctionOf( + |args: (&BalanceOf, &Weight, &CodeHash, &Vec)| args.1 + MINIMUM_WEIGHT, + DispatchClass::Normal, + Pays::Yes + )] pub fn instantiate( origin, #[compact] endowment: BalanceOf, #[compact] gas_limit: Gas, code_hash: CodeHash, data: Vec - ) -> DispatchResult { + ) -> DispatchResultWithPostInfo { let origin = ensure_signed(origin)?; + let mut gas_meter = GasMeter::new(gas_limit); - Self::execute_wasm(origin, gas_limit, |ctx, gas_meter| { + let result = Self::execute_wasm(origin, &mut gas_meter, |ctx, gas_meter| { ctx.instantiate(endowment, gas_meter, &code_hash, data) .map(|(_address, output)| output) - }) - .map(|_| ()) - .map_err(|e| e.reason.into()) + }); + gas_meter.into_dispatch_result(result.map_err(|e| e.reason)) } /// Allows block producers to claim a small reward for evicting a contract. If a block producer @@ -658,10 +582,6 @@ decl_module! { T::Currency::deposit_into_existing(&rewarded, T::SurchargeReward::get())?; } } - - fn on_finalize() { - GasSpent::kill(); - } } } @@ -678,7 +598,8 @@ impl Module { gas_limit: Gas, input_data: Vec, ) -> ExecResult { - Self::execute_wasm(origin, gas_limit, |ctx, gas_meter| { + let mut gas_meter = GasMeter::new(gas_limit); + Self::execute_wasm(origin, &mut gas_meter, |ctx, gas_meter| { ctx.call(dest, value, gas_meter, input_data) }) } @@ -712,38 +633,21 @@ impl Module { impl Module { fn execute_wasm( origin: T::AccountId, - gas_limit: Gas, + gas_meter: &mut GasMeter, func: impl FnOnce(&mut ExecutionContext, &mut GasMeter) -> ExecResult ) -> ExecResult { - // Pay for the gas upfront. - // - // NOTE: it is very important to avoid any state changes before - // paying for the gas. - let (mut gas_meter, imbalance) = - try_or_exec_error!( - gas::buy_gas::(&origin, gas_limit), - // We don't have a spare buffer here in the first place, so create a new empty one. - Vec::new() - ); - let cfg = Config::preload(); let vm = WasmVm::new(&cfg.schedule); let loader = WasmLoader::new(&cfg.schedule); let mut ctx = ExecutionContext::top_level(origin.clone(), &cfg, &vm, &loader); - let result = func(&mut ctx, &mut gas_meter); + let result = func(&mut ctx, gas_meter); if result.as_ref().map(|output| output.is_success()).unwrap_or(false) { // Commit all changes that made it thus far into the persistent storage. DirectAccountDb.commit(ctx.overlay.into_change_set()); } - // Refund cost of the unused gas. - // - // NOTE: This should go after the commit to the storage, since the storage changes - // can alter the balance of the caller. - gas::refund_unused_gas::(&origin, gas_meter, imbalance); - // Execute deferred actions. ctx.deferred.into_iter().for_each(|deferred| { use self::exec::DeferredAction::*; @@ -759,7 +663,13 @@ impl Module { origin: who, call, } => { + let info = call.get_dispatch_info(); let result = call.dispatch(RawOrigin::Signed(who.clone()).into()); + let post_info = match result { + Ok(post_info) => post_info, + Err(err) => err.post_info, + }; + gas_meter.refund(post_info.calc_unspent(&info)); Self::deposit_event(RawEvent::Dispatched(who, result.is_ok())); } RestoreTo { @@ -917,8 +827,6 @@ decl_event! { decl_storage! { trait Store for Module as Contracts { - /// Gas spent so far in this block. - GasSpent get(fn gas_spent): Gas; /// Current cost schedule for contracts. CurrentSchedule get(fn current_schedule) config(): Schedule = Schedule::default(); /// A mapping from an original code hash to the original code, untouched by instrumentation. @@ -929,8 +837,6 @@ decl_storage! { pub AccountCounter: u64 = 0; /// The code associated with a given account. pub ContractInfoOf: map hasher(twox_64_concat) T::AccountId => Option>; - /// The price of one unit of gas. - GasPrice get(fn gas_price) config(): BalanceOf = 1.into(); } } @@ -944,7 +850,6 @@ pub struct Config { pub tombstone_deposit: BalanceOf, pub max_depth: u32, pub max_value_size: u32, - pub contract_account_instantiate_fee: BalanceOf, } impl Config { @@ -955,7 +860,6 @@ impl Config { tombstone_deposit: T::TombstoneDeposit::get(), max_depth: T::MaxDepth::get(), max_value_size: T::MaxValueSize::get(), - contract_account_instantiate_fee: T::ContractFee::get(), } } } @@ -994,6 +898,9 @@ pub struct Schedule { /// Base gas cost to instantiate a contract. pub instantiate_base_cost: Gas, + /// Base gas cost to dispatch a runtime call. + pub dispatch_base_cost: Gas, + /// Gas cost per one byte read from the sandbox memory. pub sandbox_data_read_cost: Gas, @@ -1003,6 +910,9 @@ pub struct Schedule { /// Cost for a simple balance transfer. pub transfer_cost: Gas, + /// Cost for instantiating a new contract. + pub instantiate_cost: Gas, + /// The maximum number of topics supported by an event. pub max_event_topics: u32, @@ -1038,10 +948,12 @@ impl Default for Schedule { event_per_topic_cost: 1, event_base_cost: 1, call_base_cost: 135, + dispatch_base_cost: 135, instantiate_base_cost: 175, sandbox_data_read_cost: 1, sandbox_data_write_cost: 1, transfer_cost: 100, + instantiate_cost: 200, max_event_topics: 4, max_stack_height: 64 * 1024, max_memory_pages: 16, @@ -1052,67 +964,3 @@ impl Default for Schedule { } } -/// `SignedExtension` that checks if a transaction would exhausts the block gas limit. -#[derive(Encode, Decode, Clone, Eq, PartialEq)] -pub struct CheckBlockGasLimit(PhantomData); - -impl Default for CheckBlockGasLimit { - fn default() -> Self { - Self(PhantomData) - } -} - -impl sp_std::fmt::Debug for CheckBlockGasLimit { - #[cfg(feature = "std")] - fn fmt(&self, f: &mut sp_std::fmt::Formatter) -> sp_std::fmt::Result { - write!(f, "CheckBlockGasLimit") - } - - #[cfg(not(feature = "std"))] - fn fmt(&self, _: &mut sp_std::fmt::Formatter) -> sp_std::fmt::Result { - Ok(()) - } -} - -impl SignedExtension for CheckBlockGasLimit { - const IDENTIFIER: &'static str = "CheckBlockGasLimit"; - type AccountId = T::AccountId; - type Call = ::Call; - type AdditionalSigned = (); - type Pre = (); - - fn additional_signed(&self) -> sp_std::result::Result<(), TransactionValidityError> { Ok(()) } - - fn validate( - &self, - _: &Self::AccountId, - call: &Self::Call, - _: &DispatchInfoOf, - _: usize, - ) -> TransactionValidity { - let call = match call.is_sub_type() { - Some(call) => call, - None => return Ok(ValidTransaction::default()), - }; - - match call { - Call::claim_surcharge(_, _) | Call::update_schedule(_) => - Ok(ValidTransaction::default()), - Call::put_code(gas_limit, _) - | Call::call(_, _, gas_limit, _) - | Call::instantiate(_, gas_limit, _, _) - => { - // Check if the specified amount of gas is available in the current block. - // This cannot underflow since `gas_spent` is never greater than `T::BlockGasLimit`. - let gas_available = T::BlockGasLimit::get() - >::gas_spent(); - if *gas_limit > gas_available { - // gas limit reached, revert the transaction and retry again in the future - InvalidTransaction::ExhaustsResources.into() - } else { - Ok(ValidTransaction::default()) - } - }, - Call::__PhantomItem(_, _) => unreachable!("Variant is never constructed"), - } - } -} diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index 452a4517dbe68..30a516fc6ff53 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -22,20 +22,21 @@ use crate::{ BalanceOf, ComputeDispatchFee, ContractAddressFor, ContractInfo, ContractInfoOf, GenesisConfig, Module, RawAliveContractInfo, RawEvent, Trait, TrieId, TrieIdFromParentCounter, Schedule, - TrieIdGenerator, CheckBlockGasLimit, account_db::{AccountDb, DirectAccountDb, OverlayAccountDb}, + TrieIdGenerator, account_db::{AccountDb, DirectAccountDb, OverlayAccountDb}, }; use assert_matches::assert_matches; use hex_literal::*; use codec::{Decode, Encode, KeyedVec}; use sp_runtime::{ Perbill, BuildStorage, transaction_validity::{InvalidTransaction, ValidTransaction}, - traits::{BlakeTwo256, Hash, IdentityLookup, SignedExtension}, + traits::{BlakeTwo256, Hash, IdentityLookup, SignedExtension, Convert}, testing::{Digest, DigestItem, Header, UintAuthorityId, H256}, }; use frame_support::{ - assert_ok, assert_err, impl_outer_dispatch, impl_outer_event, impl_outer_origin, parameter_types, + assert_ok, assert_err, assert_err_ignore_postinfo, impl_outer_dispatch, impl_outer_event, + impl_outer_origin, parameter_types, storage::child, StorageMap, StorageValue, traits::{Currency, Get}, - weights::{DispatchInfo, DispatchClass, Weight, Pays}, + weights::{DispatchInfo, DispatchClass, Weight, PostDispatchInfo, Pays}, }; use std::{cell::RefCell, sync::atomic::{AtomicUsize, Ordering}}; use sp_core::storage::well_known_keys; @@ -70,9 +71,6 @@ impl_outer_dispatch! { thread_local! { static EXISTENTIAL_DEPOSIT: RefCell = RefCell::new(0); - static TRANSFER_FEE: RefCell = RefCell::new(0); - static INSTANTIATION_FEE: RefCell = RefCell::new(0); - static BLOCK_GAS_LIMIT: RefCell = RefCell::new(0); } pub struct ExistentialDeposit; @@ -80,16 +78,6 @@ impl Get for ExistentialDeposit { fn get() -> u64 { EXISTENTIAL_DEPOSIT.with(|v| *v.borrow()) } } -pub struct TransferFee; -impl Get for TransferFee { - fn get() -> u64 { TRANSFER_FEE.with(|v| *v.borrow()) } -} - -pub struct BlockGasLimit; -impl Get for BlockGasLimit { - fn get() -> u64 { BLOCK_GAS_LIMIT.with(|v| *v.borrow()) } -} - #[derive(Clone, Eq, PartialEq, Debug)] pub struct Test; parameter_types! { @@ -142,24 +130,37 @@ parameter_types! { pub const RentByteFee: u64 = 4; pub const RentDepositOffset: u64 = 10_000; pub const SurchargeReward: u64 = 150; - pub const TransactionBaseFee: u64 = 2; - pub const TransactionByteFee: u64 = 6; - pub const ContractFee: u64 = 21; - pub const CallBaseFee: u64 = 135; - pub const InstantiateBaseFee: u64 = 175; pub const MaxDepth: u32 = 100; pub const MaxValueSize: u32 = 16_384; } -impl Trait for Test { + +parameter_types! { + pub const TransactionBaseFee: u64 = 0; + pub const TransactionByteFee: u64 = 0; +} + +impl Convert> for Test { + fn convert(w: Weight) -> BalanceOf { + w + } +} + +impl pallet_transaction_payment::Trait for Test { type Currency = Balances; + type OnTransactionPayment = (); + type TransactionBaseFee = TransactionBaseFee; + type TransactionByteFee = TransactionByteFee; + type WeightToFee = Test; + type FeeMultiplierUpdate = (); +} + +impl Trait for Test { type Time = Timestamp; type Randomness = Randomness; type Call = Call; type DetermineContractAddress = DummyContractAddressFor; type Event = MetaEvent; - type ComputeDispatchFee = DummyComputeDispatchFee; type TrieIdGenerator = DummyTrieIdGenerator; - type GasPayment = (); type RentPayment = (); type SignedClaimHandicap = SignedClaimHandicap; type TombstoneDeposit = TombstoneDeposit; @@ -167,14 +168,8 @@ impl Trait for Test { type RentByteFee = RentByteFee; type RentDepositOffset = RentDepositOffset; type SurchargeReward = SurchargeReward; - type TransactionBaseFee = TransactionBaseFee; - type TransactionByteFee = TransactionByteFee; - type ContractFee = ContractFee; - type CallBaseFee = CallBaseFee; - type InstantiateBaseFee = InstantiateBaseFee; type MaxDepth = MaxDepth; type MaxValueSize = MaxValueSize; - type BlockGasLimit = BlockGasLimit; } type Balances = pallet_balances::Module; @@ -221,19 +216,11 @@ const DJANGO: u64 = 4; pub struct ExtBuilder { existential_deposit: u64, - gas_price: u64, - block_gas_limit: u64, - transfer_fee: u64, - instantiation_fee: u64, } impl Default for ExtBuilder { fn default() -> Self { Self { existential_deposit: 1, - gas_price: 2, - block_gas_limit: 100_000_000, - transfer_fee: 0, - instantiation_fee: 0, } } } @@ -242,27 +229,8 @@ impl ExtBuilder { self.existential_deposit = existential_deposit; self } - pub fn gas_price(mut self, gas_price: u64) -> Self { - self.gas_price = gas_price; - self - } - pub fn block_gas_limit(mut self, block_gas_limit: u64) -> Self { - self.block_gas_limit = block_gas_limit; - self - } - pub fn transfer_fee(mut self, transfer_fee: u64) -> Self { - self.transfer_fee = transfer_fee; - self - } - pub fn instantiation_fee(mut self, instantiation_fee: u64) -> Self { - self.instantiation_fee = instantiation_fee; - self - } pub fn set_associated_consts(&self) { EXISTENTIAL_DEPOSIT.with(|v| *v.borrow_mut() = self.existential_deposit); - TRANSFER_FEE.with(|v| *v.borrow_mut() = self.transfer_fee); - INSTANTIATION_FEE.with(|v| *v.borrow_mut() = self.instantiation_fee); - BLOCK_GAS_LIMIT.with(|v| *v.borrow_mut() = self.block_gas_limit); } pub fn build(self) -> sp_io::TestExternalities { self.set_associated_consts(); @@ -270,12 +238,11 @@ impl ExtBuilder { pallet_balances::GenesisConfig:: { balances: vec![], }.assimilate_storage(&mut t).unwrap(); - GenesisConfig:: { + GenesisConfig { current_schedule: Schedule { enable_println: true, ..Default::default() }, - gas_price: self.gas_price, }.assimilate_storage(&mut t).unwrap(); let mut ext = sp_io::TestExternalities::new(t); ext.execute_with(|| System::set_block_number(1)); @@ -293,17 +260,21 @@ fn compile_module(wabt_module: &str) Ok((wasm, code_hash)) } -// Perform a simple transfer to a non-existent account supplying way more gas than needed. -// Then we check that the all unused gas is refunded. +// Perform a simple transfer to a non-existent account. +// Then we check that only the base costs are returned as actual costs. #[test] -fn refunds_unused_gas() { - ExtBuilder::default().gas_price(2).build().execute_with(|| { +fn returns_base_call_cost() { + ExtBuilder::default().build().execute_with(|| { Balances::deposit_creating(&ALICE, 100_000_000); - assert_ok!(Contracts::call(Origin::signed(ALICE), BOB, 0, 100_000, Vec::new())); - - // 2 * 135 - gas price multiplied by the call base fee. - assert_eq!(Balances::free_balance(ALICE), 100_000_000 - (2 * 135)); + assert_eq!( + Contracts::call(Origin::signed(ALICE), BOB, 0, 100_000, Vec::new()), + Ok( + PostDispatchInfo { + actual_weight: Some(135), + } + ) + ); }); } @@ -496,7 +467,7 @@ fn dispatch_call() { Origin::signed(ALICE), BOB, // newly created account 0, - 100_000, + 1_000_000_000_000, vec![], )); @@ -615,12 +586,12 @@ fn dispatch_call_not_dispatched_after_top_level_transaction_failure() { // Call the newly instantiated contract. The contract is expected to dispatch a call // and then trap. - assert_err!( + assert_err_ignore_postinfo!( Contracts::call( Origin::signed(ALICE), BOB, // newly created account 0, - 100_000, + 1_000_000_000_000, vec![], ), "contract trapped during execution" @@ -691,7 +662,7 @@ fn run_out_of_gas() { // Call the contract with a fixed gas limit. It must run out of gas because it just // loops forever. - assert_err!( + assert_err_ignore_postinfo!( Contracts::call( Origin::signed(ALICE), BOB, // newly created account @@ -1006,7 +977,7 @@ fn removals(trigger_call: impl Fn() -> bool) { assert_eq!(Balances::free_balance(BOB), 50 + Balances::minimum_balance()); // Transfer funds - assert_ok!(Contracts::call(Origin::signed(ALICE), BOB, 0, 100_000, call::transfer())); + assert_ok!(Contracts::call(Origin::signed(ALICE), BOB, 0, 100_000_000_000, call::transfer())); assert_eq!(ContractInfoOf::::get(BOB).unwrap().get_alive().unwrap().rent_allowance, 1_000); assert_eq!(Balances::free_balance(BOB), Balances::minimum_balance()); @@ -1051,7 +1022,7 @@ fn call_removed_contract() { initialize_block(10); // Calling contract should remove contract and fail. - assert_err!( + assert_err_ignore_postinfo!( Contracts::call(Origin::signed(ALICE), BOB, 0, 100_000, call::null()), "contract has been evicted" ); @@ -1065,7 +1036,7 @@ fn call_removed_contract() { ]); // Subsequent contract calls should also fail. - assert_err!( + assert_err_ignore_postinfo!( Contracts::call(Origin::signed(ALICE), BOB, 0, 100_000, call::null()), "contract has been evicted" ); @@ -1192,7 +1163,7 @@ fn restoration(test_different_storage: bool, test_restore_to_with_dirty_storage: .get_alive().unwrap().code_hash; // Call `BOB`, which makes it pay rent. Since the rent allowance is set to 0 // we expect that it will get removed leaving tombstone. - assert_err!( + assert_err_ignore_postinfo!( Contracts::call(Origin::signed(ALICE), BOB, 0, 100_000, call::null()), "contract has been evicted" ); @@ -1372,7 +1343,7 @@ fn storage_max_value_limit() { )); // Call contract with too large a storage value. - assert_err!( + assert_err_ignore_postinfo!( Contracts::call( Origin::signed(ALICE), BOB, @@ -1418,23 +1389,6 @@ fn deploy_and_call_other_contract() { }); } -#[test] -fn deploy_works_without_gas_price() { - let (wasm, code_hash) = compile_module::(&load_wasm("get_runtime_storage.wat")) - .unwrap(); - ExtBuilder::default().existential_deposit(50).gas_price(0).build().execute_with(|| { - Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), 100_000, wasm)); - assert_ok!(Contracts::instantiate( - Origin::signed(ALICE), - 100, - 100_000, - code_hash.into(), - vec![], - )); - }); -} - #[test] fn cannot_self_destruct_through_draning() { let (wasm, code_hash) = compile_module::(&load_wasm("drain.wat")).unwrap(); @@ -1459,7 +1413,7 @@ fn cannot_self_destruct_through_draning() { // Call BOB with no input data, forcing it to run until out-of-balance // and eventually trapping because below existential deposit. - assert_err!( + assert_err_ignore_postinfo!( Contracts::call( Origin::signed(ALICE), BOB, @@ -1497,7 +1451,7 @@ fn cannot_self_destruct_while_live() { // Call BOB with input data, forcing it make a recursive call to itself to // self-destruct, resulting in a trap. - assert_err!( + assert_err_ignore_postinfo!( Contracts::call( Origin::signed(ALICE), BOB, @@ -1548,7 +1502,7 @@ fn self_destruct_works() { 100_000, vec![], ), - Ok(()) + Ok(_) ); // Check that account is gone @@ -1614,7 +1568,7 @@ fn cannot_self_destruct_in_constructor() { // Fail to instantiate the BOB because the call that is issued in the deploy // function exhausts all balances which puts it below the existential deposit. - assert_err!( + assert_err_ignore_postinfo!( Contracts::instantiate( Origin::signed(ALICE), 100_000, @@ -1627,23 +1581,6 @@ fn cannot_self_destruct_in_constructor() { }); } -#[test] -fn check_block_gas_limit_works() { - ExtBuilder::default().block_gas_limit(50).build().execute_with(|| { - let info = DispatchInfo { weight: 100, class: DispatchClass::Normal, pays_fee: Pays::Yes }; - let check = CheckBlockGasLimit::(Default::default()); - let call: Call = crate::Call::put_code(1000, vec![]).into(); - - assert_eq!( - check.validate(&0, &call, &info, 0), InvalidTransaction::ExhaustsResources.into(), - ); - - let call: Call = crate::Call::update_schedule(Default::default()).into(); - assert_eq!(check.validate(&0, &call, &info, 0), Ok(Default::default())); - }); -} - -#[test] fn get_runtime_storage() { let (wasm, code_hash) = compile_module::(&load_wasm("get_runtime_storage.wat")) .unwrap(); diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index 8911fb72b6130..b2fbce09e737e 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -157,7 +157,7 @@ mod tests { use crate::gas::{Gas, GasMeter}; use crate::tests::{Test, Call}; use crate::wasm::prepare::prepare_contract; - use crate::CodeHash; + use crate::{CodeHash, BalanceOf}; use wabt; use hex_literal::hex; use assert_matches::assert_matches; @@ -373,6 +373,9 @@ mod tests { ) ) } + fn get_weight_price(&self) -> BalanceOf { + 1312_u32.into() + } } impl Ext for &mut MockExt { @@ -478,6 +481,9 @@ 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 execute( @@ -544,7 +550,7 @@ mod tests { CODE_TRANSFER, vec![], &mut mock_ext, - &mut GasMeter::with_limit(50_000, 1), + &mut GasMeter::new(50_000), ).unwrap(); assert_eq!( @@ -604,7 +610,7 @@ mod tests { CODE_CALL, vec![], &mut mock_ext, - &mut GasMeter::with_limit(50_000, 1), + &mut GasMeter::new(50_000), ).unwrap(); assert_eq!( @@ -666,7 +672,7 @@ mod tests { CODE_INSTANTIATE, vec![], &mut mock_ext, - &mut GasMeter::with_limit(50_000, 1), + &mut GasMeter::new(50_000), ).unwrap(); assert_eq!( @@ -709,7 +715,7 @@ mod tests { CODE_TERMINATE, vec![], &mut mock_ext, - &mut GasMeter::with_limit(50_000, 1), + &mut GasMeter::new(50_000), ).unwrap(); assert_eq!( @@ -767,7 +773,7 @@ mod tests { &CODE_TRANSFER_LIMITED_GAS, vec![], &mut mock_ext, - &mut GasMeter::with_limit(50_000, 1), + &mut GasMeter::new(50_000), ).unwrap(); assert_eq!( @@ -860,7 +866,7 @@ mod tests { CODE_GET_STORAGE, vec![], mock_ext, - &mut GasMeter::with_limit(50_000, 1), + &mut GasMeter::new(50_000), ).unwrap(); assert_eq!(output, ExecReturnValue { status: STATUS_SUCCESS, data: [0x22; 32].to_vec() }); @@ -924,7 +930,7 @@ mod tests { CODE_CALLER, vec![], MockExt::default(), - &mut GasMeter::with_limit(50_000, 1), + &mut GasMeter::new(50_000), ).unwrap(); } @@ -986,7 +992,7 @@ mod tests { CODE_ADDRESS, vec![], MockExt::default(), - &mut GasMeter::with_limit(50_000, 1), + &mut GasMeter::new(50_000), ).unwrap(); } @@ -1041,7 +1047,7 @@ mod tests { #[test] fn balance() { - let mut gas_meter = GasMeter::with_limit(50_000, 1); + let mut gas_meter = GasMeter::new(50_000); let _ = execute( CODE_BALANCE, vec![], @@ -1101,7 +1107,7 @@ mod tests { #[test] fn gas_price() { - let mut gas_meter = GasMeter::with_limit(50_000, 1312); + let mut gas_meter = GasMeter::new(50_000); let _ = execute( CODE_GAS_PRICE, vec![], @@ -1159,7 +1165,7 @@ mod tests { #[test] fn gas_left() { - let mut gas_meter = GasMeter::with_limit(50_000, 1312); + let mut gas_meter = GasMeter::new(50_000); let output = execute( CODE_GAS_LEFT, @@ -1224,7 +1230,7 @@ mod tests { #[test] fn value_transferred() { - let mut gas_meter = GasMeter::with_limit(50_000, 1); + let mut gas_meter = GasMeter::new(50_000); let _ = execute( CODE_VALUE_TRANSFERRED, vec![], @@ -1260,7 +1266,7 @@ mod tests { CODE_DISPATCH_CALL, vec![], &mut mock_ext, - &mut GasMeter::with_limit(50_000, 1), + &mut GasMeter::new(1_000_000_000_000), ).unwrap(); assert_eq!( @@ -1300,7 +1306,7 @@ mod tests { CODE_RETURN_FROM_START_FN, vec![], MockExt::default(), - &mut GasMeter::with_limit(50_000, 1), + &mut GasMeter::new(50_000), ).unwrap(); assert_eq!(output, ExecReturnValue { status: STATUS_SUCCESS, data: vec![1, 2, 3, 4] }); @@ -1357,7 +1363,7 @@ mod tests { #[test] fn now() { - let mut gas_meter = GasMeter::with_limit(50_000, 1); + let mut gas_meter = GasMeter::new(50_000); let _ = execute( CODE_TIMESTAMP_NOW, vec![], @@ -1416,7 +1422,7 @@ mod tests { #[test] fn minimum_balance() { - let mut gas_meter = GasMeter::with_limit(50_000, 1); + let mut gas_meter = GasMeter::new(50_000); let _ = execute( CODE_MINIMUM_BALANCE, vec![], @@ -1475,7 +1481,7 @@ mod tests { #[test] fn tombstone_deposit() { - let mut gas_meter = GasMeter::with_limit(50_000, 1); + let mut gas_meter = GasMeter::new(50_000); let _ = execute( CODE_TOMBSTONE_DEPOSIT, vec![], @@ -1543,7 +1549,7 @@ mod tests { #[test] fn random() { - let mut gas_meter = GasMeter::with_limit(50_000, 1); + let mut gas_meter = GasMeter::new(50_000); let output = execute( CODE_RANDOM, @@ -1588,7 +1594,7 @@ mod tests { #[test] fn deposit_event() { let mut mock_ext = MockExt::default(); - let mut gas_meter = GasMeter::with_limit(50_000, 1); + let mut gas_meter = GasMeter::new(50_000); let _ = execute( CODE_DEPOSIT_EVENT, vec![], @@ -1634,7 +1640,7 @@ mod tests { #[test] fn deposit_event_max_topics() { // Checks that the runtime traps if there are more than `max_topic_events` topics. - let mut gas_meter = GasMeter::with_limit(50_000, 1); + let mut gas_meter = GasMeter::new(50_000); assert_matches!( execute( @@ -1678,7 +1684,7 @@ mod tests { #[test] fn deposit_event_duplicates() { // Checks that the runtime traps if there are duplicates. - let mut gas_meter = GasMeter::with_limit(50_000, 1); + let mut gas_meter = GasMeter::new(50_000); assert_matches!( execute( @@ -1749,7 +1755,7 @@ mod tests { CODE_BLOCK_NUMBER, vec![], MockExt::default(), - &mut GasMeter::with_limit(50_000, 1), + &mut GasMeter::new(50_000), ).unwrap(); } @@ -1789,7 +1795,7 @@ mod tests { CODE_SIMPLE_ASSERT, input_data, MockExt::default(), - &mut GasMeter::with_limit(50_000, 1), + &mut GasMeter::new(50_000), ).unwrap(); assert_eq!(output.data.len(), 0); @@ -1805,7 +1811,7 @@ mod tests { CODE_SIMPLE_ASSERT, input_data, MockExt::default(), - &mut GasMeter::with_limit(50_000, 1), + &mut GasMeter::new(50_000), ).err().unwrap(); assert_eq!(error.buffer.capacity(), 1_234); @@ -1859,7 +1865,7 @@ mod tests { CODE_RETURN_WITH_DATA, hex!("00112233445566778899").to_vec(), MockExt::default(), - &mut GasMeter::with_limit(50_000, 1), + &mut GasMeter::new(50_000), ).unwrap(); assert_eq!(output, ExecReturnValue { status: 0, data: hex!("445566778899").to_vec() }); @@ -1872,7 +1878,7 @@ mod tests { CODE_RETURN_WITH_DATA, hex!("112233445566778899").to_vec(), MockExt::default(), - &mut GasMeter::with_limit(50_000, 1), + &mut GasMeter::new(50_000), ).unwrap(); assert_eq!(output, ExecReturnValue { status: 17, data: hex!("5566778899").to_vec() }); @@ -1958,7 +1964,7 @@ mod tests { #[test] fn get_runtime_storage() { - let mut gas_meter = GasMeter::with_limit(50_000, 1); + let mut gas_meter = GasMeter::new(50_000); let mock_ext = MockExt::default(); // "\01\02\03\04" - Some(0x14144020) diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index 7cede5542fc6f..b7c13b2f7bbb6 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -16,11 +16,11 @@ //! Environment definition of the wasm smart-contract runtime. -use crate::{Schedule, Trait, CodeHash, ComputeDispatchFee, BalanceOf}; +use crate::{Schedule, Trait, CodeHash, BalanceOf}; use crate::exec::{ Ext, ExecResult, ExecError, ExecReturnValue, StorageKey, TopicOf, STATUS_SUCCESS, }; -use crate::gas::{Gas, GasMeter, Token, GasMeterResult, approx_gas_for_balance}; +use crate::gas::{Gas, GasMeter, Token, GasMeterResult}; use sp_sandbox; use frame_system; use sp_std::{prelude::*, mem, convert::TryInto}; @@ -32,6 +32,7 @@ use sp_io::hashing::{ blake2_128, sha2_256, }; +use frame_support::weights::GetDispatchInfo; /// The value returned from ext_call and ext_instantiate contract external functions if the call or /// instantiation traps. This value is chosen as if the execution does not trap, the return value @@ -153,8 +154,8 @@ pub enum RuntimeToken { /// The given number of bytes is read from the sandbox memory and /// is returned as the return data buffer of the call. ReturnData(u32), - /// Dispatch fee calculated by `T::ComputeDispatchFee`. - ComputedDispatchFee(Gas), + // Dispatched a call with the given weight. + DispatchWithWeight(Gas), /// (topic_count, data_bytes): A buffer of the given size is posted as an event indexed with the /// given number of topics. DepositEvent(u32, u32), @@ -195,7 +196,7 @@ impl Token for RuntimeToken { data_and_topics_cost.checked_add(metadata.event_base_cost) ) }, - ComputedDispatchFee(gas) => Some(gas), + DispatchWithWeight(gas) => gas.checked_add(metadata.dispatch_base_cost), }; value.unwrap_or_else(|| Bounded::max_value()) @@ -692,7 +693,7 @@ define_env!(Env, , // The data is encoded as T::Balance. The current contents of the scratch buffer are overwritten. ext_gas_price(ctx) => { ctx.scratch_buf.clear(); - ctx.gas_meter.gas_price().encode_to(&mut ctx.scratch_buf); + ctx.ext.get_weight_price().encode_to(&mut ctx.scratch_buf); Ok(()) }, @@ -783,16 +784,14 @@ define_env!(Env, , let call: <::T as Trait>::Call = read_sandbox_memory_as(ctx, call_ptr, call_len)?; - // Charge gas for dispatching this call. - let fee = { - let balance_fee = <::T as Trait>::ComputeDispatchFee::compute_dispatch_fee(&call); - approx_gas_for_balance(ctx.gas_meter.gas_price(), balance_fee) - }; + // We already deducted the len costs when reading from the sandbox. + // Bill on the actual weight of the dispatched call. + let info = call.get_dispatch_info(); charge_gas( &mut ctx.gas_meter, ctx.schedule, &mut ctx.special_trap, - RuntimeToken::ComputedDispatchFee(fee) + RuntimeToken::DispatchWithWeight(info.weight) )?; ctx.ext.note_dispatch_call(call); diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index df987e0b0bac6..18a071c6de4d0 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -193,10 +193,6 @@ macro_rules! assert_noop { } } -/// Panic if an expression doesn't evaluate to an `Err`. -/// -/// Used as `assert_err!(expression_to_assert, expected_err_expression)`. - /// Assert an expression returns an error specified. /// /// Used as `assert_err!(expression_to_assert, expected_error_expression)` @@ -208,6 +204,18 @@ macro_rules! assert_err { } } +/// Assert an expression returns an error specified +/// +/// This can be used on`DispatchResultWithPostInfo` when the post info should +/// be ignored. +#[macro_export] +#[cfg(feature = "std")] +macro_rules! assert_err_ignore_postinfo { + ( $x:expr , $y:expr $(,)? ) => { + assert_err!($x.map(|_| ()).map_err(|e| e.error), $y); + } +} + /// Panic if an expression doesn't evaluate to `Ok`. /// /// Used as `assert_ok!(expression_to_assert, expected_ok_expression)`, diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index 95845990b8e43..6caca4c23ba36 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -47,7 +47,7 @@ use sp_runtime::{ }, traits::{ Zero, Saturating, SignedExtension, SaturatedConversion, Convert, Dispatchable, - DispatchInfoOf, PostDispatchInfoOf, + DispatchInfoOf, PostDispatchInfoOf, UniqueSaturatedFrom, UniqueSaturatedInto, }, }; use pallet_transaction_payment_rpc_runtime_api::RuntimeDispatchInfo; @@ -116,9 +116,7 @@ decl_module! { } } -impl Module where - T::Call: Dispatchable, -{ +impl Module { /// Query the data that we know about the fee of a given `call`. /// /// As this module is not and cannot be aware of the internals of a signed extension, it only @@ -133,6 +131,7 @@ impl Module where where T: Send + Sync, BalanceOf: Send + Sync, + T::Call: Dispatchable, { // NOTE: we can actually make it understand `ChargeTransactionPayment`, but would be some // hassle for sure. We have to make it aware of the index of `ChargeTransactionPayment` in @@ -165,7 +164,9 @@ impl Module where len: u32, info: &DispatchInfoOf, tip: BalanceOf, - ) -> BalanceOf { + ) -> BalanceOf where + T::Call: Dispatchable, + { if info.pays_fee == Pays::Yes { let len = >::from(len); let per_byte = T::TransactionByteFee::get(); @@ -188,11 +189,12 @@ impl Module where /// /// 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. - pub fn weight_to_fee_with_adjustment(weight: Weight) -> BalanceOf where - BalanceOf: From + pub fn weight_to_fee_with_adjustment(weight: Weight) -> Balance where + Balance: UniqueSaturatedFrom { - NextFeeMultiplier::get().saturated_multiply_accumulate( - Self::weight_to_fee(weight) + let fee = UniqueSaturatedInto::::unique_saturated_into(Self::weight_to_fee(weight)); + UniqueSaturatedFrom::unique_saturated_from( + NextFeeMultiplier::get().saturated_multiply_accumulate(fee) ) } From 3f5b8a1784af51d73d117c1495abe312a37266ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Tue, 21 Apr 2020 14:14:50 +0200 Subject: [PATCH 2/8] Rescale the schedule to new gas definitaion (=1 weight) --- frame/contracts/src/lib.rs | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index ce0b93002b9bb..8d89e99492cfe 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -936,24 +936,27 @@ pub struct Schedule { pub max_subject_len: u32, } +// This is 500 (2 instruction per nano second on 2GHZ) * 1000x slowdown through wasmi +const WASM_INSTRUCTION_COST: Gas = 500_000; + impl Default for Schedule { fn default() -> Schedule { Schedule { version: 0, - put_code_per_byte_cost: 1, - grow_mem_cost: 1, - regular_op_cost: 1, - return_data_per_byte_cost: 1, - event_data_per_byte_cost: 1, - event_per_topic_cost: 1, - event_base_cost: 1, - call_base_cost: 135, - dispatch_base_cost: 135, - instantiate_base_cost: 175, - sandbox_data_read_cost: 1, - sandbox_data_write_cost: 1, - transfer_cost: 100, - instantiate_cost: 200, + put_code_per_byte_cost: WASM_INSTRUCTION_COST, + grow_mem_cost: WASM_INSTRUCTION_COST, + regular_op_cost: WASM_INSTRUCTION_COST, + return_data_per_byte_cost: WASM_INSTRUCTION_COST, + event_data_per_byte_cost: WASM_INSTRUCTION_COST, + event_per_topic_cost: WASM_INSTRUCTION_COST, + event_base_cost: WASM_INSTRUCTION_COST, + call_base_cost: 135 * WASM_INSTRUCTION_COST, + dispatch_base_cost: 135 * WASM_INSTRUCTION_COST, + instantiate_base_cost: 175 * WASM_INSTRUCTION_COST, + sandbox_data_read_cost: WASM_INSTRUCTION_COST, + sandbox_data_write_cost: WASM_INSTRUCTION_COST, + transfer_cost: 100 * WASM_INSTRUCTION_COST, + instantiate_cost: 200 * WASM_INSTRUCTION_COST, max_event_topics: 4, max_stack_height: 64 * 1024, max_memory_pages: 16, From 32226e57ac5dd21ca9bbb21d4add180e4dfb24d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Wed, 22 Apr 2020 13:00:05 +0200 Subject: [PATCH 3/8] Increase supplied gas_limit in tests --- bin/node/executor/tests/basic.rs | 11 ++- frame/contracts/src/exec.rs | 49 +++++----- frame/contracts/src/tests.rs | 157 ++++++++++++++++--------------- frame/contracts/src/wasm/mod.rs | 68 ++++++------- 4 files changed, 149 insertions(+), 136 deletions(-) diff --git a/bin/node/executor/tests/basic.rs b/bin/node/executor/tests/basic.rs index 758ea1a32e841..8ba99d907ea99 100644 --- a/bin/node/executor/tests/basic.rs +++ b/bin/node/executor/tests/basic.rs @@ -606,13 +606,18 @@ fn deploying_wasm_contract_should_work() { CheckedExtrinsic { signed: Some((charlie(), signed_extra(0, 0))), function: Call::Contracts( - pallet_contracts::Call::put_code::(10_000, transfer_code) + pallet_contracts::Call::put_code::(500_000_000, transfer_code) ), }, CheckedExtrinsic { signed: Some((charlie(), signed_extra(1, 0))), function: Call::Contracts( - pallet_contracts::Call::instantiate::(1 * DOLLARS, 10_000, transfer_ch, Vec::new()) + pallet_contracts::Call::instantiate::( + 1 * DOLLARS, + 500_000_000, + transfer_ch, + Vec::new() + ) ), }, CheckedExtrinsic { @@ -621,7 +626,7 @@ fn deploying_wasm_contract_should_work() { pallet_contracts::Call::call::( pallet_indices::address::Address::Id(addr.clone()), 10, - 10_000, + 500_000_000, vec![0x00, 0x01, 0x02, 0x03] ) ), diff --git a/frame/contracts/src/exec.rs b/frame/contracts/src/exec.rs index 027a10bb4de02..67809bbd4026b 100644 --- a/frame/contracts/src/exec.rs +++ b/frame/contracts/src/exec.rs @@ -896,6 +896,7 @@ mod tests { use crate::{ account_db::AccountDb, gas::GasMeter, tests::{ExtBuilder, Test}, exec::{ExecReturnValue, ExecError, STATUS_SUCCESS}, CodeHash, Config, + gas::Gas, }; use std::{cell::RefCell, rc::Rc, collections::HashMap, marker::PhantomData}; use assert_matches::assert_matches; @@ -905,6 +906,8 @@ mod tests { const BOB: u64 = 2; const CHARLIE: u64 = 3; + const GAS_LIMIT: Gas = 10_000_000_000; + impl<'a, T, V, L> ExecutionContext<'a, T, V, L> where T: crate::Trait { @@ -1010,7 +1013,7 @@ mod tests { #[test] fn it_works() { let value = Default::default(); - let mut gas_meter = GasMeter::::new(10000); + let mut gas_meter = GasMeter::::new(GAS_LIMIT); let data = vec![]; let vm = MockVm::new(); @@ -1051,7 +1054,7 @@ mod tests { ctx.overlay.set_balance(&origin, 100); ctx.overlay.set_balance(&dest, 0); - let mut gas_meter = GasMeter::::new(1000); + let mut gas_meter = GasMeter::::new(GAS_LIMIT); let result = ctx.call(dest, 0, &mut gas_meter, vec![]); assert_matches!(result, Ok(_)); @@ -1071,7 +1074,7 @@ mod tests { ctx.overlay.set_balance(&origin, 100); - let mut gas_meter = GasMeter::::new(1000); + let mut gas_meter = GasMeter::::new(GAS_LIMIT); let result = ctx.instantiate(1, &mut gas_meter, &code, vec![]); assert_matches!(result, Ok(_)); @@ -1100,7 +1103,7 @@ mod tests { let output = ctx.call( dest, 55, - &mut GasMeter::::new(1000), + &mut GasMeter::::new(GAS_LIMIT), vec![], ).unwrap(); @@ -1133,7 +1136,7 @@ mod tests { let output = ctx.call( dest, 55, - &mut GasMeter::::new(1000), + &mut GasMeter::::new(GAS_LIMIT), vec![], ).unwrap(); @@ -1159,7 +1162,7 @@ mod tests { ctx.overlay.set_balance(&origin, 100); ctx.overlay.set_balance(&dest, 0); - let mut gas_meter = GasMeter::::new(1000); + let mut gas_meter = GasMeter::::new(GAS_LIMIT); let result = ctx.call(dest, 50, &mut gas_meter, vec![]); assert_matches!(result, Ok(_)); @@ -1184,7 +1187,7 @@ mod tests { ctx.overlay.set_balance(&origin, 100); ctx.overlay.set_balance(&dest, 15); - let mut gas_meter = GasMeter::::new(1000); + let mut gas_meter = GasMeter::::new(GAS_LIMIT); let result = ctx.call(dest, 50, &mut gas_meter, vec![]); assert_matches!(result, Ok(_)); @@ -1212,7 +1215,7 @@ mod tests { ctx.overlay.set_balance(&origin, 100); ctx.overlay.set_balance(&dest, 15); - let mut gas_meter = GasMeter::::new(1000); + let mut gas_meter = GasMeter::::new(GAS_LIMIT); let result = ctx.instantiate(50, &mut gas_meter, &code, vec![]); assert_matches!(result, Ok(_)); @@ -1246,7 +1249,7 @@ mod tests { let result = ctx.call( dest, 100, - &mut GasMeter::::new(1000), + &mut GasMeter::::new(GAS_LIMIT), vec![], ); @@ -1283,7 +1286,7 @@ mod tests { let result = ctx.call( dest, 0, - &mut GasMeter::::new(1000), + &mut GasMeter::::new(GAS_LIMIT), vec![], ); @@ -1314,7 +1317,7 @@ mod tests { let result = ctx.call( dest, 0, - &mut GasMeter::::new(1000), + &mut GasMeter::::new(GAS_LIMIT), vec![], ); @@ -1342,7 +1345,7 @@ mod tests { let result = ctx.call( BOB, 0, - &mut GasMeter::::new(10000), + &mut GasMeter::::new(GAS_LIMIT), vec![1, 2, 3, 4], ); assert_matches!(result, Ok(_)); @@ -1367,7 +1370,7 @@ mod tests { let result = ctx.instantiate( 1, - &mut GasMeter::::new(10000), + &mut GasMeter::::new(GAS_LIMIT), &input_data_ch, vec![1, 2, 3, 4], ); @@ -1417,7 +1420,7 @@ mod tests { let result = ctx.call( BOB, value, - &mut GasMeter::::new(100000), + &mut GasMeter::::new(GAS_LIMIT), vec![], ); @@ -1463,7 +1466,7 @@ mod tests { let result = ctx.call( dest, 0, - &mut GasMeter::::new(10000), + &mut GasMeter::::new(GAS_LIMIT), vec![], ); @@ -1504,7 +1507,7 @@ mod tests { let result = ctx.call( BOB, 0, - &mut GasMeter::::new(10000), + &mut GasMeter::::new(GAS_LIMIT), vec![], ); @@ -1526,7 +1529,7 @@ mod tests { assert_matches!( ctx.instantiate( 0, // <- zero endowment - &mut GasMeter::::new(10000), + &mut GasMeter::::new(GAS_LIMIT), &dummy_ch, vec![], ), @@ -1552,7 +1555,7 @@ mod tests { let instantiated_contract_address = assert_matches!( ctx.instantiate( 100, - &mut GasMeter::::new(10000), + &mut GasMeter::::new(GAS_LIMIT), &dummy_ch, vec![], ), @@ -1592,7 +1595,7 @@ mod tests { let instantiated_contract_address = assert_matches!( ctx.instantiate( 100, - &mut GasMeter::::new(10000), + &mut GasMeter::::new(GAS_LIMIT), &dummy_ch, vec![], ), @@ -1637,7 +1640,7 @@ mod tests { ctx.overlay.instantiate_contract(&BOB, instantiator_ch).unwrap(); assert_matches!( - ctx.call(BOB, 20, &mut GasMeter::::new(1000), vec![]), + ctx.call(BOB, 20, &mut GasMeter::::new(GAS_LIMIT), vec![]), Ok(_) ); @@ -1697,7 +1700,7 @@ mod tests { ctx.overlay.instantiate_contract(&BOB, instantiator_ch).unwrap(); assert_matches!( - ctx.call(BOB, 20, &mut GasMeter::::new(1000), vec![]), + ctx.call(BOB, 20, &mut GasMeter::::new(GAS_LIMIT), vec![]), Ok(_) ); @@ -1734,7 +1737,7 @@ mod tests { assert_matches!( ctx.instantiate( 100, - &mut GasMeter::::new(10000), + &mut GasMeter::::new(GAS_LIMIT), &terminate_ch, vec![], ), @@ -1770,7 +1773,7 @@ mod tests { let result = ctx.instantiate( 1, - &mut GasMeter::::new(10000), + &mut GasMeter::::new(GAS_LIMIT), &rent_allowance_ch, vec![], ); diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index 30a516fc6ff53..23d9dd5877195 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -23,6 +23,7 @@ use crate::{ BalanceOf, ComputeDispatchFee, ContractAddressFor, ContractInfo, ContractInfoOf, GenesisConfig, Module, RawAliveContractInfo, RawEvent, Trait, TrieId, TrieIdFromParentCounter, Schedule, TrieIdGenerator, account_db::{AccountDb, DirectAccountDb, OverlayAccountDb}, + gas::Gas, }; use assert_matches::assert_matches; use hex_literal::*; @@ -214,6 +215,8 @@ const BOB: u64 = 2; const CHARLIE: u64 = 3; const DJANGO: u64 = 4; +const GAS_LIMIT: Gas = 10_000_000_000; + pub struct ExtBuilder { existential_deposit: u64, } @@ -268,10 +271,10 @@ fn returns_base_call_cost() { Balances::deposit_creating(&ALICE, 100_000_000); assert_eq!( - Contracts::call(Origin::signed(ALICE), BOB, 0, 100_000, Vec::new()), + Contracts::call(Origin::signed(ALICE), BOB, 0, GAS_LIMIT, Vec::new()), Ok( PostDispatchInfo { - actual_weight: Some(135), + actual_weight: Some(67500000), } ) ); @@ -359,13 +362,13 @@ fn instantiate_and_call_and_deposit_event() { ExtBuilder::default().existential_deposit(100).build().execute_with(|| { Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), 100_000, wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm)); // Check at the end to get hash on error easily let creation = Contracts::instantiate( Origin::signed(ALICE), 100, - 100_000, + GAS_LIMIT, code_hash.into(), vec![], ); @@ -433,7 +436,7 @@ fn dispatch_call() { ExtBuilder::default().existential_deposit(50).build().execute_with(|| { Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), 100_000, wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm)); // Let's keep this assert even though it's redundant. If you ever need to update the // wasm source this test will fail and will show you the actual hash. @@ -458,7 +461,7 @@ fn dispatch_call() { assert_ok!(Contracts::instantiate( Origin::signed(ALICE), 100, - 100_000, + GAS_LIMIT, code_hash.into(), vec![], )); @@ -467,7 +470,7 @@ fn dispatch_call() { Origin::signed(ALICE), BOB, // newly created account 0, - 1_000_000_000_000, + GAS_LIMIT, vec![], )); @@ -554,7 +557,7 @@ fn dispatch_call_not_dispatched_after_top_level_transaction_failure() { ExtBuilder::default().existential_deposit(50).build().execute_with(|| { Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), 100_000, wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm)); // Let's keep this assert even though it's redundant. If you ever need to update the // wasm source this test will fail and will show you the actual hash. @@ -579,7 +582,7 @@ fn dispatch_call_not_dispatched_after_top_level_transaction_failure() { assert_ok!(Contracts::instantiate( Origin::signed(ALICE), 100, - 100_000, + GAS_LIMIT, code_hash.into(), vec![], )); @@ -591,7 +594,7 @@ fn dispatch_call_not_dispatched_after_top_level_transaction_failure() { Origin::signed(ALICE), BOB, // newly created account 0, - 1_000_000_000_000, + GAS_LIMIT, vec![], ), "contract trapped during execution" @@ -650,12 +653,12 @@ fn run_out_of_gas() { .execute_with(|| { Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), 100_000, wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm)); assert_ok!(Contracts::instantiate( Origin::signed(ALICE), 100, - 100_000, + GAS_LIMIT, code_hash.into(), vec![], )); @@ -667,7 +670,7 @@ fn run_out_of_gas() { Origin::signed(ALICE), BOB, // newly created account 0, - 1000, + 67_500_000, vec![], ), "ran out of gas during contract execution" @@ -696,7 +699,7 @@ fn test_set_rent_code_and_hash() { ExtBuilder::default().existential_deposit(50).build().execute_with(|| { Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), 100_000, wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm)); // If you ever need to update the wasm source this test will fail // and will show you the actual hash. @@ -728,21 +731,21 @@ fn storage_size() { ExtBuilder::default().existential_deposit(50).build().execute_with(|| { // Create Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), 100_000, wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm)); assert_ok!(Contracts::instantiate( Origin::signed(ALICE), 30_000, - 100_000, code_hash.into(), + GAS_LIMIT, code_hash.into(), ::Balance::from(1_000u32).encode() // rent allowance )); let bob_contract = ContractInfoOf::::get(BOB).unwrap().get_alive().unwrap(); assert_eq!(bob_contract.storage_size, ::StorageSizeOffset::get() + 4); - assert_ok!(Contracts::call(Origin::signed(ALICE), BOB, 0, 100_000, call::set_storage_4_byte())); + assert_ok!(Contracts::call(Origin::signed(ALICE), BOB, 0, GAS_LIMIT, call::set_storage_4_byte())); let bob_contract = ContractInfoOf::::get(BOB).unwrap().get_alive().unwrap(); assert_eq!(bob_contract.storage_size, ::StorageSizeOffset::get() + 4 + 4); - assert_ok!(Contracts::call(Origin::signed(ALICE), BOB, 0, 100_000, call::remove_storage_4_byte())); + assert_ok!(Contracts::call(Origin::signed(ALICE), BOB, 0, GAS_LIMIT, call::remove_storage_4_byte())); let bob_contract = ContractInfoOf::::get(BOB).unwrap().get_alive().unwrap(); assert_eq!(bob_contract.storage_size, ::StorageSizeOffset::get() + 4); }); @@ -765,11 +768,11 @@ fn deduct_blocks() { ExtBuilder::default().existential_deposit(50).build().execute_with(|| { // Create Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), 100_000, wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm)); assert_ok!(Contracts::instantiate( Origin::signed(ALICE), 30_000, - 100_000, code_hash.into(), + GAS_LIMIT, code_hash.into(), ::Balance::from(1_000u32).encode() // rent allowance )); @@ -781,7 +784,7 @@ fn deduct_blocks() { initialize_block(5); // Trigger rent through call - assert_ok!(Contracts::call(Origin::signed(ALICE), BOB, 0, 100_000, call::null())); + assert_ok!(Contracts::call(Origin::signed(ALICE), BOB, 0, GAS_LIMIT, call::null())); // Check result let rent = (8 + 4 - 3) // storage size = size_offset + deploy_set_storage - deposit_offset @@ -796,7 +799,7 @@ fn deduct_blocks() { initialize_block(12); // Trigger rent through call - assert_ok!(Contracts::call(Origin::signed(ALICE), BOB, 0, 100_000, call::null())); + assert_ok!(Contracts::call(Origin::signed(ALICE), BOB, 0, GAS_LIMIT, call::null())); // Check result let rent_2 = (8 + 4 - 2) // storage size = size_offset + deploy_set_storage - deposit_offset @@ -808,7 +811,7 @@ fn deduct_blocks() { assert_eq!(Balances::free_balance(BOB), 30_000 - rent - rent_2); // Second call on same block should have no effect on rent - assert_ok!(Contracts::call(Origin::signed(ALICE), BOB, 0, 100_000, call::null())); + assert_ok!(Contracts::call(Origin::signed(ALICE), BOB, 0, GAS_LIMIT, call::null())); let bob_contract = ContractInfoOf::::get(BOB).unwrap().get_alive().unwrap(); assert_eq!(bob_contract.rent_allowance, 1_000 - rent - rent_2); @@ -821,7 +824,7 @@ fn deduct_blocks() { fn call_contract_removals() { removals(|| { // Call on already-removed account might fail, and this is fine. - Contracts::call(Origin::signed(ALICE), BOB, 0, 100_000, call::null()); + Contracts::call(Origin::signed(ALICE), BOB, 0, GAS_LIMIT, call::null()); true }); } @@ -859,11 +862,11 @@ fn claim_surcharge(blocks: u64, trigger_call: impl Fn() -> bool, removes: bool) ExtBuilder::default().existential_deposit(50).build().execute_with(|| { // Create Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), 100_000, wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm)); assert_ok!(Contracts::instantiate( Origin::signed(ALICE), 100, - 100_000, code_hash.into(), + GAS_LIMIT, code_hash.into(), ::Balance::from(1_000u32).encode() // rent allowance )); @@ -892,11 +895,11 @@ fn removals(trigger_call: impl Fn() -> bool) { ExtBuilder::default().existential_deposit(50).build().execute_with(|| { // Create Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), 100_000, wasm.clone())); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm.clone())); assert_ok!(Contracts::instantiate( Origin::signed(ALICE), 100, - 100_000, code_hash.into(), + GAS_LIMIT, code_hash.into(), ::Balance::from(1_000u32).encode() // rent allowance )); @@ -928,11 +931,11 @@ fn removals(trigger_call: impl Fn() -> bool) { ExtBuilder::default().existential_deposit(50).build().execute_with(|| { // Create Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), 100_000, wasm.clone())); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm.clone())); assert_ok!(Contracts::instantiate( Origin::signed(ALICE), 1_000, - 100_000, code_hash.into(), + GAS_LIMIT, code_hash.into(), ::Balance::from(100u32).encode() // rent allowance )); @@ -963,11 +966,11 @@ fn removals(trigger_call: impl Fn() -> bool) { ExtBuilder::default().existential_deposit(50).build().execute_with(|| { // Create Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), 100_000, wasm.clone())); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm.clone())); assert_ok!(Contracts::instantiate( Origin::signed(ALICE), 50+Balances::minimum_balance(), - 100_000, code_hash.into(), + GAS_LIMIT, code_hash.into(), ::Balance::from(1_000u32).encode() // rent allowance )); @@ -977,7 +980,7 @@ fn removals(trigger_call: impl Fn() -> bool) { assert_eq!(Balances::free_balance(BOB), 50 + Balances::minimum_balance()); // Transfer funds - assert_ok!(Contracts::call(Origin::signed(ALICE), BOB, 0, 100_000_000_000, call::transfer())); + assert_ok!(Contracts::call(Origin::signed(ALICE), BOB, 0, GAS_LIMIT, call::transfer())); assert_eq!(ContractInfoOf::::get(BOB).unwrap().get_alive().unwrap().rent_allowance, 1_000); assert_eq!(Balances::free_balance(BOB), Balances::minimum_balance()); @@ -1007,23 +1010,23 @@ fn call_removed_contract() { ExtBuilder::default().existential_deposit(50).build().execute_with(|| { // Create Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), 100_000, wasm.clone())); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm.clone())); assert_ok!(Contracts::instantiate( Origin::signed(ALICE), 100, - 100_000, code_hash.into(), + GAS_LIMIT, code_hash.into(), ::Balance::from(1_000u32).encode() // rent allowance )); // Calling contract should succeed. - assert_ok!(Contracts::call(Origin::signed(ALICE), BOB, 0, 100_000, call::null())); + assert_ok!(Contracts::call(Origin::signed(ALICE), BOB, 0, GAS_LIMIT, call::null())); // Advance blocks initialize_block(10); // Calling contract should remove contract and fail. assert_err_ignore_postinfo!( - Contracts::call(Origin::signed(ALICE), BOB, 0, 100_000, call::null()), + Contracts::call(Origin::signed(ALICE), BOB, 0, GAS_LIMIT, call::null()), "contract has been evicted" ); // Calling a contract that is about to evict shall emit an event. @@ -1037,7 +1040,7 @@ fn call_removed_contract() { // Subsequent contract calls should also fail. assert_err_ignore_postinfo!( - Contracts::call(Origin::signed(ALICE), BOB, 0, 100_000, call::null()), + Contracts::call(Origin::signed(ALICE), BOB, 0, GAS_LIMIT, call::null()), "contract has been evicted" ); }) @@ -1051,11 +1054,11 @@ fn default_rent_allowance_on_instantiate() { ExtBuilder::default().existential_deposit(50).build().execute_with(|| { // Create Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), 100_000, wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm)); assert_ok!(Contracts::instantiate( Origin::signed(ALICE), 30_000, - 100_000, + GAS_LIMIT, code_hash.into(), vec![], )); @@ -1068,7 +1071,7 @@ fn default_rent_allowance_on_instantiate() { initialize_block(5); // Trigger rent through call - assert_ok!(Contracts::call(Origin::signed(ALICE), BOB, 0, 100_000, call::null())); + assert_ok!(Contracts::call(Origin::signed(ALICE), BOB, 0, GAS_LIMIT, call::null())); // Check contract is still alive let bob_contract = ContractInfoOf::::get(BOB).unwrap().get_alive(); @@ -1104,8 +1107,8 @@ fn restoration(test_different_storage: bool, test_restore_to_with_dirty_storage: ExtBuilder::default().existential_deposit(50).build().execute_with(|| { Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), 100_000, restoration_wasm)); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), 100_000, set_rent_wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, restoration_wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, set_rent_wasm)); // If you ever need to update the wasm source this test will fail // and will show you the actual hash. @@ -1137,7 +1140,7 @@ fn restoration(test_different_storage: bool, test_restore_to_with_dirty_storage: assert_ok!(Contracts::instantiate( Origin::signed(ALICE), 30_000, - 100_000, + GAS_LIMIT, set_rent_code_hash.into(), ::Balance::from(0u32).encode() )); @@ -1150,7 +1153,7 @@ fn restoration(test_different_storage: bool, test_restore_to_with_dirty_storage: if test_different_storage { assert_ok!(Contracts::call( Origin::signed(ALICE), - BOB, 0, 100_000, + BOB, 0, GAS_LIMIT, call::set_storage_4_byte()) ); } @@ -1164,7 +1167,7 @@ fn restoration(test_different_storage: bool, test_restore_to_with_dirty_storage: // Call `BOB`, which makes it pay rent. Since the rent allowance is set to 0 // we expect that it will get removed leaving tombstone. assert_err_ignore_postinfo!( - Contracts::call(Origin::signed(ALICE), BOB, 0, 100_000, call::null()), + Contracts::call(Origin::signed(ALICE), BOB, 0, GAS_LIMIT, call::null()), "contract has been evicted" ); assert!(ContractInfoOf::::get(BOB).unwrap().get_tombstone().is_some()); @@ -1186,7 +1189,7 @@ fn restoration(test_different_storage: bool, test_restore_to_with_dirty_storage: assert_ok!(Contracts::instantiate( Origin::signed(CHARLIE), 30_000, - 100_000, + GAS_LIMIT, restoration_code_hash.into(), ::Balance::from(0u32).encode() )); @@ -1206,7 +1209,7 @@ fn restoration(test_different_storage: bool, test_restore_to_with_dirty_storage: Origin::signed(ALICE), DJANGO, 0, - 100_000, + GAS_LIMIT, vec![], )); @@ -1320,11 +1323,11 @@ fn storage_max_value_limit() { ExtBuilder::default().existential_deposit(50).build().execute_with(|| { // Create Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), 100_000, wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm)); assert_ok!(Contracts::instantiate( Origin::signed(ALICE), 30_000, - 100_000, + GAS_LIMIT, code_hash.into(), vec![], )); @@ -1338,7 +1341,7 @@ fn storage_max_value_limit() { Origin::signed(ALICE), BOB, 0, - 100_000, + GAS_LIMIT, Encode::encode(&self::MaxValueSize::get()), )); @@ -1348,7 +1351,7 @@ fn storage_max_value_limit() { Origin::signed(ALICE), BOB, 0, - 100_000, + GAS_LIMIT, Encode::encode(&(self::MaxValueSize::get() + 1)), ), "contract trapped during execution" @@ -1366,13 +1369,13 @@ fn deploy_and_call_other_contract() { ExtBuilder::default().existential_deposit(50).build().execute_with(|| { // Create Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), 100_000, callee_wasm)); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), 100_000, caller_wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, callee_wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, caller_wasm)); assert_ok!(Contracts::instantiate( Origin::signed(ALICE), 100_000, - 100_000, + GAS_LIMIT, caller_code_hash.into(), vec![], )); @@ -1383,7 +1386,7 @@ fn deploy_and_call_other_contract() { Origin::signed(ALICE), BOB, 0, - 200_000, + GAS_LIMIT, callee_code_hash.as_ref().to_vec(), )); }); @@ -1394,13 +1397,13 @@ fn cannot_self_destruct_through_draning() { let (wasm, code_hash) = compile_module::(&load_wasm("drain.wat")).unwrap(); ExtBuilder::default().existential_deposit(50).build().execute_with(|| { Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), 100_000, wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm)); // Instantiate the BOB contract. assert_ok!(Contracts::instantiate( Origin::signed(ALICE), 100_000, - 100_000, + GAS_LIMIT, code_hash.into(), vec![], )); @@ -1418,7 +1421,7 @@ fn cannot_self_destruct_through_draning() { Origin::signed(ALICE), BOB, 0, - 100_000, + GAS_LIMIT, vec![], ), "contract trapped during execution" @@ -1432,13 +1435,13 @@ fn cannot_self_destruct_while_live() { .unwrap(); ExtBuilder::default().existential_deposit(50).build().execute_with(|| { Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), 100_000, wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm)); // Instantiate the BOB contract. assert_ok!(Contracts::instantiate( Origin::signed(ALICE), 100_000, - 100_000, + GAS_LIMIT, code_hash.into(), vec![], )); @@ -1456,7 +1459,7 @@ fn cannot_self_destruct_while_live() { Origin::signed(ALICE), BOB, 0, - 100_000, + GAS_LIMIT, vec![0], ), "contract trapped during execution" @@ -1476,13 +1479,13 @@ fn self_destruct_works() { .unwrap(); ExtBuilder::default().existential_deposit(50).build().execute_with(|| { Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), 100_000, wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm)); // Instantiate the BOB contract. assert_ok!(Contracts::instantiate( Origin::signed(ALICE), 100_000, - 100_000, + GAS_LIMIT, code_hash.into(), vec![], )); @@ -1499,7 +1502,7 @@ fn self_destruct_works() { Origin::signed(ALICE), BOB, 0, - 100_000, + GAS_LIMIT, vec![], ), Ok(_) @@ -1525,15 +1528,15 @@ fn destroy_contract_and_transfer_funds() { ExtBuilder::default().existential_deposit(50).build().execute_with(|| { // Create Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), 100_000, callee_wasm)); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), 100_000, caller_wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, callee_wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, caller_wasm)); // This deploys the BOB contract, which in turn deploys the CHARLIE contract during // construction. assert_ok!(Contracts::instantiate( Origin::signed(ALICE), 200_000, - 100_000, + GAS_LIMIT, caller_code_hash.into(), callee_code_hash.as_ref().to_vec(), )); @@ -1549,7 +1552,7 @@ fn destroy_contract_and_transfer_funds() { Origin::signed(ALICE), BOB, 0, - 100_000, + GAS_LIMIT, CHARLIE.encode(), )); @@ -1564,7 +1567,7 @@ fn cannot_self_destruct_in_constructor() { compile_module::(&load_wasm("self_destructing_constructor.wat")).unwrap(); ExtBuilder::default().existential_deposit(50).build().execute_with(|| { Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), 100_000, wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm)); // Fail to instantiate the BOB because the call that is issued in the deploy // function exhausts all balances which puts it below the existential deposit. @@ -1572,7 +1575,7 @@ fn cannot_self_destruct_in_constructor() { Contracts::instantiate( Origin::signed(ALICE), 100_000, - 100_000, + GAS_LIMIT, code_hash.into(), vec![], ), @@ -1592,11 +1595,11 @@ fn get_runtime_storage() { 0x14144020u32.to_le_bytes().to_vec().as_ref() ); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), 100_000, wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm)); assert_ok!(Contracts::instantiate( Origin::signed(ALICE), 100, - 100_000, + GAS_LIMIT, code_hash.into(), vec![], )); @@ -1604,7 +1607,7 @@ fn get_runtime_storage() { Origin::signed(ALICE), BOB, 0, - 100_000, + GAS_LIMIT, vec![], )); }); @@ -1616,13 +1619,13 @@ fn crypto_hashes() { ExtBuilder::default().existential_deposit(50).build().execute_with(|| { Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), 100_000, wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm)); // Instantiate the CRYPTO_HASHES contract. assert_ok!(Contracts::instantiate( Origin::signed(ALICE), 100_000, - 100_000, + GAS_LIMIT, code_hash.into(), vec![], )); @@ -1651,7 +1654,7 @@ fn crypto_hashes() { ALICE, BOB, 0, - 100_000, + GAS_LIMIT, params, ).unwrap(); assert_eq!(result.status, 0); diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index b2fbce09e737e..cb69cd689b265 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -163,6 +163,8 @@ mod tests { use assert_matches::assert_matches; use sp_runtime::DispatchError; + const GAS_LIMIT: Gas = 10_000_000_000; + #[derive(Debug, PartialEq, Eq)] struct DispatchEntry(Call); @@ -550,7 +552,7 @@ mod tests { CODE_TRANSFER, vec![], &mut mock_ext, - &mut GasMeter::new(50_000), + &mut GasMeter::new(GAS_LIMIT), ).unwrap(); assert_eq!( @@ -559,7 +561,7 @@ mod tests { to: 7, value: 153, data: Vec::new(), - gas_left: 49978, + gas_left: 9989000000, }] ); } @@ -610,7 +612,7 @@ mod tests { CODE_CALL, vec![], &mut mock_ext, - &mut GasMeter::new(50_000), + &mut GasMeter::new(GAS_LIMIT), ).unwrap(); assert_eq!( @@ -619,7 +621,7 @@ mod tests { to: 9, value: 6, data: vec![1, 2, 3, 4], - gas_left: 49971, + gas_left: 9985500000, }] ); } @@ -672,7 +674,7 @@ mod tests { CODE_INSTANTIATE, vec![], &mut mock_ext, - &mut GasMeter::new(50_000), + &mut GasMeter::new(GAS_LIMIT), ).unwrap(); assert_eq!( @@ -681,7 +683,7 @@ mod tests { code_hash: [0x11; 32].into(), endowment: 3, data: vec![1, 2, 3, 4], - gas_left: 49947, + gas_left: 9973500000, }] ); } @@ -715,14 +717,14 @@ mod tests { CODE_TERMINATE, vec![], &mut mock_ext, - &mut GasMeter::new(50_000), + &mut GasMeter::new(GAS_LIMIT), ).unwrap(); assert_eq!( &mock_ext.terminations, &[TerminationEntry { beneficiary: 0x09, - gas_left: 49989, + gas_left: 9994500000, }] ); } @@ -773,7 +775,7 @@ mod tests { &CODE_TRANSFER_LIMITED_GAS, vec![], &mut mock_ext, - &mut GasMeter::new(50_000), + &mut GasMeter::new(GAS_LIMIT), ).unwrap(); assert_eq!( @@ -866,7 +868,7 @@ mod tests { CODE_GET_STORAGE, vec![], mock_ext, - &mut GasMeter::new(50_000), + &mut GasMeter::new(GAS_LIMIT), ).unwrap(); assert_eq!(output, ExecReturnValue { status: STATUS_SUCCESS, data: [0x22; 32].to_vec() }); @@ -930,7 +932,7 @@ mod tests { CODE_CALLER, vec![], MockExt::default(), - &mut GasMeter::new(50_000), + &mut GasMeter::new(GAS_LIMIT), ).unwrap(); } @@ -992,7 +994,7 @@ mod tests { CODE_ADDRESS, vec![], MockExt::default(), - &mut GasMeter::new(50_000), + &mut GasMeter::new(GAS_LIMIT), ).unwrap(); } @@ -1047,7 +1049,7 @@ mod tests { #[test] fn balance() { - let mut gas_meter = GasMeter::new(50_000); + let mut gas_meter = GasMeter::new(GAS_LIMIT); let _ = execute( CODE_BALANCE, vec![], @@ -1107,7 +1109,7 @@ mod tests { #[test] fn gas_price() { - let mut gas_meter = GasMeter::new(50_000); + let mut gas_meter = GasMeter::new(GAS_LIMIT); let _ = execute( CODE_GAS_PRICE, vec![], @@ -1165,7 +1167,7 @@ mod tests { #[test] fn gas_left() { - let mut gas_meter = GasMeter::new(50_000); + let mut gas_meter = GasMeter::new(GAS_LIMIT); let output = execute( CODE_GAS_LEFT, @@ -1175,7 +1177,7 @@ mod tests { ).unwrap(); let gas_left = Gas::decode(&mut output.data.as_slice()).unwrap(); - assert!(gas_left < 50_000, "gas_left must be less than initial"); + assert!(gas_left < GAS_LIMIT, "gas_left must be less than initial"); assert!(gas_left > gas_meter.gas_left(), "gas_left must be greater than final"); } @@ -1230,7 +1232,7 @@ mod tests { #[test] fn value_transferred() { - let mut gas_meter = GasMeter::new(50_000); + let mut gas_meter = GasMeter::new(GAS_LIMIT); let _ = execute( CODE_VALUE_TRANSFERRED, vec![], @@ -1266,7 +1268,7 @@ mod tests { CODE_DISPATCH_CALL, vec![], &mut mock_ext, - &mut GasMeter::new(1_000_000_000_000), + &mut GasMeter::new(GAS_LIMIT), ).unwrap(); assert_eq!( @@ -1306,7 +1308,7 @@ mod tests { CODE_RETURN_FROM_START_FN, vec![], MockExt::default(), - &mut GasMeter::new(50_000), + &mut GasMeter::new(GAS_LIMIT), ).unwrap(); assert_eq!(output, ExecReturnValue { status: STATUS_SUCCESS, data: vec![1, 2, 3, 4] }); @@ -1363,7 +1365,7 @@ mod tests { #[test] fn now() { - let mut gas_meter = GasMeter::new(50_000); + let mut gas_meter = GasMeter::new(GAS_LIMIT); let _ = execute( CODE_TIMESTAMP_NOW, vec![], @@ -1422,7 +1424,7 @@ mod tests { #[test] fn minimum_balance() { - let mut gas_meter = GasMeter::new(50_000); + let mut gas_meter = GasMeter::new(GAS_LIMIT); let _ = execute( CODE_MINIMUM_BALANCE, vec![], @@ -1481,7 +1483,7 @@ mod tests { #[test] fn tombstone_deposit() { - let mut gas_meter = GasMeter::new(50_000); + let mut gas_meter = GasMeter::new(GAS_LIMIT); let _ = execute( CODE_TOMBSTONE_DEPOSIT, vec![], @@ -1549,7 +1551,7 @@ mod tests { #[test] fn random() { - let mut gas_meter = GasMeter::new(50_000); + let mut gas_meter = GasMeter::new(GAS_LIMIT); let output = execute( CODE_RANDOM, @@ -1594,7 +1596,7 @@ mod tests { #[test] fn deposit_event() { let mut mock_ext = MockExt::default(); - let mut gas_meter = GasMeter::new(50_000); + let mut gas_meter = GasMeter::new(GAS_LIMIT); let _ = execute( CODE_DEPOSIT_EVENT, vec![], @@ -1607,7 +1609,7 @@ mod tests { vec![0x00, 0x01, 0x2a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xe5, 0x14, 0x00]) ]); - assert_eq!(gas_meter.gas_left(), 49934); + assert_eq!(gas_meter.gas_left(), 9967000000); } const CODE_DEPOSIT_EVENT_MAX_TOPICS: &str = r#" @@ -1640,7 +1642,7 @@ mod tests { #[test] fn deposit_event_max_topics() { // Checks that the runtime traps if there are more than `max_topic_events` topics. - let mut gas_meter = GasMeter::new(50_000); + let mut gas_meter = GasMeter::new(GAS_LIMIT); assert_matches!( execute( @@ -1684,7 +1686,7 @@ mod tests { #[test] fn deposit_event_duplicates() { // Checks that the runtime traps if there are duplicates. - let mut gas_meter = GasMeter::new(50_000); + let mut gas_meter = GasMeter::new(GAS_LIMIT); assert_matches!( execute( @@ -1755,7 +1757,7 @@ mod tests { CODE_BLOCK_NUMBER, vec![], MockExt::default(), - &mut GasMeter::new(50_000), + &mut GasMeter::new(GAS_LIMIT), ).unwrap(); } @@ -1795,7 +1797,7 @@ mod tests { CODE_SIMPLE_ASSERT, input_data, MockExt::default(), - &mut GasMeter::new(50_000), + &mut GasMeter::new(GAS_LIMIT), ).unwrap(); assert_eq!(output.data.len(), 0); @@ -1811,7 +1813,7 @@ mod tests { CODE_SIMPLE_ASSERT, input_data, MockExt::default(), - &mut GasMeter::new(50_000), + &mut GasMeter::new(GAS_LIMIT), ).err().unwrap(); assert_eq!(error.buffer.capacity(), 1_234); @@ -1865,7 +1867,7 @@ mod tests { CODE_RETURN_WITH_DATA, hex!("00112233445566778899").to_vec(), MockExt::default(), - &mut GasMeter::new(50_000), + &mut GasMeter::new(GAS_LIMIT), ).unwrap(); assert_eq!(output, ExecReturnValue { status: 0, data: hex!("445566778899").to_vec() }); @@ -1878,7 +1880,7 @@ mod tests { CODE_RETURN_WITH_DATA, hex!("112233445566778899").to_vec(), MockExt::default(), - &mut GasMeter::new(50_000), + &mut GasMeter::new(GAS_LIMIT), ).unwrap(); assert_eq!(output, ExecReturnValue { status: 17, data: hex!("5566778899").to_vec() }); @@ -1964,7 +1966,7 @@ mod tests { #[test] fn get_runtime_storage() { - let mut gas_meter = GasMeter::new(50_000); + let mut gas_meter = GasMeter::new(GAS_LIMIT); let mock_ext = MockExt::default(); // "\01\02\03\04" - Some(0x14144020) From 014d7d808f6b905be43e32d4d0c0dfaf85f5ab80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Wed, 22 Apr 2020 14:41:20 +0200 Subject: [PATCH 4/8] Apply style suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Tomasz Drwięga --- frame/contracts/src/gas.rs | 4 ++-- frame/contracts/src/wasm/runtime.rs | 2 +- frame/support/src/lib.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/frame/contracts/src/gas.rs b/frame/contracts/src/gas.rs index 0881394880ce8..f9c13ebe4ab90 100644 --- a/frame/contracts/src/gas.rs +++ b/frame/contracts/src/gas.rs @@ -188,9 +188,9 @@ impl GasMeter { self.gas_left } - // Turn this GasMeter into a DispatchResult that contains the actually used gas + /// Turn this GasMeter into a DispatchResult that contains the actually used gas. pub fn into_dispatch_result(self, result: Result) -> DispatchResultWithPostInfo where - E: Into + E: Into, { let post_info = PostDispatchInfo { actual_weight: Some(self.gas_spent()), diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index b7c13b2f7bbb6..f87f5d1ef53cc 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -154,7 +154,7 @@ pub enum RuntimeToken { /// The given number of bytes is read from the sandbox memory and /// is returned as the return data buffer of the call. ReturnData(u32), - // Dispatched a call with the given weight. + /// Dispatched a call with the given weight. DispatchWithWeight(Gas), /// (topic_count, data_bytes): A buffer of the given size is posted as an event indexed with the /// given number of topics. diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 18a071c6de4d0..b0e7395e9ff2f 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -204,7 +204,7 @@ macro_rules! assert_err { } } -/// Assert an expression returns an error specified +/// Assert an expression returns an error specified. /// /// This can be used on`DispatchResultWithPostInfo` when the post info should /// be ignored. From 71a82f3a7b652ca304057c40c09bd8001e963781 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Wed, 22 Apr 2020 14:51:35 +0200 Subject: [PATCH 5/8] Fix possible overflow in fn refund --- frame/contracts/src/gas.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/contracts/src/gas.rs b/frame/contracts/src/gas.rs index f9c13ebe4ab90..38f231c008f8b 100644 --- a/frame/contracts/src/gas.rs +++ b/frame/contracts/src/gas.rs @@ -147,7 +147,7 @@ impl GasMeter { // This can be used after dispatching a runtime call to refund gas that was not // used by the dispatchable. pub fn refund(&mut self, gas: Gas) { - self.gas_left = (self.gas_left + gas).max(self.gas_limit); + self.gas_left = self.gas_left.saturating_add(gas).max(self.gas_limit); } /// Allocate some amount of gas and perform some work with From 39ef81d3beb93eb41303a9faacfdaffbcb57449f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Wed, 22 Apr 2020 16:55:07 +0200 Subject: [PATCH 6/8] Remove gas metering from fn put_code() --- bin/node/executor/tests/basic.rs | 2 +- frame/contracts/src/lib.rs | 14 ++++--- frame/contracts/src/tests.rs | 52 +++++++++++++------------- frame/contracts/src/wasm/code_cache.rs | 31 +-------------- 4 files changed, 36 insertions(+), 63 deletions(-) diff --git a/bin/node/executor/tests/basic.rs b/bin/node/executor/tests/basic.rs index 8ba99d907ea99..3b50630e2d626 100644 --- a/bin/node/executor/tests/basic.rs +++ b/bin/node/executor/tests/basic.rs @@ -606,7 +606,7 @@ fn deploying_wasm_contract_should_work() { CheckedExtrinsic { signed: Some((charlie(), signed_extra(0, 0))), function: Call::Contracts( - pallet_contracts::Call::put_code::(500_000_000, transfer_code) + pallet_contracts::Call::put_code::(transfer_code) ), }, CheckedExtrinsic { diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 8d89e99492cfe..40b42a710a855 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -469,23 +469,21 @@ decl_module! { /// Stores the given binary Wasm code into the chain's storage and returns its `codehash`. /// You can instantiate contracts only with stored code. #[weight = FunctionOf( - |args: (&Weight, &Vec)| args.0 + MINIMUM_WEIGHT, + |args: (&Vec,)| Module::::calc_code_put_costs(args.0) + MINIMUM_WEIGHT, DispatchClass::Normal, Pays::Yes )] pub fn put_code( origin, - #[compact] gas_limit: Gas, code: Vec - ) -> DispatchResultWithPostInfo { + ) -> DispatchResult { ensure_signed(origin)?; - let mut gas_meter = GasMeter::new(gas_limit); let schedule = >::current_schedule(); - let result = wasm::save_code::(code, &mut gas_meter, &schedule); + let result = wasm::save_code::(code, &schedule); if let Ok(code_hash) = result { Self::deposit_event(RawEvent::CodeStored(code_hash)); } - gas_meter.into_dispatch_result(result) + result.map(|_| ()).map_err(Into::into) } /// Makes a call to an account, optionally transferring some balance. @@ -631,6 +629,10 @@ impl Module { } impl Module { + fn calc_code_put_costs(code: &Vec) -> Gas { + >::current_schedule().put_code_per_byte_cost.saturating_mul(code.len() as Gas) + } + fn execute_wasm( origin: T::AccountId, gas_meter: &mut GasMeter, diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index 23d9dd5877195..dd228b0644d2b 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -362,7 +362,7 @@ fn instantiate_and_call_and_deposit_event() { ExtBuilder::default().existential_deposit(100).build().execute_with(|| { Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), wasm)); // Check at the end to get hash on error easily let creation = Contracts::instantiate( @@ -436,7 +436,7 @@ fn dispatch_call() { ExtBuilder::default().existential_deposit(50).build().execute_with(|| { Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), wasm)); // Let's keep this assert even though it's redundant. If you ever need to update the // wasm source this test will fail and will show you the actual hash. @@ -557,7 +557,7 @@ fn dispatch_call_not_dispatched_after_top_level_transaction_failure() { ExtBuilder::default().existential_deposit(50).build().execute_with(|| { Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), wasm)); // Let's keep this assert even though it's redundant. If you ever need to update the // wasm source this test will fail and will show you the actual hash. @@ -653,7 +653,7 @@ fn run_out_of_gas() { .execute_with(|| { Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), wasm)); assert_ok!(Contracts::instantiate( Origin::signed(ALICE), @@ -699,7 +699,7 @@ fn test_set_rent_code_and_hash() { ExtBuilder::default().existential_deposit(50).build().execute_with(|| { Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), wasm)); // If you ever need to update the wasm source this test will fail // and will show you the actual hash. @@ -731,7 +731,7 @@ fn storage_size() { ExtBuilder::default().existential_deposit(50).build().execute_with(|| { // Create Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), wasm)); assert_ok!(Contracts::instantiate( Origin::signed(ALICE), 30_000, @@ -768,7 +768,7 @@ fn deduct_blocks() { ExtBuilder::default().existential_deposit(50).build().execute_with(|| { // Create Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), wasm)); assert_ok!(Contracts::instantiate( Origin::signed(ALICE), 30_000, @@ -862,7 +862,7 @@ fn claim_surcharge(blocks: u64, trigger_call: impl Fn() -> bool, removes: bool) ExtBuilder::default().existential_deposit(50).build().execute_with(|| { // Create Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), wasm)); assert_ok!(Contracts::instantiate( Origin::signed(ALICE), 100, @@ -895,7 +895,7 @@ fn removals(trigger_call: impl Fn() -> bool) { ExtBuilder::default().existential_deposit(50).build().execute_with(|| { // Create Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm.clone())); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), wasm.clone())); assert_ok!(Contracts::instantiate( Origin::signed(ALICE), 100, @@ -931,7 +931,7 @@ fn removals(trigger_call: impl Fn() -> bool) { ExtBuilder::default().existential_deposit(50).build().execute_with(|| { // Create Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm.clone())); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), wasm.clone())); assert_ok!(Contracts::instantiate( Origin::signed(ALICE), 1_000, @@ -966,7 +966,7 @@ fn removals(trigger_call: impl Fn() -> bool) { ExtBuilder::default().existential_deposit(50).build().execute_with(|| { // Create Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm.clone())); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), wasm.clone())); assert_ok!(Contracts::instantiate( Origin::signed(ALICE), 50+Balances::minimum_balance(), @@ -1010,7 +1010,7 @@ fn call_removed_contract() { ExtBuilder::default().existential_deposit(50).build().execute_with(|| { // Create Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm.clone())); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), wasm.clone())); assert_ok!(Contracts::instantiate( Origin::signed(ALICE), 100, @@ -1054,7 +1054,7 @@ fn default_rent_allowance_on_instantiate() { ExtBuilder::default().existential_deposit(50).build().execute_with(|| { // Create Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), wasm)); assert_ok!(Contracts::instantiate( Origin::signed(ALICE), 30_000, @@ -1107,8 +1107,8 @@ fn restoration(test_different_storage: bool, test_restore_to_with_dirty_storage: ExtBuilder::default().existential_deposit(50).build().execute_with(|| { Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, restoration_wasm)); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, set_rent_wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), restoration_wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), set_rent_wasm)); // If you ever need to update the wasm source this test will fail // and will show you the actual hash. @@ -1323,7 +1323,7 @@ fn storage_max_value_limit() { ExtBuilder::default().existential_deposit(50).build().execute_with(|| { // Create Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), wasm)); assert_ok!(Contracts::instantiate( Origin::signed(ALICE), 30_000, @@ -1369,8 +1369,8 @@ fn deploy_and_call_other_contract() { ExtBuilder::default().existential_deposit(50).build().execute_with(|| { // Create Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, callee_wasm)); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, caller_wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), callee_wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), caller_wasm)); assert_ok!(Contracts::instantiate( Origin::signed(ALICE), @@ -1397,7 +1397,7 @@ fn cannot_self_destruct_through_draning() { let (wasm, code_hash) = compile_module::(&load_wasm("drain.wat")).unwrap(); ExtBuilder::default().existential_deposit(50).build().execute_with(|| { Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), wasm)); // Instantiate the BOB contract. assert_ok!(Contracts::instantiate( @@ -1435,7 +1435,7 @@ fn cannot_self_destruct_while_live() { .unwrap(); ExtBuilder::default().existential_deposit(50).build().execute_with(|| { Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), wasm)); // Instantiate the BOB contract. assert_ok!(Contracts::instantiate( @@ -1479,7 +1479,7 @@ fn self_destruct_works() { .unwrap(); ExtBuilder::default().existential_deposit(50).build().execute_with(|| { Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), wasm)); // Instantiate the BOB contract. assert_ok!(Contracts::instantiate( @@ -1528,8 +1528,8 @@ fn destroy_contract_and_transfer_funds() { ExtBuilder::default().existential_deposit(50).build().execute_with(|| { // Create Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, callee_wasm)); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, caller_wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), callee_wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), caller_wasm)); // This deploys the BOB contract, which in turn deploys the CHARLIE contract during // construction. @@ -1567,7 +1567,7 @@ fn cannot_self_destruct_in_constructor() { compile_module::(&load_wasm("self_destructing_constructor.wat")).unwrap(); ExtBuilder::default().existential_deposit(50).build().execute_with(|| { Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), wasm)); // Fail to instantiate the BOB because the call that is issued in the deploy // function exhausts all balances which puts it below the existential deposit. @@ -1595,7 +1595,7 @@ fn get_runtime_storage() { 0x14144020u32.to_le_bytes().to_vec().as_ref() ); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), wasm)); assert_ok!(Contracts::instantiate( Origin::signed(ALICE), 100, @@ -1619,7 +1619,7 @@ fn crypto_hashes() { ExtBuilder::default().existential_deposit(50).build().execute_with(|| { Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), GAS_LIMIT, wasm)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), wasm)); // Instantiate the CRYPTO_HASHES contract. assert_ok!(Contracts::instantiate( diff --git a/frame/contracts/src/wasm/code_cache.rs b/frame/contracts/src/wasm/code_cache.rs index cb942a25892cd..ba7a02356d282 100644 --- a/frame/contracts/src/wasm/code_cache.rs +++ b/frame/contracts/src/wasm/code_cache.rs @@ -26,49 +26,20 @@ //! this guarantees that every instrumented contract code in cache cannot have the version equal to the current one. //! Thus, before executing a contract it should be reinstrument with new schedule. -use crate::gas::{Gas, GasMeter, Token}; use crate::wasm::{prepare, runtime::Env, PrefabWasmModule}; use crate::{CodeHash, CodeStorage, PristineCode, Schedule, Trait}; use sp_std::prelude::*; -use sp_runtime::traits::{Hash, Bounded}; +use sp_runtime::traits::Hash; use frame_support::StorageMap; -/// Gas metering token that used for charging storing code into the code storage. -/// -/// Specifies the code length in bytes. -#[cfg_attr(test, derive(Debug, PartialEq, Eq))] -#[derive(Copy, Clone)] -pub struct PutCodeToken(u32); - -impl Token for PutCodeToken { - type Metadata = Schedule; - - fn calculate_amount(&self, metadata: &Schedule) -> Gas { - metadata - .put_code_per_byte_cost - .checked_mul(self.0.into()) - .unwrap_or_else(|| Bounded::max_value()) - } -} - /// Put code in the storage. The hash of code is used as a key and is returned /// as a result of this function. /// /// This function instruments the given code and caches it in the storage. pub fn save( original_code: Vec, - gas_meter: &mut GasMeter, schedule: &Schedule, ) -> Result, &'static str> { - // The first time instrumentation is on the user. However, consequent reinstrumentation - // due to the schedule changes is on governance system. - if gas_meter - .charge(schedule, PutCodeToken(original_code.len() as u32)) - .is_out_of_gas() - { - return Err("there is not enough gas for storing the code"); - } - let prefab_module = prepare::prepare_contract::(&original_code, schedule)?; let code_hash = T::Hashing::hash(&original_code); From 6b9aa7f564a66da5b3b098e4c7f94508427a6762 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Wed, 22 Apr 2020 18:42:31 +0200 Subject: [PATCH 7/8] Typo fix Co-Authored-By: Sergei Pepyakin --- frame/contracts/src/exec.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/contracts/src/exec.rs b/frame/contracts/src/exec.rs index 67809bbd4026b..9cc1c50260db9 100644 --- a/frame/contracts/src/exec.rs +++ b/frame/contracts/src/exec.rs @@ -204,7 +204,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. + /// Returns the price of one weight unit. fn get_weight_price(&self) -> BalanceOf; } From 27ad6b91223e5894daf097b7d50c085a7c509755 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Wed, 22 Apr 2020 19:43:13 +0200 Subject: [PATCH 8/8] Improve comment on WASM_INSTRUCTION_COST --- frame/contracts/src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 40b42a710a855..8a17ada6fc324 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -938,7 +938,9 @@ pub struct Schedule { pub max_subject_len: u32, } -// This is 500 (2 instruction per nano second on 2GHZ) * 1000x slowdown through wasmi +// 500 (2 instructions per nano second on 2GHZ) * 1000x slowdown through wasmi +// This is a wild guess and should be viewed as a rough estimation. +// Proper benchmarks are needed before this value and its derivatives can be used in production. const WASM_INSTRUCTION_COST: Gas = 500_000; impl Default for Schedule {