[pallet-revive] fix tracing collect

prdoc/pr_9736.prdoc

@@ -0,0 +1,12 @@
+title: '[pallet-revive] fix tracing collect'
+doc:
+- audience: Runtime Dev
+  description: |-
+    Fix tracing collection
+
+    collect_trace should take `self` instead of `&mut self`, to avoid reusing the tracer state when tracing multiple transactions in a block.
+
+    Fix https://github.com/paritytech/contract-issues/issues/156https://github.com/paritytech/contract-issues/issues/156
+crates:
+- name: pallet-revive
+  bump: patch

substrate/frame/revive/src/evm/api/debug_rpc_types.rs

@@ -291,6 +291,9 @@ pub struct CallTrace<Gas = U256> {
 	/// Type of call.
 	#[serde(rename = "type")]
 	pub call_type: CallType,
+	/// Number of child calls entered (for log position calculation)
+	#[serde(skip)]
+	pub child_call_count: u32,
 }
 
 /// A log emitted during a call.

substrate/frame/revive/src/evm/tracing.rs

@@ -60,7 +60,7 @@ where
 	}
 
 	/// Collect the traces and return them.
-	pub fn collect_trace(&mut self) -> Option<Trace> {
+	pub fn collect_trace(self) -> Option<Trace> {
 		match self {
 			Tracer::CallTracer(inner) => inner.collect_trace().map(Trace::Call),
 			Tracer::PrestateTracer(inner) => Some(inner.collect_trace().into()),

substrate/frame/revive/src/evm/tracing/call_tracing.rs

@@ -51,8 +51,8 @@ impl<Gas, GasMapper> CallTracer<Gas, GasMapper> {
 	}
 
 	/// Collect the traces and return them.
-	pub fn collect_trace(&mut self) -> Option<CallTrace<Gas>> {
-		core::mem::take(&mut self.traces).pop()
+	pub fn collect_trace(mut self) -> Option<CallTrace<Gas>> {
+		self.traces.pop()
 	}
 }
 
@@ -71,6 +71,13 @@ impl<Gas: Default, GasMapper: Fn(Weight) -> Gas> Tracing for CallTracer<Gas, Gas
 		input: &[u8],
 		gas_left: Weight,
 	) {
+		// Increment parent's child call count.
+		if let Some(&index) = self.current_stack.last() {
+			if let Some(trace) = self.traces.get_mut(index) {
+				trace.child_call_count += 1;
+			}
+		}
+
 		if self.traces.is_empty() || !self.config.only_top_call {
 			let (call_type, input) = match self.code_with_salt.take() {
 				Some((Code::Upload(v), salt)) => (
@@ -121,12 +128,17 @@ impl<Gas: Default, GasMapper: Fn(Weight) -> Gas> Tracing for CallTracer<Gas, Gas
 		}
 
 		let current_index = self.current_stack.last().unwrap();
-		let position = self.traces[*current_index].calls.len() as u32;
-		let log =
-			CallLog { address, topics: topics.to_vec(), data: data.to_vec().into(), position };
 
-		let current_index = *self.current_stack.last().unwrap();
-		self.traces[current_index].logs.push(log);
+		if let Some(trace) = self.traces.get_mut(*current_index) {
+			let log = CallLog {
+				address,
+				topics: topics.to_vec(),
+				data: data.to_vec().into(),
+				position: trace.child_call_count,
+			};
+
+			trace.logs.push(log);
+		}
 	}
 
 	fn exit_child_span(&mut self, output: &ExecReturnValue, gas_used: Weight) {

substrate/frame/revive/src/evm/tracing/prestate_tracing.rs

@@ -63,9 +63,8 @@ where
 	}
 
 	/// Collect the traces and return them.
-	pub fn collect_trace(&mut self) -> PrestateTrace {
-		let trace = core::mem::take(&mut self.trace);
-		let (mut pre, mut post) = trace;
+	pub fn collect_trace(self) -> PrestateTrace {
+		let (mut pre, mut post) = self.trace;
 		let include_code = !self.config.disable_code;
 
 		let is_empty = |info: &PrestateTraceInfo| {

substrate/frame/revive/src/lib.rs

@@ -2059,11 +2059,11 @@ macro_rules! impl_runtime_apis_plus_revive {
 					tracer_type: $crate::evm::TracerType,
 				) -> Vec<(u32, $crate::evm::Trace)> {
 					use $crate::{sp_runtime::traits::Block, tracing::trace};
-					let mut tracer = $crate::Pallet::<Self>::evm_tracer(tracer_type);
 					let mut traces = vec![];
 					let (header, extrinsics) = block.deconstruct();
 					<$Executive>::initialize_block(&header);
 					for (index, ext) in extrinsics.into_iter().enumerate() {
+						let mut tracer = $crate::Pallet::<Self>::evm_tracer(tracer_type.clone());
 						let t = tracer.as_tracing();
 						let _ = trace(t, || <$Executive>::apply_extrinsic(ext));
 
@@ -2104,7 +2104,7 @@ macro_rules! impl_runtime_apis_plus_revive {
 					tracer_type: $crate::evm::TracerType,
 				) -> Result<$crate::evm::Trace, $crate::EthTransactError> {
 					use $crate::tracing::trace;
-					let mut tracer = $crate::Pallet::<Self>::evm_tracer(tracer_type);
+					let mut tracer = $crate::Pallet::<Self>::evm_tracer(tracer_type.clone());
 					let t = tracer.as_tracing();
 
 					t.watch_address(&tx.from.unwrap_or_default());
@@ -2116,7 +2116,7 @@ macro_rules! impl_runtime_apis_plus_revive {
 					} else if let Err(err) = result {
 						Err(err)
 					} else {
-						Ok(tracer.empty_trace())
+						Ok($crate::Pallet::<Self>::evm_tracer(tracer_type).empty_trace())
 					}
 				}
 

substrate/frame/revive/src/tests/pvm.rs

@@ -4051,8 +4051,9 @@ fn call_tracing_works() {
 
 		let tracer_configs = vec![
 			 CallTracerConfig{ with_logs: false, only_top_call: false},
-			 CallTracerConfig{ with_logs: false, only_top_call: false},
+			 CallTracerConfig{ with_logs: true, only_top_call: false},
 			 CallTracerConfig{ with_logs: false, only_top_call: true},
+			 CallTracerConfig{ with_logs: true, only_top_call: true},
 		];
 
 		// Verify that the first trace report the same weight reported by bare_call
@@ -4154,12 +4155,15 @@ fn call_tracing_works() {
 													..Default::default()
 												}
 											],
+											child_call_count: 1,
 											..Default::default()
 										},
 									],
+									child_call_count: 2,
 									..Default::default()
 								},
 							],
+							child_call_count: 2,
 							..Default::default()
 						},
 					]
@@ -4179,6 +4183,7 @@ fn call_tracing_works() {
 					logs: logs.clone(),
 					value: Some(U256::from(0)),
 					calls: calls,
+					child_call_count: 2,
 					..Default::default()
 				};
 
@@ -4244,6 +4249,7 @@ fn create_call_tracing_works() {
 					call_type: CallType::Create2,
 					..Default::default()
 				},],
+				child_call_count: 1,
 				..Default::default()
 			}
 		);