Cross-contract calling: simple debugger (#14678)

* Provide basic breakpoints

* Rename to Observer

* Rename feature. Single trait. Borrow-checker

* : frame_system::Config

* Confused type name

* Minor bugs

* pub trait

* No unnecessary cloning

* Make node compile with all features

* Move everything debug-related to a single module

* Add docs and implementation for ()

* fmt

* Make it feature-gated or for tests

* Prepare testing kit

* Testcase

* Fmt

* Use feature in dev-deps

* ?

* feature propagation

* AAAA

* lol, that doesn't make much sense to me

* Turn on

* clippy

* Remove self dep

* fmt, feature-gating test

* Noop to trigger CI

* idk

* add feature to pipeline

* Corrupt test to see if it is actually being run

* Revert change

* Doc for conf type

* Review

* Imports

* ...

* Remove debug for kitchen-sink

* Move test

* Fix imports

* I must have already tried this one...

Author: pmikolajczyk41

bin/node/runtime/Cargo.toml

@@ -373,3 +373,4 @@ try-runtime = [
 	"frame-election-provider-support/try-runtime",
 	"sp-runtime/try-runtime"
 ]
+unsafe-debug = ["pallet-contracts/unsafe-debug"]

bin/node/runtime/src/lib.rs

@@ -1258,6 +1258,8 @@ impl pallet_contracts::Config for Runtime {
 	type Migrations = pallet_contracts::migration::codegen::BenchMigrations;
 	type MaxDelegateDependencies = ConstU32<32>;
 	type CodeHashLockupDepositPercent = CodeHashLockupDepositPercent;
+	#[cfg(feature = "unsafe-debug")]
+	type Debug = ();
 }
 
 impl pallet_sudo::Config for Runtime {

frame/contracts/Cargo.toml

@@ -112,5 +112,6 @@ try-runtime = [
 	"pallet-proxy/try-runtime",
 	"pallet-timestamp/try-runtime",
 	"pallet-utility/try-runtime",
-	"sp-runtime/try-runtime"
+	"sp-runtime/try-runtime",
 ]
+unsafe-debug = []

frame/contracts/src/exec.rs

@@ -15,6 +15,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#[cfg(feature = "unsafe-debug")]
+use crate::unsafe_debug::ExecutionObserver;
 use crate::{
 	gas::GasMeter,
 	storage::{self, DepositAccount, WriteOutcome},
@@ -344,7 +346,7 @@ pub trait Ext: sealing::Sealed {
 }
 
 /// Describes the different functions that can be exported by an [`Executable`].
-#[derive(Clone, Copy, PartialEq)]
+#[derive(Clone, Copy, PartialEq, Eq, Debug)]
 pub enum ExportedFunction {
 	/// The constructor function which is executed on deployment of a contract.
 	Constructor,
@@ -904,11 +906,21 @@ where
 			// Every non delegate call or instantiate also optionally transfers the balance.
 			self.initial_transfer()?;
 
+			#[cfg(feature = "unsafe-debug")]
+			let (code_hash, input_clone) = {
+				let code_hash = *executable.code_hash();
+				T::Debug::before_call(&code_hash, entry_point, &input_data);
+				(code_hash, input_data.clone())
+			};
+
 			// Call into the Wasm blob.
 			let output = executable
 				.execute(self, &entry_point, input_data)
 				.map_err(|e| ExecError { error: e.error, origin: ErrorOrigin::Callee })?;
 
+			#[cfg(feature = "unsafe-debug")]
+			T::Debug::after_call(&code_hash, entry_point, input_clone, &output);
+
 			// Avoid useless work that would be reverted anyways.
 			if output.did_revert() {
 				return Ok(output)

frame/contracts/src/lib.rs

@@ -97,6 +97,7 @@ mod wasm;
 
 pub mod chain_extension;
 pub mod migration;
+pub mod unsafe_debug;
 pub mod weights;
 
 #[cfg(test)]
@@ -351,6 +352,14 @@ pub mod pallet {
 		/// type Migrations = (v10::Migration<Runtime, Currency>,);
 		/// ```
 		type Migrations: MigrateSequence;
+
+		/// Type that provides debug handling for the contract execution process.
+		///
+		/// # Warning
+		///
+		/// Do **not** use it in a production environment or for benchmarking purposes.
+		#[cfg(feature = "unsafe-debug")]
+		type Debug: unsafe_debug::UnsafeDebug<Self>;
 	}
 
 	#[pallet::hooks]

frame/contracts/src/tests.rs

@@ -16,6 +16,7 @@
 // limitations under the License.
 
 mod pallet_dummy;
+mod unsafe_debug;
 
 use self::test_utils::{ensure_stored, expected_deposit, hash};
 use crate::{
@@ -464,6 +465,8 @@ impl Config for Test {
 	type Migrations = crate::migration::codegen::BenchMigrations;
 	type CodeHashLockupDepositPercent = CodeHashLockupDepositPercent;
 	type MaxDelegateDependencies = MaxDelegateDependencies;
+	#[cfg(feature = "unsafe-debug")]
+	type Debug = unsafe_debug::TestDebugger;
 }
 
 pub const ALICE: AccountId32 = AccountId32::new([1u8; 32]);

frame/contracts/src/tests/unsafe_debug.rs

@@ -0,0 +1,138 @@
+#![cfg(feature = "unsafe-debug")]
+
+use super::*;
+use crate::unsafe_debug::{ExecutionObserver, ExportedFunction};
+use frame_support::traits::Currency;
+use pallet_contracts_primitives::ExecReturnValue;
+use pretty_assertions::assert_eq;
+use std::cell::RefCell;
+
+#[derive(Clone, PartialEq, Eq, Debug)]
+struct DebugFrame {
+	code_hash: CodeHash<Test>,
+	call: ExportedFunction,
+	input: Vec<u8>,
+	result: Option<Vec<u8>>,
+}
+
+thread_local! {
+	static DEBUG_EXECUTION_TRACE: RefCell<Vec<DebugFrame>> = RefCell::new(Vec::new());
+}
+
+pub struct TestDebugger;
+
+impl ExecutionObserver<CodeHash<Test>> for TestDebugger {
+	fn before_call(code_hash: &CodeHash<Test>, entry_point: ExportedFunction, input_data: &[u8]) {
+		DEBUG_EXECUTION_TRACE.with(|d| {
+			d.borrow_mut().push(DebugFrame {
+				code_hash: code_hash.clone(),
+				call: entry_point,
+				input: input_data.to_vec(),
+				result: None,
+			})
+		});
+	}
+
+	fn after_call(
+		code_hash: &CodeHash<Test>,
+		entry_point: ExportedFunction,
+		input_data: Vec<u8>,
+		output: &ExecReturnValue,
+	) {
+		DEBUG_EXECUTION_TRACE.with(|d| {
+			d.borrow_mut().push(DebugFrame {
+				code_hash: code_hash.clone(),
+				call: entry_point,
+				input: input_data,
+				result: Some(output.data.clone()),
+			})
+		});
+	}
+}
+
+#[test]
+fn unsafe_debugging_works() {
+	let (wasm_caller, code_hash_caller) = compile_module::<Test>("call").unwrap();
+	let (wasm_callee, code_hash_callee) = compile_module::<Test>("store_call").unwrap();
+
+	fn current_stack() -> Vec<DebugFrame> {
+		DEBUG_EXECUTION_TRACE.with(|stack| stack.borrow().clone())
+	}
+
+	fn deploy(wasm: Vec<u8>) -> AccountId32 {
+		Contracts::bare_instantiate(
+			ALICE,
+			0,
+			GAS_LIMIT,
+			None,
+			Code::Upload(wasm),
+			vec![],
+			vec![],
+			DebugInfo::Skip,
+			CollectEvents::Skip,
+		)
+		.result
+		.unwrap()
+		.account_id
+	}
+
+	fn constructor_frame(hash: CodeHash<Test>, after: bool) -> DebugFrame {
+		DebugFrame {
+			code_hash: hash,
+			call: ExportedFunction::Constructor,
+			input: vec![],
+			result: if after { Some(vec![]) } else { None },
+		}
+	}
+
+	fn call_frame(hash: CodeHash<Test>, args: Vec<u8>, after: bool) -> DebugFrame {
+		DebugFrame {
+			code_hash: hash,
+			call: ExportedFunction::Call,
+			input: args,
+			result: if after { Some(vec![]) } else { None },
+		}
+	}
+
+	ExtBuilder::default().existential_deposit(200).build().execute_with(|| {
+		let _ = Balances::deposit_creating(&ALICE, 1_000_000);
+
+		assert_eq!(current_stack(), vec![]);
+
+		let addr_caller = deploy(wasm_caller);
+		let addr_callee = deploy(wasm_callee);
+
+		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),
+			]
+		);
+
+		let main_args = (100u32, &addr_callee).encode();
+		let inner_args = (100u32).encode();
+
+		assert_ok!(Contracts::call(
+			RuntimeOrigin::signed(ALICE),
+			addr_caller,
+			0,
+			GAS_LIMIT,
+			None,
+			main_args.clone()
+		));
+
+		let stack_top = current_stack()[4..].to_vec();
+		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),
+			]
+		);
+	});
+}

frame/contracts/src/unsafe_debug.rs

@@ -0,0 +1,47 @@
+#![cfg(feature = "unsafe-debug")]
+
+pub use crate::exec::ExportedFunction;
+use crate::{CodeHash, Vec};
+use pallet_contracts_primitives::ExecReturnValue;
+
+/// Umbrella trait for all interfaces that serves for debugging, but are not suitable for any
+/// production or benchmarking use.
+pub trait UnsafeDebug<T: frame_system::Config>: ExecutionObserver<CodeHash<T>> {}
+
+impl<T: frame_system::Config, D> UnsafeDebug<T> for D where D: ExecutionObserver<CodeHash<T>> {}
+
+/// Defines the interface between pallet contracts and the outside observer.
+///
+/// The intended use is the environment, where the observer holds directly the whole runtime
+/// (externalities) and thus can react to the execution breakpoints synchronously.
+///
+/// This definitely *should not* be used in any production or benchmarking setting, since handling
+/// callbacks might be arbitrarily expensive and thus significantly influence performance.
+pub trait ExecutionObserver<CodeHash> {
+	/// Called just before the execution of a contract.
+	///
+	/// # Arguments
+	///
+	/// * `code_hash` - The code hash of the contract being called.
+	/// * `entry_point` - Describes whether the call is the constructor or a regular call.
+	/// * `input_data` - The raw input data of the call.
+	fn before_call(_code_hash: &CodeHash, _entry_point: ExportedFunction, _input_data: &[u8]) {}
+
+	/// Called just after the execution of a contract.
+	///
+	/// # Arguments
+	///
+	/// * `code_hash` - The code hash of the contract being called.
+	/// * `entry_point` - Describes whether the call was the constructor or a regular call.
+	/// * `input_data` - The raw input data of the call.
+	/// * `output` - The raw output of the call.
+	fn after_call(
+		_code_hash: &CodeHash,
+		_entry_point: ExportedFunction,
+		_input_data: Vec<u8>,
+		_output: &ExecReturnValue,
+	) {
+	}
+}
+
+impl<CodeHash> ExecutionObserver<CodeHash> for () {}

scripts/ci/gitlab/pipeline/test.yml

@@ -221,7 +221,7 @@ test-linux-stable:
       --locked
       --release
       --verbose
-      --features runtime-benchmarks,try-runtime,experimental
+      --features runtime-benchmarks,try-runtime,experimental,unsafe-debug
       --manifest-path ./bin/node/cli/Cargo.toml
       --partition count:${CI_NODE_INDEX}/${CI_NODE_TOTAL}
     # we need to update cache only from one job