Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Contracts: Update Config::Debug #14789

Merged
merged 26 commits into from Aug 24, 2023
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion bin/node/runtime/Cargo.toml
Expand Up @@ -373,4 +373,3 @@ try-runtime = [
"frame-election-provider-support/try-runtime",
"sp-runtime/try-runtime"
]
unsafe-debug = ["pallet-contracts/unsafe-debug"]
3 changes: 1 addition & 2 deletions bin/node/runtime/src/lib.rs
Expand Up @@ -1263,8 +1263,7 @@ 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 = ();
type Tracing = ();
}

impl pallet_sudo::Config for Runtime {
Expand Down
1 change: 0 additions & 1 deletion frame/contracts/Cargo.toml
Expand Up @@ -114,4 +114,3 @@ try-runtime = [
"pallet-utility/try-runtime",
"sp-runtime/try-runtime",
]
unsafe-debug = []
14 changes: 4 additions & 10 deletions frame/contracts/src/exec.rs
Expand Up @@ -15,11 +15,10 @@
// 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, meter::Diff, WriteOutcome},
tracing::{CallSpan, Tracing},
BalanceOf, CodeHash, CodeInfo, CodeInfoOf, Config, ContractInfo, ContractInfoOf,
DebugBufferVec, Determinism, Error, Event, Nonce, Origin, Pallet as Contracts, Schedule,
WasmBlob, LOG_TARGET,
Expand Down Expand Up @@ -908,20 +907,15 @@ 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())
};
let call_span =
T::Tracing::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 })?;

#[cfg(feature = "unsafe-debug")]
T::Debug::after_call(&code_hash, entry_point, input_clone, &output);
call_span.after_call(&output);

// Avoid useless work that would be reverted anyways.
if output.did_revert() {
Expand Down
12 changes: 4 additions & 8 deletions frame/contracts/src/lib.rs
Expand Up @@ -97,7 +97,7 @@ mod wasm;

pub mod chain_extension;
pub mod migration;
pub mod unsafe_debug;
pub mod tracing;
pub mod weights;

#[cfg(test)]
Expand Down Expand Up @@ -141,6 +141,7 @@ pub use crate::{
migration::{MigrateSequence, Migration, NoopMigration},
pallet::*,
schedule::{HostFnWeights, InstructionWeights, Limits, Schedule},
tracing::Tracing,
wasm::Determinism,
};
pub use weights::WeightInfo;
Expand Down Expand Up @@ -353,13 +354,8 @@ pub mod pallet {
/// ```
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>;
/// Defines methods to capture and trace contract calls for improved observability.
type Tracing: Tracing<Self>;
}

#[pallet::hooks]
Expand Down
10 changes: 6 additions & 4 deletions frame/contracts/src/tests.rs
Expand Up @@ -16,9 +16,12 @@
// limitations under the License.

mod pallet_dummy;
mod unsafe_debug;
mod test_tracing;

use self::test_utils::{ensure_stored, expected_deposit, hash};
use self::{
test_tracing::TestTracing,
test_utils::{ensure_stored, expected_deposit, hash},
};
use crate::{
self as pallet_contracts,
chain_extension::{
Expand Down Expand Up @@ -479,8 +482,7 @@ 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;
type Tracing = TestTracing;
}

pub const ALICE: AccountId32 = AccountId32::new([1u8; 32]);
Expand Down
@@ -1,7 +1,5 @@
#![cfg(feature = "unsafe-debug")]

use super::*;
use crate::unsafe_debug::{ExecutionObserver, ExportedFunction};
use crate::tracing::{CallSpan, ExportedFunction, Tracing};
use frame_support::traits::Currency;
use pallet_contracts_primitives::ExecReturnValue;
use pretty_assertions::assert_eq;
Expand All @@ -19,31 +17,40 @@ thread_local! {
static DEBUG_EXECUTION_TRACE: RefCell<Vec<DebugFrame>> = RefCell::new(Vec::new());
}

pub struct TestDebugger;
pub struct TestTracing;
pub struct TestCallSpan {
code_hash: CodeHash<Test>,
call: ExportedFunction,
input: Vec<u8>,
}

impl Tracing<Test> for TestTracing {
type CallSpan = TestCallSpan;

impl ExecutionObserver<CodeHash<Test>> for TestDebugger {
fn before_call(code_hash: &CodeHash<Test>, entry_point: ExportedFunction, input_data: &[u8]) {
fn new_call_span(
code_hash: &CodeHash<Test>,
entry_point: ExportedFunction,
input_data: &[u8],
) -> TestCallSpan {
DEBUG_EXECUTION_TRACE.with(|d| {
d.borrow_mut().push(DebugFrame {
code_hash: code_hash.clone(),
code_hash: *code_hash,
call: entry_point,
input: input_data.to_vec(),
result: None,
})
});
TestCallSpan { code_hash: *code_hash, call: entry_point, input: input_data.to_vec() }
}
}

fn after_call(
code_hash: &CodeHash<Test>,
entry_point: ExportedFunction,
input_data: Vec<u8>,
output: &ExecReturnValue,
) {
impl CallSpan for TestCallSpan {
fn after_call(self, output: &ExecReturnValue) {
DEBUG_EXECUTION_TRACE.with(|d| {
d.borrow_mut().push(DebugFrame {
code_hash: code_hash.clone(),
call: entry_point,
input: input_data,
code_hash: self.code_hash,
call: self.call,
input: self.input,
result: Some(output.data.clone()),
})
});
Expand Down
48 changes: 48 additions & 0 deletions frame/contracts/src/tracing.rs
@@ -0,0 +1,48 @@
pub use crate::exec::ExportedFunction;
use crate::{CodeHash, Config, LOG_TARGET};
use pallet_contracts_primitives::ExecReturnValue;

/// CallSpan defines methods to capture contract calls, enabling external observers to
/// measure, trace, and react to contract interactions.
pub trait Tracing<T: Config> {
type CallSpan: CallSpan;

/// Creates a new call span to encompass the upcoming contract execution.
///
/// This method should be invoked just before the execution of a contract and
/// marks the beginning of a traceable span of execution.
///
/// # 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 new_call_span(
code_hash: &CodeHash<T>,
entry_point: ExportedFunction,
input_data: &[u8],
) -> Self::CallSpan;
}

pub trait CallSpan {
/// Called just after the execution of a contract.
///
/// # Arguments
///
/// * `output` - The raw output of the call.
fn after_call(self, output: &ExecReturnValue);
}

impl<T: Config> Tracing<T> for () {
type CallSpan = ();

fn new_call_span(code_hash: &CodeHash<T>, entry_point: ExportedFunction, input_data: &[u8]) {
log::trace!(target: LOG_TARGET, "call {entry_point:?} hash: {code_hash:?}, input_data: {input_data:?}")
}
}

impl CallSpan for () {
fn after_call(self, output: &ExecReturnValue) {
log::trace!(target: LOG_TARGET, "call result {output:?}")
}
}
47 changes: 0 additions & 47 deletions frame/contracts/src/unsafe_debug.rs

This file was deleted.

2 changes: 1 addition & 1 deletion scripts/ci/gitlab/pipeline/test.yml
Expand Up @@ -223,7 +223,7 @@ test-linux-stable:
--locked
--release
--verbose
--features runtime-benchmarks,try-runtime,experimental,unsafe-debug
--features runtime-benchmarks,try-runtime,experimental
--manifest-path ./bin/node/cli/Cargo.toml
--partition count:${CI_NODE_INDEX}/${CI_NODE_TOTAL}
# we need to update cache only from one job
Expand Down