From 7c84bc1f5fce6350997e0dcba795f1782769d263 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Miko=C5=82ajczyk?= Date: Thu, 31 Aug 2023 10:59:01 +0200 Subject: [PATCH 01/13] Setup trait and invoking --- substrate/frame/contracts/src/debug.rs | 21 ++++++++++++++++++--- substrate/frame/contracts/src/exec.rs | 14 +++++++++----- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/substrate/frame/contracts/src/debug.rs b/substrate/frame/contracts/src/debug.rs index d92379a806dd..aed40ecc93b7 100644 --- a/substrate/frame/contracts/src/debug.rs +++ b/substrate/frame/contracts/src/debug.rs @@ -15,14 +15,14 @@ // See the License for the specific language governing permissions and // limitations under the License. -pub use crate::exec::ExportedFunction; +pub use crate::exec::{ExportedFunction, ExecResult}; use crate::{CodeHash, Config, LOG_TARGET}; use pallet_contracts_primitives::ExecReturnValue; /// Umbrella trait for all interfaces that serves for debugging. -pub trait Debugger: Tracing {} +pub trait Debugger: Tracing + CallInterceptor {} -impl Debugger for V where V: Tracing {} +impl Debugger for V where V: Tracing + CallInterceptor {} /// Defines methods to capture contract calls, enabling external observers to /// measure, trace, and react to contract interactions. @@ -70,3 +70,18 @@ impl CallSpan for () { log::trace!(target: LOG_TARGET, "call result {output:?}") } } + +pub enum CallInterceptorResult { + Proceed, + Stop(ExecResult), +} + +pub trait CallInterceptor { + fn intercept_call() -> CallInterceptorResult; +} + +impl CallInterceptor for () { + fn intercept_call() -> CallInterceptorResult { + CallInterceptorResult::Proceed + } +} diff --git a/substrate/frame/contracts/src/exec.rs b/substrate/frame/contracts/src/exec.rs index 1ba44220ff8d..b89ed9f6ce77 100644 --- a/substrate/frame/contracts/src/exec.rs +++ b/substrate/frame/contracts/src/exec.rs @@ -16,7 +16,7 @@ // limitations under the License. use crate::{ - debug::{CallSpan, Tracing}, + debug::{CallSpan, Tracing, CallInterceptor, CallInterceptorResult}, gas::GasMeter, storage::{self, meter::Diff, WriteOutcome}, BalanceOf, CodeHash, CodeInfo, CodeInfoOf, Config, ContractInfo, ContractInfoOf, @@ -910,10 +910,14 @@ where let call_span = T::Debug::new_call_span(executable.code_hash(), entry_point, &input_data); - // Call into the Wasm blob. - let output = executable - .execute(self, &entry_point, input_data) - .map_err(|e| ExecError { error: e.error, origin: ErrorOrigin::Callee })?; + let output = match T::Debug::intercept_call() { + // Call into the Wasm blob. + CallInterceptorResult::Proceed => executable + .execute(self, &entry_point, input_data) + .map_err(|e| ExecError { error: e.error, origin: ErrorOrigin::Callee })?, + // Return the output of the interceptor. + CallInterceptorResult::Stop(output) => output?, + }; call_span.after_call(&output); From 5123152c446bd9890b901908c5738349e65e85f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Miko=C5=82ajczyk?= Date: Thu, 31 Aug 2023 11:49:09 +0200 Subject: [PATCH 02/13] Fill arguments --- substrate/frame/contracts/src/debug.rs | 42 +++++++++++++++++++------- substrate/frame/contracts/src/exec.rs | 10 +++--- 2 files changed, 37 insertions(+), 15 deletions(-) diff --git a/substrate/frame/contracts/src/debug.rs b/substrate/frame/contracts/src/debug.rs index aed40ecc93b7..a9cb08f1cce7 100644 --- a/substrate/frame/contracts/src/debug.rs +++ b/substrate/frame/contracts/src/debug.rs @@ -15,14 +15,14 @@ // See the License for the specific language governing permissions and // limitations under the License. -pub use crate::exec::{ExportedFunction, ExecResult}; +pub use crate::exec::{ExecResult, ExportedFunction}; use crate::{CodeHash, Config, LOG_TARGET}; use pallet_contracts_primitives::ExecReturnValue; /// Umbrella trait for all interfaces that serves for debugging. -pub trait Debugger: Tracing + CallInterceptor {} +pub trait Debugger: Tracing + CallInterceptor {} -impl Debugger for V where V: Tracing + CallInterceptor {} +impl Debugger for V where V: Tracing + CallInterceptor {} /// Defines methods to capture contract calls, enabling external observers to /// measure, trace, and react to contract interactions. @@ -37,11 +37,11 @@ pub trait Tracing { /// /// # Arguments /// - /// * `code_hash` - The code hash of the contract being called. + /// * `contract_address` - The address of the contract that is about to be executed. /// * `entry_point` - Describes whether the call is the constructor or a regular call. /// * `input_data` - The raw input data of the call. fn new_call_span( - code_hash: &CodeHash, + contract_address: &T::AccountId, entry_point: ExportedFunction, input_data: &[u8], ) -> Self::CallSpan; @@ -60,8 +60,12 @@ pub trait CallSpan { impl Tracing for () { type CallSpan = (); - fn new_call_span(code_hash: &CodeHash, entry_point: ExportedFunction, input_data: &[u8]) { - log::trace!(target: LOG_TARGET, "call {entry_point:?} hash: {code_hash:?}, input_data: {input_data:?}") + fn new_call_span( + contract_address: &T::AccountId, + entry_point: ExportedFunction, + input_data: &[u8], + ) { + log::trace!(target: LOG_TARGET, "call {entry_point:?} account: {contract_address:?}, input_data: {input_data:?}") } } @@ -76,12 +80,28 @@ pub enum CallInterceptorResult { Stop(ExecResult), } -pub trait CallInterceptor { - fn intercept_call() -> CallInterceptorResult; +pub trait CallInterceptor { + /// Allows to intercept contract calls and decide whether they should be executed or not. + /// If the call is intercepted, the mocked result of the call is returned. + /// + /// # Arguments + /// + /// * `contract_address` - The address of the contract that is about to be executed. + /// * `entry_point` - Describes whether the call is the constructor or a regular call. + /// * `input_data` - The raw input data of the call. + fn intercept_call( + contract_address: &T::AccountId, + entry_point: ExportedFunction, + input_data: &[u8], + ) -> CallInterceptorResult; } -impl CallInterceptor for () { - fn intercept_call() -> CallInterceptorResult { +impl CallInterceptor for () { + fn intercept_call( + _contract_address: &T::AccountId, + _entry_point: ExportedFunction, + _input_data: &[u8], + ) -> CallInterceptorResult { CallInterceptorResult::Proceed } } diff --git a/substrate/frame/contracts/src/exec.rs b/substrate/frame/contracts/src/exec.rs index b89ed9f6ce77..ea6e691d2af0 100644 --- a/substrate/frame/contracts/src/exec.rs +++ b/substrate/frame/contracts/src/exec.rs @@ -16,7 +16,7 @@ // limitations under the License. use crate::{ - debug::{CallSpan, Tracing, CallInterceptor, CallInterceptorResult}, + debug::{CallInterceptor, CallInterceptorResult, CallSpan, Tracing}, gas::GasMeter, storage::{self, meter::Diff, WriteOutcome}, BalanceOf, CodeHash, CodeInfo, CodeInfoOf, Config, ContractInfo, ContractInfoOf, @@ -907,10 +907,12 @@ where // Every non delegate call or instantiate also optionally transfers the balance. self.initial_transfer()?; - let call_span = - T::Debug::new_call_span(executable.code_hash(), entry_point, &input_data); + let contract_address = &top_frame!(self).account_id; - let output = match T::Debug::intercept_call() { + let call_span = T::Debug::new_call_span(contract_address, entry_point, &input_data); + let interception_result = + T::Debug::intercept_call(contract_address, entry_point, &input_data); + let output = match interception_result { // Call into the Wasm blob. CallInterceptorResult::Proceed => executable .execute(self, &entry_point, input_data) From 66b2a841fad1a03471052515531c731dc69c6313 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Miko=C5=82ajczyk?= Date: Thu, 31 Aug 2023 11:49:35 +0200 Subject: [PATCH 03/13] Unused import --- substrate/frame/contracts/src/debug.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/contracts/src/debug.rs b/substrate/frame/contracts/src/debug.rs index a9cb08f1cce7..1f1bbc52b9f0 100644 --- a/substrate/frame/contracts/src/debug.rs +++ b/substrate/frame/contracts/src/debug.rs @@ -16,7 +16,7 @@ // limitations under the License. pub use crate::exec::{ExecResult, ExportedFunction}; -use crate::{CodeHash, Config, LOG_TARGET}; +use crate::{Config, LOG_TARGET}; use pallet_contracts_primitives::ExecReturnValue; /// Umbrella trait for all interfaces that serves for debugging. From 25268571e5b44ec43ca7656c52ffb44b597a42e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Miko=C5=82ajczyk?= Date: Thu, 31 Aug 2023 12:40:29 +0200 Subject: [PATCH 04/13] Add testcase --- substrate/frame/contracts/src/debug.rs | 2 +- .../frame/contracts/src/tests/test_debug.rs | 118 ++++++++++++++---- 2 files changed, 96 insertions(+), 24 deletions(-) diff --git a/substrate/frame/contracts/src/debug.rs b/substrate/frame/contracts/src/debug.rs index 1f1bbc52b9f0..fc8dba74f653 100644 --- a/substrate/frame/contracts/src/debug.rs +++ b/substrate/frame/contracts/src/debug.rs @@ -17,7 +17,7 @@ pub use crate::exec::{ExecResult, ExportedFunction}; use crate::{Config, LOG_TARGET}; -use pallet_contracts_primitives::ExecReturnValue; +pub use pallet_contracts_primitives::ExecReturnValue; /// Umbrella trait for all interfaces that serves for debugging. pub trait Debugger: Tracing + CallInterceptor {} diff --git a/substrate/frame/contracts/src/tests/test_debug.rs b/substrate/frame/contracts/src/tests/test_debug.rs index c7862c7f03dd..6f5ef197ad9c 100644 --- a/substrate/frame/contracts/src/tests/test_debug.rs +++ b/substrate/frame/contracts/src/tests/test_debug.rs @@ -16,7 +16,12 @@ // limitations under the License. use super::*; -use crate::debug::{CallSpan, ExportedFunction, Tracing}; +use crate::{ + debug::{ + CallInterceptor, CallInterceptorResult, CallSpan, ExportedFunction, Tracing, + }, + AccountIdOf, +}; use frame_support::traits::Currency; use pallet_contracts_primitives::ExecReturnValue; use pretty_assertions::assert_eq; @@ -24,7 +29,7 @@ use std::cell::RefCell; #[derive(Clone, PartialEq, Eq, Debug)] struct DebugFrame { - code_hash: CodeHash, + contract_account: AccountId32, call: ExportedFunction, input: Vec, result: Option>, @@ -32,11 +37,12 @@ struct DebugFrame { thread_local! { static DEBUG_EXECUTION_TRACE: RefCell> = RefCell::new(Vec::new()); + static INTERCEPTED_ADDRESS: RefCell> = RefCell::new(None); } pub struct TestDebug; pub struct TestCallSpan { - code_hash: CodeHash, + contract_account: AccountId32, call: ExportedFunction, input: Vec, } @@ -45,19 +51,42 @@ impl Tracing for TestDebug { type CallSpan = TestCallSpan; fn new_call_span( - code_hash: &CodeHash, + contract_account: &AccountIdOf, entry_point: ExportedFunction, input_data: &[u8], ) -> TestCallSpan { DEBUG_EXECUTION_TRACE.with(|d| { d.borrow_mut().push(DebugFrame { - code_hash: *code_hash, + contract_account: contract_account.clone(), call: entry_point, input: input_data.to_vec(), result: None, }) }); - TestCallSpan { code_hash: *code_hash, call: entry_point, input: input_data.to_vec() } + TestCallSpan { + contract_account: contract_account.clone(), + call: entry_point, + input: input_data.to_vec(), + } + } +} + +impl CallInterceptor for TestDebug { + fn intercept_call( + contract_address: &::AccountId, + _entry_point: ExportedFunction, + _input_data: &[u8], + ) -> CallInterceptorResult { + INTERCEPTED_ADDRESS.with(|i| { + if i.borrow().as_ref() == Some(contract_address) { + CallInterceptorResult::Stop(Ok(ExecReturnValue { + flags: ReturnFlags::REVERT, + data: vec![], + })) + } else { + CallInterceptorResult::Proceed + } + }) } } @@ -65,7 +94,7 @@ impl CallSpan for TestCallSpan { fn after_call(self, output: &ExecReturnValue) { DEBUG_EXECUTION_TRACE.with(|d| { d.borrow_mut().push(DebugFrame { - code_hash: self.code_hash, + contract_account: self.contract_account, call: self.call, input: self.input, result: Some(output.data.clone()), @@ -76,8 +105,8 @@ impl CallSpan for TestCallSpan { #[test] fn unsafe_debugging_works() { - let (wasm_caller, code_hash_caller) = compile_module::("call").unwrap(); - let (wasm_callee, code_hash_callee) = compile_module::("store_call").unwrap(); + let (wasm_caller, _) = compile_module::("call").unwrap(); + let (wasm_callee, _) = compile_module::("store_call").unwrap(); fn current_stack() -> Vec { DEBUG_EXECUTION_TRACE.with(|stack| stack.borrow().clone()) @@ -100,18 +129,18 @@ fn unsafe_debugging_works() { .account_id } - fn constructor_frame(hash: CodeHash, after: bool) -> DebugFrame { + fn constructor_frame(contract_account: AccountId32, after: bool) -> DebugFrame { DebugFrame { - code_hash: hash, + contract_account, call: ExportedFunction::Constructor, input: vec![], result: if after { Some(vec![]) } else { None }, } } - fn call_frame(hash: CodeHash, args: Vec, after: bool) -> DebugFrame { + fn call_frame(contract_account: AccountId32, args: Vec, after: bool) -> DebugFrame { DebugFrame { - code_hash: hash, + contract_account, call: ExportedFunction::Call, input: args, result: if after { Some(vec![]) } else { None }, @@ -129,19 +158,19 @@ fn unsafe_debugging_works() { assert_eq!( current_stack(), vec![ - constructor_frame(code_hash_caller, false), - constructor_frame(code_hash_caller, true), - constructor_frame(code_hash_callee, false), - constructor_frame(code_hash_callee, true), + constructor_frame(addr_caller.clone(), false), + constructor_frame(addr_caller.clone(), true), + constructor_frame(addr_callee.clone(), false), + constructor_frame(addr_callee.clone(), true), ] ); - let main_args = (100u32, &addr_callee).encode(); + let main_args = (100u32, &addr_callee.clone()).encode(); let inner_args = (100u32).encode(); assert_ok!(Contracts::call( RuntimeOrigin::signed(ALICE), - addr_caller, + addr_caller.clone(), 0, GAS_LIMIT, None, @@ -152,11 +181,54 @@ fn unsafe_debugging_works() { assert_eq!( stack_top, vec![ - call_frame(code_hash_caller, main_args.clone(), false), - call_frame(code_hash_callee, inner_args.clone(), false), - call_frame(code_hash_callee, inner_args, true), - call_frame(code_hash_caller, main_args, true), + call_frame(addr_caller.clone(), main_args.clone(), false), + call_frame(addr_callee.clone(), inner_args.clone(), false), + call_frame(addr_callee, inner_args, true), + call_frame(addr_caller, main_args, true), ] ); }); } + +#[test] +fn call_interception_works() { + let (wasm, _) = compile_module::("dummy").unwrap(); + + ExtBuilder::default().existential_deposit(200).build().execute_with(|| { + let _ = Balances::deposit_creating(&ALICE, 1_000_000); + + let account_id = Contracts::bare_instantiate( + ALICE, + 0, + GAS_LIMIT, + None, + Code::Upload(wasm), + vec![], + // some salt to ensure that the address of this contract is unique among all tests + vec![0x41, 0x41, 0x41, 0x41], + DebugInfo::Skip, + CollectEvents::Skip, + ) + .result + .unwrap() + .account_id; + + // no interception yet + assert_ok!(Contracts::call( + RuntimeOrigin::signed(ALICE), + account_id.clone(), + 0, + GAS_LIMIT, + None, + vec![], + )); + + // intercept calls to this contract + INTERCEPTED_ADDRESS.with(|i| *i.borrow_mut() = Some(account_id.clone())); + + assert_err_ignore_postinfo!( + Contracts::call(RuntimeOrigin::signed(ALICE), account_id, 0, GAS_LIMIT, None, vec![],), + >::ContractReverted, + ); + }); +} From 65634694f852ea1c128c81124bb0227673940af0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Miko=C5=82ajczyk?= Date: Thu, 31 Aug 2023 12:51:11 +0200 Subject: [PATCH 05/13] fmt --- substrate/frame/contracts/src/tests/test_debug.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/substrate/frame/contracts/src/tests/test_debug.rs b/substrate/frame/contracts/src/tests/test_debug.rs index 6f5ef197ad9c..e0cd27686e04 100644 --- a/substrate/frame/contracts/src/tests/test_debug.rs +++ b/substrate/frame/contracts/src/tests/test_debug.rs @@ -17,9 +17,7 @@ use super::*; use crate::{ - debug::{ - CallInterceptor, CallInterceptorResult, CallSpan, ExportedFunction, Tracing, - }, + debug::{CallInterceptor, CallInterceptorResult, CallSpan, ExportedFunction, Tracing}, AccountIdOf, }; use frame_support::traits::Currency; From ffdfe5d5cb379d75d319854dda038da352ba03e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Miko=C5=82ajczyk?= Date: Tue, 5 Sep 2023 15:16:57 +0200 Subject: [PATCH 06/13] Remove CallInterceptorResult --- substrate/frame/contracts/src/debug.rs | 17 +++++++++-------- substrate/frame/contracts/src/exec.rs | 6 +++--- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/substrate/frame/contracts/src/debug.rs b/substrate/frame/contracts/src/debug.rs index fc8dba74f653..ce3db654a7a1 100644 --- a/substrate/frame/contracts/src/debug.rs +++ b/substrate/frame/contracts/src/debug.rs @@ -75,11 +75,6 @@ impl CallSpan for () { } } -pub enum CallInterceptorResult { - Proceed, - Stop(ExecResult), -} - pub trait CallInterceptor { /// Allows to intercept contract calls and decide whether they should be executed or not. /// If the call is intercepted, the mocked result of the call is returned. @@ -89,11 +84,17 @@ pub trait CallInterceptor { /// * `contract_address` - The address of the contract that is about to be executed. /// * `entry_point` - Describes whether the call is the constructor or a regular call. /// * `input_data` - The raw input data of the call. + /// + /// # Returns + /// + /// * `Some(ExecResult)` - If the call is intercepted, the mocked result of the call is + /// returned. + /// * `None` - If the call is not intercepted, the call should be executed normally. fn intercept_call( contract_address: &T::AccountId, entry_point: ExportedFunction, input_data: &[u8], - ) -> CallInterceptorResult; + ) -> Option; } impl CallInterceptor for () { @@ -101,7 +102,7 @@ impl CallInterceptor for () { _contract_address: &T::AccountId, _entry_point: ExportedFunction, _input_data: &[u8], - ) -> CallInterceptorResult { - CallInterceptorResult::Proceed + ) -> Option { + None } } diff --git a/substrate/frame/contracts/src/exec.rs b/substrate/frame/contracts/src/exec.rs index cfdfb2a94fdd..b8f595b0a1f8 100644 --- a/substrate/frame/contracts/src/exec.rs +++ b/substrate/frame/contracts/src/exec.rs @@ -16,7 +16,7 @@ // limitations under the License. use crate::{ - debug::{CallInterceptor, CallInterceptorResult, CallSpan, Tracing}, + debug::{CallInterceptor, CallSpan, Tracing}, gas::GasMeter, storage::{self, meter::Diff, WriteOutcome}, BalanceOf, CodeHash, CodeInfo, CodeInfoOf, Config, ContractInfo, ContractInfoOf, @@ -915,11 +915,11 @@ where T::Debug::intercept_call(contract_address, entry_point, &input_data); let output = match interception_result { // Call into the Wasm blob. - CallInterceptorResult::Proceed => executable + None => executable .execute(self, &entry_point, input_data) .map_err(|e| ExecError { error: e.error, origin: ErrorOrigin::Callee })?, // Return the output of the interceptor. - CallInterceptorResult::Stop(output) => output?, + Some(output) => output?, }; call_span.after_call(&output); From b9399af67fa4ce0eca941e88ada72bc235255dbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Miko=C5=82ajczyk?= Date: Tue, 5 Sep 2023 15:27:01 +0200 Subject: [PATCH 07/13] Adapt test --- substrate/frame/contracts/src/tests/test_debug.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/substrate/frame/contracts/src/tests/test_debug.rs b/substrate/frame/contracts/src/tests/test_debug.rs index e0cd27686e04..78f3ee14fe90 100644 --- a/substrate/frame/contracts/src/tests/test_debug.rs +++ b/substrate/frame/contracts/src/tests/test_debug.rs @@ -17,7 +17,7 @@ use super::*; use crate::{ - debug::{CallInterceptor, CallInterceptorResult, CallSpan, ExportedFunction, Tracing}, + debug::{CallInterceptor, CallSpan, ExecResult, ExportedFunction, Tracing}, AccountIdOf, }; use frame_support::traits::Currency; @@ -74,15 +74,12 @@ impl CallInterceptor for TestDebug { contract_address: &::AccountId, _entry_point: ExportedFunction, _input_data: &[u8], - ) -> CallInterceptorResult { + ) -> Option { INTERCEPTED_ADDRESS.with(|i| { if i.borrow().as_ref() == Some(contract_address) { - CallInterceptorResult::Stop(Ok(ExecReturnValue { - flags: ReturnFlags::REVERT, - data: vec![], - })) + Some(Ok(ExecReturnValue { flags: ReturnFlags::REVERT, data: vec![] })) } else { - CallInterceptorResult::Proceed + None } }) } From b09a6779ee2c6f36bfa04b79e110bc60899992d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Miko=C5=82ajczyk?= Date: Tue, 5 Sep 2023 16:30:06 +0200 Subject: [PATCH 08/13] Review --- substrate/frame/contracts/src/debug.rs | 1 + substrate/frame/contracts/src/exec.rs | 17 ++++++------- .../frame/contracts/src/tests/test_debug.rs | 24 +++++++++---------- 3 files changed, 20 insertions(+), 22 deletions(-) diff --git a/substrate/frame/contracts/src/debug.rs b/substrate/frame/contracts/src/debug.rs index ce3db654a7a1..efefada4d829 100644 --- a/substrate/frame/contracts/src/debug.rs +++ b/substrate/frame/contracts/src/debug.rs @@ -75,6 +75,7 @@ impl CallSpan for () { } } +/// Provides an interface for intercepting contract calls. pub trait CallInterceptor { /// Allows to intercept contract calls and decide whether they should be executed or not. /// If the call is intercepted, the mocked result of the call is returned. diff --git a/substrate/frame/contracts/src/exec.rs b/substrate/frame/contracts/src/exec.rs index b8f595b0a1f8..eb40f45de184 100644 --- a/substrate/frame/contracts/src/exec.rs +++ b/substrate/frame/contracts/src/exec.rs @@ -911,16 +911,13 @@ where let contract_address = &top_frame!(self).account_id; let call_span = T::Debug::new_call_span(contract_address, entry_point, &input_data); - let interception_result = - T::Debug::intercept_call(contract_address, entry_point, &input_data); - let output = match interception_result { - // Call into the Wasm blob. - None => executable - .execute(self, &entry_point, input_data) - .map_err(|e| ExecError { error: e.error, origin: ErrorOrigin::Callee })?, - // Return the output of the interceptor. - Some(output) => output?, - }; + + let output = T::Debug::intercept_call(contract_address, entry_point, &input_data) + .unwrap_or_else(|| { + executable + .execute(self, &entry_point, input_data) + .map_err(|e| ExecError { error: e.error, origin: ErrorOrigin::Callee }) + })?; call_span.after_call(&output); diff --git a/substrate/frame/contracts/src/tests/test_debug.rs b/substrate/frame/contracts/src/tests/test_debug.rs index 78f3ee14fe90..ee16dc0b0254 100644 --- a/substrate/frame/contracts/src/tests/test_debug.rs +++ b/substrate/frame/contracts/src/tests/test_debug.rs @@ -124,18 +124,18 @@ fn unsafe_debugging_works() { .account_id } - fn constructor_frame(contract_account: AccountId32, after: bool) -> DebugFrame { + fn constructor_frame(contract_account: &AccountId32, after: bool) -> DebugFrame { DebugFrame { - contract_account, + contract_account: contract_account.clone(), call: ExportedFunction::Constructor, input: vec![], result: if after { Some(vec![]) } else { None }, } } - fn call_frame(contract_account: AccountId32, args: Vec, after: bool) -> DebugFrame { + fn call_frame(contract_account: &AccountId32, args: Vec, after: bool) -> DebugFrame { DebugFrame { - contract_account, + contract_account: contract_account.clone(), call: ExportedFunction::Call, input: args, result: if after { Some(vec![]) } else { None }, @@ -153,10 +153,10 @@ fn unsafe_debugging_works() { assert_eq!( current_stack(), vec![ - constructor_frame(addr_caller.clone(), false), - constructor_frame(addr_caller.clone(), true), - constructor_frame(addr_callee.clone(), false), - constructor_frame(addr_callee.clone(), true), + constructor_frame(&addr_caller, false), + constructor_frame(&addr_caller, true), + constructor_frame(&addr_callee, false), + constructor_frame(&addr_callee, true), ] ); @@ -176,10 +176,10 @@ fn unsafe_debugging_works() { assert_eq!( stack_top, vec![ - call_frame(addr_caller.clone(), main_args.clone(), false), - call_frame(addr_callee.clone(), inner_args.clone(), false), - call_frame(addr_callee, inner_args, true), - call_frame(addr_caller, main_args, true), + call_frame(&addr_caller, main_args.clone(), false), + call_frame(&addr_callee, inner_args.clone(), false), + call_frame(&addr_callee, inner_args, true), + call_frame(&addr_caller, main_args, true), ] ); }); From e36da54a156ccb3d35bab8d1e710ed2eca19c06f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Miko=C5=82ajczyk?= Date: Tue, 5 Sep 2023 16:54:09 +0200 Subject: [PATCH 09/13] Expected --- substrate/frame/contracts/src/debug.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/substrate/frame/contracts/src/debug.rs b/substrate/frame/contracts/src/debug.rs index efefada4d829..da6e0584495b 100644 --- a/substrate/frame/contracts/src/debug.rs +++ b/substrate/frame/contracts/src/debug.rs @@ -86,11 +86,12 @@ pub trait CallInterceptor { /// * `entry_point` - Describes whether the call is the constructor or a regular call. /// * `input_data` - The raw input data of the call. /// - /// # Returns + /// # Expected behavior /// - /// * `Some(ExecResult)` - If the call is intercepted, the mocked result of the call is - /// returned. - /// * `None` - If the call is not intercepted, the call should be executed normally. + /// This method should return: + /// * `Some(ExecResult)` - if the call should be intercepted and the mocked result of the call + /// is returned. + /// * `None` - otherwise, i.e. the call should be executed normally fn intercept_call( contract_address: &T::AccountId, entry_point: ExportedFunction, From 5ec45b4538094262a081ff356f291cfae52b3ba3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Miko=C5=82ajczyk?= Date: Wed, 6 Sep 2023 08:40:37 +0200 Subject: [PATCH 10/13] fmt --- polkadot/xcm/src/v3/multiasset.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/polkadot/xcm/src/v3/multiasset.rs b/polkadot/xcm/src/v3/multiasset.rs index a5a74368289d..188555318c8c 100644 --- a/polkadot/xcm/src/v3/multiasset.rs +++ b/polkadot/xcm/src/v3/multiasset.rs @@ -34,13 +34,13 @@ use crate::v2::{ WildMultiAsset as OldWildMultiAsset, }; use alloc::{vec, vec::Vec}; +use bounded_collections::{BoundedVec, ConstU32}; use core::{ cmp::Ordering, convert::{TryFrom, TryInto}, }; use parity_scale_codec::{self as codec, Decode, Encode, MaxEncodedLen}; use scale_info::TypeInfo; -use bounded_collections::{BoundedVec, ConstU32}; /// A general identifier for an instance of a non-fungible asset class. #[derive( @@ -518,7 +518,8 @@ impl MaxEncodedLen for MultiAssets { impl Decode for MultiAssets { fn decode(input: &mut I) -> Result { - let bounded_instructions = BoundedVec::>::decode(input)?; + let bounded_instructions = + BoundedVec::>::decode(input)?; Self::from_sorted_and_deduplicated(bounded_instructions.into_inner()) .map_err(|()| "Out of order".into()) } From 3690ee0b7bc3e2a5e8e14d1f55f3d7953f6918ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Miko=C5=82ajczyk?= Date: Thu, 28 Sep 2023 17:23:33 +0200 Subject: [PATCH 11/13] rename testcase --- substrate/frame/contracts/src/tests/test_debug.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/contracts/src/tests/test_debug.rs b/substrate/frame/contracts/src/tests/test_debug.rs index ee16dc0b0254..89ce1fa3ec09 100644 --- a/substrate/frame/contracts/src/tests/test_debug.rs +++ b/substrate/frame/contracts/src/tests/test_debug.rs @@ -99,7 +99,7 @@ impl CallSpan for TestCallSpan { } #[test] -fn unsafe_debugging_works() { +fn debugging_works() { let (wasm_caller, _) = compile_module::("call").unwrap(); let (wasm_callee, _) = compile_module::("store_call").unwrap(); From 5ed54ea0f0a171591bdf0cd103a91e997c80ce75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Miko=C5=82ajczyk?= Date: Thu, 28 Sep 2023 17:56:45 +0200 Subject: [PATCH 12/13] exportedfunction by ref --- substrate/frame/contracts/src/debug.rs | 4 ++-- substrate/frame/contracts/src/exec.rs | 2 +- substrate/frame/contracts/src/tests/test_debug.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/substrate/frame/contracts/src/debug.rs b/substrate/frame/contracts/src/debug.rs index da6e0584495b..6050969f2169 100644 --- a/substrate/frame/contracts/src/debug.rs +++ b/substrate/frame/contracts/src/debug.rs @@ -94,7 +94,7 @@ pub trait CallInterceptor { /// * `None` - otherwise, i.e. the call should be executed normally fn intercept_call( contract_address: &T::AccountId, - entry_point: ExportedFunction, + entry_point: &ExportedFunction, input_data: &[u8], ) -> Option; } @@ -102,7 +102,7 @@ pub trait CallInterceptor { impl CallInterceptor for () { fn intercept_call( _contract_address: &T::AccountId, - _entry_point: ExportedFunction, + _entry_point: &ExportedFunction, _input_data: &[u8], ) -> Option { None diff --git a/substrate/frame/contracts/src/exec.rs b/substrate/frame/contracts/src/exec.rs index 4afe504351ae..9090aa9cb112 100644 --- a/substrate/frame/contracts/src/exec.rs +++ b/substrate/frame/contracts/src/exec.rs @@ -912,7 +912,7 @@ where let call_span = T::Debug::new_call_span(contract_address, entry_point, &input_data); - let output = T::Debug::intercept_call(contract_address, entry_point, &input_data) + let output = T::Debug::intercept_call(contract_address, &entry_point, &input_data) .unwrap_or_else(|| { executable .execute(self, &entry_point, input_data) diff --git a/substrate/frame/contracts/src/tests/test_debug.rs b/substrate/frame/contracts/src/tests/test_debug.rs index 89ce1fa3ec09..2d7ed4743657 100644 --- a/substrate/frame/contracts/src/tests/test_debug.rs +++ b/substrate/frame/contracts/src/tests/test_debug.rs @@ -72,7 +72,7 @@ impl Tracing for TestDebug { impl CallInterceptor for TestDebug { fn intercept_call( contract_address: &::AccountId, - _entry_point: ExportedFunction, + _entry_point: &ExportedFunction, _input_data: &[u8], ) -> Option { INTERCEPTED_ADDRESS.with(|i| { From a6a9be7f138b93d60b99f77e1e634495fa8a840f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Miko=C5=82ajczyk?= Date: Fri, 29 Sep 2023 10:20:14 +0200 Subject: [PATCH 13/13] Dummy commit to retrigger CI --- substrate/frame/contracts/src/debug.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/contracts/src/debug.rs b/substrate/frame/contracts/src/debug.rs index 6050969f2169..e22a841e6fb7 100644 --- a/substrate/frame/contracts/src/debug.rs +++ b/substrate/frame/contracts/src/debug.rs @@ -91,7 +91,7 @@ pub trait CallInterceptor { /// This method should return: /// * `Some(ExecResult)` - if the call should be intercepted and the mocked result of the call /// is returned. - /// * `None` - otherwise, i.e. the call should be executed normally + /// * `None` - otherwise, i.e. the call should be executed normally. fn intercept_call( contract_address: &T::AccountId, entry_point: &ExportedFunction,