[contracts] Make debug buffer work like a FIFO pipe (#12953)

* make debug buffer work like a FIFO pipe

* remove unused Error type

* Remove panics

* Update frame/contracts/src/exec.rs

Co-authored-by: Sasha Gryaznov <hi@agryaznov.com>

Co-authored-by: Alexander Theißen <alex.theissen@me.com>

Author: agryaznov

frame/contracts/src/exec.rs

@@ -279,7 +279,7 @@ pub trait Ext: sealing::Sealed {
 	/// when the code is executing on-chain.
 	///
 	/// Returns `true` if debug message recording is enabled. Otherwise `false` is returned.
-	fn append_debug_buffer(&mut self, msg: &str) -> Result<bool, DispatchError>;
+	fn append_debug_buffer(&mut self, msg: &str) -> bool;
 
 	/// Call some dispatchable and return the result.
 	fn call_runtime(&self, call: <Self::T as Config>::RuntimeCall) -> DispatchResultWithPostInfo;
@@ -1334,16 +1334,34 @@ where
 		&mut self.top_frame_mut().nested_gas
 	}
 
-	fn append_debug_buffer(&mut self, msg: &str) -> Result<bool, DispatchError> {
+	fn append_debug_buffer(&mut self, msg: &str) -> bool {
 		if let Some(buffer) = &mut self.debug_message {
-			if !msg.is_empty() {
-				buffer
-					.try_extend(&mut msg.bytes())
-					.map_err(|_| Error::<T>::DebugBufferExhausted)?;
-			}
-			Ok(true)
+			let mut msg = msg.bytes();
+			let num_drain = {
+				let capacity = DebugBufferVec::<T>::bound().checked_sub(buffer.len()).expect(
+					"
+					`buffer` is of type `DebugBufferVec`,
+					`DebugBufferVec` is a `BoundedVec`,
+					`BoundedVec::len()` <= `BoundedVec::bound()`;
+					qed
+				",
+				);
+				msg.len().saturating_sub(capacity).min(buffer.len())
+			};
+			buffer.drain(0..num_drain);
+			buffer
+				.try_extend(&mut msg)
+				.map_err(|_| {
+					log::debug!(
+						target: "runtime::contracts",
+						"Debug message to big (size={}) for debug buffer (bound={})",
+						msg.len(), DebugBufferVec::<T>::bound(),
+					);
+				})
+				.ok();
+			true
 		} else {
-			Ok(false)
+			false
 		}
 	}
 
@@ -2511,12 +2529,8 @@ mod tests {
 	#[test]
 	fn printing_works() {
 		let code_hash = MockLoader::insert(Call, |ctx, _| {
-			ctx.ext
-				.append_debug_buffer("This is a test")
-				.expect("Maximum allowed debug buffer size exhausted!");
-			ctx.ext
-				.append_debug_buffer("More text")
-				.expect("Maximum allowed debug buffer size exhausted!");
+			ctx.ext.append_debug_buffer("This is a test");
+			ctx.ext.append_debug_buffer("More text");
 			exec_success()
 		});
 
@@ -2549,12 +2563,8 @@ mod tests {
 	#[test]
 	fn printing_works_on_fail() {
 		let code_hash = MockLoader::insert(Call, |ctx, _| {
-			ctx.ext
-				.append_debug_buffer("This is a test")
-				.expect("Maximum allowed debug buffer size exhausted!");
-			ctx.ext
-				.append_debug_buffer("More text")
-				.expect("Maximum allowed debug buffer size exhausted!");
+			ctx.ext.append_debug_buffer("This is a test");
+			ctx.ext.append_debug_buffer("More text");
 			exec_trapped()
 		});
 
@@ -2587,7 +2597,7 @@ mod tests {
 	#[test]
 	fn debug_buffer_is_limited() {
 		let code_hash = MockLoader::insert(Call, move |ctx, _| {
-			ctx.ext.append_debug_buffer("overflowing bytes")?;
+			ctx.ext.append_debug_buffer("overflowing bytes");
 			exec_success()
 		});
 
@@ -2602,20 +2612,22 @@ mod tests {
 			set_balance(&ALICE, min_balance * 10);
 			place_contract(&BOB, code_hash);
 			let mut storage_meter = storage::meter::Meter::new(&ALICE, Some(0), 0).unwrap();
-			assert_err!(
-				MockStack::run_call(
-					ALICE,
-					BOB,
-					&mut gas_meter,
-					&mut storage_meter,
-					&schedule,
-					0,
-					vec![],
-					Some(&mut debug_buffer),
-					Determinism::Deterministic,
-				)
-				.map_err(|e| e.error),
-				Error::<Test>::DebugBufferExhausted
+			MockStack::run_call(
+				ALICE,
+				BOB,
+				&mut gas_meter,
+				&mut storage_meter,
+				&schedule,
+				0,
+				vec![],
+				Some(&mut debug_buffer),
+				Determinism::Deterministic,
+			)
+			.unwrap();
+			assert_eq!(
+				&String::from_utf8(debug_buffer[DebugBufferVec::<Test>::bound() - 17..].to_vec())
+					.unwrap(),
+				"overflowing bytes"
 			);
 		});
 	}

frame/contracts/src/lib.rs

@@ -857,9 +857,6 @@ pub mod pallet {
 		CodeRejected,
 		/// An indetermistic code was used in a context where this is not permitted.
 		Indeterministic,
-		/// The debug buffer size used during contract execution exceeded the limit determined by
-		/// the `MaxDebugBufferLen` pallet config parameter.
-		DebugBufferExhausted,
 	}
 
 	/// A mapping from an original code hash to the original code, untouched by instrumentation.

frame/contracts/src/wasm/mod.rs

@@ -588,9 +588,9 @@ mod tests {
 		fn gas_meter(&mut self) -> &mut GasMeter<Self::T> {
 			&mut self.gas_meter
 		}
-		fn append_debug_buffer(&mut self, msg: &str) -> Result<bool, DispatchError> {
+		fn append_debug_buffer(&mut self, msg: &str) -> bool {
 			self.debug_buffer.extend(msg.as_bytes());
-			Ok(true)
+			true
 		}
 		fn call_runtime(
 			&self,

frame/contracts/src/wasm/runtime.rs

@@ -2380,11 +2380,11 @@ pub mod env {
 		str_len: u32,
 	) -> Result<ReturnCode, TrapReason> {
 		ctx.charge_gas(RuntimeCosts::DebugMessage)?;
-		if ctx.ext.append_debug_buffer("")? {
+		if ctx.ext.append_debug_buffer("") {
 			let data = ctx.read_sandbox_memory(memory, str_ptr, str_len)?;
 			let msg =
 				core::str::from_utf8(&data).map_err(|_| <Error<E::T>>::DebugMessageInvalidUTF8)?;
-			ctx.ext.append_debug_buffer(msg)?;
+			ctx.ext.append_debug_buffer(msg);
 			return Ok(ReturnCode::Success)
 		}
 		Ok(ReturnCode::LoggingDisabled)