From f8539a04aa1e6f49b9030e9ec0e5b44ecae9ab30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Mon, 27 Jul 2020 11:15:26 +0200 Subject: [PATCH 01/14] seal: Rework ext_transfer, ext_instantiate, ext_call error handling * Deny calling plain accounts (must use transfer now) * Return proper module error rather than ad-hoc strings * Return the correct error codes from call,instantiate (documentation was wrong) * Make ext_transfer fallible again to make it consistent with ext_call --- frame/contracts/fixtures/caller_contract.wat | 4 +- .../fixtures/destroy_and_transfer.wat | 8 +- frame/contracts/fixtures/drain.wat | 11 +- .../fixtures/self_destructing_constructor.wat | 39 +--- frame/contracts/fixtures/set_rent.wat | 9 +- frame/contracts/src/exec.rs | 188 ++++++++++------ frame/contracts/src/gas.rs | 11 +- frame/contracts/src/lib.rs | 17 +- frame/contracts/src/storage.rs | 1 + frame/contracts/src/tests.rs | 37 ++-- frame/contracts/src/wasm/mod.rs | 32 +-- frame/contracts/src/wasm/runtime.rs | 204 ++++++++++++------ 12 files changed, 342 insertions(+), 219 deletions(-) diff --git a/frame/contracts/fixtures/caller_contract.wat b/frame/contracts/fixtures/caller_contract.wat index 369007834dc7b..ee2e16098d595 100644 --- a/frame/contracts/fixtures/caller_contract.wat +++ b/frame/contracts/fixtures/caller_contract.wat @@ -89,7 +89,7 @@ (call $ext_instantiate (i32.const 24) ;; Pointer to the code hash. (i32.const 32) ;; Length of the code hash. - (i64.const 200) ;; How much gas to devote for the execution. + (i64.const 187500000) ;; Just enough to pay for the instantiate (i32.const 0) ;; Pointer to the buffer with value to transfer (i32.const 8) ;; Length of the buffer with value to transfer. (i32.const 8) ;; Pointer to input data buffer address @@ -206,7 +206,7 @@ (call $ext_call (i32.const 16) ;; Pointer to "callee" address. (i32.const 8) ;; Length of "callee" address. - (i64.const 100) ;; How much gas to devote for the execution. + (i64.const 117500000) ;; Just enough to make the call (i32.const 0) ;; Pointer to the buffer with value to transfer (i32.const 8) ;; Length of the buffer with value to transfer. (i32.const 8) ;; Pointer to input data buffer address diff --git a/frame/contracts/fixtures/destroy_and_transfer.wat b/frame/contracts/fixtures/destroy_and_transfer.wat index ee191aa019bfb..3f8a8c89b02cd 100644 --- a/frame/contracts/fixtures/destroy_and_transfer.wat +++ b/frame/contracts/fixtures/destroy_and_transfer.wat @@ -3,6 +3,7 @@ (import "env" "ext_get_storage" (func $ext_get_storage (param i32 i32 i32) (result i32))) (import "env" "ext_set_storage" (func $ext_set_storage (param i32 i32 i32))) (import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32 i32 i32) (result i32))) + (import "env" "ext_transfer" (func $ext_transfer (param i32 i32 i32 i32) (result i32))) (import "env" "ext_instantiate" (func $ext_instantiate (param i32 i32 i64 i32 i32 i32 i32 i32 i32 i32 i32) (result i32))) (import "env" "memory" (memory 1 1)) @@ -139,16 +140,11 @@ ;; does not keep the contract alive. (call $assert (i32.eq - (call $ext_call + (call $ext_transfer (i32.const 80) ;; Pointer to destination address (i32.const 8) ;; Length of destination address - (i64.const 0) ;; How much gas to devote for the execution. 0 = all. (i32.const 0) ;; Pointer to the buffer with value to transfer (i32.const 8) ;; Length of the buffer with value to transfer - (i32.const 0) ;; Pointer to input data buffer address - (i32.const 1) ;; Length of input data buffer - (i32.const 4294967295) ;; u32 max sentinel value: do not copy output - (i32.const 0) ;; Length is ignored in this case ) (i32.const 0) ) diff --git a/frame/contracts/fixtures/drain.wat b/frame/contracts/fixtures/drain.wat index 1b3172b2a01a4..22422bb859d0f 100644 --- a/frame/contracts/fixtures/drain.wat +++ b/frame/contracts/fixtures/drain.wat @@ -1,6 +1,6 @@ (module (import "env" "ext_balance" (func $ext_balance (param i32 i32))) - (import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32 i32 i32) (result i32))) + (import "env" "ext_transfer" (func $ext_transfer (param i32 i32 i32 i32) (result i32))) (import "env" "memory" (memory 1 1)) ;; [0, 8) reserved for $ext_balance output @@ -36,18 +36,13 @@ ;; Self-destruct by sending full balance to the 0 address. (call $assert (i32.eq - (call $ext_call + (call $ext_transfer (i32.const 16) ;; Pointer to destination address (i32.const 8) ;; Length of destination address - (i64.const 0) ;; How much gas to devote for the execution. 0 = all. (i32.const 0) ;; Pointer to the buffer with value to transfer (i32.const 8) ;; Length of the buffer with value to transfer - (i32.const 0) ;; Pointer to input data buffer address - (i32.const 0) ;; Length of input data buffer - (i32.const 4294967295) ;; u32 max sentinel value: do not copy output - (i32.const 0) ;; Length is ignored in this case ) - (i32.const 0) + (i32.const 4) ;; ReturnCode::BelowSubsistenceThreshold ) ) ) diff --git a/frame/contracts/fixtures/self_destructing_constructor.wat b/frame/contracts/fixtures/self_destructing_constructor.wat index 3b99db001cd38..ece5679f4f691 100644 --- a/frame/contracts/fixtures/self_destructing_constructor.wat +++ b/frame/contracts/fixtures/self_destructing_constructor.wat @@ -1,15 +1,7 @@ (module - (import "env" "ext_balance" (func $ext_balance (param i32 i32))) - (import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32 i32 i32) (result i32))) + (import "env" "ext_terminate" (func $ext_terminate (param i32 i32))) (import "env" "memory" (memory 1 1)) - ;; [0, 8) reserved for $ext_balance output - - ;; [8, 16) length of the buffer - (data (i32.const 8) "\08") - - ;; [16, inf) zero initialized - (func $assert (param i32) (block $ok (br_if $ok @@ -20,33 +12,10 @@ ) (func (export "deploy") - ;; Send entire remaining balance to the 0 address. - (call $ext_balance (i32.const 0) (i32.const 8)) - - ;; Balance should be encoded as a u64. - (call $assert - (i32.eq - (i32.load (i32.const 8)) - (i32.const 8) - ) - ) - ;; Self-destruct by sending full balance to the 0 address. - (call $assert - (i32.eq - (call $ext_call - (i32.const 16) ;; Pointer to destination address - (i32.const 8) ;; Length of destination address - (i64.const 0) ;; How much gas to devote for the execution. 0 = all. - (i32.const 0) ;; Pointer to the buffer with value to transfer - (i32.const 8) ;; Length of the buffer with value to transfer - (i32.const 0) ;; Pointer to input data buffer address - (i32.const 0) ;; Length of input data buffer - (i32.const 4294967295) ;; u32 max sentinel value: do not copy output - (i32.const 0) ;; Length is ignored in this case - ) - (i32.const 0) - ) + (call $ext_terminate + (i32.const 0) ;; Pointer to destination address + (i32.const 8) ;; Length of destination address ) ) diff --git a/frame/contracts/fixtures/set_rent.wat b/frame/contracts/fixtures/set_rent.wat index 4e6424e720104..ba52e9ed752ce 100644 --- a/frame/contracts/fixtures/set_rent.wat +++ b/frame/contracts/fixtures/set_rent.wat @@ -1,5 +1,5 @@ (module - (import "env" "ext_transfer" (func $ext_transfer (param i32 i32 i32 i32))) + (import "env" "ext_transfer" (func $ext_transfer (param i32 i32 i32 i32) (result i32))) (import "env" "ext_set_storage" (func $ext_set_storage (param i32 i32 i32))) (import "env" "ext_clear_storage" (func $ext_clear_storage (param i32))) (import "env" "ext_set_rent_allowance" (func $ext_set_rent_allowance (param i32 i32))) @@ -24,7 +24,12 @@ ;; transfer 50 to CHARLIE (func $call_2 - (call $ext_transfer (i32.const 68) (i32.const 8) (i32.const 76) (i32.const 8)) + (call $assert + (i32.eq + (call $ext_transfer (i32.const 68) (i32.const 8) (i32.const 76) (i32.const 8)) + (i32.const 0) + ) + ) ) ;; do nothing diff --git a/frame/contracts/src/exec.rs b/frame/contracts/src/exec.rs index e8965692aa243..7f32aaf35c42c 100644 --- a/frame/contracts/src/exec.rs +++ b/frame/contracts/src/exec.rs @@ -14,9 +14,11 @@ // You should have received a copy of the GNU General Public License // along with Substrate. If not, see . -use super::{CodeHash, Config, ContractAddressFor, Event, RawEvent, Trait, - TrieId, BalanceOf, ContractInfo, TrieIdGenerator}; -use crate::{gas::{Gas, GasMeter, Token}, rent, storage, Error, ContractInfoOf}; +use crate::{ + CodeHash, Config, ContractAddressFor, Event, RawEvent, Trait, + TrieId, BalanceOf, ContractInfo, TrieIdGenerator, + gas::{Gas, GasMeter, Token}, rent, storage, Error, ContractInfoOf +}; use bitflags::bitflags; use sp_std::prelude::*; use sp_runtime::traits::{Bounded, Zero, Convert, Saturating}; @@ -69,7 +71,34 @@ impl ExecReturnValue { } } -pub type ExecResult = Result; +/// 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 during the supervisor code execution. +#[cfg_attr(test, derive(PartialEq, Eq, Debug))] +pub enum ErrorOrigin { + /// The error happened during the setup phase of the contract that is to be called. + Supervisor, + /// The error happened during execution of the called contract. + Callee, +} + +#[cfg_attr(test, derive(PartialEq, Eq, Debug))] +pub struct ExecError { + pub error: DispatchError, + pub origin: ErrorOrigin, +} + +impl> From for ExecError { + fn from(error: T) -> Self { + Self { + error: error.into(), + origin: ErrorOrigin::Supervisor, + } + } +} + +/// The error returned from functions calling into contract execution. +pub type ExecResult = Result; /// An interface that provides access to the external environment in which the /// smart-contract is executed. @@ -99,7 +128,7 @@ pub trait Ext { value: BalanceOf, gas_meter: &mut GasMeter, input_data: Vec, - ) -> Result<(AccountIdOf, ExecReturnValue), DispatchError>; + ) -> Result<(AccountIdOf, ExecReturnValue), ExecError>; /// Transfer some amount of funds into the specified account. fn transfer( @@ -307,31 +336,31 @@ where input_data: Vec, ) -> ExecResult { if self.depth == self.config.max_depth as usize { - Err("reached maximum depth, cannot make a call")? + Err(Error::::MaxCallDepthReached)? } if gas_meter .charge(self.config, ExecFeeToken::Call) .is_out_of_gas() { - Err("not enough gas to pay base call fee")? + Err(Error::::OutOfGas)? } // Assumption: `collect_rent` doesn't collide with overlay because // `collect_rent` will be done on first call and destination contract and balance // cannot be changed before the first call - let contract_info = rent::collect_rent::(&dest); - - // Calls to dead contracts always fail. - if let Some(ContractInfo::Tombstone(_)) = contract_info { - Err("contract has been evicted")? + // We do not allow 'calling' plain accounts. For transfering value + // `ext_transfer` must be used. + let contract = if let Some(ContractInfo::Alive(info)) = rent::collect_rent::(&dest) { + info + } else { + Err(Error::::InvalidContractCalled)? }; let transactor_kind = self.transactor_kind(); let caller = self.self_account.clone(); - let dest_trie_id = contract_info.and_then(|i| i.as_alive().map(|i| i.trie_id.clone())); - self.with_nested_context(dest.clone(), dest_trie_id, |nested| { + self.with_nested_context(dest.clone(), Some(contract.trie_id.clone()), |nested| { if value > BalanceOf::::zero() { transfer( gas_meter, @@ -344,22 +373,15 @@ where )? } - // If code_hash is not none, then the destination account is a live contract, otherwise - // it is a regular account since tombstone accounts have already been rejected. - match storage::code_hash::(&dest) { - Ok(dest_code_hash) => { - let executable = nested.loader.load_main(&dest_code_hash)?; - let output = nested.vm - .execute( - &executable, - nested.new_call_context(caller, value), - input_data, - gas_meter, - )?; - Ok(output) - } - Err(storage::ContractAbsentError) => Ok(ExecReturnValue { flags: ReturnFlags::empty(), data: Vec::new() }), - } + let executable = nested.loader.load_main(&contract.code_hash) + .map_err(|_| Error::::CodeNotFound)?; + let output = nested.vm.execute( + &executable, + nested.new_call_context(caller, value), + input_data, + gas_meter, + ).map_err(|e| ExecError { error: e.error, origin: ErrorOrigin::Callee })?; + Ok(output) }) } @@ -369,16 +391,16 @@ where gas_meter: &mut GasMeter, code_hash: &CodeHash, input_data: Vec, - ) -> Result<(T::AccountId, ExecReturnValue), DispatchError> { + ) -> Result<(T::AccountId, ExecReturnValue), ExecError> { if self.depth == self.config.max_depth as usize { - Err("reached maximum depth, cannot instantiate")? + Err(Error::::MaxCallDepthReached)? } if gas_meter .charge(self.config, ExecFeeToken::Instantiate) .is_out_of_gas() { - Err("not enough gas to pay base instantiate fee")? + Err(Error::::OutOfGas)? } let transactor_kind = self.transactor_kind(); @@ -416,21 +438,21 @@ where nested, )?; - let executable = nested.loader.load_init(&code_hash)?; + let executable = nested.loader.load_init(&code_hash) + .map_err(|_| Error::::CodeNotFound)?; let output = nested.vm .execute( &executable, nested.new_call_context(caller.clone(), endowment), input_data, gas_meter, - )?; + ).map_err(|e| ExecError { error: e.error, origin: ErrorOrigin::Callee })?; - // Error out if insufficient remaining balance. // We need each contract that exists to be above the subsistence threshold // in order to keep up the guarantuee that we always leave a tombstone behind // with the exception of a contract that called `ext_terminate`. - if T::Currency::free_balance(&dest) < nested.config.subsistence_threshold() { - Err("insufficient remaining balance")? + if T::Currency::total_balance(&dest) < nested.config.subsistence_threshold() { + Err(Error::::NewContractNotFunded)? } // Deposit an instantiation event. @@ -569,7 +591,7 @@ fn transfer<'a, T: Trait, V: Vm, L: Loader>( }; if gas_meter.charge(ctx.config, token).is_out_of_gas() { - Err("not enough gas to pay transfer fee")? + Err(Error::::OutOfGas)? } // Only ext_terminate is allowed to bring the sender below the subsistence @@ -580,13 +602,15 @@ fn transfer<'a, T: Trait, V: Vm, L: Loader>( ensure!( T::Currency::total_balance(transactor).saturating_sub(value) >= ctx.config.subsistence_threshold(), - Error::::InsufficientBalance, + Error::::BelowSubsistenceThreshold, ); ExistenceRequirement::KeepAlive }, (_, PlainAccount) => ExistenceRequirement::KeepAlive, }; - T::Currency::transfer(transactor, dest, value, existence_requirement)?; + + T::Currency::transfer(transactor, dest, value, existence_requirement) + .map_err(|_| Error::::TransferFailed)?; Ok(()) } @@ -653,7 +677,7 @@ where endowment: BalanceOf, gas_meter: &mut GasMeter, input_data: Vec, - ) -> Result<(AccountIdOf, ExecReturnValue), DispatchError> { + ) -> Result<(AccountIdOf, ExecReturnValue), ExecError> { self.ctx.instantiate(endowment, gas_meter, code_hash, input_data) } @@ -837,13 +861,13 @@ fn deposit_event( mod tests { use super::{ BalanceOf, Event, ExecFeeToken, ExecResult, ExecutionContext, Ext, Loader, - RawEvent, TransferFeeKind, TransferFeeToken, Vm, ReturnFlags, + RawEvent, TransferFeeKind, TransferFeeToken, Vm, ReturnFlags, ExecError, ErrorOrigin }; use crate::{ gas::GasMeter, tests::{ExtBuilder, Test, MetaEvent}, exec::ExecReturnValue, CodeHash, Config, gas::Gas, - storage, + storage, Error }; use crate::tests::test_utils::{place_contract, set_balance, get_balance}; use sp_runtime::DispatchError; @@ -999,11 +1023,19 @@ mod tests { let mut gas_meter = GasMeter::::new(GAS_LIMIT); - let result = ctx.call(dest, 0, &mut gas_meter, vec![]); + let result = super::transfer( + &mut gas_meter, + super::TransferCause::Call, + super::TransactorKind::PlainAccount, + &origin, + &dest, + 0, + &mut ctx, + ); assert_matches!(result, Ok(_)); let mut toks = gas_meter.tokens().iter(); - match_tokens!(toks, ExecFeeToken::Call,); + match_tokens!(toks, TransferFeeToken { kind: TransferFeeKind::Transfer },); }); // This test verifies that base fee for instantiation is taken. @@ -1043,14 +1075,18 @@ mod tests { set_balance(&origin, 100); set_balance(&dest, 0); - let output = ctx.call( - dest, + let mut gas_meter = GasMeter::::new(GAS_LIMIT); + + super::transfer( + &mut gas_meter, + super::TransferCause::Call, + super::TransactorKind::PlainAccount, + &origin, + &dest, 55, - &mut GasMeter::::new(GAS_LIMIT), - vec![], + &mut ctx, ).unwrap(); - assert!(output.is_success()); assert_eq!(get_balance(&origin), 45); assert_eq!(get_balance(&dest), 55); }); @@ -1107,13 +1143,20 @@ mod tests { let mut gas_meter = GasMeter::::new(GAS_LIMIT); - let result = ctx.call(dest, 50, &mut gas_meter, vec![]); + let result = super::transfer( + &mut gas_meter, + super::TransferCause::Call, + super::TransactorKind::PlainAccount, + &origin, + &dest, + 50, + &mut ctx, + ); assert_matches!(result, Ok(_)); let mut toks = gas_meter.tokens().iter(); match_tokens!( toks, - ExecFeeToken::Call, TransferFeeToken { kind: TransferFeeKind::Transfer, }, @@ -1132,13 +1175,20 @@ mod tests { let mut gas_meter = GasMeter::::new(GAS_LIMIT); - let result = ctx.call(dest, 50, &mut gas_meter, vec![]); + let result = super::transfer( + &mut gas_meter, + super::TransferCause::Call, + super::TransactorKind::PlainAccount, + &origin, + &dest, + 50, + &mut ctx, + ); assert_matches!(result, Ok(_)); let mut toks = gas_meter.tokens().iter(); match_tokens!( toks, - ExecFeeToken::Call, TransferFeeToken { kind: TransferFeeKind::Transfer, }, @@ -1189,16 +1239,19 @@ mod tests { let mut ctx = ExecutionContext::top_level(origin, &cfg, &vm, &loader); set_balance(&origin, 0); - let result = ctx.call( - dest, - 100, + let result = super::transfer( &mut GasMeter::::new(GAS_LIMIT), - vec![], + super::TransferCause::Call, + super::TransactorKind::PlainAccount, + &origin, + &dest, + 100, + &mut ctx, ); - assert_matches!( + assert_eq!( result, - Err(DispatchError::Module { message: Some("InsufficientBalance"), .. }) + Err(Error::::TransferFailed.into()) ); assert_eq!(get_balance(&origin), 0); assert_eq!(get_balance(&dest), 0); @@ -1335,9 +1388,9 @@ mod tests { if !*reached_bottom { // We are first time here, it means we just reached bottom. // Verify that we've got proper error and set `reached_bottom`. - assert_matches!( + assert_eq!( r, - Err(DispatchError::Other("reached maximum depth, cannot make a call")) + Err(Error::::MaxCallDepthReached.into()) ); *reached_bottom = true; } else { @@ -1604,7 +1657,10 @@ mod tests { ctx.gas_meter, vec![] ), - Err(DispatchError::Other("It's a trap!")) + Err(ExecError { + error: DispatchError::Other("It's a trap!"), + origin: ErrorOrigin::Callee, + }) ); exec_success() @@ -1648,14 +1704,14 @@ mod tests { let mut ctx = ExecutionContext::top_level(ALICE, &cfg, &vm, &loader); set_balance(&ALICE, 1000); - assert_matches!( + assert_eq!( ctx.instantiate( 100, &mut GasMeter::::new(GAS_LIMIT), &terminate_ch, vec![], ), - Err(DispatchError::Other("insufficient remaining balance")) + Err(Error::::NewContractNotFunded.into()) ); assert_eq!( diff --git a/frame/contracts/src/gas.rs b/frame/contracts/src/gas.rs index d6c7f82753ebd..decaf11b796f7 100644 --- a/frame/contracts/src/gas.rs +++ b/frame/contracts/src/gas.rs @@ -14,11 +14,11 @@ // You should have received a copy of the GNU General Public License // along with Substrate. If not, see . -use crate::Trait; +use crate::{Trait, exec::ExecError}; use sp_std::marker::PhantomData; use sp_runtime::traits::Zero; use frame_support::dispatch::{ - DispatchError, DispatchResultWithPostInfo, PostDispatchInfo, DispatchErrorWithPostInfo, + DispatchResultWithPostInfo, PostDispatchInfo, DispatchErrorWithPostInfo, }; #[cfg(test)] @@ -189,8 +189,9 @@ impl GasMeter { } /// Turn this GasMeter into a DispatchResult that contains the actually used gas. - pub fn into_dispatch_result(self, result: Result) -> DispatchResultWithPostInfo where - E: Into, + pub fn into_dispatch_result(self, result: Result) -> DispatchResultWithPostInfo + where + E: Into, { let post_info = PostDispatchInfo { actual_weight: Some(self.gas_spent()), @@ -199,7 +200,7 @@ impl GasMeter { result .map(|_| post_info) - .map_err(|e| DispatchErrorWithPostInfo { post_info, error: e.into() }) + .map_err(|e| DispatchErrorWithPostInfo { post_info, error: e.into().error }) } #[cfg(test)] diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 6d0b481dd0d47..c63b8b4aa8dde 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -420,9 +420,24 @@ decl_error! { /// the subsistence threshold. No transfer is allowed to do this in order to allow /// for a tombstone to be created. Use `ext_terminate` to remove a contract without /// leaving a tombstone behind. - InsufficientBalance, + BelowSubsistenceThreshold, + /// The newly created contract is below the subsistence threshold after executing + /// its contructor. No contracts are allowed to exist below that threshold. + NewContractNotFunded, + /// Performing the requested transfer failed for a reason originating in the + /// chosen currency implementation of the runtime. Most probably the balance is + /// too low or locks are placed on it. + TransferFailed, + /// Performing a call was denied because the calling depth reached the limit + /// of what is specified in the schedule. + MaxCallDepthReached, + /// The contract that was called is either no contract at all (a plain account) + /// or is a tombstone. + InvalidContractCalled, /// The code supplied to `put_code` exceeds the limit specified in the current schedule. CodeTooLarge, + /// No code could be found at the supplied code hash. + CodeNotFound, } } diff --git a/frame/contracts/src/storage.rs b/frame/contracts/src/storage.rs index 4c5ad892a967b..3740952778fd3 100644 --- a/frame/contracts/src/storage.rs +++ b/frame/contracts/src/storage.rs @@ -149,6 +149,7 @@ pub fn set_rent_allowance( } /// Returns the code hash of the contract specified by `account` ID. +#[cfg(test)] pub fn code_hash(account: &AccountIdOf) -> Result, ContractAbsentError> { >::get(account) .and_then(|i| i.as_alive().map(|i| i.code_hash)) diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index 5b2d7feb3d128..ae5844970507b 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -32,6 +32,7 @@ use frame_support::{ impl_outer_origin, parameter_types, StorageMap, StorageValue, traits::{Currency, Get}, weights::{Weight, PostDispatchInfo}, + dispatch::DispatchErrorWithPostInfo, }; use std::cell::RefCell; use frame_system::{self as system, EventRecord, Phase}; @@ -279,19 +280,23 @@ where Ok((wasm_binary, code_hash)) } -// Perform a simple transfer to a non-existent account. +// Perform a call to a plain account. +// The actual transfer fails because we can only call contracts. // Then we check that only the base costs are returned as actual costs. #[test] -fn returns_base_call_cost() { +fn calling_plain_account_fails() { ExtBuilder::default().build().execute_with(|| { let _ = Balances::deposit_creating(&ALICE, 100_000_000); assert_eq!( Contracts::call(Origin::signed(ALICE), BOB, 0, GAS_LIMIT, Vec::new()), - Ok( - PostDispatchInfo { - actual_weight: Some(67500000), - pays_fee: Default::default(), + Err( + DispatchErrorWithPostInfo { + error: Error::::InvalidContractCalled.into(), + post_info: PostDispatchInfo { + actual_weight: Some(67500000), + pays_fee: Default::default(), + }, } ) ); @@ -987,7 +992,7 @@ fn call_removed_contract() { // Calling contract should remove contract and fail. assert_err_ignore_postinfo!( Contracts::call(Origin::signed(ALICE), BOB, 0, GAS_LIMIT, call::null()), - "contract has been evicted" + Error::::InvalidContractCalled ); // Calling a contract that is about to evict shall emit an event. assert_eq!(System::events(), vec![ @@ -1001,7 +1006,7 @@ fn call_removed_contract() { // Subsequent contract calls should also fail. assert_err_ignore_postinfo!( Contracts::call(Origin::signed(ALICE), BOB, 0, GAS_LIMIT, call::null()), - "contract has been evicted" + Error::::InvalidContractCalled ); }) } @@ -1128,7 +1133,7 @@ fn restoration(test_different_storage: bool, test_restore_to_with_dirty_storage: // we expect that it will get removed leaving tombstone. assert_err_ignore_postinfo!( Contracts::call(Origin::signed(ALICE), BOB, 0, GAS_LIMIT, call::null()), - "contract has been evicted" + Error::::InvalidContractCalled ); assert!(ContractInfoOf::::get(BOB).unwrap().get_tombstone().is_some()); assert_eq!(System::events(), vec![ @@ -1373,17 +1378,16 @@ fn cannot_self_destruct_through_draning() { Some(ContractInfo::Alive(_)) ); - // Call BOB with no input data, forcing it to run until out-of-balance - // and eventually trapping because below existential deposit. - assert_err_ignore_postinfo!( + // Call BOB which makes it send all funds to the zero address + // The contract code asserts that the correct error value is returned. + assert_ok!( Contracts::call( Origin::signed(ALICE), BOB, 0, GAS_LIMIT, vec![], - ), - "contract trapped during execution" + ) ); }); } @@ -1535,8 +1539,7 @@ fn cannot_self_destruct_in_constructor() { let _ = Balances::deposit_creating(&ALICE, 1_000_000); assert_ok!(Contracts::put_code(Origin::signed(ALICE), wasm)); - // Fail to instantiate the BOB because the call that is issued in the deploy - // function exhausts all balances which puts it below the existential deposit. + // Fail to instantiate the BOB because the contructor calls ext_terminate. assert_err_ignore_postinfo!( Contracts::instantiate( Origin::signed(ALICE), @@ -1545,7 +1548,7 @@ fn cannot_self_destruct_in_constructor() { code_hash.into(), vec![], ), - "contract trapped during execution" + Error::::NewContractNotFunded, ); }); } diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index 68dbae896b0c6..4fdf3307a58e2 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -152,7 +152,7 @@ mod tests { use super::*; use std::collections::HashMap; use sp_core::H256; - use crate::exec::{Ext, StorageKey, ExecReturnValue, ReturnFlags}; + use crate::exec::{Ext, StorageKey, ExecReturnValue, ReturnFlags, ExecError, ErrorOrigin}; use crate::gas::{Gas, GasMeter}; use crate::tests::{Test, Call}; use crate::wasm::prepare::prepare_contract; @@ -225,7 +225,7 @@ mod tests { endowment: u64, gas_meter: &mut GasMeter, data: Vec, - ) -> Result<(u64, ExecReturnValue), DispatchError> { + ) -> Result<(u64, ExecReturnValue), ExecError> { self.instantiates.push(InstantiateEntry { code_hash: code_hash.clone(), endowment, @@ -365,7 +365,7 @@ mod tests { value: u64, gas_meter: &mut GasMeter, input_data: Vec, - ) -> Result<(u64, ExecReturnValue), DispatchError> { + ) -> Result<(u64, ExecReturnValue), ExecError> { (**self).instantiate(code, value, gas_meter, input_data) } fn transfer( @@ -483,14 +483,16 @@ mod tests { ;; value_ptr: u32, ;; value_len: u32, ;;) -> u32 - (import "env" "ext_transfer" (func $ext_transfer (param i32 i32 i32 i32))) + (import "env" "ext_transfer" (func $ext_transfer (param i32 i32 i32 i32) (result i32))) (import "env" "memory" (memory 1 1)) (func (export "call") - (call $ext_transfer - (i32.const 4) ;; Pointer to "account" address. - (i32.const 8) ;; Length of "account" address. - (i32.const 12) ;; Pointer to the buffer with value to transfer - (i32.const 8) ;; Length of the buffer with value to transfer. + (drop + (call $ext_transfer + (i32.const 4) ;; Pointer to "account" address. + (i32.const 8) ;; Length of "account" address. + (i32.const 12) ;; Pointer to the buffer with value to transfer + (i32.const 8) ;; Length of the buffer with value to transfer. + ) ) ) (func (export "deploy")) @@ -521,7 +523,7 @@ mod tests { to: 7, value: 153, data: Vec::new(), - gas_left: 9989500000, + gas_left: 9989000000, }] ); } @@ -1510,7 +1512,10 @@ mod tests { MockExt::default(), &mut gas_meter ), - Err(DispatchError::Other("contract trapped during execution")) + Err(ExecError { + error: DispatchError::Other("contract trapped during execution"), + origin: ErrorOrigin::Supervisor, + }) ); } @@ -1552,7 +1557,10 @@ mod tests { MockExt::default(), &mut gas_meter ), - Err(DispatchError::Other("contract trapped during execution")) + Err(ExecError { + error: DispatchError::Other("contract trapped during execution"), + origin: ErrorOrigin::Supervisor, + }) ); } diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index ab93076f57b2a..b8be41277993c 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -18,7 +18,7 @@ use crate::{Schedule, Trait, CodeHash, BalanceOf, Error}; use crate::exec::{ - Ext, ExecResult, ExecReturnValue, StorageKey, TopicOf, ReturnFlags, + Ext, ExecResult, ExecReturnValue, StorageKey, TopicOf, ReturnFlags, ExecError }; use crate::gas::{Gas, GasMeter, Token, GasMeterResult}; use crate::wasm::env_def::ConvertibleToWasm; @@ -43,14 +43,26 @@ pub enum ReturnCode { Success = 0, /// The called function trapped and has its state changes reverted. /// In this case no output buffer is returned. - /// Can only be returned from `ext_call` and `ext_instantiate`. CalleeTrapped = 1, /// The called function ran to completion but decided to revert its state. /// An output buffer is returned when one was supplied. - /// Can only be returned from `ext_call` and `ext_instantiate`. CalleeReverted = 2, /// The passed key does not exist in storage. KeyNotFound = 3, + /// Transfer failed because it would have brought the senders total balance below the + /// subsistence deposit. + BelowSubsistenceThreshold = 4, + /// Transfer failed for other reasons. Most probably reserved or locked balance of the + /// sender prevents the transfer. + TransferFailed = 5, + /// The newly created contract is below the subsistence threshold after executing + // its contructor. + NewContractNotFunded = 6, + /// No code could be found at the supplied code hash. + CodeNotFound = 7, + /// The contract that was called is either no contract at all (a plain account) + /// or is a tombstone. + InvalidContractCalled = 8, } impl ConvertibleToWasm for ReturnCode { @@ -66,7 +78,7 @@ impl ConvertibleToWasm for ReturnCode { } impl From for ReturnCode { - fn from(from: ExecReturnValue) -> ReturnCode { + fn from(from: ExecReturnValue) -> Self { if from.flags.contains(ReturnFlags::REVERT) { Self::CalleeReverted } else { @@ -379,7 +391,7 @@ fn write_sandbox_output( let len: u32 = read_sandbox_memory_as(ctx, out_len_ptr, 4)?; if len < buf_len { - Err(map_err(ctx, Error::::OutputBufferTooSmall))? + Err(store_err(ctx, Error::::OutputBufferTooSmall))? } charge_gas( @@ -398,7 +410,7 @@ fn write_sandbox_output( /// Stores a DispatchError returned from an Ext function into the trap_reason. /// /// This allows through supervisor generated errors to the caller. -fn map_err(ctx: &mut Runtime, err: Error) -> sp_sandbox::HostError where +fn store_err(ctx: &mut Runtime, err: Error) -> sp_sandbox::HostError where E: Ext, Error: Into, { @@ -406,12 +418,86 @@ fn map_err(ctx: &mut Runtime, err: Error) -> sp_sandbox::HostError sp_sandbox::HostError } +/// Fallible conversion of `DispatchError` to `ReturnCode`. +fn err_into_return_code(from: DispatchError) -> Result { + use ReturnCode::*; + + let below_sub = Error::::BelowSubsistenceThreshold.into(); + let transfer_failed = Error::::TransferFailed.into(); + let not_funded = Error::::NewContractNotFunded.into(); + let no_code = Error::::CodeNotFound.into(); + let invalid_contract = Error::::InvalidContractCalled.into(); + + match from { + x if x == below_sub => Ok(BelowSubsistenceThreshold), + x if x == transfer_failed => Ok(TransferFailed), + x if x == not_funded => Ok(NewContractNotFunded), + x if x == no_code => Ok(CodeNotFound), + x if x == invalid_contract => Ok(InvalidContractCalled), + err => Err(err) + } +} + +/// Fallible conversion of a `ExecResult` to `ReturnCode`. +fn exec_into_return_code(from: ExecResult) -> Result { + use crate::exec::ErrorOrigin::Callee; + + let ExecError { error, origin } = match from { + Ok(retval) => return Ok(retval.into()), + Err(err) => err, + }; + + match (error, origin) { + (_, Callee) => Ok(ReturnCode::CalleeTrapped), + (err, _) => err_into_return_code::(err) + } +} + +/// Used by Runtime API that calls into other contracts. +/// +/// Those need to transform the the `ExecResult` returned from the execution into +/// a `ReturnCode`. If this conversion fails because the `ExecResult` constitutes a +/// a fatal error then this error is stored in the `ExecutionContext` so it can be +/// extracted for display in the UI. +fn map_exec_result(ctx: &mut Runtime, result: ExecResult) + -> Result +{ + match exec_into_return_code::(result) { + Ok(code) => Ok(code), + Err(err) => Err(store_err(ctx, err)), + } +} + +/// Try to convert an error into a `ReturnCode`. +/// +/// Used to decide between fatal and non-fatal errors. +fn map_dispatch_result(ctx: &mut Runtime, result: Result) + -> Result +{ + let err = if let Err(err) = result { + err + } else { + return Ok(ReturnCode::Success) + }; + + match err_into_return_code::(err) { + Ok(code) => Ok(code), + Err(err) => Err(store_err(ctx, err)), + } +} + // *********************************************************** // * AFTER MAKING A CHANGE MAKE SURE TO UPDATE COMPLEXITY.MD * // *********************************************************** // Define a function `fn init_env() -> HostFunctionSet` that returns // a function set which can be imported by an executed contract. +// +// # Note +// +// Any input that leads to a out of bound error (reading or writing) or failing to decode +// data passed to the supervisor will lead to a trap. This is not documented explicitly +// for every function. define_env!(Env, , // Account for used gas. Traps if gas used is greater than gas limit. @@ -441,7 +527,7 @@ define_env!(Env, , // - `value_ptr`: pointer into the linear memory where the value to set is placed. // - `value_len`: the length of the value in bytes. // - // # Errors + // # Traps // // - If value length exceeds the configured maximum value length of a storage entry. // - Upon trying to set an empty storage entry (value length is 0). @@ -480,12 +566,7 @@ define_env!(Env, , // // # Errors // - // If there is no entry under the given key then this function will return - // `ReturnCode::KeyNotFound`. - // - // # Traps - // - // Traps if the supplied buffer length is smaller than the size of the stored value. + // `ReturnCode::KeyNotFound` ext_get_storage(ctx, key_ptr: u32, out_ptr: u32, out_len_ptr: u32) -> ReturnCode => { let mut key: StorageKey = [0; 32]; read_sandbox_memory_into_buf(ctx, key_ptr, &mut key)?; @@ -508,24 +589,24 @@ define_env!(Env, , // Should be decodable as a `T::Balance`. Traps otherwise. // - value_len: length of the value buffer. // - // # Traps + // # Errors // - // Traps if the transfer wasn't succesful. This can happen when the value transfered - // brings the sender below the existential deposit. Use `ext_terminate` to remove - // the caller contract. + // `ReturnCode::BelowSubsistenceThreshold` + // `ReturnCode::TransferFailed` ext_transfer( ctx, account_ptr: u32, account_len: u32, value_ptr: u32, value_len: u32 - ) => { + ) -> ReturnCode => { let callee: <::T as frame_system::Trait>::AccountId = read_sandbox_memory_as(ctx, account_ptr, account_len)?; let value: BalanceOf<::T> = read_sandbox_memory_as(ctx, value_ptr, value_len)?; - ctx.ext.transfer(&callee, value, ctx.gas_meter).map_err(|e| map_err(ctx, e)) + let result = ctx.ext.transfer(&callee, value, ctx.gas_meter); + map_dispatch_result(ctx, result) }, // Make a call to another contract. @@ -551,17 +632,14 @@ define_env!(Env, , // // # Errors // - // `ReturnCode::CalleeReverted`: The callee ran to completion but decided to have its - // changes reverted. The delivery of the output buffer is still possible. - // `ReturnCode::CalleeTrapped`: The callee trapped during execution. All changes are reverted - // and no output buffer is delivered. - // - // # Traps + // An error means that the call wasn't succesful output buffer is returned unless + // stated otherwise. // - // - Transfer of balance failed. This call can not bring the sender below the existential - // deposit. Use `ext_terminate` to remove the caller. - // - Callee does not exist. - // - Supplied output buffer is too small. + // `ReturnCode::CalleeReverted`: Output buffer is returned. + // `ReturnCode::CalleeTrapped` + // `ReturnCode::BelowSubsistenceThreshold` + // `ReturnCode::TransferFailed` + // `ReturnCode::InvalidConractCalled` ext_call( ctx, callee_ptr: u32, @@ -594,22 +672,16 @@ define_env!(Env, , nested_meter, input_data, ) - .map_err(|_| ()) } // there is not enough gas to allocate for the nested call. - None => Err(()), + None => Err(Error::<::T>::OutOfGas.into()), } }); - match call_outcome { - Ok(output) => { - write_sandbox_output(ctx, output_ptr, output_len_ptr, &output.data, true)?; - Ok(output.into()) - }, - Err(_) => { - Ok(ReturnCode::CalleeTrapped) - }, + if let Ok(output) = &call_outcome { + write_sandbox_output(ctx, output_ptr, output_len_ptr, &output.data, true)?; } + map_exec_result(ctx, call_outcome) }, // Instantiate a contract with the specified code hash. @@ -643,19 +715,18 @@ define_env!(Env, , // // # Errors // - // `ReturnCode::CalleeReverted`: The callee's constructor ran to completion but decided to have - // its changes reverted. The delivery of the output buffer is still possible but the - // account was not created and no address is returned. - // `ReturnCode::CalleeTrapped`: The callee trapped during execution. All changes are reverted - // and no output buffer is delivered. The accounts was not created and no address is - // returned. + // Please consult the `ReturnCode` enum declaration for more information on those + // errors. Here we only note things specific to this function. // - // # Traps + // An error means that the account wasn't created and no address or output buffer + // is returned unless stated otherwise. // - // - Transfer of balance failed. This call can not bring the sender below the existential - // deposit. Use `ext_terminate` to remove the caller. - // - Code hash does not exist. - // - Supplied output buffers are too small. + // `ReturnCode::CalleeReverted`: Output buffer is returned. + // `ReturnCode::CalleeTrapped` + // `ReturnCode::BelowSubsistenceThreshold` + // `ReturnCode::TransferFailed` + // `ReturnCode::NewContractNotFunded` + // `ReturnCode::CodeNotFound` ext_instantiate( ctx, code_hash_ptr: u32, @@ -690,26 +761,20 @@ define_env!(Env, , nested_meter, input_data ) - .map_err(|_| ()) } // there is not enough gas to allocate for the nested call. - None => Err(()), + None => Err(Error::<::T>::OutOfGas.into()), } }); - match instantiate_outcome { - Ok((address, output)) => { - if !output.flags.contains(ReturnFlags::REVERT) { - write_sandbox_output( - ctx, address_ptr, address_len_ptr, &address.encode(), true - )?; - } - write_sandbox_output(ctx, output_ptr, output_len_ptr, &output.data, true)?; - Ok(output.into()) - }, - Err(_) => { - Ok(ReturnCode::CalleeTrapped) - }, + if let Ok((address, output)) = &instantiate_outcome { + if !output.flags.contains(ReturnFlags::REVERT) { + write_sandbox_output( + ctx, address_ptr, address_len_ptr, &address.encode(), true + )?; + } + write_sandbox_output(ctx, output_ptr, output_len_ptr, &output.data, true)?; } + map_exec_result(ctx, instantiate_outcome.map(|(_id, retval)| retval)) }, // Remove the calling account and transfer remaining balance. @@ -722,6 +787,10 @@ define_env!(Env, , // where all remaining funds of the caller are transfered. // Should be decodable as an `T::AccountId`. Traps otherwise. // - beneficiary_len: length of the address buffer. + // + // # Traps + // + // - The contract is live i.e is already on the call stack. ext_terminate( ctx, beneficiary_ptr: u32, @@ -939,6 +1008,11 @@ define_env!(Env, , // encodes the rent allowance that must be set in the case of successful restoration. // `delta_ptr` is the pointer to the start of a buffer that has `delta_count` storage keys // laid out sequentially. + // + // # Traps + // + // - Tombstone hashes do not match + // - Calling cantract is live i.e is already on the call stack. ext_restore_to( ctx, dest_ptr: u32, From 2a19ceb8859e1e4470954ee1e4836440f22b4eeb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Wed, 29 Jul 2020 18:41:30 +0200 Subject: [PATCH 02/14] seal: Improve error messages on memory access failures --- frame/contracts/src/lib.rs | 4 ++ frame/contracts/src/wasm/mod.rs | 71 +++++++++++++++++++++++++++++ frame/contracts/src/wasm/runtime.rs | 12 ++--- 3 files changed, 81 insertions(+), 6 deletions(-) diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index c63b8b4aa8dde..815655f3c095c 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -438,6 +438,10 @@ decl_error! { CodeTooLarge, /// No code could be found at the supplied code hash. CodeNotFound, + /// A buffer outside of sandbox memory was passed to a contract API function. + OutOfBounds, + /// Input passed to a contract API function failed to decode as expected type. + DecodingFailed, } } diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index 4fdf3307a58e2..8c2cd60ddfb20 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -1674,4 +1674,75 @@ mod tests { assert_eq!(output, ExecReturnValue { flags: ReturnFlags::REVERT, data: hex!("5566778899").to_vec() }); assert!(!output.is_success()); } + + const CODE_OUT_OF_BOUNDS_ACCESS: &str = r#" +(module + (import "env" "ext_terminate" (func $ext_terminate (param i32 i32))) + (import "env" "memory" (memory 1 1)) + + (func (export "deploy")) + + (func (export "call") + (call $ext_terminate + (i32.const 65536) ;; Pointer to "account" address (out of bound). + (i32.const 8) ;; Length of "account" address. + ) + ) +) +"#; + + #[test] + fn contract_out_of_bounds_access() { + let mut mock_ext = MockExt::default(); + let result = execute( + CODE_OUT_OF_BOUNDS_ACCESS, + vec![], + &mut mock_ext, + &mut GasMeter::new(GAS_LIMIT), + ); + + assert_eq!( + result, + Err(ExecError { + error: Error::::OutOfBounds.into(), + origin: ErrorOrigin::Supervisor, + }) + ); + } + + const CODE_DECODE_FAILURE: &str = r#" +(module + (import "env" "ext_terminate" (func $ext_terminate (param i32 i32))) + (import "env" "memory" (memory 1 1)) + + (func (export "deploy")) + + (func (export "call") + (call $ext_terminate + (i32.const 0) ;; Pointer to "account" address. + (i32.const 4) ;; Length of "account" address (too small -> decode fail). + ) + ) +) +"#; + + #[test] + fn contract_decode_failure() { + let mut mock_ext = MockExt::default(); + let result = execute( + CODE_DECODE_FAILURE, + vec![], + &mut mock_ext, + &mut GasMeter::new(GAS_LIMIT), + ); + + assert_eq!( + result, + Err(ExecError { + error: Error::::DecodingFailed.into(), + origin: ErrorOrigin::Supervisor, + }) + ); + } + } diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index b8be41277993c..8b0500cb1de22 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -292,7 +292,8 @@ fn read_sandbox_memory( )?; let mut buf = vec![0u8; len as usize]; - ctx.memory.get(ptr, buf.as_mut_slice()).map_err(|_| sp_sandbox::HostError)?; + ctx.memory.get(ptr, buf.as_mut_slice()) + .map_err(|_| store_err(ctx, Error::::OutOfBounds))?; Ok(buf) } @@ -316,7 +317,7 @@ fn read_sandbox_memory_into_buf( RuntimeToken::ReadMemory(buf.len() as u32), )?; - ctx.memory.get(ptr, buf).map_err(Into::into) + ctx.memory.get(ptr, buf).map_err(|_| store_err(ctx, Error::::OutOfBounds)) } /// Read designated chunk from the sandbox memory, consuming an appropriate amount of @@ -334,7 +335,7 @@ fn read_sandbox_memory_as( len: u32, ) -> Result { let buf = read_sandbox_memory(ctx, ptr, len)?; - D::decode(&mut &buf[..]).map_err(|_| sp_sandbox::HostError) + D::decode(&mut &buf[..]).map_err(|_| store_err(ctx, Error::::DecodingFailed)) } /// Write the given buffer to the designated location in the sandbox memory, consuming @@ -357,9 +358,8 @@ fn write_sandbox_memory( RuntimeToken::WriteMemory(buf.len() as u32), )?; - ctx.memory.set(ptr, buf)?; - - Ok(()) + ctx.memory.set(ptr, buf) + .map_err(|_| store_err(ctx, Error::::OutOfBounds)) } /// Write the given buffer and its length to the designated locations in sandbox memory. From 694dbc10e5525464d06679ce58f83ed088483ea9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Wed, 29 Jul 2020 18:52:31 +0200 Subject: [PATCH 03/14] seal: Convert contract trapped to module error --- frame/contracts/src/lib.rs | 2 ++ frame/contracts/src/tests.rs | 6 +++--- frame/contracts/src/wasm/mod.rs | 11 +++++------ frame/contracts/src/wasm/runtime.rs | 2 +- 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 815655f3c095c..d05d6593f6f40 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -442,6 +442,8 @@ decl_error! { OutOfBounds, /// Input passed to a contract API function failed to decode as expected type. DecodingFailed, + /// Contract trapped during execution. + ContractTrapped, } } diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index ae5844970507b..e2fc4b9cc9065 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -1186,7 +1186,7 @@ fn restoration(test_different_storage: bool, test_restore_to_with_dirty_storage: assert_err_ignore_postinfo!( perform_the_restoration(), - "contract trapped during execution" + Error::::ContractTrapped, ); assert!(ContractInfoOf::::get(BOB).unwrap().get_tombstone().is_some()); @@ -1314,7 +1314,7 @@ fn storage_max_value_limit() { GAS_LIMIT, Encode::encode(&(self::MaxValueSize::get() + 1)), ), - "contract trapped during execution" + Error::::ContractTrapped, ); }); } @@ -1427,7 +1427,7 @@ fn cannot_self_destruct_while_live() { GAS_LIMIT, vec![0], ), - "contract trapped during execution" + Error::::ContractTrapped, ); // Check that BOB is still alive. diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index 8c2cd60ddfb20..e701e72a64840 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -156,9 +156,8 @@ mod tests { use crate::gas::{Gas, GasMeter}; use crate::tests::{Test, Call}; use crate::wasm::prepare::prepare_contract; - use crate::{CodeHash, BalanceOf}; + use crate::{CodeHash, BalanceOf, Error}; use hex_literal::hex; - use assert_matches::assert_matches; use sp_runtime::DispatchError; use frame_support::weights::Weight; @@ -1505,7 +1504,7 @@ mod tests { // Checks that the runtime traps if there are more than `max_topic_events` topics. let mut gas_meter = GasMeter::new(GAS_LIMIT); - assert_matches!( + assert_eq!( execute( CODE_DEPOSIT_EVENT_MAX_TOPICS, vec![], @@ -1513,7 +1512,7 @@ mod tests { &mut gas_meter ), Err(ExecError { - error: DispatchError::Other("contract trapped during execution"), + error: Error::::ContractTrapped.into(), origin: ErrorOrigin::Supervisor, }) ); @@ -1550,7 +1549,7 @@ mod tests { // Checks that the runtime traps if there are duplicates. let mut gas_meter = GasMeter::new(GAS_LIMIT); - assert_matches!( + assert_eq!( execute( CODE_DEPOSIT_EVENT_DUPLICATES, vec![], @@ -1558,7 +1557,7 @@ mod tests { &mut gas_meter ), Err(ExecError { - error: DispatchError::Other("contract trapped during execution"), + error: Error::::ContractTrapped.into(), origin: ErrorOrigin::Supervisor, }) ); diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index 8b0500cb1de22..0e03ea1d11cc7 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -190,7 +190,7 @@ pub(crate) fn to_execution_result( Err("validation error")?, // Any other kind of a trap should result in a failure. Err(sp_sandbox::Error::Execution) | Err(sp_sandbox::Error::OutOfBounds) => - Err("contract trapped during execution")?, + Err(Error::::ContractTrapped)? } } From 3039e5b4c05dd06d29977770452637619fb59be0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Thu, 30 Jul 2020 12:36:15 +0200 Subject: [PATCH 04/14] seal: Add additional tests for transfer, call, instantiate These tests verify that those functions return the error types which are declared in its docs. --- frame/contracts/fixtures/call_return_code.wat | 46 ++++ .../fixtures/instantiate_return_code.wat | 49 ++++ frame/contracts/fixtures/ok_trap_revert.wat | 35 +++ .../fixtures/transfer_return_code.wat | 32 +++ frame/contracts/src/lib.rs | 1 + frame/contracts/src/tests.rs | 224 +++++++++++++++++- frame/contracts/src/wasm/mod.rs | 1 + frame/contracts/src/wasm/runtime.rs | 2 +- 8 files changed, 387 insertions(+), 3 deletions(-) create mode 100644 frame/contracts/fixtures/call_return_code.wat create mode 100644 frame/contracts/fixtures/instantiate_return_code.wat create mode 100644 frame/contracts/fixtures/ok_trap_revert.wat create mode 100644 frame/contracts/fixtures/transfer_return_code.wat diff --git a/frame/contracts/fixtures/call_return_code.wat b/frame/contracts/fixtures/call_return_code.wat new file mode 100644 index 0000000000000..69af1ef71202e --- /dev/null +++ b/frame/contracts/fixtures/call_return_code.wat @@ -0,0 +1,46 @@ +;; This calls Django (4) and transfers 100 balance during this call and copies the return code +;; of this call to the output buffer. +;; It also forwards its input to the callee. +(module + (import "env" "ext_input" (func $ext_input (param i32 i32))) + (import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32 i32 i32) (result i32))) + (import "env" "ext_return" (func $ext_return (param i32 i32 i32))) + (import "env" "memory" (memory 1 1)) + + ;; [0, 8) address of django + (data (i32.const 0) "\04\00\00\00\00\00\00\00") + + ;; [8, 16) 100 balance + (data (i32.const 8) "\64\00\00\00\00\00\00\00") + + ;; [16, 20) here we store the return code of the transfer + + ;; [20, 24) here we store the input data + + ;; [24, 28) size of the input data + (data (i32.const 24) "\04") + + (func (export "deploy")) + + (func (export "call") + (call $ext_input (i32.const 20) (i32.const 24)) + + (i32.store + (i32.const 16) + (call $ext_call + (i32.const 0) ;; Pointer to "callee" address. + (i32.const 8) ;; Length of "callee" address. + (i64.const 0) ;; How much gas to devote for the execution. 0 = all. + (i32.const 8) ;; Pointer to the buffer with value to transfer + (i32.const 8) ;; Length of the buffer with value to transfer. + (i32.const 20) ;; Pointer to input data buffer address + (i32.load (i32.const 24)) ;; Length of input data buffer + (i32.const 4294967295) ;; u32 max sentinel value: do not copy output + (i32.const 0) ;; Ptr to output buffer len + ) + ) + + ;; exit with success and take transfer return code to the output buffer + (call $ext_return (i32.const 0) (i32.const 16) (i32.const 4)) + ) +) \ No newline at end of file diff --git a/frame/contracts/fixtures/instantiate_return_code.wat b/frame/contracts/fixtures/instantiate_return_code.wat new file mode 100644 index 0000000000000..771691962f157 --- /dev/null +++ b/frame/contracts/fixtures/instantiate_return_code.wat @@ -0,0 +1,49 @@ +;; This instantiats Charlie (3) and transfers 100 balance during this call and copies the return code +;; of this call to the output buffer. +;; The first 32 byte of input is the code hash to instantiate +;; The rest of the input is forwarded to the constructor of the callee +(module + (import "env" "ext_input" (func $ext_input (param i32 i32))) + (import "env" "ext_instantiate" (func $ext_instantiate (param i32 i32 i64 i32 i32 i32 i32 i32 i32 i32 i32) (result i32))) + (import "env" "ext_return" (func $ext_return (param i32 i32 i32))) + (import "env" "memory" (memory 1 1)) + + ;; [0, 8) address of django + (data (i32.const 0) "\04\00\00\00\00\00\00\00") + + ;; [8, 16) 100 balance + (data (i32.const 8) "\64\00\00\00\00\00\00\00") + + ;; [16, 20) here we store the return code of the transfer + + ;; [20, 24) size of the input buffer + (data (i32.const 20) "\FF") + + ;; [24, inf) input buffer + + (func (export "deploy")) + + (func (export "call") + (call $ext_input (i32.const 24) (i32.const 20)) + + (i32.store + (i32.const 16) + (call $ext_instantiate + (i32.const 24) ;; Pointer to the code hash. + (i32.const 32) ;; Length of the code hash. + (i64.const 0) ;; How much gas to devote for the execution. 0 = all. + (i32.const 8) ;; Pointer to the buffer with value to transfer + (i32.const 8) ;; Length of the buffer with value to transfer. + (i32.const 56) ;; Pointer to input data buffer address + (i32.sub (i32.load (i32.const 20)) (i32.const 32)) ;; Length of input data buffer + (i32.const 4294967295) ;; u32 max sentinel value: do not copy address + (i32.const 0) ;; Length is ignored in this case + (i32.const 4294967295) ;; u32 max sentinel value: do not copy output + (i32.const 0) ;; Length is ignored in this case + ) + ) + + ;; exit with success and take transfer return code to the output buffer + (call $ext_return (i32.const 0) (i32.const 16) (i32.const 4)) + ) +) \ No newline at end of file diff --git a/frame/contracts/fixtures/ok_trap_revert.wat b/frame/contracts/fixtures/ok_trap_revert.wat new file mode 100644 index 0000000000000..5877e55d0e70e --- /dev/null +++ b/frame/contracts/fixtures/ok_trap_revert.wat @@ -0,0 +1,35 @@ +(module + (import "env" "ext_input" (func $ext_input (param i32 i32))) + (import "env" "ext_return" (func $ext_return (param i32 i32 i32))) + (import "env" "memory" (memory 1 1)) + + (func (export "deploy") + (call $ok_trap_revert) + ) + + (func (export "call") + (call $ok_trap_revert) + ) + + (func $ok_trap_revert + (i32.store (i32.const 4) (i32.const 4)) + (call $ext_input (i32.const 0) (i32.const 4)) + (block $IF_2 + (block $IF_1 + (block $IF_0 + (br_table $IF_0 $IF_1 $IF_2 + (i32.load8_u (i32.const 0)) + ) + (unreachable) + ) + ;; 0 = return with success + return + ) + ;; 1 = revert + (call $ext_return (i32.const 1) (i32.const 0) (i32.const 0)) + (unreachable) + ) + ;; 2 = trap + (unreachable) + ) +) \ No newline at end of file diff --git a/frame/contracts/fixtures/transfer_return_code.wat b/frame/contracts/fixtures/transfer_return_code.wat new file mode 100644 index 0000000000000..919b0eff40178 --- /dev/null +++ b/frame/contracts/fixtures/transfer_return_code.wat @@ -0,0 +1,32 @@ +;; This transfers 100 balance to the zero account and copies the return code +;; of this transfer to the output buffer. +(module + (import "env" "ext_transfer" (func $ext_transfer (param i32 i32 i32 i32) (result i32))) + (import "env" "ext_return" (func $ext_return (param i32 i32 i32))) + (import "env" "memory" (memory 1 1)) + + ;; [0, 8) zero-adress + (data (i32.const 0) "\00\00\00\00\00\00\00\00") + + ;; [8, 16) 100 balance + (data (i32.const 8) "\64\00\00\00\00\00\00\00") + + ;; [16, 20) here we store the return code of the transfer + + (func (export "deploy")) + + (func (export "call") + (i32.store + (i32.const 16) + (call $ext_transfer + (i32.const 0) ;; ptr to destination address + (i32.const 8) ;; length of destination address + (i32.const 8) ;; ptr to value to transfer + (i32.const 8) ;; length of value to transfer + ) + ) + + ;; exit with success and take transfer return code to the output buffer + (call $ext_return (i32.const 0) (i32.const 16) (i32.const 4)) + ) +) \ No newline at end of file diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index d05d6593f6f40..8949e6e604875 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -95,6 +95,7 @@ use crate::wasm::{WasmLoader, WasmVm}; pub use crate::gas::{Gas, GasMeter}; pub use crate::exec::{ExecResult, ExecReturnValue}; +pub use crate::wasm::ReturnCode as RuntimeReturnCode; #[cfg(feature = "std")] use serde::{Serialize, Deserialize}; diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index e2fc4b9cc9065..12b833af3a654 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -17,7 +17,7 @@ use crate::{ BalanceOf, ContractAddressFor, ContractInfo, ContractInfoOf, GenesisConfig, Module, RawAliveContractInfo, RawEvent, Trait, TrieId, Schedule, TrieIdGenerator, gas::Gas, - Error, + Error, Config, RuntimeReturnCode, }; use assert_matches::assert_matches; use hex_literal::*; @@ -30,7 +30,7 @@ use sp_runtime::{ use frame_support::{ assert_ok, assert_err_ignore_postinfo, impl_outer_dispatch, impl_outer_event, impl_outer_origin, parameter_types, StorageMap, StorageValue, - traits::{Currency, Get}, + traits::{Currency, Get, ReservableCurrency}, weights::{Weight, PostDispatchInfo}, dispatch::DispatchErrorWithPostInfo, }; @@ -64,6 +64,7 @@ impl_outer_dispatch! { } } +#[macro_use] pub mod test_utils { use super::{Test, Balances}; use crate::{ContractInfoOf, TrieIdGenerator, CodeHash}; @@ -90,6 +91,12 @@ pub mod test_utils { pub fn get_balance(who: &u64) -> u64 { Balances::free_balance(who) } + macro_rules! assert_return_code { + ( $x:expr , $y:expr $(,)? ) => {{ + use sp_std::convert::TryInto; + assert_eq!(u32::from_le_bytes($x.data[..].try_into().unwrap()), $y as u32); + }} + } } thread_local! { @@ -1606,3 +1613,216 @@ fn crypto_hashes() { } }) } + +#[test] +fn transfer_return_code() { + let (wasm, code_hash) = compile_module::("transfer_return_code").unwrap(); + ExtBuilder::default().existential_deposit(50).build().execute_with(|| { + let subsistence = Config::::subsistence_threshold_uncached(); + let _ = Balances::deposit_creating(&ALICE, 10 * subsistence); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), wasm)); + + assert_ok!( + Contracts::instantiate( + Origin::signed(ALICE), + subsistence, + GAS_LIMIT, + code_hash.into(), + vec![], + ), + ); + + // Contract has only the minimal balance so any transfer will return BelowSubsistence. + let result = Contracts::bare_call( + ALICE, + BOB, + 0, + GAS_LIMIT, + vec![], + ).0.unwrap(); + assert_return_code!(result, RuntimeReturnCode::BelowSubsistenceThreshold); + + // Contract has enough total balance in order to not go below the subsistence + // threshold when transfering 100 balance but this balance is reserved so + // the transfer still fails but with another return code. + Balances::make_free_balance_be(&BOB, subsistence + 100); + Balances::reserve(&BOB, subsistence + 100).unwrap(); + let result = Contracts::bare_call( + ALICE, + BOB, + 0, + GAS_LIMIT, + vec![], + ).0.unwrap(); + assert_return_code!(result, RuntimeReturnCode::TransferFailed); + }); +} + +#[test] +fn call_return_code() { + let (caller_code, caller_hash) = compile_module::("call_return_code").unwrap(); + let (callee_code, callee_hash) = compile_module::("ok_trap_revert").unwrap(); + ExtBuilder::default().existential_deposit(50).build().execute_with(|| { + let subsistence = Config::::subsistence_threshold_uncached(); + let _ = Balances::deposit_creating(&ALICE, 10 * subsistence); + let _ = Balances::deposit_creating(&CHARLIE, 10 * subsistence); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), caller_code)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), callee_code)); + + assert_ok!( + Contracts::instantiate( + Origin::signed(ALICE), + subsistence, + GAS_LIMIT, + caller_hash.into(), + vec![0], + ), + ); + + // Contract calls into Django which is no valid contract + let result = Contracts::bare_call( + ALICE, + BOB, + 0, + GAS_LIMIT, + vec![0], + ).0.unwrap(); + assert_return_code!(result, RuntimeReturnCode::InvalidContractCalled); + + assert_ok!( + Contracts::instantiate( + Origin::signed(CHARLIE), + subsistence, + GAS_LIMIT, + callee_hash.into(), + vec![0], + ), + ); + + // Contract has only the minimal balance so any transfer will return BelowSubsistence. + let result = Contracts::bare_call( + ALICE, + BOB, + 0, + GAS_LIMIT, + vec![0], + ).0.unwrap(); + assert_return_code!(result, RuntimeReturnCode::BelowSubsistenceThreshold); + + // Contract has enough total balance in order to not go below the subsistence + // threshold when transfering 100 balance but this balance is reserved so + // the transfer still fails but with another return code. + Balances::make_free_balance_be(&BOB, subsistence + 100); + Balances::reserve(&BOB, subsistence + 100).unwrap(); + let result = Contracts::bare_call( + ALICE, + BOB, + 0, + GAS_LIMIT, + vec![0], + ).0.unwrap(); + assert_return_code!(result, RuntimeReturnCode::TransferFailed); + + // Contract has enough balance but callee reverts because "1" is passed. + Balances::make_free_balance_be(&BOB, subsistence + 1000); + let result = Contracts::bare_call( + ALICE, + BOB, + 0, + GAS_LIMIT, + vec![1], + ).0.unwrap(); + assert_return_code!(result, RuntimeReturnCode::CalleeReverted); + + // Contract has enough balance but callee traps because "2" is passed. + let result = Contracts::bare_call( + ALICE, + BOB, + 0, + GAS_LIMIT, + vec![2], + ).0.unwrap(); + assert_return_code!(result, RuntimeReturnCode::CalleeTrapped); + + }); +} + +#[test] +fn instantiate_return_code() { + let (caller_code, caller_hash) = compile_module::("instantiate_return_code").unwrap(); + let (callee_code, callee_hash) = compile_module::("ok_trap_revert").unwrap(); + ExtBuilder::default().existential_deposit(50).build().execute_with(|| { + let subsistence = Config::::subsistence_threshold_uncached(); + let _ = Balances::deposit_creating(&ALICE, 10 * subsistence); + let _ = Balances::deposit_creating(&CHARLIE, 10 * subsistence); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), caller_code)); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), callee_code)); + let callee_hash = callee_hash.as_ref().to_vec(); + + assert_ok!( + Contracts::instantiate( + Origin::signed(ALICE), + subsistence, + GAS_LIMIT, + caller_hash.into(), + vec![], + ), + ); + + // Contract has only the minimal balance so any transfer will return BelowSubsistence. + let result = Contracts::bare_call( + ALICE, + BOB, + 0, + GAS_LIMIT, + vec![0; 33], + ).0.unwrap(); + assert_return_code!(result, RuntimeReturnCode::BelowSubsistenceThreshold); + + // Contract has enough total balance in order to not go below the subsistence + // threshold when transfering 100 balance but this balance is reserved so + // the transfer still fails but with another return code. + Balances::make_free_balance_be(&BOB, subsistence + 100); + Balances::reserve(&BOB, subsistence + 100).unwrap(); + let result = Contracts::bare_call( + ALICE, + BOB, + 0, + GAS_LIMIT, + vec![0; 33], + ).0.unwrap(); + assert_return_code!(result, RuntimeReturnCode::TransferFailed); + + // Contract has enough balance but the passed code hash is invalid + Balances::make_free_balance_be(&BOB, subsistence + 1000); + let result = Contracts::bare_call( + ALICE, + BOB, + 0, + GAS_LIMIT, + vec![0; 33], + ).0.unwrap(); + assert_return_code!(result, RuntimeReturnCode::CodeNotFound); + + // Contract has enough balance but callee reverts because "1" is passed. + let result = Contracts::bare_call( + ALICE, + BOB, + 0, + GAS_LIMIT, + callee_hash.iter().cloned().chain(sp_std::iter::once(1)).collect(), + ).0.unwrap(); + assert_return_code!(result, RuntimeReturnCode::CalleeReverted); + + // Contract has enough balance but callee traps because "2" is passed. + let result = Contracts::bare_call( + ALICE, + BOB, + 0, + GAS_LIMIT, + callee_hash.iter().cloned().chain(sp_std::iter::once(2)).collect(), + ).0.unwrap(); + assert_return_code!(result, RuntimeReturnCode::CalleeTrapped); + + }); +} diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index e701e72a64840..96dd7daae6b06 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -36,6 +36,7 @@ use self::runtime::{to_execution_result, Runtime}; use self::code_cache::load as load_code; pub use self::code_cache::save as save_code; +pub use self::runtime::ReturnCode; /// A prepared wasm module ready for execution. #[derive(Clone, Encode, Decode)] diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index 0e03ea1d11cc7..0525f6cba6e2f 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -639,7 +639,7 @@ define_env!(Env, , // `ReturnCode::CalleeTrapped` // `ReturnCode::BelowSubsistenceThreshold` // `ReturnCode::TransferFailed` - // `ReturnCode::InvalidConractCalled` + // `ReturnCode::InvalidContractCalled` ext_call( ctx, callee_ptr: u32, From 4c88bb8f9cec76df2873ae851bf8b59d9c42e86f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Fri, 31 Jul 2020 15:15:15 +0200 Subject: [PATCH 05/14] Make it more pronounced that to_execution_result handles trap_reason --- frame/contracts/src/wasm/runtime.rs | 55 ++++++++++++++++------------- 1 file changed, 31 insertions(+), 24 deletions(-) diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index 0525f6cba6e2f..e232b7288c1cb 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -143,35 +143,42 @@ impl<'a, E: Ext + 'a> Runtime<'a, E> { } } +/// Converts the sandbox result and the runtime state into the execution outcome. +/// +/// It evaluates information stored in the `trap_reason` variable of the runtime and +/// bases the outcome on the value if this variable. Only if `trap_reason` is `None` +/// the result of the sandbox is evaluated. pub(crate) fn to_execution_result( runtime: Runtime, sandbox_result: Result, ) -> ExecResult { - match runtime.trap_reason { - // The trap was the result of the execution `return` host function. - Some(TrapReason::Return(ReturnData{ flags, data })) => { - let flags = ReturnFlags::from_bits(flags).ok_or_else(|| - "used reserved bit in return flags" - )?; - return Ok(ExecReturnValue { - flags, - data, - }) - }, - Some(TrapReason::Termination) => { - return Ok(ExecReturnValue { - flags: ReturnFlags::empty(), - data: Vec::new(), - }) - }, - Some(TrapReason::Restoration) => { - return Ok(ExecReturnValue { - flags: ReturnFlags::empty(), - data: Vec::new(), - }) + // If a trap reason is set we base our decision solely on that. + if let Some(trap_reason) = runtime.trap_reason { + return match trap_reason { + // The trap was the result of the execution `return` host function. + TrapReason::Return(ReturnData{ flags, data }) => { + let flags = ReturnFlags::from_bits(flags).ok_or_else(|| + "used reserved bit in return flags" + )?; + Ok(ExecReturnValue { + flags, + data, + }) + }, + TrapReason::Termination => { + Ok(ExecReturnValue { + flags: ReturnFlags::empty(), + data: Vec::new(), + }) + }, + TrapReason::Restoration => { + Ok(ExecReturnValue { + flags: ReturnFlags::empty(), + data: Vec::new(), + }) + }, + TrapReason::SupervisorError(error) => Err(error)?, } - Some(TrapReason::SupervisorError(error)) => Err(error)?, - None => (), } // Check the exact type of the error. From af38fe3aeee8d4d1822533a077aec3969c0181f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Fri, 31 Jul 2020 15:26:48 +0200 Subject: [PATCH 06/14] Improve ReturnCode docs --- frame/contracts/src/wasm/runtime.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index e232b7288c1cb..9fb5f50648a9c 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -36,7 +36,7 @@ use sp_io::hashing::{ sha2_256, }; -/// Every error that can be returned from a runtime API call. +/// Every error that can be returned to a contract when it calls any of the host functions. #[repr(u32)] pub enum ReturnCode { /// API call successful. From be22f99cc2520d85cbca49767b0a3914d4d72ba0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Fri, 31 Jul 2020 15:50:04 +0200 Subject: [PATCH 07/14] Fix whitespace issues in wat files --- frame/contracts/fixtures/call_return_code.wat | 44 +++++++++---------- .../fixtures/instantiate_return_code.wat | 44 +++++++++---------- .../fixtures/transfer_return_code.wat | 33 +++++++------- 3 files changed, 58 insertions(+), 63 deletions(-) diff --git a/frame/contracts/fixtures/call_return_code.wat b/frame/contracts/fixtures/call_return_code.wat index 69af1ef71202e..51ae1af3ca227 100644 --- a/frame/contracts/fixtures/call_return_code.wat +++ b/frame/contracts/fixtures/call_return_code.wat @@ -7,40 +7,38 @@ (import "env" "ext_return" (func $ext_return (param i32 i32 i32))) (import "env" "memory" (memory 1 1)) - ;; [0, 8) address of django - (data (i32.const 0) "\04\00\00\00\00\00\00\00") + ;; [0, 8) address of django + (data (i32.const 0) "\04\00\00\00\00\00\00\00") ;; [8, 16) 100 balance (data (i32.const 8) "\64\00\00\00\00\00\00\00") ;; [16, 20) here we store the return code of the transfer - ;; [20, 24) here we store the input data + ;; [20, 24) here we store the input data - ;; [24, 28) size of the input data - (data (i32.const 24) "\04") + ;; [24, 28) size of the input data + (data (i32.const 24) "\04") - (func (export "deploy")) + (func (export "deploy")) (func (export "call") - (call $ext_input (i32.const 20) (i32.const 24)) - - (i32.store - (i32.const 16) - (call $ext_call - (i32.const 0) ;; Pointer to "callee" address. - (i32.const 8) ;; Length of "callee" address. - (i64.const 0) ;; How much gas to devote for the execution. 0 = all. - (i32.const 8) ;; Pointer to the buffer with value to transfer - (i32.const 8) ;; Length of the buffer with value to transfer. - (i32.const 20) ;; Pointer to input data buffer address + (call $ext_input (i32.const 20) (i32.const 24)) + (i32.store + (i32.const 16) + (call $ext_call + (i32.const 0) ;; Pointer to "callee" address. + (i32.const 8) ;; Length of "callee" address. + (i64.const 0) ;; How much gas to devote for the execution. 0 = all. + (i32.const 8) ;; Pointer to the buffer with value to transfer + (i32.const 8) ;; Length of the buffer with value to transfer. + (i32.const 20) ;; Pointer to input data buffer address (i32.load (i32.const 24)) ;; Length of input data buffer (i32.const 4294967295) ;; u32 max sentinel value: do not copy output (i32.const 0) ;; Ptr to output buffer len ) - ) - - ;; exit with success and take transfer return code to the output buffer - (call $ext_return (i32.const 0) (i32.const 16) (i32.const 4)) - ) -) \ No newline at end of file + ) + ;; exit with success and take transfer return code to the output buffer + (call $ext_return (i32.const 0) (i32.const 16) (i32.const 4)) + ) +) diff --git a/frame/contracts/fixtures/instantiate_return_code.wat b/frame/contracts/fixtures/instantiate_return_code.wat index 771691962f157..120bfc414e00b 100644 --- a/frame/contracts/fixtures/instantiate_return_code.wat +++ b/frame/contracts/fixtures/instantiate_return_code.wat @@ -8,42 +8,40 @@ (import "env" "ext_return" (func $ext_return (param i32 i32 i32))) (import "env" "memory" (memory 1 1)) - ;; [0, 8) address of django - (data (i32.const 0) "\04\00\00\00\00\00\00\00") + ;; [0, 8) address of django + (data (i32.const 0) "\04\00\00\00\00\00\00\00") ;; [8, 16) 100 balance (data (i32.const 8) "\64\00\00\00\00\00\00\00") ;; [16, 20) here we store the return code of the transfer - ;; [20, 24) size of the input buffer - (data (i32.const 20) "\FF") + ;; [20, 24) size of the input buffer + (data (i32.const 20) "\FF") - ;; [24, inf) input buffer + ;; [24, inf) input buffer - (func (export "deploy")) + (func (export "deploy")) (func (export "call") - (call $ext_input (i32.const 24) (i32.const 20)) - - (i32.store - (i32.const 16) + (call $ext_input (i32.const 24) (i32.const 20)) + (i32.store + (i32.const 16) (call $ext_instantiate - (i32.const 24) ;; Pointer to the code hash. - (i32.const 32) ;; Length of the code hash. - (i64.const 0) ;; How much gas to devote for the execution. 0 = all. - (i32.const 8) ;; Pointer to the buffer with value to transfer - (i32.const 8) ;; Length of the buffer with value to transfer. - (i32.const 56) ;; Pointer to input data buffer address - (i32.sub (i32.load (i32.const 20)) (i32.const 32)) ;; Length of input data buffer + (i32.const 24) ;; Pointer to the code hash. + (i32.const 32) ;; Length of the code hash. + (i64.const 0) ;; How much gas to devote for the execution. 0 = all. + (i32.const 8) ;; Pointer to the buffer with value to transfer + (i32.const 8) ;; Length of the buffer with value to transfer. + (i32.const 56) ;; Pointer to input data buffer address + (i32.sub (i32.load (i32.const 20)) (i32.const 32)) ;; Length of input data buffer (i32.const 4294967295) ;; u32 max sentinel value: do not copy address (i32.const 0) ;; Length is ignored in this case (i32.const 4294967295) ;; u32 max sentinel value: do not copy output (i32.const 0) ;; Length is ignored in this case ) - ) - - ;; exit with success and take transfer return code to the output buffer - (call $ext_return (i32.const 0) (i32.const 16) (i32.const 4)) - ) -) \ No newline at end of file + ) + ;; exit with success and take transfer return code to the output buffer + (call $ext_return (i32.const 0) (i32.const 16) (i32.const 4)) + ) +) diff --git a/frame/contracts/fixtures/transfer_return_code.wat b/frame/contracts/fixtures/transfer_return_code.wat index 919b0eff40178..87d186811e757 100644 --- a/frame/contracts/fixtures/transfer_return_code.wat +++ b/frame/contracts/fixtures/transfer_return_code.wat @@ -5,28 +5,27 @@ (import "env" "ext_return" (func $ext_return (param i32 i32 i32))) (import "env" "memory" (memory 1 1)) - ;; [0, 8) zero-adress - (data (i32.const 0) "\00\00\00\00\00\00\00\00") + ;; [0, 8) zero-adress + (data (i32.const 0) "\00\00\00\00\00\00\00\00") ;; [8, 16) 100 balance (data (i32.const 8) "\64\00\00\00\00\00\00\00") ;; [16, 20) here we store the return code of the transfer - (func (export "deploy")) + (func (export "deploy")) (func (export "call") - (i32.store - (i32.const 16) - (call $ext_transfer - (i32.const 0) ;; ptr to destination address - (i32.const 8) ;; length of destination address - (i32.const 8) ;; ptr to value to transfer - (i32.const 8) ;; length of value to transfer - ) - ) - - ;; exit with success and take transfer return code to the output buffer - (call $ext_return (i32.const 0) (i32.const 16) (i32.const 4)) - ) -) \ No newline at end of file + (i32.store + (i32.const 16) + (call $ext_transfer + (i32.const 0) ;; ptr to destination address + (i32.const 8) ;; length of destination address + (i32.const 8) ;; ptr to value to transfer + (i32.const 8) ;; length of value to transfer + ) + ) + ;; exit with success and take transfer return code to the output buffer + (call $ext_return (i32.const 0) (i32.const 16) (i32.const 4)) + ) +) From c095acce35fbdf46302c94d8d327e933fa11e11a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Fri, 31 Jul 2020 16:01:53 +0200 Subject: [PATCH 08/14] Improve ReturnCode doc --- frame/contracts/src/wasm/runtime.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index 9fb5f50648a9c..c837257d716ef 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -49,14 +49,14 @@ pub enum ReturnCode { CalleeReverted = 2, /// The passed key does not exist in storage. KeyNotFound = 3, - /// Transfer failed because it would have brought the senders total balance below the - /// subsistence deposit. + /// Transfer failed because it would have brought the sender's total balance below the + /// subsistence threshold. BelowSubsistenceThreshold = 4, /// Transfer failed for other reasons. Most probably reserved or locked balance of the /// sender prevents the transfer. TransferFailed = 5, /// The newly created contract is below the subsistence threshold after executing - // its contructor. + /// its constructor. NewContractNotFunded = 6, /// No code could be found at the supplied code hash. CodeNotFound = 7, From afc7e0713fa325bb59ea958abf6531947ba87bc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Fri, 31 Jul 2020 16:02:21 +0200 Subject: [PATCH 09/14] Improve ErrorOrigin doc and variant naming --- frame/contracts/src/exec.rs | 9 +++++---- frame/contracts/src/wasm/mod.rs | 8 ++++---- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/frame/contracts/src/exec.rs b/frame/contracts/src/exec.rs index 7f32aaf35c42c..c10a1aa9b5c17 100644 --- a/frame/contracts/src/exec.rs +++ b/frame/contracts/src/exec.rs @@ -73,11 +73,12 @@ impl ExecReturnValue { /// 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 during the supervisor code execution. +/// 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 during the setup phase of the contract that is to be called. - Supervisor, + /// 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, } @@ -92,7 +93,7 @@ impl> From for ExecError { fn from(error: T) -> Self { Self { error: error.into(), - origin: ErrorOrigin::Supervisor, + origin: ErrorOrigin::Caller, } } } diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index 96dd7daae6b06..7f985e90b66b6 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -1514,7 +1514,7 @@ mod tests { ), Err(ExecError { error: Error::::ContractTrapped.into(), - origin: ErrorOrigin::Supervisor, + origin: ErrorOrigin::Caller, }) ); } @@ -1559,7 +1559,7 @@ mod tests { ), Err(ExecError { error: Error::::ContractTrapped.into(), - origin: ErrorOrigin::Supervisor, + origin: ErrorOrigin::Caller, }) ); } @@ -1705,7 +1705,7 @@ mod tests { result, Err(ExecError { error: Error::::OutOfBounds.into(), - origin: ErrorOrigin::Supervisor, + origin: ErrorOrigin::Caller, }) ); } @@ -1740,7 +1740,7 @@ mod tests { result, Err(ExecError { error: Error::::DecodingFailed.into(), - origin: ErrorOrigin::Supervisor, + origin: ErrorOrigin::Caller, }) ); } From 97ffa452fe7c7a3e3cde5aff38236a3da58758bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Fri, 31 Jul 2020 16:11:44 +0200 Subject: [PATCH 10/14] Improve docs on ExecResult and ExecError --- frame/contracts/src/exec.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/frame/contracts/src/exec.rs b/frame/contracts/src/exec.rs index c10a1aa9b5c17..8517d2810ec1b 100644 --- a/frame/contracts/src/exec.rs +++ b/frame/contracts/src/exec.rs @@ -83,9 +83,12 @@ pub enum ErrorOrigin { 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, } @@ -98,7 +101,8 @@ impl> From for ExecError { } } -/// The error returned from functions calling into contract execution. +/// 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 From f5e712716af6ca7686692c0ae3ddd0bc0c7f9da3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Fri, 31 Jul 2020 16:18:36 +0200 Subject: [PATCH 11/14] Encode u32 sentinel value as hex --- frame/contracts/fixtures/call_return_code.wat | 2 +- frame/contracts/fixtures/instantiate_return_code.wat | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/contracts/fixtures/call_return_code.wat b/frame/contracts/fixtures/call_return_code.wat index 51ae1af3ca227..d724f90446245 100644 --- a/frame/contracts/fixtures/call_return_code.wat +++ b/frame/contracts/fixtures/call_return_code.wat @@ -34,7 +34,7 @@ (i32.const 8) ;; Length of the buffer with value to transfer. (i32.const 20) ;; Pointer to input data buffer address (i32.load (i32.const 24)) ;; Length of input data buffer - (i32.const 4294967295) ;; u32 max sentinel value: do not copy output + (i32.const 0xffffffff) ;; u32 max sentinel value: do not copy output (i32.const 0) ;; Ptr to output buffer len ) ) diff --git a/frame/contracts/fixtures/instantiate_return_code.wat b/frame/contracts/fixtures/instantiate_return_code.wat index 120bfc414e00b..bce80ca01faac 100644 --- a/frame/contracts/fixtures/instantiate_return_code.wat +++ b/frame/contracts/fixtures/instantiate_return_code.wat @@ -35,9 +35,9 @@ (i32.const 8) ;; Length of the buffer with value to transfer. (i32.const 56) ;; Pointer to input data buffer address (i32.sub (i32.load (i32.const 20)) (i32.const 32)) ;; Length of input data buffer - (i32.const 4294967295) ;; u32 max sentinel value: do not copy address + (i32.const 0xffffffff) ;; u32 max sentinel value: do not copy address (i32.const 0) ;; Length is ignored in this case - (i32.const 4294967295) ;; u32 max sentinel value: do not copy output + (i32.const 0xffffffff) ;; u32 max sentinel value: do not copy output (i32.const 0) ;; Length is ignored in this case ) ) From 36324ce10dbbfad53fd3292e70fa1dc59304b36e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Fri, 31 Jul 2020 16:31:17 +0200 Subject: [PATCH 12/14] with_nested_context no longer accepts an Option for trie --- frame/contracts/src/exec.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/frame/contracts/src/exec.rs b/frame/contracts/src/exec.rs index 8517d2810ec1b..b873bcf7715e4 100644 --- a/frame/contracts/src/exec.rs +++ b/frame/contracts/src/exec.rs @@ -316,12 +316,12 @@ where } } - fn nested<'b, 'c: 'b>(&'c self, dest: T::AccountId, trie_id: Option) + fn nested<'b, 'c: 'b>(&'c self, dest: T::AccountId, trie_id: TrieId) -> ExecutionContext<'b, T, V, L> { ExecutionContext { caller: Some(self), - self_trie_id: trie_id, + self_trie_id: Some(trie_id), self_account: dest, depth: self.depth + 1, config: self.config, @@ -365,7 +365,7 @@ where let transactor_kind = self.transactor_kind(); let caller = self.self_account.clone(); - self.with_nested_context(dest.clone(), Some(contract.trie_id.clone()), |nested| { + self.with_nested_context(dest.clone(), contract.trie_id.clone(), |nested| { if value > BalanceOf::::zero() { transfer( gas_meter, @@ -421,7 +421,7 @@ where // Generate it now. let dest_trie_id = ::TrieIdGenerator::trie_id(&dest); - let output = self.with_nested_context(dest.clone(), Some(dest_trie_id), |nested| { + let output = self.with_nested_context(dest.clone(), dest_trie_id, |nested| { storage::place_contract::( &dest, nested @@ -486,7 +486,7 @@ where } /// Execute the given closure within a nested execution context. - fn with_nested_context(&mut self, dest: T::AccountId, trie_id: Option, func: F) + fn with_nested_context(&mut self, dest: T::AccountId, trie_id: TrieId, func: F) -> ExecResult where F: FnOnce(&mut ExecutionContext) -> ExecResult { From 9d9ea7bfccdc1008e399827038671014df706c1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Fri, 31 Jul 2020 16:33:54 +0200 Subject: [PATCH 13/14] Fix successful typo --- frame/contracts/src/wasm/runtime.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index c837257d716ef..b4f4ec66a2da1 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -108,7 +108,7 @@ enum TrapReason { SupervisorError(DispatchError), /// Signals that trap was generated in response to call `ext_return` host function. Return(ReturnData), - /// Signals that a trap was generated in response to a succesful call to the + /// Signals that a trap was generated in response to a successful call to the /// `ext_terminate` host function. Termination, /// Signals that a trap was generated because of a successful restoration. @@ -639,7 +639,7 @@ define_env!(Env, , // // # Errors // - // An error means that the call wasn't succesful output buffer is returned unless + // An error means that the call wasn't successful output buffer is returned unless // stated otherwise. // // `ReturnCode::CalleeReverted`: Output buffer is returned. From 12374ebb5735843208362c730d58630e5b745ed6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Mon, 3 Aug 2020 10:12:56 +0200 Subject: [PATCH 14/14] Rename InvalidContractCalled to NotCallable --- frame/contracts/src/exec.rs | 2 +- frame/contracts/src/lib.rs | 2 +- frame/contracts/src/tests.rs | 10 +++++----- frame/contracts/src/wasm/runtime.rs | 8 ++++---- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/frame/contracts/src/exec.rs b/frame/contracts/src/exec.rs index b873bcf7715e4..a2fb50dd3f319 100644 --- a/frame/contracts/src/exec.rs +++ b/frame/contracts/src/exec.rs @@ -359,7 +359,7 @@ where let contract = if let Some(ContractInfo::Alive(info)) = rent::collect_rent::(&dest) { info } else { - Err(Error::::InvalidContractCalled)? + Err(Error::::NotCallable)? }; let transactor_kind = self.transactor_kind(); diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 8949e6e604875..24e5ece5bb8d8 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -434,7 +434,7 @@ decl_error! { MaxCallDepthReached, /// The contract that was called is either no contract at all (a plain account) /// or is a tombstone. - InvalidContractCalled, + NotCallable, /// The code supplied to `put_code` exceeds the limit specified in the current schedule. CodeTooLarge, /// No code could be found at the supplied code hash. diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index 12b833af3a654..37ded30a6934b 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -299,7 +299,7 @@ fn calling_plain_account_fails() { Contracts::call(Origin::signed(ALICE), BOB, 0, GAS_LIMIT, Vec::new()), Err( DispatchErrorWithPostInfo { - error: Error::::InvalidContractCalled.into(), + error: Error::::NotCallable.into(), post_info: PostDispatchInfo { actual_weight: Some(67500000), pays_fee: Default::default(), @@ -999,7 +999,7 @@ fn call_removed_contract() { // Calling contract should remove contract and fail. assert_err_ignore_postinfo!( Contracts::call(Origin::signed(ALICE), BOB, 0, GAS_LIMIT, call::null()), - Error::::InvalidContractCalled + Error::::NotCallable ); // Calling a contract that is about to evict shall emit an event. assert_eq!(System::events(), vec![ @@ -1013,7 +1013,7 @@ fn call_removed_contract() { // Subsequent contract calls should also fail. assert_err_ignore_postinfo!( Contracts::call(Origin::signed(ALICE), BOB, 0, GAS_LIMIT, call::null()), - Error::::InvalidContractCalled + Error::::NotCallable ); }) } @@ -1140,7 +1140,7 @@ fn restoration(test_different_storage: bool, test_restore_to_with_dirty_storage: // we expect that it will get removed leaving tombstone. assert_err_ignore_postinfo!( Contracts::call(Origin::signed(ALICE), BOB, 0, GAS_LIMIT, call::null()), - Error::::InvalidContractCalled + Error::::NotCallable ); assert!(ContractInfoOf::::get(BOB).unwrap().get_tombstone().is_some()); assert_eq!(System::events(), vec![ @@ -1687,7 +1687,7 @@ fn call_return_code() { GAS_LIMIT, vec![0], ).0.unwrap(); - assert_return_code!(result, RuntimeReturnCode::InvalidContractCalled); + assert_return_code!(result, RuntimeReturnCode::NotCallable); assert_ok!( Contracts::instantiate( diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index b4f4ec66a2da1..ed97a4dae3c9f 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -62,7 +62,7 @@ pub enum ReturnCode { CodeNotFound = 7, /// The contract that was called is either no contract at all (a plain account) /// or is a tombstone. - InvalidContractCalled = 8, + NotCallable = 8, } impl ConvertibleToWasm for ReturnCode { @@ -433,14 +433,14 @@ fn err_into_return_code(from: DispatchError) -> Result::TransferFailed.into(); let not_funded = Error::::NewContractNotFunded.into(); let no_code = Error::::CodeNotFound.into(); - let invalid_contract = Error::::InvalidContractCalled.into(); + let invalid_contract = Error::::NotCallable.into(); match from { x if x == below_sub => Ok(BelowSubsistenceThreshold), x if x == transfer_failed => Ok(TransferFailed), x if x == not_funded => Ok(NewContractNotFunded), x if x == no_code => Ok(CodeNotFound), - x if x == invalid_contract => Ok(InvalidContractCalled), + x if x == invalid_contract => Ok(NotCallable), err => Err(err) } } @@ -646,7 +646,7 @@ define_env!(Env, , // `ReturnCode::CalleeTrapped` // `ReturnCode::BelowSubsistenceThreshold` // `ReturnCode::TransferFailed` - // `ReturnCode::InvalidContractCalled` + // `ReturnCode::NotCallable` ext_call( ctx, callee_ptr: u32,