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

Integrate pallet_contracts gas with the weight system #5712

Merged
merged 8 commits into from
Apr 24, 2020
Merged

Integrate pallet_contracts gas with the weight system #5712

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4ce3163
Integrate pallet_contracts gas with the weight system
athei Apr 14, 2020
3f5b8a1
Rescale the schedule to new gas definitaion (=1 weight)
athei Apr 21, 2020
32226e5
Increase supplied gas_limit in tests
athei Apr 22, 2020
014d7d8
Apply style suggestions from code review
athei Apr 22, 2020
71a82f3
Fix possible overflow in fn refund
athei Apr 22, 2020
39ef81d
Remove gas metering from fn put_code()
athei Apr 22, 2020
6b9aa7f
Typo fix
athei Apr 22, 2020
27ad6b9
Improve comment on WASM_INSTRUCTION_COST
athei Apr 22, 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: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion bin/node/cli/src/chain_spec.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 1 addition & 2 deletions bin/node/cli/src/factory_impl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ impl<Number> FactoryState<Number> {
frame_system::CheckNonce::from(index),
frame_system::CheckWeight::new(),
pallet_transaction_payment::ChargeTransactionPayment::from(0),
Default::default(),
)
}
}
Expand Down Expand Up @@ -122,7 +121,7 @@ impl RuntimeAdapter for FactoryState<Number> {
(*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 {
Expand Down
3 changes: 1 addition & 2 deletions bin/node/cli/src/service.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
11 changes: 8 additions & 3 deletions bin/node/executor/tests/basic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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::<Runtime>(10_000, transfer_code)
pallet_contracts::Call::put_code::<Runtime>(transfer_code)
),
},
CheckedExtrinsic {
signed: Some((charlie(), signed_extra(1, 0))),
function: Call::Contracts(
pallet_contracts::Call::instantiate::<Runtime>(1 * DOLLARS, 10_000, transfer_ch, Vec::new())
pallet_contracts::Call::instantiate::<Runtime>(
1 * DOLLARS,
500_000_000,
transfer_ch,
Vec::new()
)
),
},
CheckedExtrinsic {
Expand All @@ -621,7 +626,7 @@ fn deploying_wasm_contract_should_work() {
pallet_contracts::Call::call::<Runtime>(
pallet_indices::address::Address::Id(addr.clone()),
10,
10_000,
500_000_000,
vec![0x00, 0x01, 0x02, 0x03]
)
),
Expand Down
17 changes: 1 addition & 16 deletions bin/node/runtime/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down Expand Up @@ -456,40 +455,28 @@ 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;
pub const SurchargeReward: Balance = 150 * DOLLARS;
}

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<Runtime>;
type ComputeDispatchFee = pallet_contracts::DefaultDispatchFeeComputor<Runtime>;
type TrieIdGenerator = pallet_contracts::TrieIdFromParentCounter<Runtime>;
type GasPayment = ();
type RentPayment = ();
type SignedClaimHandicap = pallet_contracts::DefaultSignedClaimHandicap;
type TombstoneDeposit = TombstoneDeposit;
type StorageSizeOffset = pallet_contracts::DefaultStorageSizeOffset;
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 {
Expand Down Expand Up @@ -532,7 +519,6 @@ impl<LocalCall> frame_system::offchain::CreateSignedTransaction<LocalCall> for R
frame_system::CheckNonce::<Runtime>::from(nonce),
frame_system::CheckWeight::<Runtime>::new(),
pallet_transaction_payment::ChargeTransactionPayment::<Runtime>::from(tip),
Default::default(),
);
let raw_payload = SignedPayload::new(call, extra).map_err(|e| {
debug::warn!("Unable to create signed payload: {:?}", e);
Expand Down Expand Up @@ -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<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 @@ -720,7 +706,6 @@ pub type SignedExtra = (
frame_system::CheckNonce<Runtime>,
frame_system::CheckWeight<Runtime>,
pallet_transaction_payment::ChargeTransactionPayment<Runtime>,
pallet_contracts::CheckBlockGasLimit<Runtime>,
Copy link
Contributor

Choose a reason for hiding this comment

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

As a curious note, this could have been a breaking change.

Copy link
Member Author

Choose a reason for hiding this comment

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

It brakes chains which use the contracts module as they will be referencing the now gone signed extension.

What are you saying here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, what do you mean by "referencing"? I was thinking that this could have been a breaking change if CheckBlockGasLimit required including some data into the extrinsics extras (but AFAIK it didn't).

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay then you need to define "braking". Because every chain node compiled against an older version of substrate that uses the contracts pallet will no longer compile after this change. That is what I call braking change.

And yes you are right. It did only validation. No inclusion of additional data.

Copy link
Contributor

Choose a reason for hiding this comment

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

By "breaking" i meant that the format of the extrinsics would have been incompatible.

Copy link
Member Author

Choose a reason for hiding this comment

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

What would be the implication of that? I mean you would import the old extrinsics with a compatible runtime then upgrade the runtime and expect other extrinsics.

);
/// 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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 0 additions & 1 deletion bin/node/testing/src/keyring.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
)
}

Expand Down
2 changes: 0 additions & 2 deletions bin/utils/subkey/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -708,7 +708,6 @@ fn create_extrinsic<C: Crypto>(
frame_system::CheckNonce::<Runtime>::from(i),
frame_system::CheckWeight::<Runtime>::new(),
pallet_transaction_payment::ChargeTransactionPayment::<Runtime>::from(f),
Default::default(),
)
};
let raw_payload = SignedPayload::from_raw(
Expand All @@ -721,7 +720,6 @@ fn create_extrinsic<C: Crypto>(
(),
(),
(),
(),
),
);
let signature = raw_payload.using_encoded(|payload| signer.sign(payload)).into_runtime();
Expand Down
1 change: 1 addition & 0 deletions frame/contracts/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
7 changes: 4 additions & 3 deletions frame/contracts/rpc/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading