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 27 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 @@ -276,7 +276,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
19 changes: 14 additions & 5 deletions bin/node/executor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ mod tests {
Fixed64, traits::{Header as HeaderT, Hash as HashT, Convert}, ApplyExtrinsicResult,
transaction_validity::InvalidTransaction,
};
use pallet_contracts::ContractAddressFor;
use sc_executor::{NativeExecutor, WasmExecutionMethod};
use frame_system::{EventRecord, Phase};
use node_runtime::{
Expand Down Expand Up @@ -311,6 +310,10 @@ mod tests {
ext
}

/// Construct a fake block.
///
/// `extrinsics` must be a list of valid extrinsics, i.e. none of the extrinsics for example
/// can report `ExhaustResources`. Otherwise, this function panics.
fn construct_block(
env: &mut TestExternalities<Blake2Hasher>,
number: BlockNumber,
Expand Down Expand Up @@ -346,13 +349,17 @@ mod tests {
).0.unwrap();

for i in extrinsics.iter() {
executor_call::<NeverNativeValue, fn() -> _>(
let r = executor_call::<NeverNativeValue, fn() -> _>(
env,
"BlockBuilder_apply_extrinsic",
&i.encode(),
true,
None,
).0.unwrap();
).0.expect("execution failed").into_encoded();
pepyakin marked this conversation as resolved.
Show resolved Hide resolved
match ApplyExtrinsicResult::decode(&mut &r[..]).expect("apply result deserialization failed") {
Ok(_) => {},
Err(e) => panic!("panic while applying extrinsic: {:?}", e),
}
}

let header = match executor_call::<NeverNativeValue, fn() -> _>(
Expand Down Expand Up @@ -727,6 +734,8 @@ mod tests {

#[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 @@ -748,7 +757,7 @@ mod tests {
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 @@ -763,7 +772,7 @@ mod tests {
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
20 changes: 15 additions & 5 deletions bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -391,11 +391,21 @@ impl pallet_treasury::Trait for Runtime {
}

parameter_types! {
pub const ContractTransferFee: Balance = 1 * CENTS;
pub const ContractCreationFee: Balance = 1 * CENTS;
// NOTE Those are significantly lower than than corresponding fees for transferring funds.
pepyakin marked this conversation as resolved.
Show resolved Hide resolved
pepyakin marked this conversation as resolved.
Show resolved Hide resolved
// 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.
pub const ContractTransferFee: Balance = 1000;
pub const ContractCreationFee: Balance = 1000;
pub const ContractFee: Balance = 1000;
pepyakin marked this conversation as resolved.
Show resolved Hide resolved

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;
Expand Down Expand Up @@ -428,7 +438,7 @@ 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>;
}

impl pallet_sudo::Trait for Runtime {
Expand Down Expand Up @@ -548,7 +558,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,
Contracts: pallet_contracts::{Module, Call, Storage, Config, Event<T>},
Sudo: pallet_sudo,
ImOnline: pallet_im_online::{Module, Call, Storage, Event<T>, ValidateUnsigned, Config<T>},
AuthorityDiscovery: pallet_authority_discovery::{Module, Call, Config},
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 @@ -82,7 +82,6 @@ pub fn config(support_changes_trie: bool, code: Option<&[u8]>) -> GenesisConfig
}),
pallet_contracts: Some(ContractsConfig {
current_schedule: Default::default(),
gas_price: 1 * MILLICENTS,
}),
pallet_babe: Some(Default::default()),
pallet_grandpa: Some(GrandpaConfig {
Expand Down
61 changes: 3 additions & 58 deletions frame/contracts/src/gas.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,9 @@
// 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 crate::{Trait, BalanceOf};
use sp_runtime::traits::{
CheckedMul, Zero, SaturatedConversion, SimpleArithmetic, UniqueSaturatedInto,
};
use frame_support::{
traits::{Currency, ExistenceRequirement, Imbalance, OnUnbalanced, WithdrawReason}, StorageValue,
dispatch::DispatchError,
Zero, SaturatedConversion, SimpleArithmetic,
};

#[cfg(test)]
Expand Down Expand Up @@ -185,7 +180,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 +190,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
Loading