[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>

Author: 2 people

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

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 {