From cdc033945741d79ebc87a630558b4fc507a51df6 Mon Sep 17 00:00:00 2001
From: PG Herveou <pgherveou@gmail.com>
Date: Mon, 24 Feb 2025 17:56:21 +0100
Subject: [PATCH] [pallet-revive] tracing should wrap around call stack
execution (#7676)
Fix tracing should wrap around the entire call stack execution
---------
Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
---
prdoc/pr_7676.prdoc | 7 ++++
substrate/frame/revive/src/exec.rs | 67 +++++++++++++++---------------
2 files changed, 41 insertions(+), 33 deletions(-)
create mode 100644 prdoc/pr_7676.prdoc
diff --git a/prdoc/pr_7676.prdoc b/prdoc/pr_7676.prdoc
new file mode 100644
index 00000000000..eea0b8c749f
--- /dev/null
+++ b/prdoc/pr_7676.prdoc
@@ -0,0 +1,7 @@
+title: '[pallet-revive] tracing should wrap around call stack execution'
+doc:
+- audience: Runtime Dev
+ description: Fix tracing should wrap around the entire call stack execution
+crates:
+- name: pallet-revive
+ bump: minor
diff --git a/substrate/frame/revive/src/exec.rs b/substrate/frame/revive/src/exec.rs
index 721254180bf..e13212f5e5e 100644
--- a/substrate/frame/revive/src/exec.rs
+++ b/substrate/frame/revive/src/exec.rs
@@ -1015,10 +1015,21 @@ where
fn run(&mut self, executable: E, input_data: Vec<u8>) -> Result<(), ExecError> {
let frame = self.top_frame();
let entry_point = frame.entry_point;
- let is_delegate_call = frame.delegate.is_some();
let delegated_code_hash =
if frame.delegate.is_some() { Some(*executable.code_hash()) } else { None };
+ if_tracing(|tracer| {
+ tracer.enter_child_span(
+ self.caller().account_id().map(T::AddressMapper::to_address).unwrap_or_default(),
+ T::AddressMapper::to_address(&frame.account_id),
+ frame.delegate.is_some(),
+ frame.read_only,
+ frame.value_transferred,
+ &input_data,
+ frame.nested_gas.gas_left(),
+ );
+ });
+
// The output of the caller frame will be replaced by the output of this run.
// It is also not accessible from nested frames.
// Hence we drop it early to save the memory.
@@ -1036,8 +1047,6 @@ where
let do_transaction = || -> ExecResult {
let caller = self.caller();
let frame = top_frame_mut!(self);
- let read_only = frame.read_only;
- let value_transferred = frame.value_transferred;
let account_id = &frame.account_id.clone();
// We need to make sure that the contract's account exists before calling its
@@ -1081,35 +1090,10 @@ where
)?;
}
- let contract_address = T::AddressMapper::to_address(account_id);
- let maybe_caller_address = caller.account_id().map(T::AddressMapper::to_address);
let code_deposit = executable.code_info().deposit();
-
- if_tracing(|tracer| {
- tracer.enter_child_span(
- maybe_caller_address.unwrap_or_default(),
- contract_address,
- is_delegate_call,
- read_only,
- value_transferred,
- &input_data,
- frame.nested_gas.gas_left(),
- );
- });
-
- let output = executable.execute(self, entry_point, input_data).map_err(|e| {
- if_tracing(|tracer| {
- tracer.exit_child_span_with_error(
- e.error,
- top_frame_mut!(self).nested_gas.gas_consumed(),
- );
- });
- ExecError { error: e.error, origin: ErrorOrigin::Callee }
- })?;
-
- if_tracing(|tracer| {
- tracer.exit_child_span(&output, top_frame_mut!(self).nested_gas.gas_consumed());
- });
+ let output = executable
+ .execute(self, entry_point, input_data)
+ .map_err(|e| ExecError { error: e.error, origin: ErrorOrigin::Callee })?;
// Avoid useless work that would be reverted anyways.
if output.did_revert() {
@@ -1157,10 +1141,27 @@ where
let (success, output) = match transaction_outcome {
// `with_transactional` executed successfully, and we have the expected output.
- Ok((success, output)) => (success, output),
+ Ok((success, output)) => {
+ if_tracing(|tracer| {
+ let gas_consumed = top_frame!(self).nested_gas.gas_consumed();
+ match &output {
+ Ok(output) => tracer.exit_child_span(&output, gas_consumed),
+ Err(e) => tracer.exit_child_span_with_error(e.error.into(), gas_consumed),
+ }
+ });
+
+ (success, output)
+ },
// `with_transactional` returned an error, and we propagate that error and note no state
// has changed.
- Err(error) => (false, Err(error.into())),
+ Err(error) => {
+ if_tracing(|tracer| {
+ let gas_consumed = top_frame!(self).nested_gas.gas_consumed();
+ tracer.exit_child_span_with_error(error.into(), gas_consumed);
+ });
+
+ (false, Err(error.into()))
+ },
};
if success {
--
GitLab