Skip to content

Commit

Permalink
[pallet-revive] Remove debug buffer (paritytech#7163)
Browse files Browse the repository at this point in the history
Remove the `debug_buffer` feature

---------

Co-authored-by: command-bot <>
Co-authored-by: Cyrill Leutwiler <cyrill@parity.io>
Co-authored-by: Alexander Theißen <alex.theissen@me.com>
  • Loading branch information
3 people authored and Nathy-bajo committed Jan 21, 2025
1 parent a0c032e commit 8895686
Show file tree
Hide file tree
Showing 23 changed files with 54 additions and 767 deletions.
15 changes: 3 additions & 12 deletions cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -952,11 +952,6 @@ parameter_types! {
pub CodeHashLockupDepositPercent: Perbill = Perbill::from_percent(30);
}

type EventRecord = frame_system::EventRecord<
<Runtime as frame_system::Config>::RuntimeEvent,
<Runtime as frame_system::Config>::Hash,
>;

impl pallet_revive::Config for Runtime {
type Time = Timestamp;
type Currency = Balances;
Expand Down Expand Up @@ -2073,7 +2068,7 @@ impl_runtime_apis! {
}
}

impl pallet_revive::ReviveApi<Block, AccountId, Balance, Nonce, BlockNumber, EventRecord> for Runtime
impl pallet_revive::ReviveApi<Block, AccountId, Balance, Nonce, BlockNumber> for Runtime
{
fn balance(address: H160) -> U256 {
Revive::evm_balance(&address)
Expand Down Expand Up @@ -2108,7 +2103,7 @@ impl_runtime_apis! {
gas_limit: Option<Weight>,
storage_deposit_limit: Option<Balance>,
input_data: Vec<u8>,
) -> pallet_revive::ContractResult<pallet_revive::ExecReturnValue, Balance, EventRecord> {
) -> pallet_revive::ContractResult<pallet_revive::ExecReturnValue, Balance> {
let blockweights= <Runtime as frame_system::Config>::BlockWeights::get();
Revive::bare_call(
RuntimeOrigin::signed(origin),
Expand All @@ -2117,8 +2112,6 @@ impl_runtime_apis! {
gas_limit.unwrap_or(blockweights.max_block),
pallet_revive::DepositLimit::Balance(storage_deposit_limit.unwrap_or(u128::MAX)),
input_data,
pallet_revive::DebugInfo::UnsafeDebug,
pallet_revive::CollectEvents::UnsafeCollect,
)
}

Expand All @@ -2130,7 +2123,7 @@ impl_runtime_apis! {
code: pallet_revive::Code,
data: Vec<u8>,
salt: Option<[u8; 32]>,
) -> pallet_revive::ContractResult<pallet_revive::InstantiateReturnValue, Balance, EventRecord>
) -> pallet_revive::ContractResult<pallet_revive::InstantiateReturnValue, Balance>
{
let blockweights= <Runtime as frame_system::Config>::BlockWeights::get();
Revive::bare_instantiate(
Expand All @@ -2141,8 +2134,6 @@ impl_runtime_apis! {
code,
data,
salt,
pallet_revive::DebugInfo::UnsafeDebug,
pallet_revive::CollectEvents::UnsafeCollect,
)
}

Expand Down
13 changes: 13 additions & 0 deletions prdoc/pr_7163.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
title: '[pallet-revive] Remove debug buffer'
doc:
- audience: Runtime Dev
description: Remove the `debug_buffer` feature
crates:
- name: asset-hub-westend-runtime
bump: minor
- name: pallet-revive
bump: major
- name: pallet-revive-proc-macro
bump: minor
- name: pallet-revive-uapi
bump: minor
10 changes: 3 additions & 7 deletions substrate/bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3212,7 +3212,7 @@ impl_runtime_apis! {
}
}

impl pallet_revive::ReviveApi<Block, AccountId, Balance, Nonce, BlockNumber, EventRecord> for Runtime
impl pallet_revive::ReviveApi<Block, AccountId, Balance, Nonce, BlockNumber> for Runtime
{
fn balance(address: H160) -> U256 {
Revive::evm_balance(&address)
Expand Down Expand Up @@ -3247,16 +3247,14 @@ impl_runtime_apis! {
gas_limit: Option<Weight>,
storage_deposit_limit: Option<Balance>,
input_data: Vec<u8>,
) -> pallet_revive::ContractResult<pallet_revive::ExecReturnValue, Balance, EventRecord> {
) -> pallet_revive::ContractResult<pallet_revive::ExecReturnValue, Balance> {
Revive::bare_call(
RuntimeOrigin::signed(origin),
dest,
value,
gas_limit.unwrap_or(RuntimeBlockWeights::get().max_block),
pallet_revive::DepositLimit::Balance(storage_deposit_limit.unwrap_or(u128::MAX)),
input_data,
pallet_revive::DebugInfo::UnsafeDebug,
pallet_revive::CollectEvents::UnsafeCollect,
)
}

Expand All @@ -3268,7 +3266,7 @@ impl_runtime_apis! {
code: pallet_revive::Code,
data: Vec<u8>,
salt: Option<[u8; 32]>,
) -> pallet_revive::ContractResult<pallet_revive::InstantiateReturnValue, Balance, EventRecord>
) -> pallet_revive::ContractResult<pallet_revive::InstantiateReturnValue, Balance>
{
Revive::bare_instantiate(
RuntimeOrigin::signed(origin),
Expand All @@ -3278,8 +3276,6 @@ impl_runtime_apis! {
code,
data,
salt,
pallet_revive::DebugInfo::UnsafeDebug,
pallet_revive::CollectEvents::UnsafeCollect,
)
}

Expand Down
23 changes: 0 additions & 23 deletions substrate/frame/revive/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,29 +49,6 @@ This module executes PolkaVM smart contracts. These can potentially be written i
RISC-V. For now, the only officially supported languages are Solidity (via [`revive`](https://github.com/xermicus/revive))
and Rust (check the `fixtures` directory for Rust examples).

## Debugging

Contracts can emit messages to the client when called as RPC through the
[`debug_message`](https://paritytech.github.io/substrate/master/pallet_revive/trait.SyscallDocs.html#tymethod.debug_message)
API.

Those messages are gathered into an internal buffer and sent to the RPC client. It is up to the individual client if
and how those messages are presented to the user.

This buffer is also printed as a debug message. In order to see these messages on the node console the log level for the
`runtime::revive` target needs to be raised to at least the `debug` level. However, those messages are easy to
overlook because of the noise generated by block production. A good starting point for observing them on the console is
using this command line in the root directory of the Substrate repository:

```bash
cargo run --release -- --dev -lerror,runtime::revive=debug
```

This raises the log level of `runtime::revive` to `debug` and all other targets to `error` in order to prevent them
from spamming the console.

`--dev`: Use a dev chain spec `--tmp`: Use temporary storage for chain data (the chain state is deleted on exit)

## Host function tracing

For contract authors, it can be a helpful debugging tool to see which host functions are called, with which arguments,
Expand Down

This file was deleted.

This file was deleted.

33 changes: 0 additions & 33 deletions substrate/frame/revive/fixtures/contracts/debug_message_works.rs

This file was deleted.

7 changes: 1 addition & 6 deletions substrate/frame/revive/proc-macro/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -510,12 +510,7 @@ fn expand_functions(def: &EnvDef) -> TokenStream2 {
quote! {
// wrap body in closure to make sure the tracing is always executed
let result = (|| #body)();
if ::log::log_enabled!(target: "runtime::revive::strace", ::log::Level::Trace) {
use core::fmt::Write;
let mut msg = alloc::string::String::default();
let _ = core::write!(&mut msg, #trace_fmt_str, #( #trace_fmt_args, )* result);
self.ext().append_debug_buffer(&msg);
}
::log::trace!(target: "runtime::revive::strace", #trace_fmt_str, #( #trace_fmt_args, )* result);
result
}
};
Expand Down
15 changes: 1 addition & 14 deletions substrate/frame/revive/src/benchmarking/call_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use crate::{
storage::meter::Meter,
transient_storage::MeterEntry,
wasm::{PreparedCall, Runtime},
BalanceOf, Config, DebugBuffer, Error, GasMeter, MomentOf, Origin, WasmBlob, Weight,
BalanceOf, Config, Error, GasMeter, MomentOf, Origin, WasmBlob, Weight,
};
use alloc::{vec, vec::Vec};
use frame_benchmarking::benchmarking;
Expand All @@ -38,7 +38,6 @@ pub struct CallSetup<T: Config> {
gas_meter: GasMeter<T>,
storage_meter: Meter<T>,
value: BalanceOf<T>,
debug_message: Option<DebugBuffer>,
data: Vec<u8>,
transient_storage_size: u32,
}
Expand Down Expand Up @@ -91,7 +90,6 @@ where
gas_meter: GasMeter::new(Weight::MAX),
storage_meter,
value: 0u32.into(),
debug_message: None,
data: vec![],
transient_storage_size: 0,
}
Expand Down Expand Up @@ -122,16 +120,6 @@ where
self.transient_storage_size = size;
}

/// Set the debug message.
pub fn enable_debug_message(&mut self) {
self.debug_message = Some(Default::default());
}

/// Get the debug message.
pub fn debug_message(&self) -> Option<DebugBuffer> {
self.debug_message.clone()
}

/// Get the call's input data.
pub fn data(&self) -> Vec<u8> {
self.data.clone()
Expand All @@ -150,7 +138,6 @@ where
&mut self.gas_meter,
&mut self.storage_meter,
self.value,
self.debug_message.as_mut(),
);
if self.transient_storage_size > 0 {
Self::with_transient_storage(&mut ext.0, self.transient_storage_size).unwrap();
Expand Down
28 changes: 0 additions & 28 deletions substrate/frame/revive/src/benchmarking/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,6 @@ where
Code::Upload(module.code),
data,
salt,
DebugInfo::Skip,
CollectEvents::Skip,
);

let address = outcome.result?.addr;
Expand Down Expand Up @@ -1047,32 +1045,6 @@ mod benchmarks {
);
}

// Benchmark debug_message call
// Whereas this function is used in RPC mode only, it still should be secured
// against an excessive use.
//
// i: size of input in bytes up to maximum allowed contract memory or maximum allowed debug
// buffer size, whichever is less.
#[benchmark]
fn seal_debug_message(
i: Linear<0, { (limits::code::BLOB_BYTES).min(limits::DEBUG_BUFFER_BYTES) }>,
) {
let mut setup = CallSetup::<T>::default();
setup.enable_debug_message();
let (mut ext, _) = setup.ext();
let mut runtime = crate::wasm::Runtime::<_, [u8]>::new(&mut ext, vec![]);
// Fill memory with printable ASCII bytes.
let mut memory = (0..i).zip((32..127).cycle()).map(|i| i.1).collect::<Vec<_>>();

let result;
#[block]
{
result = runtime.bench_debug_message(memory.as_mut_slice(), 0, i);
}
assert_ok!(result);
assert_eq!(setup.debug_message().unwrap().len() as u32, i);
}

#[benchmark(skip_meta, pov_mode = Measured)]
fn get_storage_empty() -> Result<(), BenchmarkError> {
let max_key_len = limits::STORAGE_KEY_BYTES;
Expand Down
Loading

0 comments on commit 8895686

Please sign in to comment.