Skip to content

Commit

Permalink
Stabilize seal_debug_message (paritytech#9550)
Browse files Browse the repository at this point in the history
* Stableize `seal_debug_message`

* Update changelog

* Enable more tests

* Cargo fmt
  • Loading branch information
atenjin committed Sep 6, 2021
1 parent d8c5d3d commit 6cb34cf
Show file tree
Hide file tree
Showing 10 changed files with 14 additions and 27 deletions.
5 changes: 3 additions & 2 deletions frame/contracts/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,10 @@ In other words: Upgrading this pallet will not break pre-existing contracts.

### Changed

- Replaced `seal_println` with the **unstable** `seal_debug_message` API which allows
output to an RPC client.
- Replaced `seal_println` with the `seal_debug_message` API which allows outputting debug
messages to the console and RPC clients.
[#8773](https://github.com/paritytech/substrate/pull/8773)
[#9550](https://github.com/paritytech/substrate/pull/9550)

- Make storage and fields of `Schedule` private to the crate.
[#8359](https://github.com/paritytech/substrate/pull/8359)
Expand Down
2 changes: 1 addition & 1 deletion frame/contracts/fixtures/debug_message_invalid_utf8.wat
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
;; Emit a "Hello World!" debug message
(module
(import "__unstable__" "seal_debug_message" (func $seal_debug_message (param i32 i32) (result i32)))
(import "seal0" "seal_debug_message" (func $seal_debug_message (param i32 i32) (result i32)))
(import "env" "memory" (memory 1 1))

(data (i32.const 0) "\fc")
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
;; Emit a "Hello World!" debug message but assume that logging is disabled.
(module
(import "__unstable__" "seal_debug_message" (func $seal_debug_message (param i32 i32) (result i32)))
(import "seal0" "seal_debug_message" (func $seal_debug_message (param i32 i32) (result i32)))
(import "env" "memory" (memory 1 1))

(data (i32.const 0) "Hello World!")
Expand Down
2 changes: 1 addition & 1 deletion frame/contracts/fixtures/debug_message_works.wat
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
;; Emit a "Hello World!" debug message
(module
(import "__unstable__" "seal_debug_message" (func $seal_debug_message (param i32 i32) (result i32)))
(import "seal0" "seal_debug_message" (func $seal_debug_message (param i32 i32) (result i32)))
(import "env" "memory" (memory 1 1))

(data (i32.const 0) "Hello World!")
Expand Down
2 changes: 1 addition & 1 deletion frame/contracts/src/benchmarking/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1036,7 +1036,7 @@ benchmarks! {
let code = WasmModule::<T>::from(ModuleDefinition {
memory: Some(ImportedMemory { min_pages: 1, max_pages: 1 }),
imported_functions: vec![ImportedFunction {
module: "__unstable__",
module: "seal0",
name: "seal_debug_message",
params: vec![ValueType::I32, ValueType::I32],
return_type: Some(ValueType::I32),
Expand Down
3 changes: 0 additions & 3 deletions frame/contracts/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2856,7 +2856,6 @@ fn reinstrument_does_charge() {
}

#[test]
#[cfg(feature = "unstable-interface")]
fn debug_message_works() {
let (wasm, code_hash) = compile_module::<Test>("debug_message_works").unwrap();

Expand Down Expand Up @@ -2888,7 +2887,6 @@ fn debug_message_works() {
}

#[test]
#[cfg(feature = "unstable-interface")]
fn debug_message_logging_disabled() {
let (wasm, code_hash) = compile_module::<Test>("debug_message_logging_disabled").unwrap();

Expand Down Expand Up @@ -2928,7 +2926,6 @@ fn debug_message_logging_disabled() {
}

#[test]
#[cfg(feature = "unstable-interface")]
fn debug_message_invalid_utf8() {
let (wasm, code_hash) = compile_module::<Test>("debug_message_invalid_utf8").unwrap();

Expand Down
6 changes: 2 additions & 4 deletions frame/contracts/src/wasm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2108,11 +2108,10 @@ mod tests {
}

#[test]
#[cfg(feature = "unstable-interface")]
fn debug_message_works() {
const CODE_DEBUG_MESSAGE: &str = r#"
(module
(import "__unstable__" "seal_debug_message" (func $seal_debug_message (param i32 i32) (result i32)))
(import "seal0" "seal_debug_message" (func $seal_debug_message (param i32 i32) (result i32)))
(import "env" "memory" (memory 1 1))
(data (i32.const 0) "Hello World!")
Expand All @@ -2139,11 +2138,10 @@ mod tests {
}

#[test]
#[cfg(feature = "unstable-interface")]
fn debug_message_invalid_utf8_fails() {
const CODE_DEBUG_MESSAGE_FAIL: &str = r#"
(module
(import "__unstable__" "seal_debug_message" (func $seal_debug_message (param i32 i32) (result i32)))
(import "seal0" "seal_debug_message" (func $seal_debug_message (param i32 i32) (result i32)))
(import "env" "memory" (memory 1 1))
(data (i32.const 0) "\fc")
Expand Down
13 changes: 1 addition & 12 deletions frame/contracts/src/wasm/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ pub enum ReturnCode {
NotCallable = 8,
/// The call to `seal_debug_message` had no effect because debug message
/// recording was disabled.
#[cfg(feature = "unstable-interface")]
LoggingDisabled = 9,
}

Expand Down Expand Up @@ -196,7 +195,6 @@ pub enum RuntimeCosts {
/// Weight of calling `seal_deposit_event` with the given number of topics and event size.
DepositEvent{num_topic: u32, len: u32},
/// Weight of calling `seal_debug_message`.
#[cfg(feature = "unstable-interface")]
DebugMessage,
/// Weight of calling `seal_set_rent_allowance`.
SetRentAllowance,
Expand Down Expand Up @@ -265,7 +263,6 @@ impl RuntimeCosts {
DepositEvent{num_topic, len} => s.deposit_event
.saturating_add(s.deposit_event_per_topic.saturating_mul(num_topic.into()))
.saturating_add(s.deposit_event_per_byte.saturating_mul(len.into())),
#[cfg(feature = "unstable-interface")]
DebugMessage => s.debug_message,
SetRentAllowance => s.set_rent_allowance,
SetStorage(len) => s.set_storage
Expand Down Expand Up @@ -2008,15 +2005,7 @@ define_env!(Env, <E: Ext>,
// not being executed as an RPC. For example, they could allow users to disable logging
// through compile time flags (cargo features) for on-chain deployment. Additionally, the
// return value of this function can be cached in order to prevent further calls at runtime.
[__unstable__] seal_debug_message(ctx, str_ptr: u32, str_len: u32) -> ReturnCode => {
let mut protege = SealPrintln::default();
let _guard = EnvTraceGuard::<E::T, _>::new(&protege);

// We move this part outside the if closure part, may cause different behaviour to other pallet-contracts.
// But this `seal` is an unstable feature, thus we do not worry about it.
let data = ctx.read_sandbox_memory(str_ptr, str_len)?;
log::info!(target: "runtime::contracts", "[Contracts]: {}", core::str::from_utf8(&data).unwrap_or("<Invalid UTF8>"));

[seal0] seal_debug_message(ctx, str_ptr: u32, str_len: u32) -> ReturnCode => {
ctx.charge_gas(RuntimeCosts::DebugMessage)?;
if ctx.ext.append_debug_buffer("") {
// let data = ctx.read_sandbox_memory(str_ptr, str_len)?;
Expand Down
3 changes: 2 additions & 1 deletion frame/election-provider-multi-phase/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1386,7 +1386,8 @@ impl<T: Config> Pallet<T> {
// Check that assignment.who is actually a voter (defensive-only).
// NOTE: while using the index map from `voter_index` is better than a blind linear
// search, this *still* has room for optimization. Note that we had the index when
// we did `compact -> assignment` and we lost it. Ideal is to keep the index around.
// we did `solution -> assignment` and we lost it. Ideal is to keep the index
// around.

// Defensive-only: must exist in the snapshot.
let snapshot_index =
Expand Down
3 changes: 2 additions & 1 deletion frame/support/src/storage/migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,8 @@ pub fn put_storage_value<T: Encode>(module: &[u8], item: &[u8], hash: &[u8], val
frame_support::storage::unhashed::put(&key, &value);
}

/// Get a particular value in storage by the `module`, the map's `item` name and the key `hash`.
/// Remove all items under a storage prefix by the `module`, the map's `item` name and the key
/// `hash`.
pub fn remove_storage_prefix(module: &[u8], item: &[u8], hash: &[u8]) {
let mut key = vec![0u8; 32 + hash.len()];
key[0..16].copy_from_slice(&Twox128::hash(module));
Expand Down

0 comments on commit 6cb34cf

Please sign in to comment.