Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

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

Merged
merged 6 commits into from
Dec 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 48 additions & 36 deletions frame/contracts/src/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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!(
agryaznov marked this conversation as resolved.
Show resolved Hide resolved
target: "runtime::contracts",
"Debug message to big (size={}) for debug buffer (bound={})",
msg.len(), DebugBufferVec::<T>::bound(),
);
})
.ok();
true
} else {
Ok(false)
false
}
}

Expand Down Expand Up @@ -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()
});

Expand Down Expand Up @@ -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()
});

Expand Down Expand Up @@ -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()
});

Expand All @@ -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"
);
});
}
Expand Down
3 changes: 0 additions & 3 deletions frame/contracts/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions frame/contracts/src/wasm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions frame/contracts/src/wasm/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down