From f6aafe83ec149a8798d35b4e0842116fdc8d8fc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Thu, 4 Mar 2021 09:17:44 +0100 Subject: [PATCH 1/5] contracts: Remove types and storage from the public interface --- bin/node/cli/src/chain_spec.rs | 7 +- bin/node/runtime/src/lib.rs | 6 +- frame/contracts/src/benchmarking/mod.rs | 1 + frame/contracts/src/lib.rs | 179 ++++-------------------- frame/contracts/src/schedule.rs | 32 ++++- frame/contracts/src/storage.rs | 125 +++++++++++++++-- frame/contracts/src/tests.rs | 3 +- frame/contracts/src/wasm/prepare.rs | 2 +- frame/contracts/src/wasm/runtime.rs | 3 +- 9 files changed, 183 insertions(+), 175 deletions(-) diff --git a/bin/node/cli/src/chain_spec.rs b/bin/node/cli/src/chain_spec.rs index ae1418981f167..c30710d236acc 100644 --- a/bin/node/cli/src/chain_spec.rs +++ b/bin/node/cli/src/chain_spec.rs @@ -295,10 +295,9 @@ pub fn testnet_genesis( phantom: Default::default(), }, pallet_contracts: ContractsConfig { - current_schedule: pallet_contracts::Schedule { - enable_println, // this should only be enabled on development chains - ..Default::default() - }, + // println should only be enabled on development chains + current_schedule: pallet_contracts::Schedule::default() + .enable_println(enable_println), }, pallet_sudo: SudoConfig { key: root_key, diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index bb372f31c73b9..f3cd54edecea3 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -760,11 +760,11 @@ impl pallet_tips::Config for Runtime { } parameter_types! { - pub const TombstoneDeposit: Balance = deposit( + pub TombstoneDeposit: Balance = deposit( 1, - sp_std::mem::size_of::>() as u32 + >::contract_info_size(), ); - pub const DepositPerContract: Balance = TombstoneDeposit::get(); + pub DepositPerContract: Balance = TombstoneDeposit::get(); pub const DepositPerStorageByte: Balance = deposit(0, 1); pub const DepositPerStorageItem: Balance = deposit(1, 0); pub RentFraction: Perbill = Perbill::from_rational_approximation(1u32, 30 * DAYS); diff --git a/frame/contracts/src/benchmarking/mod.rs b/frame/contracts/src/benchmarking/mod.rs index d41154e995a67..928ba9fffa9dc 100644 --- a/frame/contracts/src/benchmarking/mod.rs +++ b/frame/contracts/src/benchmarking/mod.rs @@ -36,6 +36,7 @@ use self::{ }, sandbox::Sandbox, }; +use codec::Encode; use frame_benchmarking::{benchmarks, account, whitelisted_caller, impl_benchmark_test_suite}; use frame_system::{Module as System, RawOrigin}; use parity_wasm::elements::{Instruction, ValueType, BlockType}; diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 5453e079e3ae4..a6d57fd6a6588 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -98,29 +98,24 @@ pub mod weights; #[cfg(test)] mod tests; -pub use crate::{ - wasm::PrefabWasmModule, - schedule::{Schedule, HostFnWeights, InstructionWeights, Limits}, - pallet::*, -}; +pub use crate::{pallet::*, schedule::Schedule}; use crate::{ gas::GasMeter, exec::{ExecutionContext, Executable}, rent::Rent, - storage::{Storage, DeletedContract}, + storage::{Storage, DeletedContract, ContractInfo, AliveContractInfo, TombstoneContractInfo}, weights::WeightInfo, + wasm::PrefabWasmModule, }; use sp_core::crypto::UncheckedFrom; -use sp_std::{prelude::*, marker::PhantomData, fmt::Debug}; -use codec::{Codec, Encode, Decode}; +use sp_std::prelude::*; use sp_runtime::{ traits::{ - Hash, StaticLookup, MaybeSerializeDeserialize, Member, Convert, Saturating, Zero, + Hash, StaticLookup, Convert, Saturating, Zero, }, - RuntimeDebug, Perbill, + Perbill, }; use frame_support::{ - storage::child::ChildInfo, traits::{OnUnbalanced, Currency, Get, Time, Randomness}, weights::{Weight, PostDispatchInfo, WithPostDispatchInfo}, }; @@ -129,16 +124,12 @@ use pallet_contracts_primitives::{ RentProjectionResult, GetStorageResult, ContractAccessError, ContractExecResult, }; -pub type CodeHash = ::Hash; -pub type TrieId = Vec; -pub type BalanceOf = +type CodeHash = ::Hash; +type TrieId = Vec; +type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; -pub type NegativeImbalanceOf = +type NegativeImbalanceOf = <::Currency as Currency<::AccountId>>::NegativeImbalance; -pub type AliveContractInfo = - RawAliveContractInfo, BalanceOf, ::BlockNumber>; -pub type TombstoneContractInfo = - RawTombstoneContractInfo<::Hash, ::Hashing>; #[frame_support::pallet] pub mod pallet { @@ -615,32 +606,32 @@ pub mod pallet { /// Current cost schedule for contracts. #[pallet::storage] #[pallet::getter(fn current_schedule)] - pub(super) type CurrentSchedule = StorageValue<_, Schedule, ValueQuery>; + pub(crate) type CurrentSchedule = StorageValue<_, Schedule, ValueQuery>; /// A mapping from an original code hash to the original code, untouched by instrumentation. #[pallet::storage] - pub type PristineCode = StorageMap<_, Identity, CodeHash, Vec>; + pub(crate) type PristineCode = StorageMap<_, Identity, CodeHash, Vec>; /// A mapping between an original code hash and instrumented wasm code, ready for execution. #[pallet::storage] - pub type CodeStorage = StorageMap<_, Identity, CodeHash, PrefabWasmModule>; + pub(crate) type CodeStorage = StorageMap<_, Identity, CodeHash, PrefabWasmModule>; /// The subtrie counter. #[pallet::storage] - pub type AccountCounter = StorageValue<_, u64, ValueQuery>; + pub(crate) type AccountCounter = StorageValue<_, u64, ValueQuery>; /// The code associated with a given account. /// /// TWOX-NOTE: SAFE since `AccountId` is a secure hash. #[pallet::storage] - pub type ContractInfoOf = StorageMap<_, Twox64Concat, T::AccountId, ContractInfo>; + pub(crate) type ContractInfoOf = StorageMap<_, Twox64Concat, T::AccountId, ContractInfo>; /// Evicted contracts that await child trie deletion. /// /// Child trie deletion is a heavy operation depending on the amount of storage items /// stored in said trie. Therefore this operation is performed lazily in `on_initialize`. #[pallet::storage] - pub type DeletionQueue = StorageValue<_, Vec, ValueQuery>; + pub(crate) type DeletionQueue = StorageValue<_, Vec, ValueQuery>; #[pallet::genesis_config] pub struct GenesisConfig { @@ -743,6 +734,20 @@ where T::Currency::minimum_balance().saturating_add(T::TombstoneDeposit::get()) } + /// The in-memory size in bytes of the data structure associated with each contract. + /// + /// The data structure is also put into storage for each contract. The in-storage size + /// is never larger than the in-memory representation and usually smaller due to compact + /// encoding and lack of padding. + /// + /// # Note + /// + /// This returns the in-memory size because the in-storage size (SCALE encoded) cannot + /// be efficiently determined. Treat this as an upper bound of the in-storage size. + pub fn contract_info_size() -> u32 { + sp_std::mem::size_of::>() as u32 + } + /// Store code for benchmarks which does not check nor instrument the code. #[cfg(feature = "runtime-benchmarks")] fn store_code_raw(code: Vec) -> frame_support::dispatch::DispatchResult { @@ -760,127 +765,3 @@ where self::wasm::reinstrument(module, schedule) } } - -/// Information for managing an account and its sub trie abstraction. -/// This is the required info to cache for an account -#[derive(Encode, Decode, RuntimeDebug)] -pub enum ContractInfo { - Alive(AliveContractInfo), - Tombstone(TombstoneContractInfo), -} - -impl ContractInfo { - /// If contract is alive then return some alive info - pub fn get_alive(self) -> Option> { - if let ContractInfo::Alive(alive) = self { - Some(alive) - } else { - None - } - } - /// If contract is alive then return some reference to alive info - pub fn as_alive(&self) -> Option<&AliveContractInfo> { - if let ContractInfo::Alive(ref alive) = self { - Some(alive) - } else { - None - } - } - /// If contract is alive then return some mutable reference to alive info - pub fn as_alive_mut(&mut self) -> Option<&mut AliveContractInfo> { - if let ContractInfo::Alive(ref mut alive) = self { - Some(alive) - } else { - None - } - } - - /// If contract is tombstone then return some tombstone info - pub fn get_tombstone(self) -> Option> { - if let ContractInfo::Tombstone(tombstone) = self { - Some(tombstone) - } else { - None - } - } - /// If contract is tombstone then return some reference to tombstone info - pub fn as_tombstone(&self) -> Option<&TombstoneContractInfo> { - if let ContractInfo::Tombstone(ref tombstone) = self { - Some(tombstone) - } else { - None - } - } - /// If contract is tombstone then return some mutable reference to tombstone info - pub fn as_tombstone_mut(&mut self) -> Option<&mut TombstoneContractInfo> { - if let ContractInfo::Tombstone(ref mut tombstone) = self { - Some(tombstone) - } else { - None - } - } -} - -/// Information for managing an account and its sub trie abstraction. -/// This is the required info to cache for an account. -#[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug)] -pub struct RawAliveContractInfo { - /// Unique ID for the subtree encoded as a bytes vector. - pub trie_id: TrieId, - /// The total number of bytes used by this contract. - /// - /// It is a sum of each key-value pair stored by this contract. - pub storage_size: u32, - /// The total number of key-value pairs in storage of this contract. - pub pair_count: u32, - /// The code associated with a given account. - pub code_hash: CodeHash, - /// Pay rent at most up to this value. - pub rent_allowance: Balance, - /// The amount of rent that was payed by the contract over its whole lifetime. - /// - /// A restored contract starts with a value of zero just like a new contract. - pub rent_payed: Balance, - /// Last block rent has been payed. - pub deduct_block: BlockNumber, - /// Last block child storage has been written. - pub last_write: Option, - /// This field is reserved for future evolution of format. - pub _reserved: Option<()>, -} - -impl RawAliveContractInfo { - /// Associated child trie unique id is built from the hash part of the trie id. - pub fn child_trie_info(&self) -> ChildInfo { - child_trie_info(&self.trie_id[..]) - } -} - -/// Associated child trie unique id is built from the hash part of the trie id. -pub(crate) fn child_trie_info(trie_id: &[u8]) -> ChildInfo { - ChildInfo::new_default(trie_id) -} - -#[derive(Encode, Decode, PartialEq, Eq, RuntimeDebug)] -pub struct RawTombstoneContractInfo(H, PhantomData); - -impl RawTombstoneContractInfo -where - H: Member + MaybeSerializeDeserialize+ Debug - + AsRef<[u8]> + AsMut<[u8]> + Copy + Default - + sp_std::hash::Hash + Codec, - Hasher: Hash, -{ - fn new(storage_root: &[u8], code_hash: H) -> Self { - let mut buf = Vec::new(); - storage_root.using_encoded(|encoded| buf.extend_from_slice(encoded)); - buf.extend_from_slice(code_hash.as_ref()); - RawTombstoneContractInfo(::hash(&buf[..]), PhantomData) - } -} - -impl From> for ContractInfo { - fn from(alive_info: AliveContractInfo) -> Self { - Self::Alive(alive_info) - } -} diff --git a/frame/contracts/src/schedule.rs b/frame/contracts/src/schedule.rs index 24ba83cc1b799..90c396c627775 100644 --- a/frame/contracts/src/schedule.rs +++ b/frame/contracts/src/schedule.rs @@ -39,7 +39,11 @@ pub const API_BENCHMARK_BATCH_SIZE: u32 = 100; /// as for `API_BENCHMARK_BATCH_SIZE`. pub const INSTR_BENCHMARK_BATCH_SIZE: u32 = 1_000; -/// Definition of the cost schedule and other parameterizations for wasm vm. +/// Definition of the cost schedule and other parameterizations for the wasm vm. +/// +/// Its fields are private to the crate in order to allow addition of new contract +/// callable functions without bumping to a new major version. A genesis config should +/// rely on public functions of this type. #[cfg_attr(feature = "std", derive(Serialize, Deserialize))] #[cfg_attr(feature = "std", serde(bound(serialize = "", deserialize = "")))] #[derive(Clone, Encode, Decode, PartialEq, Eq, ScheduleDebug)] @@ -53,20 +57,20 @@ pub struct Schedule { /// of all contracts which are triggered by a version comparison on call. /// Changes to other parts of the schedule should not increment the version in /// order to avoid unnecessary re-instrumentations. - pub version: u32, + pub(crate) version: u32, /// Whether the `seal_println` function is allowed to be used contracts. /// MUST only be enabled for `dev` chains, NOT for production chains - pub enable_println: bool, + pub(crate) enable_println: bool, /// Describes the upper limits on various metrics. - pub limits: Limits, + pub(crate) limits: Limits, /// The weights for individual wasm instructions. - pub instruction_weights: InstructionWeights, + pub(crate) instruction_weights: InstructionWeights, /// The weights for each imported function a contract is allowed to call. - pub host_fn_weights: HostFnWeights, + pub(crate) host_fn_weights: HostFnWeights, } /// Describes the upper limits on various metrics. @@ -602,7 +606,21 @@ struct ScheduleRules<'a, T: Config> { } impl Schedule { - pub fn rules(&self, module: &elements::Module) -> impl rules::Rules + '_ { + /// Allow contracts to call `seal_println` in order to print messages to the console. + /// + /// This should only ever be activated in development chains. The printed messages + /// can be observed on the console by setting the environment variable + /// `RUST_LOG=runtime=debug` when running the node. + /// + /// # Note + /// + /// Is set to `false` by default. + pub fn enable_println(mut self, enable: bool) -> Self { + self.enable_println = enable; + self + } + + pub(crate) fn rules(&self, module: &elements::Module) -> impl rules::Rules + '_ { ScheduleRules { schedule: &self, params: module diff --git a/frame/contracts/src/storage.rs b/frame/contracts/src/storage.rs index 970eec2003668..b729ef181e775 100644 --- a/frame/contracts/src/storage.rs +++ b/frame/contracts/src/storage.rs @@ -19,23 +19,131 @@ use crate::{ exec::{AccountIdOf, StorageKey}, - AliveContractInfo, BalanceOf, CodeHash, ContractInfo, ContractInfoOf, Config, TrieId, + BalanceOf, CodeHash, ContractInfoOf, Config, TrieId, AccountCounter, DeletionQueue, Error, weights::WeightInfo, }; -use codec::{Encode, Decode}; +use codec::{Codec, Encode, Decode}; use sp_std::prelude::*; -use sp_std::marker::PhantomData; +use sp_std::{marker::PhantomData, fmt::Debug}; use sp_io::hashing::blake2_256; -use sp_runtime::traits::{Bounded, Saturating, Zero}; +use sp_runtime::{ + RuntimeDebug, + traits::{Bounded, Saturating, Zero, Hash, Member, MaybeSerializeDeserialize}, +}; use sp_core::crypto::UncheckedFrom; use frame_support::{ dispatch::{DispatchError, DispatchResult}, - storage::child::{self, KillChildStorageResult}, + storage::child::{self, KillChildStorageResult, ChildInfo}, traits::Get, weights::Weight, }; +pub type AliveContractInfo = + RawAliveContractInfo, BalanceOf, ::BlockNumber>; +pub type TombstoneContractInfo = + RawTombstoneContractInfo<::Hash, ::Hashing>; + +/// Information for managing an account and its sub trie abstraction. +/// This is the required info to cache for an account +#[derive(Encode, Decode, RuntimeDebug)] +pub enum ContractInfo { + Alive(AliveContractInfo), + Tombstone(TombstoneContractInfo), +} + +impl ContractInfo { + /// If contract is alive then return some alive info + pub fn get_alive(self) -> Option> { + if let ContractInfo::Alive(alive) = self { + Some(alive) + } else { + None + } + } + /// If contract is alive then return some reference to alive info + pub fn as_alive(&self) -> Option<&AliveContractInfo> { + if let ContractInfo::Alive(ref alive) = self { + Some(alive) + } else { + None + } + } + + /// If contract is tombstone then return some tombstone info + pub fn get_tombstone(self) -> Option> { + if let ContractInfo::Tombstone(tombstone) = self { + Some(tombstone) + } else { + None + } + } +} + +/// Information for managing an account and its sub trie abstraction. +/// This is the required info to cache for an account. +#[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug)] +pub struct RawAliveContractInfo { + /// Unique ID for the subtree encoded as a bytes vector. + pub trie_id: TrieId, + /// The total number of bytes used by this contract. + /// + /// It is a sum of each key-value pair stored by this contract. + pub storage_size: u32, + /// The total number of key-value pairs in storage of this contract. + pub pair_count: u32, + /// The code associated with a given account. + pub code_hash: CodeHash, + /// Pay rent at most up to this value. + pub rent_allowance: Balance, + /// The amount of rent that was payed by the contract over its whole lifetime. + /// + /// A restored contract starts with a value of zero just like a new contract. + pub rent_payed: Balance, + /// Last block rent has been payed. + pub deduct_block: BlockNumber, + /// Last block child storage has been written. + pub last_write: Option, + /// This field is reserved for future evolution of format. + pub _reserved: Option<()>, +} + +impl RawAliveContractInfo { + /// Associated child trie unique id is built from the hash part of the trie id. + pub fn child_trie_info(&self) -> ChildInfo { + child_trie_info(&self.trie_id[..]) + } +} + +/// Associated child trie unique id is built from the hash part of the trie id. +fn child_trie_info(trie_id: &[u8]) -> ChildInfo { + ChildInfo::new_default(trie_id) +} + +#[derive(Encode, Decode, PartialEq, Eq, RuntimeDebug)] +pub struct RawTombstoneContractInfo(H, PhantomData); + +impl RawTombstoneContractInfo +where + H: Member + MaybeSerializeDeserialize+ Debug + + AsRef<[u8]> + AsMut<[u8]> + Copy + Default + + sp_std::hash::Hash + Codec, + Hasher: Hash, +{ + pub fn new(storage_root: &[u8], code_hash: H) -> Self { + let mut buf = Vec::new(); + storage_root.using_encoded(|encoded| buf.extend_from_slice(encoded)); + buf.extend_from_slice(code_hash.as_ref()); + RawTombstoneContractInfo(::hash(&buf[..]), PhantomData) + } +} + +impl From> for ContractInfo { + fn from(alive_info: AliveContractInfo) -> Self { + Self::Alive(alive_info) + } +} + /// An error that means that the account requested either doesn't exist or represents a tombstone /// account. #[cfg_attr(test, derive(PartialEq, Eq, Debug))] @@ -59,7 +167,7 @@ where /// The read is performed from the `trie_id` only. The `address` is not necessary. If the contract /// doesn't store under the given `key` `None` is returned. pub fn read(trie_id: &TrieId, key: &StorageKey) -> Option> { - child::get_raw(&crate::child_trie_info(&trie_id), &blake2_256(key)) + child::get_raw(&child_trie_info(&trie_id), &blake2_256(key)) } /// Update a storage entry into a contract's kv storage. @@ -87,7 +195,7 @@ where }; let hashed_key = blake2_256(key); - let child_trie_info = &crate::child_trie_info(&trie_id); + let child_trie_info = &child_trie_info(&trie_id); let opt_prev_len = child::len(&child_trie_info, &hashed_key); @@ -257,7 +365,7 @@ where let trie = &mut queue[0]; let pair_count = trie.pair_count; let outcome = child::kill_storage( - &crate::child_trie_info(&trie.trie_id), + &child_trie_info(&trie.trie_id), Some(remaining_key_budget), ); if pair_count > remaining_key_budget { @@ -290,7 +398,6 @@ where /// This generator uses inner counter for account id and applies the hash over `AccountId + /// accountid_counter`. pub fn generate_trie_id(account_id: &AccountIdOf) -> TrieId { - use sp_runtime::traits::Hash; // Note that skipping a value due to error is not an issue here. // We only need uniqueness, not sequence. let new_seed = >::mutate(|v| { diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index 2fa09e3405c1c..b2aec3d95d1a7 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -17,7 +17,7 @@ use crate::{ BalanceOf, ContractInfo, ContractInfoOf, Module, - RawAliveContractInfo, Config, Schedule, + Config, Schedule, Error, storage::Storage, chain_extension::{ Result as ExtensionResult, Environment, ChainExtension, Ext, SysConfig, RetVal, @@ -26,6 +26,7 @@ use crate::{ exec::{AccountIdOf, Executable}, wasm::PrefabWasmModule, weights::WeightInfo, wasm::ReturnCode as RuntimeReturnCode, + storage::RawAliveContractInfo, }; use assert_matches::assert_matches; use codec::Encode; diff --git a/frame/contracts/src/wasm/prepare.rs b/frame/contracts/src/wasm/prepare.rs index caf6ef88c1ba0..d710f4ecba853 100644 --- a/frame/contracts/src/wasm/prepare.rs +++ b/frame/contracts/src/wasm/prepare.rs @@ -526,7 +526,7 @@ pub mod benchmarking { #[cfg(test)] mod tests { use super::*; - use crate::{exec::Ext, Limits}; + use crate::{exec::Ext, schedule::Limits}; use std::fmt; use assert_matches::assert_matches; diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index 2ceac1c51604e..74a17d70bc6ef 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -18,10 +18,11 @@ //! Environment definition of the wasm smart-contract runtime. use crate::{ - HostFnWeights, Config, CodeHash, BalanceOf, Error, + Config, CodeHash, BalanceOf, Error, exec::{Ext, StorageKey, TopicOf}, gas::{GasMeter, Token, ChargedAmount}, wasm::env_def::ConvertibleToWasm, + schedule::HostFnWeights, }; use parity_wasm::elements::ValueType; use frame_support::{dispatch::DispatchError, ensure, traits::Get, weights::Weight}; From 6d13f5ba003003bdbc1f406b1ff264dc511ab896 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Thu, 4 Mar 2021 10:17:02 +0100 Subject: [PATCH 2/5] contracts: Remove current_schedule() getter --- frame/contracts/src/benchmarking/code.rs | 8 ++--- frame/contracts/src/benchmarking/mod.rs | 24 +++++++-------- frame/contracts/src/exec.rs | 38 ++++++++++++------------ frame/contracts/src/lib.rs | 14 ++++----- 4 files changed, 40 insertions(+), 44 deletions(-) diff --git a/frame/contracts/src/benchmarking/code.rs b/frame/contracts/src/benchmarking/code.rs index 64d2a0cf011d9..118ce038fc229 100644 --- a/frame/contracts/src/benchmarking/code.rs +++ b/frame/contracts/src/benchmarking/code.rs @@ -24,9 +24,7 @@ //! we define this simple definition of a contract that can be passed to `create_code` that //! compiles it down into a `WasmModule` that can be used as a contract's code. -use crate::Config; -use crate::Module as Contracts; - +use crate::{Config, CurrentSchedule}; use parity_wasm::elements::{ Instruction, Instructions, FuncBody, ValueType, BlockType, Section, CustomSection, }; @@ -225,7 +223,7 @@ where if def.inject_stack_metering { code = inject_limiter( code, - Contracts::::current_schedule().limits.stack_height + >::get().limits.stack_height ) .unwrap(); } @@ -505,5 +503,5 @@ where T: Config, T::AccountId: UncheckedFrom + AsRef<[u8]>, { - Contracts::::current_schedule().limits.memory_pages + >::get().limits.memory_pages } diff --git a/frame/contracts/src/benchmarking/mod.rs b/frame/contracts/src/benchmarking/mod.rs index 928ba9fffa9dc..6c66c77df3a0d 100644 --- a/frame/contracts/src/benchmarking/mod.rs +++ b/frame/contracts/src/benchmarking/mod.rs @@ -314,7 +314,7 @@ benchmarks! { let WasmModule { code, hash, .. } = WasmModule::::sized(c * 1024); Contracts::::store_code_raw(code)?; let mut module = PrefabWasmModule::from_storage_noinstr(hash)?; - let schedule = Contracts::::current_schedule(); + let schedule = >::get(); }: { Contracts::::reinstrument_module(&mut module, &schedule)?; } @@ -937,7 +937,7 @@ benchmarks! { seal_random { let r in 0 .. API_BENCHMARK_BATCHES; let pages = code::max_pages::(); - let subject_len = Contracts::::current_schedule().limits.subject_len; + let subject_len = >::get().limits.subject_len; assert!(subject_len < 1024); let code = WasmModule::::from(ModuleDefinition { memory: Some(ImportedMemory::max::()), @@ -993,7 +993,7 @@ benchmarks! { // `t`: Number of topics // `n`: Size of event payload in kb seal_deposit_event_per_topic_and_kb { - let t in 0 .. Contracts::::current_schedule().limits.event_topics; + let t in 0 .. >::get().limits.event_topics; let n in 0 .. T::MaxValueSize::get() / 1024; let mut topics = (0..API_BENCHMARK_BATCH_SIZE) .map(|n| (n * t..n * t + t).map(|i| T::Hashing::hash_of(&i)).collect::>().encode()) @@ -1923,7 +1923,7 @@ benchmarks! { // w_br_table_per_entry = w_bench instr_br_table_per_entry { - let e in 1 .. Contracts::::current_schedule().limits.br_table_size; + let e in 1 .. >::get().limits.br_table_size; let entry: Vec = [0, 1].iter() .cloned() .cycle() @@ -1979,7 +1979,7 @@ benchmarks! { // w_call_indrect = w_bench - 3 * w_param instr_call_indirect { let r in 0 .. INSTR_BENCHMARK_BATCHES; - let num_elements = Contracts::::current_schedule().limits.table_size; + let num_elements = >::get().limits.table_size; use self::code::TableSegment; let mut sbox = Sandbox::from(&WasmModule::::from(ModuleDefinition { // We need to make use of the stack here in order to trigger stack height @@ -2009,8 +2009,8 @@ benchmarks! { // linearly depend on the amount of parameters to this function. // Please note that this is not necessary with a direct call. instr_call_indirect_per_param { - let p in 0 .. Contracts::::current_schedule().limits.parameters; - let num_elements = Contracts::::current_schedule().limits.table_size; + let p in 0 .. >::get().limits.parameters; + let num_elements = >::get().limits.table_size; use self::code::TableSegment; let mut sbox = Sandbox::from(&WasmModule::::from(ModuleDefinition { // We need to make use of the stack here in order to trigger stack height @@ -2040,7 +2040,7 @@ benchmarks! { // w_local_get = w_bench - 1 * w_param instr_local_get { let r in 0 .. INSTR_BENCHMARK_BATCHES; - let max_locals = Contracts::::current_schedule().limits.stack_height; + let max_locals = >::get().limits.stack_height; let mut call_body = body::repeated_dyn(r * INSTR_BENCHMARK_BATCH_SIZE, vec![ RandomGetLocal(0, max_locals), Regular(Instruction::Drop), @@ -2057,7 +2057,7 @@ benchmarks! { // w_local_set = w_bench - 1 * w_param instr_local_set { let r in 0 .. INSTR_BENCHMARK_BATCHES; - let max_locals = Contracts::::current_schedule().limits.stack_height; + let max_locals = >::get().limits.stack_height; let mut call_body = body::repeated_dyn(r * INSTR_BENCHMARK_BATCH_SIZE, vec![ RandomI64Repeated(1), RandomSetLocal(0, max_locals), @@ -2074,7 +2074,7 @@ benchmarks! { // w_local_tee = w_bench - 2 * w_param instr_local_tee { let r in 0 .. INSTR_BENCHMARK_BATCHES; - let max_locals = Contracts::::current_schedule().limits.stack_height; + let max_locals = >::get().limits.stack_height; let mut call_body = body::repeated_dyn(r * INSTR_BENCHMARK_BATCH_SIZE, vec![ RandomI64Repeated(1), RandomTeeLocal(0, max_locals), @@ -2092,7 +2092,7 @@ benchmarks! { // w_global_get = w_bench - 1 * w_param instr_global_get { let r in 0 .. INSTR_BENCHMARK_BATCHES; - let max_globals = Contracts::::current_schedule().limits.globals; + let max_globals = >::get().limits.globals; let mut sbox = Sandbox::from(&WasmModule::::from(ModuleDefinition { call_body: Some(body::repeated_dyn(r * INSTR_BENCHMARK_BATCH_SIZE, vec![ RandomGetGlobal(0, max_globals), @@ -2108,7 +2108,7 @@ benchmarks! { // w_global_set = w_bench - 1 * w_param instr_global_set { let r in 0 .. INSTR_BENCHMARK_BATCHES; - let max_globals = Contracts::::current_schedule().limits.globals; + let max_globals = >::get().limits.globals; let mut sbox = Sandbox::from(&WasmModule::::from(ModuleDefinition { call_body: Some(body::repeated_dyn(r * INSTR_BENCHMARK_BATCH_SIZE, vec![ RandomI64Repeated(1), diff --git a/frame/contracts/src/exec.rs b/frame/contracts/src/exec.rs index 427cf1ada5ad5..010b740bcf6cf 100644 --- a/frame/contracts/src/exec.rs +++ b/frame/contracts/src/exec.rs @@ -945,7 +945,7 @@ mod tests { test_utils::{place_contract, set_balance, get_balance}, }, exec::ExportedFunction::*, - Error, Weight, + Error, Weight, CurrentSchedule, }; use sp_runtime::DispatchError; use assert_matches::assert_matches; @@ -1141,7 +1141,7 @@ mod tests { }); ExtBuilder::default().build().execute_with(|| { - let schedule = Contracts::current_schedule(); + let schedule = >::get(); let mut ctx = MockContext::top_level(ALICE, &schedule); place_contract(&BOB, exec_ch); @@ -1191,7 +1191,7 @@ mod tests { ); ExtBuilder::default().build().execute_with(|| { - let schedule = Contracts::current_schedule(); + let schedule = >::get(); let mut ctx = MockContext::top_level(origin.clone(), &schedule); place_contract(&BOB, return_ch); set_balance(&origin, 100); @@ -1251,7 +1251,7 @@ mod tests { ); ExtBuilder::default().build().execute_with(|| { - let schedule = Contracts::current_schedule(); + let schedule = >::get(); let mut ctx = MockContext::top_level(origin, &schedule); place_contract(&BOB, return_ch); @@ -1280,7 +1280,7 @@ mod tests { ); ExtBuilder::default().build().execute_with(|| { - let schedule = Contracts::current_schedule(); + let schedule = >::get(); let mut ctx = MockContext::top_level(origin, &schedule); place_contract(&BOB, return_ch); @@ -1306,7 +1306,7 @@ mod tests { // This one tests passing the input data into a contract via call. ExtBuilder::default().build().execute_with(|| { - let schedule = Contracts::current_schedule(); + let schedule = >::get(); let mut ctx = MockContext::top_level(ALICE, &schedule); place_contract(&BOB, input_data_ch); @@ -1329,7 +1329,7 @@ mod tests { // This one tests passing the input data into a contract via instantiate. ExtBuilder::default().build().execute_with(|| { - let schedule = Contracts::current_schedule(); + let schedule = >::get(); let subsistence = Contracts::::subsistence_threshold(); let mut ctx = MockContext::top_level(ALICE, &schedule); let mut gas_meter = GasMeter::::new(GAS_LIMIT); @@ -1382,7 +1382,7 @@ mod tests { }); ExtBuilder::default().build().execute_with(|| { - let schedule = Contracts::current_schedule(); + let schedule = >::get(); let mut ctx = MockContext::top_level(ALICE, &schedule); set_balance(&BOB, 1); place_contract(&BOB, recurse_ch); @@ -1430,7 +1430,7 @@ mod tests { }); ExtBuilder::default().build().execute_with(|| { - let schedule = Contracts::current_schedule(); + let schedule = >::get(); let mut ctx = MockContext::top_level(origin.clone(), &schedule); place_contract(&dest, bob_ch); place_contract(&CHARLIE, charlie_ch); @@ -1468,7 +1468,7 @@ mod tests { }); ExtBuilder::default().build().execute_with(|| { - let schedule = Contracts::current_schedule(); + let schedule = >::get(); let mut ctx = MockContext::top_level(ALICE, &schedule); place_contract(&BOB, bob_ch); place_contract(&CHARLIE, charlie_ch); @@ -1489,7 +1489,7 @@ mod tests { let dummy_ch = MockLoader::insert(Constructor, |_, _| exec_success()); ExtBuilder::default().existential_deposit(15).build().execute_with(|| { - let schedule = Contracts::current_schedule(); + let schedule = >::get(); let mut ctx = MockContext::top_level(ALICE, &schedule); let mut gas_meter = GasMeter::::new(GAS_LIMIT); let executable = MockExecutable::from_storage( @@ -1517,7 +1517,7 @@ mod tests { ); ExtBuilder::default().existential_deposit(15).build().execute_with(|| { - let schedule = Contracts::current_schedule(); + let schedule = >::get(); let mut ctx = MockContext::top_level(ALICE, &schedule); let mut gas_meter = GasMeter::::new(GAS_LIMIT); let executable = MockExecutable::from_storage( @@ -1553,7 +1553,7 @@ mod tests { ); ExtBuilder::default().existential_deposit(15).build().execute_with(|| { - let schedule = Contracts::current_schedule(); + let schedule = >::get(); let mut ctx = MockContext::top_level(ALICE, &schedule); let mut gas_meter = GasMeter::::new(GAS_LIMIT); let executable = MockExecutable::from_storage( @@ -1601,7 +1601,7 @@ mod tests { }); ExtBuilder::default().existential_deposit(15).build().execute_with(|| { - let schedule = Contracts::current_schedule(); + let schedule = >::get(); let mut ctx = MockContext::top_level(ALICE, &schedule); set_balance(&ALICE, Contracts::::subsistence_threshold() * 100); place_contract(&BOB, instantiator_ch); @@ -1650,7 +1650,7 @@ mod tests { }); ExtBuilder::default().existential_deposit(15).build().execute_with(|| { - let schedule = Contracts::current_schedule(); + let schedule = >::get(); let mut ctx = MockContext::top_level(ALICE, &schedule); set_balance(&ALICE, 1000); set_balance(&BOB, 100); @@ -1678,7 +1678,7 @@ mod tests { .existential_deposit(15) .build() .execute_with(|| { - let schedule = Contracts::current_schedule(); + let schedule = >::get(); let mut ctx = MockContext::top_level(ALICE, &schedule); let mut gas_meter = GasMeter::::new(GAS_LIMIT); let executable = MockExecutable::from_storage( @@ -1717,7 +1717,7 @@ mod tests { ExtBuilder::default().build().execute_with(|| { let subsistence = Contracts::::subsistence_threshold(); - let schedule = Contracts::current_schedule(); + let schedule = >::get(); let mut ctx = MockContext::top_level(ALICE, &schedule); let mut gas_meter = GasMeter::::new(GAS_LIMIT); let executable = MockExecutable::from_storage( @@ -1749,7 +1749,7 @@ mod tests { ExtBuilder::default().build().execute_with(|| { let subsistence = Contracts::::subsistence_threshold(); - let schedule = Contracts::current_schedule(); + let schedule = >::get(); let mut ctx = MockContext::top_level(ALICE, &schedule); let mut gas_meter = GasMeter::::new(GAS_LIMIT); set_balance(&ALICE, subsistence * 10); @@ -1797,7 +1797,7 @@ mod tests { ExtBuilder::default().build().execute_with(|| { let subsistence = Contracts::::subsistence_threshold(); - let schedule = Contracts::current_schedule(); + let schedule = >::get(); let mut ctx = MockContext::top_level(ALICE, &schedule); let mut gas_meter = GasMeter::::new(GAS_LIMIT); set_balance(&ALICE, subsistence * 100); diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index a6d57fd6a6588..512ed1aa5f289 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -239,7 +239,6 @@ pub mod pallet { } #[pallet::pallet] - #[pallet::generate_store(pub(super) trait Store)] pub struct Pallet(PhantomData); #[pallet::hooks] @@ -281,7 +280,7 @@ pub mod pallet { schedule: Schedule ) -> DispatchResultWithPostInfo { ensure_root(origin)?; - if >::current_schedule().version > schedule.version { + if schedule.version < >::get().version { Err(Error::::InvalidScheduleVersion)? } Self::deposit_event(Event::ScheduleUpdated(schedule.version)); @@ -307,7 +306,7 @@ pub mod pallet { let origin = ensure_signed(origin)?; let dest = T::Lookup::lookup(dest)?; let mut gas_meter = GasMeter::new(gas_limit); - let schedule = >::current_schedule(); + let schedule = >::get(); let mut ctx = ExecutionContext::>::top_level(origin, &schedule); let (result, code_len) = match ctx.call(dest, value, &mut gas_meter, data) { Ok((output, len)) => (Ok(output), len), @@ -356,7 +355,7 @@ pub mod pallet { let code_len = code.len() as u32; ensure!(code_len <= T::MaxCodeSize::get(), Error::::CodeTooLarge); let mut gas_meter = GasMeter::new(gas_limit); - let schedule = >::current_schedule(); + let schedule = >::get(); let executable = PrefabWasmModule::from_code(code, &schedule)?; let code_len = executable.code_len(); ensure!(code_len <= T::MaxCodeSize::get(), Error::::CodeTooLarge); @@ -388,7 +387,7 @@ pub mod pallet { ) -> DispatchResultWithPostInfo { let origin = ensure_signed(origin)?; let mut gas_meter = GasMeter::new(gas_limit); - let schedule = >::current_schedule(); + let schedule = >::get(); let executable = PrefabWasmModule::from_storage(code_hash, &schedule, &mut gas_meter)?; let mut ctx = ExecutionContext::>::top_level(origin, &schedule); let code_len = executable.code_len(); @@ -605,7 +604,6 @@ pub mod pallet { /// Current cost schedule for contracts. #[pallet::storage] - #[pallet::getter(fn current_schedule)] pub(crate) type CurrentSchedule = StorageValue<_, Schedule, ValueQuery>; /// A mapping from an original code hash to the original code, untouched by instrumentation. @@ -674,7 +672,7 @@ where input_data: Vec, ) -> ContractExecResult { let mut gas_meter = GasMeter::new(gas_limit); - let schedule = >::current_schedule(); + let schedule = >::get(); let mut ctx = ExecutionContext::>::top_level(origin, &schedule); let result = ctx.call(dest, value, &mut gas_meter, input_data); let gas_consumed = gas_meter.gas_spent(); @@ -751,7 +749,7 @@ where /// Store code for benchmarks which does not check nor instrument the code. #[cfg(feature = "runtime-benchmarks")] fn store_code_raw(code: Vec) -> frame_support::dispatch::DispatchResult { - let schedule = >::current_schedule(); + let schedule = >::get(); PrefabWasmModule::store_code_unchecked(code, &schedule)?; Ok(()) } From 87f8e6889ba675960e1c7c73313b14cf18a8ce2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Thu, 4 Mar 2021 11:24:51 +0100 Subject: [PATCH 3/5] contracts: Improve documentation --- frame/contracts/src/chain_extension.rs | 12 ++++++++-- frame/contracts/src/lib.rs | 31 +++++++++++++++----------- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/frame/contracts/src/chain_extension.rs b/frame/contracts/src/chain_extension.rs index dc6e9771775ca..4ac5300d57d7f 100644 --- a/frame/contracts/src/chain_extension.rs +++ b/frame/contracts/src/chain_extension.rs @@ -47,6 +47,12 @@ //! induces. In order to be able to charge the correct weight for the functions defined //! by a chain extension benchmarks must be written, too. In the near future this crate //! will provide the means for easier creation of those specialized benchmarks. +//! +//! # Example +//! +//! The ink! repository maintains an +//! [end-to-end example](https://github.com/paritytech/ink/tree/master/examples/rand-extension) +//! on how to use a chain extension in order to provide new features to ink! contracts. use crate::{ Error, @@ -141,8 +147,8 @@ pub enum RetVal { /// Grants the chain extension access to its parameters and execution environment. /// -/// It uses the typestate pattern to enforce the correct usage of the parameters passed -/// to the chain extension. +/// It uses [typestate programming](https://docs.rust-embedded.org/book/static-guarantees/typestate-programming.html) +/// to enforce the correct usage of the parameters passed to the chain extension. pub struct Environment<'a, 'b, E: Ext, S: state::State> { /// The actual data of this type. inner: Inner<'a, 'b, E>, @@ -376,6 +382,8 @@ mod state { pub trait BufIn: State {} pub trait BufOut: State {} + /// The initial state of an [`Environment`](`super::Environment`). + /// See [typestate programming](https://docs.rust-embedded.org/book/static-guarantees/typestate-programming.html). pub enum Init {} pub enum OnlyIn {} pub enum PrimInBufOut {} diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 512ed1aa5f289..9345fc6d3a535 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -19,17 +19,16 @@ //! //! The Contract module provides functionality for the runtime to deploy and execute WebAssembly smart-contracts. //! -//! - [`contract::Config`](./trait.Config.html) -//! - [`Call`](./enum.Call.html) +//! See [`pallet`] for more information. //! //! ## Overview //! -//! This module extends accounts based on the `Currency` trait to have smart-contract functionality. It can -//! be used with other modules that implement accounts based on `Currency`. These "smart-contract accounts" +//! This module extends accounts based on the [`Currency`] trait to have smart-contract functionality. It can +//! be used with other modules that implement accounts based on [`Currency`]. These "smart-contract accounts" //! have the ability to instantiate smart-contracts and make calls to other contract and non-contract accounts. //! -//! The smart-contract code is stored once in a `code_cache`, and later retrievable via its `code_hash`. -//! This means that multiple smart-contracts can be instantiated from the same `code_cache`, without replicating +//! The smart-contract code is stored once in a code cache, and later retrievable via its hash. +//! This means that multiple smart-contracts can be instantiated from the same hash, without replicating //! the code each time. //! //! When a smart-contract is called, its associated code is retrieved via the code hash and gets executed. @@ -59,12 +58,17 @@ //! //! ### Dispatchable functions //! -//! * `instantiate_with_code` - Deploys a new contract from the supplied wasm binary, optionally transferring -//! some balance. This instantiates a new smart contract account and calls its contract deploy -//! handler to initialize the contract. -//! * `instantiate` - The same as `instantiate_with_code` but instead of uploading new code an -//! existing `code_hash` is supplied. -//! * `call` - Makes a call to an account, optionally transferring some balance. +//! * [`Pallet::update_schedule`] - +//! ([Root Origin](https://substrate.dev/docs/en/knowledgebase/runtime/origin) Only) - +//! Set a new [`Schedule`]. +//! * [`Pallet::instantiate_with_code`] - Deploys a new contract from the supplied wasm binary, +//! optionally transferring +//! some balance. This instantiates a new smart contract account with the supplied code and +//! calls its constructor to initialize the contract. +//! * [`Pallet::instantiate`] - The same as `instantiate_with_code` but instead of uploading new +//! code an existing `code_hash` is supplied. +//! * [`Pallet::call`] - Makes a call to an account, optionally transferring some balance. +//! * [`Pallet::claim_surcharge`] - Evict a contract that cannot pay rent anymore. //! //! ## Usage //! @@ -326,7 +330,7 @@ pub mod pallet { /// * `gas_limit`: The gas limit enforced when executing the constructor. /// * `code`: The contract code to deploy in raw bytes. /// * `data`: The input data to pass to the contract constructor. - /// * `salt`: Used for the address derivation. See [`Self::contract_address`]. + /// * `salt`: Used for the address derivation. See [`Pallet::contract_address`]. /// /// Instantiation is executed as follows: /// @@ -631,6 +635,7 @@ pub mod pallet { #[pallet::storage] pub(crate) type DeletionQueue = StorageValue<_, Vec, ValueQuery>; + #[pallet::genesis_config] pub struct GenesisConfig { #[doc = "Current cost schedule for contracts."] From f979e76fee063d37cb4299d3c922dbaa38031d1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Mon, 15 Mar 2021 11:35:33 +0100 Subject: [PATCH 4/5] Update README.md --- frame/contracts/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/frame/contracts/CHANGELOG.md b/frame/contracts/CHANGELOG.md index ef69e050a2c5f..9c9a827564fa0 100644 --- a/frame/contracts/CHANGELOG.md +++ b/frame/contracts/CHANGELOG.md @@ -20,6 +20,7 @@ In other words: Upgrading this pallet will not break pre-existing contracts. ### Added +- Make storage and fields of `Schedule` private to the crate. - Add `seal_rent_params` contract callable function. ## [v3.0.0] 2021-02-25 From dd9457692637e3fa04f8d14c2ba91a33026fd77b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Mon, 15 Mar 2021 13:32:50 +0100 Subject: [PATCH 5/5] Fix integration test --- bin/node/executor/tests/basic.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/bin/node/executor/tests/basic.rs b/bin/node/executor/tests/basic.rs index 279b6a776031a..436ced50c525a 100644 --- a/bin/node/executor/tests/basic.rs +++ b/bin/node/executor/tests/basic.rs @@ -656,13 +656,10 @@ fn deploying_wasm_contract_should_work() { ).0.unwrap(); t.execute_with(|| { - // Verify that the contract constructor worked well and code of TRANSFER contract is actually deployed. - assert_eq!( - &pallet_contracts::ContractInfoOf::::get(addr) - .and_then(|c| c.get_alive()) - .unwrap() - .code_hash, - &transfer_ch + // Verify that the contract does exist by querying some of its storage items + // It does not matter that the storage item itself does not exist. + assert!( + &pallet_contracts::Pallet::::get_storage(addr, Default::default()).is_ok() ); }); }