From 537ff7008a350720715f6643c1b1343362633362 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Sun, 25 Oct 2020 11:24:52 +0100 Subject: [PATCH 01/10] contracts: Make use of existing type aliases for runtime API types --- frame/contracts/src/lib.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index cd5cbe5d32a40..40bdcd3a4b117 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -118,7 +118,7 @@ use frame_support::{ traits::{OnUnbalanced, Currency, Get, Time, Randomness}, }; use frame_system::{ensure_signed, ensure_root}; -use pallet_contracts_primitives::{RentProjection, ContractAccessError}; +use pallet_contracts_primitives::{RentProjectionResult, GetStorageResult, ContractAccessError}; use frame_support::weights::Weight; pub type CodeHash = ::Hash; @@ -650,10 +650,7 @@ impl Module { } /// Query storage of a specified contract under a specified key. - pub fn get_storage( - address: T::AccountId, - key: [u8; 32], - ) -> sp_std::result::Result>, ContractAccessError> { + pub fn get_storage(address: T::AccountId, key: [u8; 32]) -> GetStorageResult { let contract_info = ContractInfoOf::::get(&address) .ok_or(ContractAccessError::DoesntExist)? .get_alive() @@ -663,9 +660,7 @@ impl Module { Ok(maybe_value) } - pub fn rent_projection( - address: T::AccountId, - ) -> sp_std::result::Result, ContractAccessError> { + pub fn rent_projection(address: T::AccountId) -> RentProjectionResult { rent::compute_rent_projection::(&address) } From 19cbd401367d00e91f4ff80ff34a64046a4a8606 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Sun, 25 Oct 2020 12:29:12 +0100 Subject: [PATCH 02/10] contracts: Refactor the contracts call runtime API --- Cargo.lock | 2 +- bin/node/runtime/src/lib.rs | 14 +--- frame/contracts/Cargo.toml | 1 - frame/contracts/common/Cargo.toml | 1 + frame/contracts/common/src/lib.rs | 84 ++++++++++++++++++++-- frame/contracts/rpc/runtime-api/src/lib.rs | 26 +------ frame/contracts/rpc/src/lib.rs | 28 +++----- frame/contracts/src/benchmarking/mod.rs | 1 + frame/contracts/src/exec.rs | 60 +--------------- frame/contracts/src/gas.rs | 3 +- frame/contracts/src/lib.rs | 21 +++--- frame/contracts/src/tests.rs | 26 +++---- frame/contracts/src/wasm/mod.rs | 8 ++- frame/contracts/src/wasm/runtime.rs | 7 +- 14 files changed, 131 insertions(+), 151 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c165c3ccb9722..c7effdf91fb4c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4422,7 +4422,6 @@ name = "pallet-contracts" version = "2.0.0" dependencies = [ "assert_matches", - "bitflags", "frame-benchmarking", "frame-support", "frame-system", @@ -4450,6 +4449,7 @@ dependencies = [ name = "pallet-contracts-primitives" version = "2.0.0" dependencies = [ + "bitflags", "parity-scale-codec", "sp-runtime", "sp-std", diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index dfa7a4680abe8..8ffb9fc286da7 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -65,7 +65,6 @@ use pallet_im_online::sr25519::AuthorityId as ImOnlineId; use sp_authority_discovery::AuthorityId as AuthorityDiscoveryId; use pallet_transaction_payment_rpc_runtime_api::RuntimeDispatchInfo; pub use pallet_transaction_payment::{Multiplier, TargetedFeeAdjustment}; -use pallet_contracts_rpc_runtime_api::ContractExecResult; use pallet_session::{historical as pallet_session_historical}; use sp_inherents::{InherentData, CheckInherentsResult}; use static_assertions::const_assert; @@ -1124,17 +1123,8 @@ impl_runtime_apis! { value: Balance, gas_limit: u64, input_data: Vec, - ) -> ContractExecResult { - let (exec_result, gas_consumed) = - Contracts::bare_call(origin, dest.into(), value, gas_limit, input_data); - match exec_result { - Ok(v) => ContractExecResult::Success { - flags: v.flags.bits(), - data: v.data, - gas_consumed: gas_consumed, - }, - Err(_) => ContractExecResult::Error, - } + ) -> pallet_contracts_primitives::ContractExecResult { + Contracts::bare_call(origin, dest, value, gas_limit, input_data) } fn get_storage( diff --git a/frame/contracts/Cargo.toml b/frame/contracts/Cargo.toml index 41c4e893f8ca3..ffcb37385849a 100644 --- a/frame/contracts/Cargo.toml +++ b/frame/contracts/Cargo.toml @@ -13,7 +13,6 @@ readme = "README.md" targets = ["x86_64-unknown-linux-gnu"] [dependencies] -bitflags = "1.0" codec = { package = "parity-scale-codec", version = "1.3.4", default-features = false, features = ["derive"] } frame-benchmarking = { version = "2.0.0", default-features = false, path = "../benchmarking", optional = true } frame-support = { version = "2.0.0", default-features = false, path = "../support" } diff --git a/frame/contracts/common/Cargo.toml b/frame/contracts/common/Cargo.toml index 753ef9c08122f..e87cad055aff5 100644 --- a/frame/contracts/common/Cargo.toml +++ b/frame/contracts/common/Cargo.toml @@ -14,6 +14,7 @@ targets = ["x86_64-unknown-linux-gnu"] [dependencies] # This crate should not rely on any of the frame primitives. +bitflags = "1.0" codec = { package = "parity-scale-codec", version = "1.3.4", default-features = false, features = ["derive"] } sp-std = { version = "2.0.0", default-features = false, path = "../../../primitives/std" } sp-runtime = { version = "2.0.0", default-features = false, path = "../../../primitives/runtime" } diff --git a/frame/contracts/common/src/lib.rs b/frame/contracts/common/src/lib.rs index 6a74a417fa0fe..d6196e203eac1 100644 --- a/frame/contracts/common/src/lib.rs +++ b/frame/contracts/common/src/lib.rs @@ -18,13 +18,28 @@ #![cfg_attr(not(feature = "std"), no_std)] +use bitflags::bitflags; +use codec::{Decode, Encode}; +use sp_runtime::{DispatchError, RuntimeDebug}; use sp_std::prelude::*; -/// A result type of a get storage call. +/// A result type of a `bare_call` call. +/// The result of a contract exection together together with the consumed gas. +#[derive(Eq, PartialEq, Encode, Decode, RuntimeDebug)] +pub struct ContractExecResult { + pub exec_result: ExecResult, + pub gas_consumed: u64, +} + +/// A result type of a `get_storage` call. pub type GetStorageResult = Result>, ContractAccessError>; +/// A result type of a `rent_projection` call. +pub type RentProjectionResult = + Result, ContractAccessError>; + /// The possible errors that can happen querying the storage of a contract. -#[derive(Eq, PartialEq, codec::Encode, codec::Decode, sp_runtime::RuntimeDebug)] +#[derive(Eq, PartialEq, Encode, Decode, RuntimeDebug)] pub enum ContractAccessError { /// The given address doesn't point to a contract. DoesntExist, @@ -32,11 +47,7 @@ pub enum ContractAccessError { IsTombstone, } -/// A result type of a `rent_projection` call. -pub type RentProjectionResult = - Result, ContractAccessError>; - -#[derive(Eq, PartialEq, codec::Encode, codec::Decode, sp_runtime::RuntimeDebug)] +#[derive(Eq, PartialEq, Encode, Decode, RuntimeDebug)] pub enum RentProjection { /// Eviction is projected to happen at the specified block number. EvictionAt(BlockNumber), @@ -45,3 +56,62 @@ pub enum RentProjection { /// E.g. because the contract accumulated enough funds to offset the rent storage costs. NoEviction, } + +bitflags! { + /// Flags used by a contract to customize exit behaviour. + #[derive(Encode, Decode)] + pub struct ReturnFlags: u32 { + /// If this bit is set all changes made by the contract exection are rolled back. + const REVERT = 0x0000_0001; + } +} + +/// Output of a contract call or instantiation which ran to completion. +#[derive(PartialEq, Eq, Encode, Decode, RuntimeDebug)] +pub struct ExecReturnValue { + /// Flags passed along by `seal_return`. Empty when `seal_return` was never called. + pub flags: ReturnFlags, + /// Buffer passed along by `seal_return`. Empty when `seal_return` was never called. + pub data: Vec, +} + +impl ExecReturnValue { + /// We understand the absense of a revert flag as success. + pub fn is_success(&self) -> bool { + !self.flags.contains(ReturnFlags::REVERT) + } +} + +/// Call or instantiate both call into other contracts and pass through errors happening +/// in those to the caller. This enum is for the caller to distinguish whether the error +/// happened during the execution of the callee or in the current execution context. +#[derive(PartialEq, Eq, Encode, Decode, RuntimeDebug)] +pub enum ErrorOrigin { + /// The error happened in the current exeuction context rather than in the one + /// of the contract that is called into. + Caller, + /// The error happened during execution of the called contract. + Callee, +} + +/// Error returned by contract exection. +#[derive(PartialEq, Eq, Encode, Decode, RuntimeDebug)] +pub struct ExecError { + /// The reason why the execution failed. + pub error: DispatchError, + /// Origin of the error. + pub origin: ErrorOrigin, +} + +impl> From for ExecError { + fn from(error: T) -> Self { + Self { + error: error.into(), + origin: ErrorOrigin::Caller, + } + } +} + +/// The result that is returned from contract execution. It either contains the output +/// buffer or an error describing the reason for failure. +pub type ExecResult = Result; diff --git a/frame/contracts/rpc/runtime-api/src/lib.rs b/frame/contracts/rpc/runtime-api/src/lib.rs index 7d208cf7763e7..94b9fe7967c08 100644 --- a/frame/contracts/rpc/runtime-api/src/lib.rs +++ b/frame/contracts/rpc/runtime-api/src/lib.rs @@ -23,31 +23,9 @@ #![cfg_attr(not(feature = "std"), no_std)] -use codec::{Codec, Decode, Encode}; -use pallet_contracts_primitives::{GetStorageResult, RentProjectionResult}; -use sp_runtime::RuntimeDebug; +use codec::Codec; use sp_std::vec::Vec; - -/// A result of execution of a contract. -#[derive(Eq, PartialEq, Encode, Decode, RuntimeDebug)] -pub enum ContractExecResult { - /// The contract returned successfully. - /// - /// There is a status code and, optionally, some data returned by the contract. - Success { - /// Flags that the contract passed along on returning to alter its exit behaviour. - /// Described in `pallet_contracts::exec::ReturnFlags`. - flags: u32, - /// Output data returned by the contract. - /// - /// Can be empty. - data: Vec, - /// How much gas was consumed by the call. - gas_consumed: u64, - }, - /// The contract execution either trapped or returned an error. - Error, -} +use pallet_contracts_primitives::{ContractExecResult, GetStorageResult, RentProjectionResult}; sp_api::decl_runtime_apis! { /// The API to interact with contracts without using executive. diff --git a/frame/contracts/rpc/src/lib.rs b/frame/contracts/rpc/src/lib.rs index d99ed1e78a652..84df1e25a3b38 100644 --- a/frame/contracts/rpc/src/lib.rs +++ b/frame/contracts/rpc/src/lib.rs @@ -33,11 +33,9 @@ use sp_runtime::{ traits::{Block as BlockT, Header as HeaderT}, }; use std::convert::TryInto; +use pallet_contracts_primitives::ContractExecResult; -pub use self::gen_client::Client as ContractsClient; -pub use pallet_contracts_rpc_runtime_api::{ - self as runtime_api, ContractExecResult, ContractsApi as ContractsRuntimeApi, -}; +pub use pallet_contracts_rpc_runtime_api::ContractsApi as ContractsRuntimeApi; const RUNTIME_ERROR: i64 = 1; const CONTRACT_DOESNT_EXIST: i64 = 2; @@ -105,17 +103,13 @@ pub enum RpcContractExecResult { impl From for RpcContractExecResult { fn from(r: ContractExecResult) -> Self { - match r { - ContractExecResult::Success { - flags, - data, - gas_consumed - } => RpcContractExecResult::Success { - flags, - data: data.into(), - gas_consumed, + match r.exec_result { + Ok(val) => RpcContractExecResult::Success { + flags: val.flags.bits(), + data: val.data.into(), + gas_consumed: r.gas_consumed, }, - ContractExecResult::Error => RpcContractExecResult::Error(()), + _ => RpcContractExecResult::Error(()), } } } @@ -233,7 +227,7 @@ where let exec_result = api .call(&at, origin, dest, value, gas_limit, input_data.to_vec()) - .map_err(|e| runtime_error_into_rpc_err(e))?; + .map_err(runtime_error_into_rpc_err)?; Ok(exec_result.into()) } @@ -251,7 +245,7 @@ where let result = api .get_storage(&at, address, key.into()) - .map_err(|e| runtime_error_into_rpc_err(e))? + .map_err(runtime_error_into_rpc_err)? .map_err(ContractAccessError)? .map(Bytes); @@ -270,7 +264,7 @@ where let result = api .rent_projection(&at, address) - .map_err(|e| runtime_error_into_rpc_err(e))? + .map_err(runtime_error_into_rpc_err)? .map_err(ContractAccessError)?; Ok(match result { diff --git a/frame/contracts/src/benchmarking/mod.rs b/frame/contracts/src/benchmarking/mod.rs index 7c084a222a641..79863afc4419a 100644 --- a/frame/contracts/src/benchmarking/mod.rs +++ b/frame/contracts/src/benchmarking/mod.rs @@ -34,6 +34,7 @@ use frame_system::{Module as System, RawOrigin}; use parity_wasm::elements::{Instruction, ValueType, BlockType}; use sp_runtime::traits::{Hash, Bounded}; use sp_std::{default::Default, convert::{TryInto}}; +use pallet_contracts_primitives::RentProjection; /// How many batches we do per API benchmark. const API_BENCHMARK_BATCHES: u32 = 20; diff --git a/frame/contracts/src/exec.rs b/frame/contracts/src/exec.rs index bc99431c85e65..f93f262d821e9 100644 --- a/frame/contracts/src/exec.rs +++ b/frame/contracts/src/exec.rs @@ -19,7 +19,6 @@ use crate::{ TrieId, BalanceOf, ContractInfo, TrieIdGenerator, gas::GasMeter, rent, storage, Error, ContractInfoOf }; -use bitflags::bitflags; use sp_std::prelude::*; use sp_runtime::traits::{Bounded, Zero, Convert, Saturating}; use frame_support::{ @@ -28,6 +27,7 @@ use frame_support::{ weights::Weight, ensure, StorageMap, }; +use pallet_contracts_primitives::{ErrorOrigin, ExecError, ExecReturnValue, ExecResult, ReturnFlags}; pub type AccountIdOf = ::AccountId; pub type MomentOf = <::Time as Time>::Moment; @@ -38,14 +38,6 @@ pub type StorageKey = [u8; 32]; /// A type that represents a topic of an event. At the moment a hash is used. pub type TopicOf = ::Hash; -bitflags! { - /// Flags used by a contract to customize exit behaviour. - pub struct ReturnFlags: u32 { - /// If this bit is set all changes made by the contract exection are rolled back. - const REVERT = 0x0000_0001; - } -} - /// Describes whether we deal with a contract or a plain account. pub enum TransactorKind { /// Transaction was initiated from a plain account. That can be either be through a @@ -55,56 +47,6 @@ pub enum TransactorKind { Contract, } -/// Output of a contract call or instantiation which ran to completion. -#[cfg_attr(test, derive(PartialEq, Eq, Debug))] -pub struct ExecReturnValue { - /// Flags passed along by `seal_return`. Empty when `seal_return` was never called. - pub flags: ReturnFlags, - /// Buffer passed along by `seal_return`. Empty when `seal_return` was never called. - pub data: Vec, -} - -impl ExecReturnValue { - /// We understand the absense of a revert flag as success. - pub fn is_success(&self) -> bool { - !self.flags.contains(ReturnFlags::REVERT) - } -} - -/// Call or instantiate both call into other contracts and pass through errors happening -/// in those to the caller. This enum is for the caller to distinguish whether the error -/// happened during the execution of the callee or in the current execution context. -#[cfg_attr(test, derive(PartialEq, Eq, Debug))] -pub enum ErrorOrigin { - /// The error happened in the current exeuction context rather than in the one - /// of the contract that is called into. - Caller, - /// The error happened during execution of the called contract. - Callee, -} - -/// Error returned by contract exection. -#[cfg_attr(test, derive(PartialEq, Eq, Debug))] -pub struct ExecError { - /// The reason why the execution failed. - pub error: DispatchError, - /// Origin of the error. - pub origin: ErrorOrigin, -} - -impl> From for ExecError { - fn from(error: T) -> Self { - Self { - error: error.into(), - origin: ErrorOrigin::Caller, - } - } -} - -/// The result that is returned from contract execution. It either contains the output -/// buffer or an error describing the reason for failure. -pub type ExecResult = Result; - /// An interface that provides access to the external environment in which the /// smart-contract is executed. /// diff --git a/frame/contracts/src/gas.rs b/frame/contracts/src/gas.rs index decaf11b796f7..0828a220c0406 100644 --- a/frame/contracts/src/gas.rs +++ b/frame/contracts/src/gas.rs @@ -14,12 +14,13 @@ // You should have received a copy of the GNU General Public License // along with Substrate. If not, see . -use crate::{Trait, exec::ExecError}; +use crate::Trait; use sp_std::marker::PhantomData; use sp_runtime::traits::Zero; use frame_support::dispatch::{ DispatchResultWithPostInfo, PostDispatchInfo, DispatchErrorWithPostInfo, }; +use pallet_contracts_primitives::ExecError; #[cfg(test)] use std::{any::Any, fmt::Debug}; diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 40bdcd3a4b117..9f1656f35f6ee 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -97,7 +97,6 @@ use crate::exec::ExecutionContext; use crate::wasm::{WasmLoader, WasmVm}; pub use crate::gas::{Gas, GasMeter}; -pub use crate::exec::{ExecResult, ExecReturnValue}; pub use crate::wasm::ReturnCode as RuntimeReturnCode; pub use crate::weight_info::WeightInfo; pub use crate::schedule::{Schedule, HostFnWeights, InstructionWeights}; @@ -118,7 +117,9 @@ use frame_support::{ traits::{OnUnbalanced, Currency, Get, Time, Randomness}, }; use frame_system::{ensure_signed, ensure_root}; -use pallet_contracts_primitives::{RentProjectionResult, GetStorageResult, ContractAccessError}; +use pallet_contracts_primitives::{ + RentProjectionResult, GetStorageResult, ContractAccessError, ContractExecResult, ExecResult, +}; use frame_support::weights::Weight; pub type CodeHash = ::Hash; @@ -639,14 +640,16 @@ impl Module { value: BalanceOf, gas_limit: Gas, input_data: Vec, - ) -> (ExecResult, Gas) { + ) -> ContractExecResult { 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) - }), - gas_meter.gas_spent(), - ) + let exec_result = Self::execute_wasm(origin, &mut gas_meter, |ctx, gas_meter| { + ctx.call(dest, value, gas_meter, input_data) + }); + let gas_consumed = gas_meter.gas_spent(); + ContractExecResult { + exec_result, + gas_consumed, + } } /// Query storage of a specified contract under a specified key. diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index 1c14e3e35f248..c2d9ed6642559 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -1655,7 +1655,7 @@ fn crypto_hashes() { 0, GAS_LIMIT, params, - ).0.unwrap(); + ).exec_result.unwrap(); assert!(result.is_success()); let expected = hash_fn(input.as_ref()); assert_eq!(&result.data[..*expected_size], &*expected); @@ -1688,7 +1688,7 @@ fn transfer_return_code() { 0, GAS_LIMIT, vec![], - ).0.unwrap(); + ).exec_result.unwrap(); assert_return_code!(result, RuntimeReturnCode::BelowSubsistenceThreshold); // Contract has enough total balance in order to not go below the subsistence @@ -1702,7 +1702,7 @@ fn transfer_return_code() { 0, GAS_LIMIT, vec![], - ).0.unwrap(); + ).exec_result.unwrap(); assert_return_code!(result, RuntimeReturnCode::TransferFailed); }); } @@ -1735,7 +1735,7 @@ fn call_return_code() { 0, GAS_LIMIT, vec![0], - ).0.unwrap(); + ).exec_result.unwrap(); assert_return_code!(result, RuntimeReturnCode::NotCallable); assert_ok!( @@ -1755,7 +1755,7 @@ fn call_return_code() { 0, GAS_LIMIT, vec![0], - ).0.unwrap(); + ).exec_result.unwrap(); assert_return_code!(result, RuntimeReturnCode::BelowSubsistenceThreshold); // Contract has enough total balance in order to not go below the subsistence @@ -1769,7 +1769,7 @@ fn call_return_code() { 0, GAS_LIMIT, vec![0], - ).0.unwrap(); + ).exec_result.unwrap(); assert_return_code!(result, RuntimeReturnCode::TransferFailed); // Contract has enough balance but callee reverts because "1" is passed. @@ -1780,7 +1780,7 @@ fn call_return_code() { 0, GAS_LIMIT, vec![1], - ).0.unwrap(); + ).exec_result.unwrap(); assert_return_code!(result, RuntimeReturnCode::CalleeReverted); // Contract has enough balance but callee traps because "2" is passed. @@ -1790,7 +1790,7 @@ fn call_return_code() { 0, GAS_LIMIT, vec![2], - ).0.unwrap(); + ).exec_result.unwrap(); assert_return_code!(result, RuntimeReturnCode::CalleeTrapped); }); @@ -1825,7 +1825,7 @@ fn instantiate_return_code() { 0, GAS_LIMIT, vec![0; 33], - ).0.unwrap(); + ).exec_result.unwrap(); assert_return_code!(result, RuntimeReturnCode::BelowSubsistenceThreshold); // Contract has enough total balance in order to not go below the subsistence @@ -1839,7 +1839,7 @@ fn instantiate_return_code() { 0, GAS_LIMIT, vec![0; 33], - ).0.unwrap(); + ).exec_result.unwrap(); assert_return_code!(result, RuntimeReturnCode::TransferFailed); // Contract has enough balance but the passed code hash is invalid @@ -1850,7 +1850,7 @@ fn instantiate_return_code() { 0, GAS_LIMIT, vec![0; 33], - ).0.unwrap(); + ).exec_result.unwrap(); assert_return_code!(result, RuntimeReturnCode::CodeNotFound); // Contract has enough balance but callee reverts because "1" is passed. @@ -1860,7 +1860,7 @@ fn instantiate_return_code() { 0, GAS_LIMIT, callee_hash.iter().cloned().chain(sp_std::iter::once(1)).collect(), - ).0.unwrap(); + ).exec_result.unwrap(); assert_return_code!(result, RuntimeReturnCode::CalleeReverted); // Contract has enough balance but callee traps because "2" is passed. @@ -1870,7 +1870,7 @@ fn instantiate_return_code() { 0, GAS_LIMIT, callee_hash.iter().cloned().chain(sp_std::iter::once(2)).collect(), - ).0.unwrap(); + ).exec_result.unwrap(); assert_return_code!(result, RuntimeReturnCode::CalleeTrapped); }); diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index 100148b18dcd4..2440abed8ec41 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -19,7 +19,7 @@ use crate::{CodeHash, Schedule, Trait}; use crate::wasm::env_def::FunctionImplProvider; -use crate::exec::{Ext, ExecResult}; +use crate::exec::Ext; use crate::gas::GasMeter; use sp_std::prelude::*; @@ -34,6 +34,7 @@ mod runtime; use self::runtime::{to_execution_result, Runtime}; use self::code_cache::load as load_code; +use pallet_contracts_primitives::ExecResult; pub use self::code_cache::save as save_code; #[cfg(feature = "runtime-benchmarks")] @@ -155,7 +156,7 @@ mod tests { use super::*; use std::collections::HashMap; use sp_core::H256; - use crate::exec::{Ext, StorageKey, ExecReturnValue, ReturnFlags, ExecError, ErrorOrigin}; + use crate::exec::{Ext, StorageKey}; use crate::gas::{Gas, GasMeter}; use crate::tests::{Test, Call}; use crate::wasm::prepare::prepare_contract; @@ -163,6 +164,7 @@ mod tests { use hex_literal::hex; use sp_runtime::DispatchError; use frame_support::weights::Weight; + use pallet_contracts_primitives::{ExecReturnValue, ReturnFlags, ExecError, ErrorOrigin}; const GAS_LIMIT: Gas = 10_000_000_000; @@ -1361,7 +1363,7 @@ mod tests { ;; size of our buffer is 128 bytes (data (i32.const 160) "\80") - + (func $assert (param i32) (block $ok (br_if $ok diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index 7d8ce26678c09..40c2ea8f9bbf8 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -17,9 +17,7 @@ //! Environment definition of the wasm smart-contract runtime. use crate::{HostFnWeights, Schedule, Trait, CodeHash, BalanceOf, Error}; -use crate::exec::{ - Ext, ExecResult, ExecReturnValue, StorageKey, TopicOf, ReturnFlags, ExecError -}; +use crate::exec::{Ext, StorageKey, TopicOf}; use crate::gas::{Gas, GasMeter, Token, GasMeterResult}; use crate::wasm::env_def::ConvertibleToWasm; use sp_sandbox; @@ -35,6 +33,7 @@ use sp_io::hashing::{ blake2_128, sha2_256, }; +use pallet_contracts_primitives::{ExecResult, ExecReturnValue, ReturnFlags, ExecError}; /// Every error that can be returned to a contract when it calls any of the host functions. #[repr(u32)] @@ -499,7 +498,7 @@ fn err_into_return_code(from: DispatchError) -> Result(from: ExecResult) -> Result { - use crate::exec::ErrorOrigin::Callee; + use pallet_contracts_primitives::ErrorOrigin::Callee; let ExecError { error, origin } = match from { Ok(retval) => return Ok(retval.into()), From 60b93fab7c0aa456ca3f7b2fb410e02e9b3fbc27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Thu, 29 Oct 2020 11:32:53 +0100 Subject: [PATCH 03/10] review: Fix comment typo Co-authored-by: Andrew Jones --- frame/contracts/common/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/contracts/common/src/lib.rs b/frame/contracts/common/src/lib.rs index d6196e203eac1..354aa405af3d5 100644 --- a/frame/contracts/common/src/lib.rs +++ b/frame/contracts/common/src/lib.rs @@ -61,7 +61,7 @@ bitflags! { /// Flags used by a contract to customize exit behaviour. #[derive(Encode, Decode)] pub struct ReturnFlags: u32 { - /// If this bit is set all changes made by the contract exection are rolled back. + /// If this bit is set all changes made by the contract execution are rolled back. const REVERT = 0x0000_0001; } } From 5dd9c5160477f6a1a7f4c5aadbf273164d26b040 Mon Sep 17 00:00:00 2001 From: Addie Wagenknecht Date: Thu, 29 Oct 2020 14:56:21 +0100 Subject: [PATCH 04/10] Update frame/contracts/common/src/lib.rs Co-authored-by: Nikolay Volf --- frame/contracts/common/src/lib.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frame/contracts/common/src/lib.rs b/frame/contracts/common/src/lib.rs index 354aa405af3d5..6d0b1a9e06301 100644 --- a/frame/contracts/common/src/lib.rs +++ b/frame/contracts/common/src/lib.rs @@ -23,8 +23,9 @@ use codec::{Decode, Encode}; use sp_runtime::{DispatchError, RuntimeDebug}; use sp_std::prelude::*; -/// A result type of a `bare_call` call. -/// The result of a contract exection together together with the consumed gas. +/// Result type of a `bare_call` call. +/// +/// The result of a contract execution along with a gas consumed. #[derive(Eq, PartialEq, Encode, Decode, RuntimeDebug)] pub struct ContractExecResult { pub exec_result: ExecResult, From 24edf3e9743c2ba98f120ade8222a384221bbe56 Mon Sep 17 00:00:00 2001 From: Addie Wagenknecht Date: Thu, 29 Oct 2020 14:56:31 +0100 Subject: [PATCH 05/10] Update frame/contracts/common/src/lib.rs Co-authored-by: Nikolay Volf --- frame/contracts/common/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/contracts/common/src/lib.rs b/frame/contracts/common/src/lib.rs index 6d0b1a9e06301..e082b928e7391 100644 --- a/frame/contracts/common/src/lib.rs +++ b/frame/contracts/common/src/lib.rs @@ -32,7 +32,7 @@ pub struct ContractExecResult { pub gas_consumed: u64, } -/// A result type of a `get_storage` call. +/// Result type of a `get_storage` call. pub type GetStorageResult = Result>, ContractAccessError>; /// A result type of a `rent_projection` call. From c19fe0c22f1b727b2837c68e3f3ab1cbd1da42ad Mon Sep 17 00:00:00 2001 From: Addie Wagenknecht Date: Thu, 29 Oct 2020 14:56:43 +0100 Subject: [PATCH 06/10] Update frame/contracts/common/src/lib.rs Co-authored-by: Nikolay Volf --- frame/contracts/common/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/contracts/common/src/lib.rs b/frame/contracts/common/src/lib.rs index e082b928e7391..f2ced0e3b8e33 100644 --- a/frame/contracts/common/src/lib.rs +++ b/frame/contracts/common/src/lib.rs @@ -35,7 +35,7 @@ pub struct ContractExecResult { /// Result type of a `get_storage` call. pub type GetStorageResult = Result>, ContractAccessError>; -/// A result type of a `rent_projection` call. +/// Result type of a `rent_projection` call. pub type RentProjectionResult = Result, ContractAccessError>; From bf6ebfbea73d1613ccd5387b539ebd43099eeacd Mon Sep 17 00:00:00 2001 From: Addie Wagenknecht Date: Thu, 29 Oct 2020 14:57:05 +0100 Subject: [PATCH 07/10] Update frame/contracts/common/src/lib.rs Co-authored-by: Nikolay Volf --- frame/contracts/common/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frame/contracts/common/src/lib.rs b/frame/contracts/common/src/lib.rs index f2ced0e3b8e33..31b2c0e80de7f 100644 --- a/frame/contracts/common/src/lib.rs +++ b/frame/contracts/common/src/lib.rs @@ -83,6 +83,8 @@ impl ExecReturnValue { } } +/// Origin of the error. +/// /// Call or instantiate both call into other contracts and pass through errors happening /// in those to the caller. This enum is for the caller to distinguish whether the error /// happened during the execution of the callee or in the current execution context. From de3021a33ce77d0ad607770b5d6fd1cddba59150 Mon Sep 17 00:00:00 2001 From: Addie Wagenknecht Date: Thu, 29 Oct 2020 14:57:16 +0100 Subject: [PATCH 08/10] Update frame/contracts/common/src/lib.rs Co-authored-by: Nikolay Volf --- frame/contracts/common/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frame/contracts/common/src/lib.rs b/frame/contracts/common/src/lib.rs index 31b2c0e80de7f..085de65f3b832 100644 --- a/frame/contracts/common/src/lib.rs +++ b/frame/contracts/common/src/lib.rs @@ -90,6 +90,8 @@ impl ExecReturnValue { /// happened during the execution of the callee or in the current execution context. #[derive(PartialEq, Eq, Encode, Decode, RuntimeDebug)] pub enum ErrorOrigin { + /// Caller error origin. + /// /// The error happened in the current exeuction context rather than in the one /// of the contract that is called into. Caller, From 5435fbdaeadd275e0e6675963fe18e74186d5d70 Mon Sep 17 00:00:00 2001 From: Addie Wagenknecht Date: Thu, 29 Oct 2020 15:04:25 +0100 Subject: [PATCH 09/10] Update lib.rs --- frame/contracts/common/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/contracts/common/src/lib.rs b/frame/contracts/common/src/lib.rs index 085de65f3b832..9da105cf2d80a 100644 --- a/frame/contracts/common/src/lib.rs +++ b/frame/contracts/common/src/lib.rs @@ -54,7 +54,7 @@ pub enum RentProjection { EvictionAt(BlockNumber), /// No eviction is scheduled. /// - /// E.g. because the contract accumulated enough funds to offset the rent storage costs. + /// E.g. Contract accumulated enough funds to offset the rent storage costs. NoEviction, } @@ -85,8 +85,8 @@ impl ExecReturnValue { /// Origin of the error. /// -/// Call or instantiate both call into other contracts and pass through errors happening -/// in those to the caller. This enum is for the caller to distinguish whether the error +/// Call or instantiate both called into other contracts and pass through errors happening +/// in those to the caller. This enum is for the caller to distinguish whether the error /// happened during the execution of the callee or in the current execution context. #[derive(PartialEq, Eq, Encode, Decode, RuntimeDebug)] pub enum ErrorOrigin { From 31d526b478d63a2e4e88c066f6864ba97212802a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Thu, 29 Oct 2020 15:09:01 +0100 Subject: [PATCH 10/10] review: Group crate imports --- frame/contracts/src/wasm/runtime.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index 40c2ea8f9bbf8..ffc816aa4c82f 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -16,10 +16,12 @@ //! Environment definition of the wasm smart-contract runtime. -use crate::{HostFnWeights, Schedule, Trait, CodeHash, BalanceOf, Error}; -use crate::exec::{Ext, StorageKey, TopicOf}; -use crate::gas::{Gas, GasMeter, Token, GasMeterResult}; -use crate::wasm::env_def::ConvertibleToWasm; +use crate::{ + HostFnWeights, Schedule, Trait, CodeHash, BalanceOf, Error, + exec::{Ext, StorageKey, TopicOf}, + gas::{Gas, GasMeter, Token, GasMeterResult}, + wasm::env_def::ConvertibleToWasm, +}; use sp_sandbox; use parity_wasm::elements::ValueType; use frame_system;