Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

pallet-contracts: Migrate to weights mechanism #4147

Closed
wants to merge 44 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
d409505
Initial transformation towards using weights
pepyakin Oct 30, 2019
169d461
Clean
pepyakin Oct 30, 2019
64ff194
Gas price calculation for weight
pepyakin Oct 30, 2019
00dd58e
Doc
pepyakin Oct 30, 2019
c0a89b2
Eradicate gas block limit and fix the test.
pepyakin Oct 30, 2019
78bb525
Set proper gas_price
pepyakin Oct 31, 2019
a1340cd
Bump runtime version
pepyakin Oct 31, 2019
8212f50
WIP, workaround by scaling gas usage
pepyakin Nov 5, 2019
f3794f7
Merge origin/master
pepyakin Nov 18, 2019
b9b1a08
Add a screeming TODO
pepyakin Nov 18, 2019
5d63cb2
Migrate to new weight system
pepyakin Nov 18, 2019
a3c1abc
Clean and docs
pepyakin Nov 18, 2019
36c86ad
Make construct_block less of a footgun
pepyakin Nov 18, 2019
f32fd65
WIP
pepyakin Nov 19, 2019
6e925db
Fix the test
pepyakin Nov 19, 2019
dbd2d3e
Fix the tests
pepyakin Nov 19, 2019
afc8167
Merge 'origin/master' into ser-contract-weights-2
pepyakin Nov 21, 2019
d3afa08
'master' of github.com:paritytech/substrate
pepyakin Dec 9, 2019
d9f5e8b
Fix up merge
pepyakin Dec 9, 2019
aa37774
Clean imports
pepyakin Dec 10, 2019
8e4448c
A few cleanings
pepyakin Dec 10, 2019
dc85fa5
Merge branch 'master' of github.com:paritytech/substrate
pepyakin Dec 10, 2019
c24de55
Fix node-executor tests.
pepyakin Dec 10, 2019
86c09d9
Merge remote-tracking branch 'origin' into ser-contract-weights-2
pepyakin Jan 8, 2020
f9db9c0
Remove unnecessary `.map_err`.
pepyakin Jan 8, 2020
2110696
Fix the tests.
pepyakin Jan 9, 2020
5ef036f
Clean up.
pepyakin Jan 9, 2020
49d1ea5
Apply suggestions from code review
pepyakin Jan 22, 2020
9b18e0e
Update the expect message and add a comment.
pepyakin Jan 22, 2020
33a3ea6
Add associated issue and TODO for tx loophole
pepyakin Jan 22, 2020
74dbd4a
Add some clarifying comments regarding GasUsageReport.
pepyakin Jan 22, 2020
a399472
Added System::remaining_weight convenience function
pepyakin Jan 22, 2020
d6f5cef
Merge origin/master
pepyakin Jan 22, 2020
775c73c
Merge remote-tracking branch 'origin/master' into ser-contract-weights-2
pepyakin Jan 31, 2020
9fd2b90
Leverage FunctionOf for setting put_code price.
pepyakin Jan 31, 2020
b39ca97
Remove a stale TODO
pepyakin Jan 31, 2020
a5c331d
Update docs.
pepyakin Jan 31, 2020
81930e8
Rename CheckGasBlockLimit to GasWeightConversion.
pepyakin Jan 31, 2020
cc233d3
Tidy up.
pepyakin Jan 31, 2020
624b3ef
Renaming
pepyakin Jan 31, 2020
c794526
Merge master
pepyakin Feb 18, 2020
57850d0
Clean
pepyakin Feb 18, 2020
4585dc1
Bump runtime version
pepyakin Feb 18, 2020
cc8ec73
Fix the build.
pepyakin Feb 18, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion bin/node/cli/src/chain_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,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,
Expand Down
6 changes: 4 additions & 2 deletions bin/node/executor/tests/basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -587,6 +587,8 @@ const CODE_TRANSFER: &str = r#"

#[test]
fn deploying_wasm_contract_should_work() {
use pallet_contracts::ContractAddressFor;

let transfer_code = wabt::wat2wasm(CODE_TRANSFER).unwrap();
let transfer_ch = <Runtime as frame_system::Trait>::Hashing::hash(&transfer_code);

Expand All @@ -608,7 +610,7 @@ fn deploying_wasm_contract_should_work() {
CheckedExtrinsic {
signed: Some((charlie(), signed_extra(0, 0))),
function: Call::Contracts(
pallet_contracts::Call::put_code::<Runtime>(10_000, transfer_code)
pallet_contracts::Call::put_code::<Runtime>(transfer_code)
),
},
CheckedExtrinsic {
Expand All @@ -623,7 +625,7 @@ fn deploying_wasm_contract_should_work() {
pallet_contracts::Call::call::<Runtime>(
pallet_indices::address::Address::Id(addr.clone()),
10,
10_000,
100_000_000,
vec![0x00, 0x01, 0x02, 0x03]
)
),
Expand Down
30 changes: 25 additions & 5 deletions bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
// and set impl_version to 0. If only runtime
// implementation changes and behavior does not, then leave spec_version as
// is and increment impl_version.
spec_version: 221,
spec_version: 222,
impl_version: 0,
apis: RUNTIME_API_VERSIONS,
};
Expand Down Expand Up @@ -404,13 +404,31 @@ impl pallet_treasury::Trait for Runtime {
}

parameter_types! {
// NOTE Those are significantly lower than the corresponding fees for transferring funds.
// Since if we charge the fee in gas there is basically not enough gas even to make one simple
// transfer.
//
// This basically means that in order to transfer funds it would make more sense to use the
// contracts module rather than balances module.
//
// This should be fixed in nearest future by using the synchronous transfer directly. It will
// cut fees from the senders account balance directly.
//
// TODO: https://github.com/paritytech/substrate/issues/4713
pub const ContractTransferFee: Balance = 1000;
pub const ContractCreationFee: Balance = 1000;
pub const ContractFee: Balance = 1000;

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;
pub const SurchargeReward: Balance = 150 * DOLLARS;

/// These values mean that it is possible to store code up to approximatelly 250Kb.
pub const PutCodeBaseWeight: Weight = 10_000;
pub const PutCodePerByteWeight: Weight = 4;
}

impl pallet_contracts::Trait for Runtime {
Expand All @@ -437,7 +455,9 @@ impl pallet_contracts::Trait for Runtime {
type InstantiateBaseFee = pallet_contracts::DefaultInstantiateBaseFee;
type MaxDepth = pallet_contracts::DefaultMaxDepth;
type MaxValueSize = pallet_contracts::DefaultMaxValueSize;
type BlockGasLimit = pallet_contracts::DefaultBlockGasLimit;
type WeightToFee = LinearWeightToFee<WeightFeeCoefficient>;
type PutCodeBaseWeight = PutCodeBaseWeight;
type PutCodePerByteWeight = PutCodePerByteWeight;
}

impl pallet_sudo::Trait for Runtime {
Expand Down Expand Up @@ -618,7 +638,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<T>},
Contracts: pallet_contracts::{Module, Call, Config<T>, Storage, Event<T>},
Contracts: pallet_contracts::{Module, Call, Config, Storage, Event<T>},
Sudo: pallet_sudo::{Module, Call, Config<T>, Storage, Event<T>},
ImOnline: pallet_im_online::{Module, Call, Storage, Event<T>, ValidateUnsigned, Config<T>},
AuthorityDiscovery: pallet_authority_discovery::{Module, Call, Config},
Expand Down Expand Up @@ -649,7 +669,7 @@ pub type SignedExtra = (
frame_system::CheckNonce<Runtime>,
frame_system::CheckWeight<Runtime>,
pallet_transaction_payment::ChargeTransactionPayment<Runtime>,
pallet_contracts::CheckBlockGasLimit<Runtime>,
pallet_contracts::GasWeightConversion<Runtime>,
);
/// Unchecked extrinsic type as expected by this runtime.
pub type UncheckedExtrinsic = generic::UncheckedExtrinsic<Address, Call, Signature, SignedExtra>;
Expand Down
1 change: 0 additions & 1 deletion bin/node/testing/src/genesis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,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 {
Expand Down
63 changes: 3 additions & 60 deletions frame/contracts/src/gas.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,8 @@
// You should have received a copy of the GNU General Public License
// along with Substrate. If not, see <http://www.gnu.org/licenses/>.

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::{BalanceOf, Trait};
use sp_runtime::traits::{AtLeast32Bit, SaturatedConversion, Zero};

#[cfg(test)]
use std::{any::Any, fmt::Debug};
Expand Down Expand Up @@ -185,7 +178,7 @@ impl<T: Trait> GasMeter<T> {
}

/// Returns how much gas was spent.
fn spent(&self) -> Gas {
pub fn spent(&self) -> Gas {
self.limit - self.gas_left
}

Expand All @@ -195,56 +188,6 @@ impl<T: Trait> GasMeter<T> {
}
}

/// 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<T: Trait>(
transactor: &T::AccountId,
gas_limit: Gas,
) -> Result<(GasMeter<T>, NegativeImbalanceOf<T>), DispatchError> {
// Buy the specified amount of gas.
let gas_price = <Module<T>>::gas_price();
let cost = if gas_price.is_zero() {
<BalanceOf<T>>::zero()
} else {
<BalanceOf<T> as TryFrom<Gas>>::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<T: Trait>(
transactor: &T::AccountId,
gas_meter: GasMeter<T>,
imbalance: NegativeImbalanceOf<T>,
) {
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<Balance>(gas_price: Balance, balance: Balance) -> Gas
Expand Down
195 changes: 195 additions & 0 deletions frame/contracts/src/gas_weight_conv.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,195 @@
// Copyright 2020 Parity Technologies (UK) Ltd.
// This file is part of Substrate.

// Substrate is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.

// Substrate is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.

// You should have received a copy of the GNU General Public License
// along with Substrate. If not, see <http://www.gnu.org/licenses/>.

use crate::{Call, GasPrice, GasUsageReport, NegativeImbalanceOf, Trait};
use frame_support::{
storage::StorageValue,
traits::{Currency, ExistenceRequirement, Imbalance, OnUnbalanced, WithdrawReason},
weights::{DispatchInfo, Weight},
IsSubType,
};
use sp_runtime::{
traits::{CheckedDiv, Convert, SignedExtension, UniqueSaturatedInto},
transaction_validity::{
InvalidTransaction, TransactionValidity, TransactionValidityError, ValidTransaction,
},
};
use sp_std::{convert::TryFrom, fmt, marker::PhantomData, prelude::*};

/// Data which is collected during the pre-dispatch phase and needed at the post_dispatch phase.
#[cfg_attr(test, derive(Debug))]
pub struct GasCallEnvelope<AccountId, NegativeImbalance> {
/// The account id who should receive the refund from the gas leftovers.
transactor: AccountId,
/// The negative imbalance obtained by withdrawing the value up to the requested gas limit.
imbalance: NegativeImbalance,
}

/// A `SignedExtension` required to properly handle gas limits requested by contract executing
/// transactions.
#[derive(codec::Encode, codec::Decode, Clone, Eq, PartialEq)]
pub struct GasWeightConversion<T: Trait + Send + Sync>(PhantomData<T>);

impl<T: Trait + Send + Sync> GasWeightConversion<T> {
pub fn new() -> Self {
Self(PhantomData)
}

/// Perform pre-dispatch checks for the given call and origin.
fn perform_pre_dispatch_checks(
who: &T::AccountId,
call: &<T as Trait>::Call,
) -> Result<
Option<GasCallEnvelope<T::AccountId, NegativeImbalanceOf<T>>>,
TransactionValidityError,
> {
// This signed extension only deals with this module's `Call`.
let call = match call.is_sub_type() {
Some(call) => call,
None => return Ok(None),
};

match *call {
Call::claim_surcharge(_, _) | Call::update_schedule(_) | Call::put_code(_) => Ok(None),
Call::call(_, _, gas_limit, _) | Call::instantiate(_, gas_limit, _, _) => {
// Compute how much block weight this transaction can take at its limit, i.e.
// if this transaction depleted all provided gas to zero.
let gas_weight_limit = Weight::try_from(gas_limit)
.map_err(|_| InvalidTransaction::ExhaustsResources)?;
let weight_available = <frame_system::Module<T>>::remaining_weight().into();
if gas_weight_limit > weight_available {
// We discard the transaction if the requested limit exceeds the available
// amount of weight in the current block.
//
// Note that this transaction is left out only from the current block and txq
// might attempt to include this transaction again.
return Err(InvalidTransaction::ExhaustsResources.into());
}

// Compute the fee corresponding for the given gas_weight_limit and attempt
// withdrawing from the origin of this transaction.
let fee = T::WeightToFee::convert(gas_weight_limit);

// Compute and store the effective price per unit of gas.
let gas_price = {
let divisor = gas_weight_limit.unique_saturated_into();
fee.checked_div(&divisor).unwrap_or(1.into())
};

//
// The place where we set GasPrice for the execution of this transaction.
//
<GasPrice<T>>::put(gas_price);

let imbalance = match T::Currency::withdraw(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You call this from SignedExtension::validate and SignedExtension::pre_dispatch? Aren't you charging the who double by doing this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whatever we do in validate() is not persistent. The state is dropped thereafter.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But changes in pre_dispatch are persisted, right?

who,
fee,
WithdrawReason::TransactionPayment.into(),
ExistenceRequirement::KeepAlive,
) {
Ok(imbalance) => imbalance,
Err(_) => return Err(InvalidTransaction::Payment.into()),
};

Ok(Some(GasCallEnvelope {
transactor: who.clone(),
imbalance,
}))
}
Call::__PhantomItem(_, _) => unreachable!("Variant is never constructed"),
}
}
}

impl<T: Trait + Send + Sync> Default for GasWeightConversion<T> {
fn default() -> Self {
Self(PhantomData)
}
}

impl<T: Trait + Send + Sync> fmt::Debug for GasWeightConversion<T> {
#[cfg(feature = "std")]
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "GasWeightConversion")
}

#[cfg(not(feature = "std"))]
fn fmt(&self, _: &mut sp_std::fmt::Formatter) -> sp_std::fmt::Result {
Ok(())
}
}

impl<T: Trait + Send + Sync> SignedExtension for GasWeightConversion<T> {
const IDENTIFIER: &'static str = "GasWeightConversion";
type AccountId = T::AccountId;
type Call = <T as Trait>::Call;
type AdditionalSigned = ();
type DispatchInfo = DispatchInfo;
/// Optional pre-dispatch check data.
///
/// It is present only for the contract calls that operate with gas.
type Pre = Option<GasCallEnvelope<T::AccountId, NegativeImbalanceOf<T>>>;

fn additional_signed(&self) -> Result<(), TransactionValidityError> {
Ok(())
}

fn pre_dispatch(
self,
who: &Self::AccountId,
call: &Self::Call,
_: DispatchInfo,
_: usize,
) -> Result<Self::Pre, TransactionValidityError> {
Self::perform_pre_dispatch_checks(who, call)
}

fn validate(
&self,
who: &Self::AccountId,
call: &Self::Call,
_: Self::DispatchInfo,
_: usize,
) -> TransactionValidity {
Self::perform_pre_dispatch_checks(who, call).map(|_| ValidTransaction::default())
}

fn post_dispatch(pre: Self::Pre, _info: DispatchInfo, _len: usize) {
if let Some(GasCallEnvelope {
transactor,
imbalance,
}) = pre
{
// Take the report of consumed and left gas after the execution of the current
// transaction.
let (gas_left, gas_spent) = GasUsageReport::take();

// These should be OK since we don't buy more
let unused_weight = gas_left as Weight;
let spent_weight = gas_spent as Weight;

let refund = T::WeightToFee::convert(unused_weight);

// Refund the unused gas.
let refund_imbalance = T::Currency::deposit_creating(&transactor, refund);
if let Ok(imbalance) = imbalance.offset(refund_imbalance) {
T::GasPayment::on_unbalanced(imbalance);
}

<frame_system::Module<T>>::register_extra_weight_unchecked(spent_weight);
}
}
}
Loading