diff --git a/prdoc/pr_7676.prdoc b/prdoc/pr_7676.prdoc new file mode 100644 index 0000000000000000000000000000000000000000..eea0b8c749f6d72fb06fcac011f2fee34211733c --- /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 721254180bf5d69fcb0d941d7bcf6270eba29fc7..e13212f5e5ee56fd7497d75a16343b36c6e76058 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 {