From d3baa2a09c601e3cd907bbb6d54ca71bf0a1af67 Mon Sep 17 00:00:00 2001 From: pgherveou Date: Tue, 14 Jan 2025 22:40:46 +0100 Subject: [PATCH 1/5] rm debug buffer --- .../assets/asset-hub-westend/src/lib.rs | 15 +- substrate/bin/node/runtime/src/lib.rs | 10 +- substrate/frame/revive/README.md | 23 -- .../contracts/debug_message_invalid_utf8.rs | 33 --- .../debug_message_logging_disabled.rs | 33 --- .../fixtures/contracts/debug_message_works.rs | 33 --- substrate/frame/revive/proc-macro/src/lib.rs | 7 +- .../revive/src/benchmarking/call_builder.rs | 15 +- .../frame/revive/src/benchmarking/mod.rs | 28 --- substrate/frame/revive/src/exec.rs | 228 +----------------- substrate/frame/revive/src/lib.rs | 68 +----- substrate/frame/revive/src/limits.rs | 5 - substrate/frame/revive/src/primitives.rs | 53 +--- .../frame/revive/src/test_utils/builder.rs | 17 +- substrate/frame/revive/src/tests.rs | 148 +----------- .../frame/revive/src/tests/test_debug.rs | 144 +---------- substrate/frame/revive/src/wasm/mod.rs | 2 +- substrate/frame/revive/src/wasm/runtime.rs | 34 +-- substrate/frame/revive/src/weights.rs | 21 -- substrate/frame/revive/uapi/src/host.rs | 20 -- .../frame/revive/uapi/src/host/riscv64.rs | 7 - substrate/frame/revive/uapi/src/lib.rs | 3 - 22 files changed, 42 insertions(+), 905 deletions(-) delete mode 100644 substrate/frame/revive/fixtures/contracts/debug_message_invalid_utf8.rs delete mode 100644 substrate/frame/revive/fixtures/contracts/debug_message_logging_disabled.rs delete mode 100644 substrate/frame/revive/fixtures/contracts/debug_message_works.rs diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs index 71cfdc58cceb..5585dd8cfcca 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs @@ -953,11 +953,6 @@ parameter_types! { pub CodeHashLockupDepositPercent: Perbill = Perbill::from_percent(30); } -type EventRecord = frame_system::EventRecord< - ::RuntimeEvent, - ::Hash, ->; - impl pallet_revive::Config for Runtime { type Time = Timestamp; type Currency = Balances; @@ -2073,7 +2068,7 @@ impl_runtime_apis! { } } - impl pallet_revive::ReviveApi for Runtime + impl pallet_revive::ReviveApi for Runtime { fn balance(address: H160) -> U256 { Revive::evm_balance(&address) @@ -2108,7 +2103,7 @@ impl_runtime_apis! { gas_limit: Option, storage_deposit_limit: Option, input_data: Vec, - ) -> pallet_revive::ContractResult { + ) -> pallet_revive::ContractResult { let blockweights= ::BlockWeights::get(); Revive::bare_call( RuntimeOrigin::signed(origin), @@ -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, ) } @@ -2130,7 +2123,7 @@ impl_runtime_apis! { code: pallet_revive::Code, data: Vec, salt: Option<[u8; 32]>, - ) -> pallet_revive::ContractResult + ) -> pallet_revive::ContractResult { let blockweights= ::BlockWeights::get(); Revive::bare_instantiate( @@ -2141,8 +2134,6 @@ impl_runtime_apis! { code, data, salt, - pallet_revive::DebugInfo::UnsafeDebug, - pallet_revive::CollectEvents::UnsafeCollect, ) } diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index e11a009c1c3f..97728f12f5f9 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -3212,7 +3212,7 @@ impl_runtime_apis! { } } - impl pallet_revive::ReviveApi for Runtime + impl pallet_revive::ReviveApi for Runtime { fn balance(address: H160) -> U256 { Revive::evm_balance(&address) @@ -3247,7 +3247,7 @@ impl_runtime_apis! { gas_limit: Option, storage_deposit_limit: Option, input_data: Vec, - ) -> pallet_revive::ContractResult { + ) -> pallet_revive::ContractResult { Revive::bare_call( RuntimeOrigin::signed(origin), dest, @@ -3255,8 +3255,6 @@ impl_runtime_apis! { 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, ) } @@ -3268,7 +3266,7 @@ impl_runtime_apis! { code: pallet_revive::Code, data: Vec, salt: Option<[u8; 32]>, - ) -> pallet_revive::ContractResult + ) -> pallet_revive::ContractResult { Revive::bare_instantiate( RuntimeOrigin::signed(origin), @@ -3278,8 +3276,6 @@ impl_runtime_apis! { code, data, salt, - pallet_revive::DebugInfo::UnsafeDebug, - pallet_revive::CollectEvents::UnsafeCollect, ) } diff --git a/substrate/frame/revive/README.md b/substrate/frame/revive/README.md index 575920dfaac7..7538f77d10bc 100644 --- a/substrate/frame/revive/README.md +++ b/substrate/frame/revive/README.md @@ -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, diff --git a/substrate/frame/revive/fixtures/contracts/debug_message_invalid_utf8.rs b/substrate/frame/revive/fixtures/contracts/debug_message_invalid_utf8.rs deleted file mode 100644 index 6c850a9ec663..000000000000 --- a/substrate/frame/revive/fixtures/contracts/debug_message_invalid_utf8.rs +++ /dev/null @@ -1,33 +0,0 @@ -// This file is part of Substrate. - -// Copyright (C) Parity Technologies (UK) Ltd. -// SPDX-License-Identifier: Apache-2.0 - -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -//! Emit a debug message with an invalid utf-8 code. -#![no_std] -#![no_main] - -extern crate common; -use uapi::{HostFn, HostFnImpl as api}; - -#[no_mangle] -#[polkavm_derive::polkavm_export] -pub extern "C" fn deploy() {} - -#[no_mangle] -#[polkavm_derive::polkavm_export] -pub extern "C" fn call() { - api::debug_message(b"\xFC").unwrap(); -} diff --git a/substrate/frame/revive/fixtures/contracts/debug_message_logging_disabled.rs b/substrate/frame/revive/fixtures/contracts/debug_message_logging_disabled.rs deleted file mode 100644 index 0ce2b6b5628d..000000000000 --- a/substrate/frame/revive/fixtures/contracts/debug_message_logging_disabled.rs +++ /dev/null @@ -1,33 +0,0 @@ -// This file is part of Substrate. - -// Copyright (C) Parity Technologies (UK) Ltd. -// SPDX-License-Identifier: Apache-2.0 - -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -//! Emit a "Hello World!" debug message but assume that logging is disabled. -#![no_std] -#![no_main] - -extern crate common; -use uapi::{HostFn, HostFnImpl as api, ReturnErrorCode}; - -#[no_mangle] -#[polkavm_derive::polkavm_export] -pub extern "C" fn deploy() {} - -#[no_mangle] -#[polkavm_derive::polkavm_export] -pub extern "C" fn call() { - assert_eq!(api::debug_message(b"Hello World!"), Err(ReturnErrorCode::LoggingDisabled)); -} diff --git a/substrate/frame/revive/fixtures/contracts/debug_message_works.rs b/substrate/frame/revive/fixtures/contracts/debug_message_works.rs deleted file mode 100644 index 3a2509509d8f..000000000000 --- a/substrate/frame/revive/fixtures/contracts/debug_message_works.rs +++ /dev/null @@ -1,33 +0,0 @@ -// This file is part of Substrate. - -// Copyright (C) Parity Technologies (UK) Ltd. -// SPDX-License-Identifier: Apache-2.0 - -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -//! Emit a "Hello World!" debug message. -#![no_std] -#![no_main] - -extern crate common; -use uapi::{HostFn, HostFnImpl as api}; - -#[no_mangle] -#[polkavm_derive::polkavm_export] -pub extern "C" fn deploy() {} - -#[no_mangle] -#[polkavm_derive::polkavm_export] -pub extern "C" fn call() { - api::debug_message(b"Hello World!").unwrap(); -} diff --git a/substrate/frame/revive/proc-macro/src/lib.rs b/substrate/frame/revive/proc-macro/src/lib.rs index b09bdef14632..6e38063d20a6 100644 --- a/substrate/frame/revive/proc-macro/src/lib.rs +++ b/substrate/frame/revive/proc-macro/src/lib.rs @@ -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 } }; diff --git a/substrate/frame/revive/src/benchmarking/call_builder.rs b/substrate/frame/revive/src/benchmarking/call_builder.rs index 1177d47aadc3..077e18ff5f0b 100644 --- a/substrate/frame/revive/src/benchmarking/call_builder.rs +++ b/substrate/frame/revive/src/benchmarking/call_builder.rs @@ -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; @@ -38,7 +38,6 @@ pub struct CallSetup { gas_meter: GasMeter, storage_meter: Meter, value: BalanceOf, - debug_message: Option, data: Vec, transient_storage_size: u32, } @@ -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, } @@ -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 { - self.debug_message.clone() - } - /// Get the call's input data. pub fn data(&self) -> Vec { self.data.clone() @@ -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(); diff --git a/substrate/frame/revive/src/benchmarking/mod.rs b/substrate/frame/revive/src/benchmarking/mod.rs index e67c39ec0899..314bf7a6d990 100644 --- a/substrate/frame/revive/src/benchmarking/mod.rs +++ b/substrate/frame/revive/src/benchmarking/mod.rs @@ -107,8 +107,6 @@ where Code::Upload(module.code), data, salt, - DebugInfo::Skip, - CollectEvents::Skip, ); let address = outcome.result?.addr; @@ -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::::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::>(); - - 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; diff --git a/substrate/frame/revive/src/exec.rs b/substrate/frame/revive/src/exec.rs index a6a259149768..cf94a94ca476 100644 --- a/substrate/frame/revive/src/exec.rs +++ b/substrate/frame/revive/src/exec.rs @@ -24,8 +24,8 @@ use crate::{ runtime_decl_for_revive_api::{Decode, Encode, RuntimeDebugNoBound, TypeInfo}, storage::{self, meter::Diff, WriteOutcome}, transient_storage::TransientStorage, - BalanceOf, CodeInfo, CodeInfoOf, Config, ContractInfo, ContractInfoOf, DebugBuffer, Error, - Event, ImmutableData, ImmutableDataOf, Pallet as Contracts, LOG_TARGET, + BalanceOf, CodeInfo, CodeInfoOf, Config, ContractInfo, ContractInfoOf, Error, Event, + ImmutableData, ImmutableDataOf, Pallet as Contracts, }; use alloc::vec::Vec; use core::{fmt::Debug, marker::PhantomData, mem}; @@ -378,19 +378,6 @@ pub trait Ext: sealing::Sealed { /// Charges `diff` from the meter. fn charge_storage(&mut self, diff: &Diff); - /// Append a string to the debug buffer. - /// - /// It is added as-is without any additional new line. - /// - /// This is a no-op if debug message recording is disabled which is always the case - /// 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) -> bool; - - /// Returns `true` if debug message recording is enabled. Otherwise `false` is returned. - fn debug_buffer_enabled(&self) -> bool; - /// Call some dispatchable and return the result. fn call_runtime(&self, call: ::RuntimeCall) -> DispatchResultWithPostInfo; @@ -555,11 +542,6 @@ pub struct Stack<'a, T: Config, E> { frames: BoundedVec, ConstU32<{ limits::CALL_STACK_DEPTH }>>, /// Statically guarantee that each call stack has at least one frame. first_frame: Frame, - /// A text buffer used to output human readable information. - /// - /// All the bytes added to this field should be valid UTF-8. The buffer has no defined - /// structure and is intended to be shown to users as-is for debugging purposes. - debug_message: Option<&'a mut DebugBuffer>, /// Transient storage used to store data, which is kept for the duration of a transaction. transient_storage: TransientStorage, /// Whether or not actual transfer of funds should be performed. @@ -765,11 +747,6 @@ where { /// Create and run a new call stack by calling into `dest`. /// - /// # Note - /// - /// `debug_message` should only ever be set to `Some` when executing as an RPC because - /// it adds allocations and could be abused to drive the runtime into an OOM panic. - /// /// # Return Value /// /// Result<(ExecReturnValue, CodeSize), (ExecError, CodeSize)> @@ -781,7 +758,6 @@ where value: U256, input_data: Vec, skip_transfer: bool, - debug_message: Option<&'a mut DebugBuffer>, ) -> ExecResult { let dest = T::AddressMapper::to_account_id(&dest); if let Some((mut stack, executable)) = Self::new( @@ -791,7 +767,6 @@ where storage_meter, value, skip_transfer, - debug_message, )? { stack.run(executable, input_data).map(|_| stack.first_frame.last_frame_output) } else { @@ -801,11 +776,6 @@ where /// Create and run a new call stack by instantiating a new contract. /// - /// # Note - /// - /// `debug_message` should only ever be set to `Some` when executing as an RPC because - /// it adds allocations and could be abused to drive the runtime into an OOM panic. - /// /// # Return Value /// /// Result<(NewContractAccountId, ExecReturnValue), ExecError)> @@ -818,7 +788,6 @@ where input_data: Vec, salt: Option<&[u8; 32]>, skip_transfer: bool, - debug_message: Option<&'a mut DebugBuffer>, ) -> Result<(H160, ExecReturnValue), ExecError> { let (mut stack, executable) = Self::new( FrameArgs::Instantiate { @@ -832,7 +801,6 @@ where storage_meter, value, skip_transfer, - debug_message, )? .expect(FRAME_ALWAYS_EXISTS_ON_INSTANTIATE); let address = T::AddressMapper::to_address(&stack.top_frame().account_id); @@ -848,7 +816,6 @@ where gas_meter: &'a mut GasMeter, storage_meter: &'a mut storage::meter::Meter, value: BalanceOf, - debug_message: Option<&'a mut DebugBuffer>, ) -> (Self, E) { Self::new( FrameArgs::Call { @@ -861,7 +828,6 @@ where storage_meter, value.into(), false, - debug_message, ) .unwrap() .unwrap() @@ -878,7 +844,6 @@ where storage_meter: &'a mut storage::meter::Meter, value: U256, skip_transfer: bool, - debug_message: Option<&'a mut DebugBuffer>, ) -> Result, ExecError> { origin.ensure_mapped()?; let Some((first_frame, executable)) = Self::new_frame( @@ -903,7 +868,6 @@ where block_number: >::block_number(), first_frame, frames: Default::default(), - debug_message, transient_storage: TransientStorage::new(limits::TRANSIENT_STORAGE_BYTES), skip_transfer, _phantom: Default::default(), @@ -1260,13 +1224,6 @@ where } } } else { - if let Some((msg, false)) = self.debug_message.as_ref().map(|m| (m, m.is_empty())) { - log::debug!( - target: LOG_TARGET, - "Execution finished with debug buffer: {}", - core::str::from_utf8(msg).unwrap_or(""), - ); - } self.gas_meter.absorb_nested(mem::take(&mut self.first_frame.nested_gas)); if !persist { return; @@ -1769,28 +1726,6 @@ where self.top_frame_mut().nested_storage.charge(diff) } - fn debug_buffer_enabled(&self) -> bool { - self.debug_message.is_some() - } - - fn append_debug_buffer(&mut self, msg: &str) -> bool { - if let Some(buffer) = &mut self.debug_message { - buffer - .try_extend(&mut msg.bytes()) - .map_err(|_| { - log::debug!( - target: LOG_TARGET, - "Debug buffer (of {} bytes) exhausted!", - limits::DEBUG_BUFFER_BYTES, - ) - }) - .ok(); - true - } else { - false - } - } - fn call_runtime(&self, call: ::RuntimeCall) -> DispatchResultWithPostInfo { let mut origin: T::RuntimeOrigin = RawOrigin::Signed(self.account_id().clone()).into(); origin.add_filter(T::CallFilter::contains); @@ -2113,7 +2048,6 @@ mod tests { value.into(), vec![], false, - None, ), Ok(_) ); @@ -2206,7 +2140,6 @@ mod tests { value.into(), vec![], false, - None, ) .unwrap(); @@ -2247,7 +2180,6 @@ mod tests { value.into(), vec![], false, - None, )); assert_eq!(get_balance(&ALICE), 100 - value); @@ -2284,7 +2216,6 @@ mod tests { U256::zero(), vec![], false, - None, ), ExecError { error: Error::::CodeNotFound.into(), @@ -2302,7 +2233,6 @@ mod tests { U256::zero(), vec![], false, - None, )); }); } @@ -2331,7 +2261,6 @@ mod tests { 55u64.into(), vec![], false, - None, ) .unwrap(); @@ -2381,7 +2310,6 @@ mod tests { U256::zero(), vec![], false, - None, ); let output = result.unwrap(); @@ -2411,7 +2339,6 @@ mod tests { U256::zero(), vec![], false, - None, ); let output = result.unwrap(); @@ -2441,7 +2368,6 @@ mod tests { U256::zero(), vec![1, 2, 3, 4], false, - None, ); assert_matches!(result, Ok(_)); }); @@ -2478,7 +2404,6 @@ mod tests { vec![1, 2, 3, 4], Some(&[0; 32]), false, - None, ); assert_matches!(result, Ok(_)); }); @@ -2533,7 +2458,6 @@ mod tests { value.into(), vec![], false, - None, ); assert_matches!(result, Ok(_)); @@ -2598,7 +2522,6 @@ mod tests { U256::zero(), vec![], false, - None, ); assert_matches!(result, Ok(_)); @@ -2664,7 +2587,6 @@ mod tests { U256::zero(), vec![], false, - None, ); assert_matches!(result, Ok(_)); @@ -2697,7 +2619,6 @@ mod tests { U256::zero(), vec![], false, - None, ); assert_matches!(result, Ok(_)); }); @@ -2735,7 +2656,6 @@ mod tests { U256::zero(), vec![0], false, - None, ); assert_matches!(result, Ok(_)); }); @@ -2762,7 +2682,6 @@ mod tests { U256::zero(), vec![0], false, - None, ); assert_matches!(result, Ok(_)); }); @@ -2807,7 +2726,6 @@ mod tests { U256::zero(), vec![0], false, - None, ); assert_matches!(result, Ok(_)); }); @@ -2834,7 +2752,6 @@ mod tests { U256::zero(), vec![0], false, - None, ); assert_matches!(result, Ok(_)); }); @@ -2861,7 +2778,6 @@ mod tests { 1u64.into(), vec![0], false, - None, ); assert_matches!(result, Err(_)); }); @@ -2906,7 +2822,6 @@ mod tests { U256::zero(), vec![0], false, - None, ); assert_matches!(result, Ok(_)); }); @@ -2952,7 +2867,6 @@ mod tests { U256::zero(), vec![], false, - None, ); assert_matches!(result, Ok(_)); @@ -2979,7 +2893,6 @@ mod tests { vec![], Some(&[0; 32]), false, - None, ), Err(_) ); @@ -3015,7 +2928,6 @@ mod tests { vec![], Some(&[0 ;32]), false, - None, ), Ok((address, ref output)) if output.data == vec![80, 65, 83, 83] => address ); @@ -3070,7 +2982,6 @@ mod tests { vec![], Some(&[0; 32]), false, - None, ), Ok((address, ref output)) if output.data == vec![70, 65, 73, 76] => address ); @@ -3135,7 +3046,6 @@ mod tests { (min_balance * 10).into(), vec![], false, - None, ), Ok(_) ); @@ -3216,7 +3126,6 @@ mod tests { U256::zero(), vec![], false, - None, ), Ok(_) ); @@ -3260,7 +3169,6 @@ mod tests { vec![], Some(&[0; 32]), false, - None, ), Err(Error::::TerminatedInConstructor.into()) ); @@ -3325,7 +3233,6 @@ mod tests { U256::zero(), vec![0], false, - None, ); assert_matches!(result, Ok(_)); }); @@ -3388,7 +3295,6 @@ mod tests { vec![], Some(&[0; 32]), false, - None, ); assert_matches!(result, Ok(_)); }); @@ -3435,113 +3341,11 @@ mod tests { U256::zero(), vec![], false, - None, ) .unwrap(); }); } - #[test] - fn printing_works() { - let code_hash = MockLoader::insert(Call, |ctx, _| { - ctx.ext.append_debug_buffer("This is a test"); - ctx.ext.append_debug_buffer("More text"); - exec_success() - }); - - let mut debug_buffer = DebugBuffer::try_from(Vec::new()).unwrap(); - - ExtBuilder::default().build().execute_with(|| { - let min_balance = ::Currency::minimum_balance(); - - let mut gas_meter = GasMeter::::new(GAS_LIMIT); - set_balance(&ALICE, min_balance * 10); - place_contract(&BOB, code_hash); - let origin = Origin::from_account_id(ALICE); - let mut storage_meter = storage::meter::Meter::new(&origin, 0, 0).unwrap(); - MockStack::run_call( - origin, - BOB_ADDR, - &mut gas_meter, - &mut storage_meter, - U256::zero(), - vec![], - false, - Some(&mut debug_buffer), - ) - .unwrap(); - }); - - assert_eq!(&String::from_utf8(debug_buffer.to_vec()).unwrap(), "This is a testMore text"); - } - - #[test] - fn printing_works_on_fail() { - let code_hash = MockLoader::insert(Call, |ctx, _| { - ctx.ext.append_debug_buffer("This is a test"); - ctx.ext.append_debug_buffer("More text"); - exec_trapped() - }); - - let mut debug_buffer = DebugBuffer::try_from(Vec::new()).unwrap(); - - ExtBuilder::default().build().execute_with(|| { - let min_balance = ::Currency::minimum_balance(); - - let mut gas_meter = GasMeter::::new(GAS_LIMIT); - set_balance(&ALICE, min_balance * 10); - place_contract(&BOB, code_hash); - let origin = Origin::from_account_id(ALICE); - let mut storage_meter = storage::meter::Meter::new(&origin, 0, 0).unwrap(); - let result = MockStack::run_call( - origin, - BOB_ADDR, - &mut gas_meter, - &mut storage_meter, - U256::zero(), - vec![], - false, - Some(&mut debug_buffer), - ); - assert!(result.is_err()); - }); - - assert_eq!(&String::from_utf8(debug_buffer.to_vec()).unwrap(), "This is a testMore text"); - } - - #[test] - fn debug_buffer_is_limited() { - let code_hash = MockLoader::insert(Call, move |ctx, _| { - ctx.ext.append_debug_buffer("overflowing bytes"); - exec_success() - }); - - // Pre-fill the buffer almost up to its limit, leaving not enough space to the message - let debug_buf_before = DebugBuffer::try_from(vec![0u8; DebugBuffer::bound() - 5]).unwrap(); - let mut debug_buf_after = debug_buf_before.clone(); - - ExtBuilder::default().build().execute_with(|| { - let min_balance = ::Currency::minimum_balance(); - let mut gas_meter = GasMeter::::new(GAS_LIMIT); - set_balance(&ALICE, min_balance * 10); - place_contract(&BOB, code_hash); - let origin = Origin::from_account_id(ALICE); - let mut storage_meter = storage::meter::Meter::new(&origin, 0, 0).unwrap(); - MockStack::run_call( - origin, - BOB_ADDR, - &mut gas_meter, - &mut storage_meter, - U256::zero(), - vec![], - false, - Some(&mut debug_buf_after), - ) - .unwrap(); - assert_eq!(debug_buf_before, debug_buf_after); - }); - } - #[test] fn call_reentry_direct_recursion() { // call the contract passed as input with disabled reentry @@ -3569,7 +3373,6 @@ mod tests { U256::zero(), CHARLIE_ADDR.as_bytes().to_vec(), false, - None, )); // Calling into oneself fails @@ -3582,7 +3385,6 @@ mod tests { U256::zero(), BOB_ADDR.as_bytes().to_vec(), false, - None, ) .map_err(|e| e.error), >::ReentranceDenied, @@ -3633,7 +3435,6 @@ mod tests { U256::zero(), vec![0], false, - None, ) .map_err(|e| e.error), >::ReentranceDenied, @@ -3668,7 +3469,6 @@ mod tests { U256::zero(), vec![], false, - None, ) .unwrap(); @@ -3753,7 +3553,6 @@ mod tests { U256::zero(), vec![], false, - None, ) .unwrap(); @@ -3880,7 +3679,6 @@ mod tests { vec![], Some(&[0; 32]), false, - None, ) .ok(); assert_eq!(System::account_nonce(&ALICE), 0); @@ -3894,7 +3692,6 @@ mod tests { vec![], Some(&[0; 32]), false, - None, )); assert_eq!(System::account_nonce(&ALICE), 1); @@ -3907,7 +3704,6 @@ mod tests { vec![], Some(&[0; 32]), false, - None, )); assert_eq!(System::account_nonce(&ALICE), 2); @@ -3920,7 +3716,6 @@ mod tests { vec![], Some(&[0; 32]), false, - None, )); assert_eq!(System::account_nonce(&ALICE), 3); }); @@ -3989,7 +3784,6 @@ mod tests { U256::zero(), vec![], false, - None, )); }); } @@ -4101,7 +3895,6 @@ mod tests { U256::zero(), vec![], false, - None, )); }); } @@ -4141,7 +3934,6 @@ mod tests { U256::zero(), vec![], false, - None, )); }); } @@ -4181,7 +3973,6 @@ mod tests { U256::zero(), vec![], false, - None, )); }); } @@ -4235,7 +4026,6 @@ mod tests { U256::zero(), vec![], false, - None, )); }); } @@ -4292,7 +4082,6 @@ mod tests { U256::zero(), vec![], false, - None, )); }); } @@ -4368,7 +4157,6 @@ mod tests { U256::zero(), vec![], false, - None, )); }); } @@ -4439,7 +4227,6 @@ mod tests { U256::zero(), vec![0], false, - None, ); assert_matches!(result, Ok(_)); }); @@ -4478,7 +4265,6 @@ mod tests { U256::zero(), vec![], false, - None, )); }); } @@ -4541,7 +4327,6 @@ mod tests { U256::zero(), vec![0], false, - None, ); assert_matches!(result, Ok(_)); }); @@ -4575,7 +4360,6 @@ mod tests { U256::zero(), vec![], false, - None, ); assert_matches!(result, Ok(_)); }); @@ -4659,7 +4443,6 @@ mod tests { U256::zero(), vec![], false, - None, ) .unwrap() }); @@ -4728,7 +4511,6 @@ mod tests { U256::zero(), vec![0], false, - None, ); assert_matches!(result, Ok(_)); }); @@ -4800,7 +4582,6 @@ mod tests { U256::zero(), vec![], false, - None, ); assert_matches!(result, Ok(_)); }); @@ -4852,7 +4633,6 @@ mod tests { U256::zero(), vec![], false, - None, ) .unwrap() }); @@ -4922,7 +4702,6 @@ mod tests { U256::zero(), vec![], false, - None, ) .unwrap() }); @@ -4969,7 +4748,6 @@ mod tests { U256::zero(), vec![], false, - None, ) .unwrap() }); @@ -5014,7 +4792,6 @@ mod tests { U256::zero(), vec![], false, - None, ) .unwrap() }); @@ -5070,7 +4847,6 @@ mod tests { U256::zero(), vec![0], false, - None, ), Ok(_) ); diff --git a/substrate/frame/revive/src/lib.rs b/substrate/frame/revive/src/lib.rs index 04bce264a188..39dce27d91d8 100644 --- a/substrate/frame/revive/src/lib.rs +++ b/substrate/frame/revive/src/lib.rs @@ -71,7 +71,7 @@ use frame_support::{ use frame_system::{ ensure_signed, pallet_prelude::{BlockNumberFor, OriginFor}, - EventRecord, Pallet as System, + Pallet as System, }; use pallet_transaction_payment::OnChargeTransaction; use scale_info::TypeInfo; @@ -98,9 +98,6 @@ type BalanceOf = <::Currency as Inspect<::AccountId>>::Balance; type OnChargeTransactionBalanceOf = <::OnChargeTransaction as OnChargeTransaction>::Balance; type CodeVec = BoundedVec>; -type EventRecordOf = - EventRecord<::RuntimeEvent, ::Hash>; -type DebugBuffer = BoundedVec>; type ImmutableData = BoundedVec>; /// Used as a sentinel value when reading and writing contract memory. @@ -258,9 +255,9 @@ pub mod pallet { #[pallet::no_default_bounds] type InstantiateOrigin: EnsureOrigin; - /// For most production chains, it's recommended to use the `()` implementation of this - /// trait. This implementation offers additional logging when the log target - /// "runtime::revive" is set to trace. + /// Debugging utilities for contracts. + /// For production chains, it's recommended to use the `()` implementation of this + /// trait. #[pallet::no_default_bounds] type Debug: Debugger; @@ -810,9 +807,8 @@ pub mod pallet { gas_limit, DepositLimit::Balance(storage_deposit_limit), data, - DebugInfo::Skip, - CollectEvents::Skip, ); + if let Ok(return_value) = &output.result { if return_value.did_revert() { output.result = Err(>::ContractReverted.into()); @@ -848,8 +844,6 @@ pub mod pallet { Code::Existing(code_hash), data, salt, - DebugInfo::Skip, - CollectEvents::Skip, ); if let Ok(retval) = &output.result { if retval.result.did_revert() { @@ -914,8 +908,6 @@ pub mod pallet { Code::Upload(code), data, salt, - DebugInfo::Skip, - CollectEvents::Skip, ); if let Ok(retval) = &output.result { if retval.result.did_revert() { @@ -1085,16 +1077,10 @@ where gas_limit: Weight, storage_deposit_limit: DepositLimit>, data: Vec, - debug: DebugInfo, - collect_events: CollectEvents, - ) -> ContractResult, EventRecordOf> { + ) -> ContractResult> { let mut gas_meter = GasMeter::new(gas_limit); let mut storage_deposit = Default::default(); - let mut debug_message = if matches!(debug, DebugInfo::UnsafeDebug) { - Some(DebugBuffer::default()) - } else { - None - }; + let try_call = || { let origin = Origin::from_runtime_origin(origin)?; let mut storage_meter = match storage_deposit_limit { @@ -1109,7 +1095,6 @@ where Self::convert_native_to_evm(value), data, storage_deposit_limit.is_unchecked(), - debug_message.as_mut(), )?; storage_deposit = storage_meter .try_into_deposit(&origin, storage_deposit_limit.is_unchecked()) @@ -1119,18 +1104,11 @@ where Ok(result) }; let result = Self::run_guarded(try_call); - let events = if matches!(collect_events, CollectEvents::UnsafeCollect) { - Some(System::::read_events_no_consensus().map(|e| *e).collect()) - } else { - None - }; ContractResult { result: result.map_err(|r| r.error), gas_consumed: gas_meter.gas_consumed(), gas_required: gas_meter.gas_required(), storage_deposit, - debug_message: debug_message.unwrap_or_default().to_vec(), - events, } } @@ -1138,8 +1116,8 @@ where /// /// Identical to [`Self::instantiate`] or [`Self::instantiate_with_code`] but tailored towards /// being called by other code within the runtime as opposed to from an extrinsic. It returns - /// more information and allows the enablement of features that are not suitable for an - /// extrinsic (debugging, event collection). + /// more information and allows the enablement of debugging features that are not suitable for + /// an extrinsic. pub fn bare_instantiate( origin: OriginFor, value: BalanceOf, @@ -1148,14 +1126,9 @@ where code: Code, data: Vec, salt: Option<[u8; 32]>, - debug: DebugInfo, - collect_events: CollectEvents, - ) -> ContractResult, EventRecordOf> { + ) -> ContractResult> { let mut gas_meter = GasMeter::new(gas_limit); let mut storage_deposit = Default::default(); - let mut debug_message = - if debug == DebugInfo::UnsafeDebug { Some(DebugBuffer::default()) } else { None }; - let unchecked_deposit_limit = storage_deposit_limit.is_unchecked(); let mut storage_deposit_limit = match storage_deposit_limit { DepositLimit::Balance(limit) => limit, @@ -1195,7 +1168,6 @@ where data, salt.as_ref(), unchecked_deposit_limit, - debug_message.as_mut(), ); storage_deposit = storage_meter .try_into_deposit(&instantiate_origin, unchecked_deposit_limit)? @@ -1203,11 +1175,6 @@ where result }; let output = Self::run_guarded(try_instantiate); - let events = if matches!(collect_events, CollectEvents::UnsafeCollect) { - Some(System::::read_events_no_consensus().map(|e| *e).collect()) - } else { - None - }; ContractResult { result: output .map(|(addr, result)| InstantiateReturnValue { result, addr }) @@ -1215,8 +1182,6 @@ where gas_consumed: gas_meter.gas_consumed(), gas_required: gas_meter.gas_required(), storage_deposit, - debug_message: debug_message.unwrap_or_default().to_vec(), - events, } } @@ -1273,8 +1238,6 @@ where }; let input = tx.input.clone().unwrap_or_default().0; - let debug = DebugInfo::Skip; - let collect_events = CollectEvents::Skip; let extract_error = |err| { if err == Error::::TransferFailed.into() || @@ -1305,8 +1268,6 @@ where gas_limit, storage_deposit_limit, input.clone(), - debug, - collect_events, ); let data = match result.result { @@ -1363,8 +1324,6 @@ where Code::Upload(code.to_vec()), data.to_vec(), None, - debug, - collect_events, ); let returned_data = match result.result { @@ -1535,12 +1494,11 @@ environmental!(executing_contract: bool); sp_api::decl_runtime_apis! { /// The API used to dry-run contract interactions. #[api_version(1)] - pub trait ReviveApi where + pub trait ReviveApi where AccountId: Codec, Balance: Codec, Nonce: Codec, BlockNumber: Codec, - EventRecord: Codec, { /// Returns the free balance of the given `[H160]` address, using EVM decimals. fn balance(address: H160) -> U256; @@ -1558,7 +1516,7 @@ sp_api::decl_runtime_apis! { gas_limit: Option, storage_deposit_limit: Option, input_data: Vec, - ) -> ContractResult; + ) -> ContractResult; /// Instantiate a new contract. /// @@ -1571,7 +1529,7 @@ sp_api::decl_runtime_apis! { code: Code, data: Vec, salt: Option<[u8; 32]>, - ) -> ContractResult; + ) -> ContractResult; /// Perform an Ethereum call. diff --git a/substrate/frame/revive/src/limits.rs b/substrate/frame/revive/src/limits.rs index 3b55106c67d8..f101abf0ea7e 100644 --- a/substrate/frame/revive/src/limits.rs +++ b/substrate/frame/revive/src/limits.rs @@ -57,11 +57,6 @@ pub const TRANSIENT_STORAGE_BYTES: u32 = 4 * 1024; /// The maximum allowable length in bytes for (transient) storage keys. pub const STORAGE_KEY_BYTES: u32 = 128; -/// The maximum size of the debug buffer contracts can write messages to. -/// -/// The buffer will always be disabled for on-chain execution. -pub const DEBUG_BUFFER_BYTES: u32 = 2 * 1024 * 1024; - /// The page size in which PolkaVM should allocate memory chunks. pub const PAGE_SIZE: u32 = 4 * 1024; diff --git a/substrate/frame/revive/src/primitives.rs b/substrate/frame/revive/src/primitives.rs index a7127f812b4b..cf1f52c9e9f1 100644 --- a/substrate/frame/revive/src/primitives.rs +++ b/substrate/frame/revive/src/primitives.rs @@ -63,7 +63,7 @@ impl From for DepositLimit { /// `ContractsApi` version. Therefore when SCALE decoding a `ContractResult` its trailing data /// should be ignored to avoid any potential compatibility issues. #[derive(Clone, Eq, PartialEq, Encode, Decode, RuntimeDebug, TypeInfo)] -pub struct ContractResult { +pub struct ContractResult { /// How much weight was consumed during execution. pub gas_consumed: Weight, /// How much weight is required as gas limit in order to execute this call. @@ -84,26 +84,8 @@ pub struct ContractResult { /// is `Err`. This is because on error all storage changes are rolled back including the /// payment of the deposit. pub storage_deposit: StorageDeposit, - /// An optional debug message. This message is only filled when explicitly requested - /// by the code that calls into the contract. Otherwise it is empty. - /// - /// The contained bytes are valid UTF-8. This is not declared as `String` because - /// this type is not allowed within the runtime. - /// - /// Clients should not make any assumptions about the format of the buffer. - /// They should just display it as-is. It is **not** only a collection of log lines - /// provided by a contract but a formatted buffer with different sections. - /// - /// # Note - /// - /// The debug message is never generated during on-chain execution. It is reserved for - /// RPC calls. - pub debug_message: Vec, /// The execution result of the wasm code. pub result: Result, - /// The events that were emitted during execution. It is an option as event collection is - /// optional. - pub events: Option>, } /// The result of the execution of a `eth_transact` call. @@ -284,36 +266,3 @@ where } } } - -/// Determines whether events should be collected during execution. -#[derive( - Copy, Clone, PartialEq, Eq, RuntimeDebug, Decode, Encode, MaxEncodedLen, scale_info::TypeInfo, -)] -pub enum CollectEvents { - /// Collect events. - /// - /// # Note - /// - /// Events should only be collected when called off-chain, as this would otherwise - /// collect all the Events emitted in the block so far and put them into the PoV. - /// - /// **Never** use this mode for on-chain execution. - UnsafeCollect, - /// Skip event collection. - Skip, -} - -/// Determines whether debug messages will be collected. -#[derive( - Copy, Clone, PartialEq, Eq, RuntimeDebug, Decode, Encode, MaxEncodedLen, scale_info::TypeInfo, -)] -pub enum DebugInfo { - /// Collect debug messages. - /// # Note - /// - /// This should only ever be set to `UnsafeDebug` when executing as an RPC because - /// it adds allocations and could be abused to drive the runtime into an OOM panic. - UnsafeDebug, - /// Skip collection of debug messages. - Skip, -} diff --git a/substrate/frame/revive/src/test_utils/builder.rs b/substrate/frame/revive/src/test_utils/builder.rs index 8ba5e7384070..7fbb5b676439 100644 --- a/substrate/frame/revive/src/test_utils/builder.rs +++ b/substrate/frame/revive/src/test_utils/builder.rs @@ -17,9 +17,8 @@ use super::{deposit_limit, GAS_LIMIT}; use crate::{ - address::AddressMapper, AccountIdOf, BalanceOf, Code, CollectEvents, Config, ContractResult, - DebugInfo, DepositLimit, EventRecordOf, ExecReturnValue, InstantiateReturnValue, OriginFor, - Pallet, Weight, + address::AddressMapper, AccountIdOf, BalanceOf, Code, Config, ContractResult, DepositLimit, + ExecReturnValue, InstantiateReturnValue, OriginFor, Pallet, Weight, }; use frame_support::pallet_prelude::DispatchResultWithPostInfo; use paste::paste; @@ -138,9 +137,7 @@ builder!( code: Code, data: Vec, salt: Option<[u8; 32]>, - debug: DebugInfo, - collect_events: CollectEvents, - ) -> ContractResult, EventRecordOf>; + ) -> ContractResult>; /// Build the instantiate call and unwrap the result. pub fn build_and_unwrap_result(self) -> InstantiateReturnValue { @@ -164,8 +161,6 @@ builder!( code, data: vec![], salt: Some([0; 32]), - debug: DebugInfo::UnsafeDebug, - collect_events: CollectEvents::Skip, } } ); @@ -201,9 +196,7 @@ builder!( gas_limit: Weight, storage_deposit_limit: DepositLimit>, data: Vec, - debug: DebugInfo, - collect_events: CollectEvents, - ) -> ContractResult, EventRecordOf>; + ) -> ContractResult>; /// Build the call and unwrap the result. pub fn build_and_unwrap_result(self) -> ExecReturnValue { @@ -219,8 +212,6 @@ builder!( gas_limit: GAS_LIMIT, storage_deposit_limit: DepositLimit::Balance(deposit_limit::()), data: vec![], - debug: DebugInfo::UnsafeDebug, - collect_events: CollectEvents::Skip, } } ); diff --git a/substrate/frame/revive/src/tests.rs b/substrate/frame/revive/src/tests.rs index 664578bf7672..1a2e0dfdcac1 100644 --- a/substrate/frame/revive/src/tests.rs +++ b/substrate/frame/revive/src/tests.rs @@ -38,9 +38,9 @@ use crate::{ tests::test_utils::{get_contract, get_contract_checked}, wasm::Memory, weights::WeightInfo, - AccountId32Mapper, BalanceOf, Code, CodeInfoOf, CollectEvents, Config, ContractInfo, - ContractInfoOf, DebugInfo, DeletionQueueCounter, DepositLimit, Error, EthTransactError, - HoldReason, Origin, Pallet, PristineCode, H160, + AccountId32Mapper, BalanceOf, Code, CodeInfoOf, Config, ContractInfo, ContractInfoOf, + DeletionQueueCounter, DepositLimit, Error, EthTransactError, HoldReason, Origin, Pallet, + PristineCode, H160, }; use crate::test_utils::builder::Contract; @@ -2184,58 +2184,6 @@ fn refcounter() { }); } -#[test] -fn debug_message_works() { - let (wasm, _code_hash) = compile_module("debug_message_works").unwrap(); - - ExtBuilder::default().existential_deposit(50).build().execute_with(|| { - let _ = ::Currency::set_balance(&ALICE, 1_000_000); - let Contract { addr, .. } = builder::bare_instantiate(Code::Upload(wasm)) - .value(30_000) - .build_and_unwrap_contract(); - let result = builder::bare_call(addr).debug(DebugInfo::UnsafeDebug).build(); - - assert_matches!(result.result, Ok(_)); - assert_eq!(std::str::from_utf8(&result.debug_message).unwrap(), "Hello World!"); - }); -} - -#[test] -fn debug_message_logging_disabled() { - let (wasm, _code_hash) = compile_module("debug_message_logging_disabled").unwrap(); - - ExtBuilder::default().existential_deposit(50).build().execute_with(|| { - let _ = ::Currency::set_balance(&ALICE, 1_000_000); - let Contract { addr, .. } = builder::bare_instantiate(Code::Upload(wasm)) - .value(30_000) - .build_and_unwrap_contract(); - // the dispatchables always run without debugging - assert_ok!(Contracts::call( - RuntimeOrigin::signed(ALICE), - addr, - 0, - GAS_LIMIT, - deposit_limit::(), - vec![] - )); - }); -} - -#[test] -fn debug_message_invalid_utf8() { - let (wasm, _code_hash) = compile_module("debug_message_invalid_utf8").unwrap(); - - ExtBuilder::default().existential_deposit(50).build().execute_with(|| { - let _ = ::Currency::set_balance(&ALICE, 1_000_000); - let Contract { addr, .. } = builder::bare_instantiate(Code::Upload(wasm)) - .value(30_000) - .build_and_unwrap_contract(); - let result = builder::bare_call(addr).debug(DebugInfo::UnsafeDebug).build(); - assert_ok!(result.result); - assert!(result.debug_message.is_empty()); - }); -} - #[test] fn gas_estimation_for_subcalls() { let (caller_code, _caller_hash) = compile_module("call_with_limit").unwrap(); @@ -2451,79 +2399,6 @@ fn ecdsa_recover() { }) } -#[test] -fn bare_instantiate_returns_events() { - let (wasm, _code_hash) = compile_module("transfer_return_code").unwrap(); - ExtBuilder::default().existential_deposit(50).build().execute_with(|| { - let min_balance = Contracts::min_balance(); - let _ = ::Currency::set_balance(&ALICE, 1000 * min_balance); - - let result = builder::bare_instantiate(Code::Upload(wasm)) - .value(min_balance * 100) - .collect_events(CollectEvents::UnsafeCollect) - .build(); - - let events = result.events.unwrap(); - assert!(!events.is_empty()); - assert_eq!(events, System::events()); - }); -} - -#[test] -fn bare_instantiate_does_not_return_events() { - let (wasm, _code_hash) = compile_module("transfer_return_code").unwrap(); - ExtBuilder::default().existential_deposit(50).build().execute_with(|| { - let min_balance = Contracts::min_balance(); - let _ = ::Currency::set_balance(&ALICE, 1000 * min_balance); - - let result = builder::bare_instantiate(Code::Upload(wasm)).value(min_balance * 100).build(); - - let events = result.events; - assert!(!System::events().is_empty()); - assert!(events.is_none()); - }); -} - -#[test] -fn bare_call_returns_events() { - let (wasm, _code_hash) = compile_module("transfer_return_code").unwrap(); - ExtBuilder::default().existential_deposit(50).build().execute_with(|| { - let min_balance = Contracts::min_balance(); - let _ = ::Currency::set_balance(&ALICE, 1000 * min_balance); - - let Contract { addr, .. } = builder::bare_instantiate(Code::Upload(wasm)) - .value(min_balance * 100) - .build_and_unwrap_contract(); - - let result = builder::bare_call(addr).collect_events(CollectEvents::UnsafeCollect).build(); - - let events = result.events.unwrap(); - assert_return_code!(&result.result.unwrap(), RuntimeReturnCode::Success); - assert!(!events.is_empty()); - assert_eq!(events, System::events()); - }); -} - -#[test] -fn bare_call_does_not_return_events() { - let (wasm, _code_hash) = compile_module("transfer_return_code").unwrap(); - ExtBuilder::default().existential_deposit(50).build().execute_with(|| { - let min_balance = Contracts::min_balance(); - let _ = ::Currency::set_balance(&ALICE, 1000 * min_balance); - - let Contract { addr, .. } = builder::bare_instantiate(Code::Upload(wasm)) - .value(min_balance * 100) - .build_and_unwrap_contract(); - - let result = builder::bare_call(addr).build(); - - let events = result.events; - assert_return_code!(&result.result.unwrap(), RuntimeReturnCode::Success); - assert!(!System::events().is_empty()); - assert!(events.is_none()); - }); -} - #[test] fn sr25519_verify() { let (wasm, _code_hash) = compile_module("sr25519_verify").unwrap(); @@ -3327,14 +3202,11 @@ fn set_code_hash() { // First call sets new code_hash and returns 1 let result = builder::bare_call(contract_addr) .data(new_code_hash.as_ref().to_vec()) - .debug(DebugInfo::UnsafeDebug) .build_and_unwrap_result(); assert_return_code!(result, 1); // Second calls new contract code that returns 2 - let result = builder::bare_call(contract_addr) - .debug(DebugInfo::UnsafeDebug) - .build_and_unwrap_result(); + let result = builder::bare_call(contract_addr).build_and_unwrap_result(); assert_return_code!(result, 2); // Checking for the last event only @@ -4814,7 +4686,7 @@ fn skip_transfer_works() { ..Default::default() }, Weight::MAX, - |_| 0u32 + |_| 0u32, ), EthTransactError::Message(format!( "insufficient funds for gas * price + value: address {BOB_ADDR:?} have 0 (supplied gas 1)" @@ -4829,7 +4701,7 @@ fn skip_transfer_works() { ..Default::default() }, Weight::MAX, - |_| 0u32 + |_| 0u32, )); let Contract { addr, .. } = @@ -4848,7 +4720,7 @@ fn skip_transfer_works() { ..Default::default() }, Weight::MAX, - |_| 0u32 + |_| 0u32, ), EthTransactError::Message(format!( "insufficient funds for gas * price + value: address {BOB_ADDR:?} have 0 (supplied gas 1)" @@ -4866,7 +4738,7 @@ fn skip_transfer_works() { ..Default::default() }, Weight::MAX, - |_| 0u32 + |_| 0u32, ), EthTransactError::Message(format!("insufficient funds for gas * price + value: address {BOB_ADDR:?} have 0 (supplied gas 1)")) ); @@ -4875,7 +4747,7 @@ fn skip_transfer_works() { assert_ok!(Pallet::::bare_eth_transact( GenericTransaction { from: Some(BOB_ADDR), to: Some(addr), ..Default::default() }, Weight::MAX, - |_| 0u32 + |_| 0u32, )); // works when calling from a contract when no gas is specified. @@ -4887,7 +4759,7 @@ fn skip_transfer_works() { ..Default::default() }, Weight::MAX, - |_| 0u32 + |_| 0u32, )); }); } diff --git a/substrate/frame/revive/src/tests/test_debug.rs b/substrate/frame/revive/src/tests/test_debug.rs index c9e19e52ace1..314011928f64 100644 --- a/substrate/frame/revive/src/tests/test_debug.rs +++ b/substrate/frame/revive/src/tests/test_debug.rs @@ -18,59 +18,19 @@ use super::*; use crate::{ - debug::{CallInterceptor, CallSpan, ExecResult, ExportedFunction, Tracing}, + debug::{CallInterceptor, ExecResult, ExportedFunction}, primitives::ExecReturnValue, test_utils::*, - DepositLimit, }; use frame_support::traits::Currency; use pretty_assertions::assert_eq; -use sp_core::H160; use std::cell::RefCell; -#[derive(Clone, PartialEq, Eq, Debug)] -struct DebugFrame { - contract_address: sp_core::H160, - call: ExportedFunction, - input: Vec, - result: Option>, -} - thread_local! { - static DEBUG_EXECUTION_TRACE: RefCell> = RefCell::new(Vec::new()); static INTERCEPTED_ADDRESS: RefCell> = RefCell::new(None); } pub struct TestDebug; -pub struct TestCallSpan { - contract_address: sp_core::H160, - call: ExportedFunction, - input: Vec, -} - -impl Tracing for TestDebug { - type CallSpan = TestCallSpan; - - fn new_call_span( - contract_address: &crate::H160, - entry_point: ExportedFunction, - input_data: &[u8], - ) -> TestCallSpan { - DEBUG_EXECUTION_TRACE.with(|d| { - d.borrow_mut().push(DebugFrame { - contract_address: *contract_address, - call: entry_point, - input: input_data.to_vec(), - result: None, - }) - }); - TestCallSpan { - contract_address: *contract_address, - call: entry_point, - input: input_data.to_vec(), - } - } -} impl CallInterceptor for TestDebug { fn intercept_call( @@ -88,106 +48,6 @@ impl CallInterceptor for TestDebug { } } -impl CallSpan for TestCallSpan { - fn after_call(self, output: &ExecReturnValue) { - DEBUG_EXECUTION_TRACE.with(|d| { - d.borrow_mut().push(DebugFrame { - contract_address: self.contract_address, - call: self.call, - input: self.input, - result: Some(output.data.clone()), - }) - }); - } -} - -#[test] -fn debugging_works() { - let (wasm_caller, _) = compile_module("call").unwrap(); - let (wasm_callee, _) = compile_module("store_call").unwrap(); - - fn current_stack() -> Vec { - DEBUG_EXECUTION_TRACE.with(|stack| stack.borrow().clone()) - } - - fn deploy(wasm: Vec) -> H160 { - Contracts::bare_instantiate( - RuntimeOrigin::signed(ALICE), - 0, - GAS_LIMIT, - DepositLimit::Balance(deposit_limit::()), - Code::Upload(wasm), - vec![], - Some([0u8; 32]), - DebugInfo::Skip, - CollectEvents::Skip, - ) - .result - .unwrap() - .addr - } - - fn constructor_frame(contract_address: &H160, after: bool) -> DebugFrame { - DebugFrame { - contract_address: *contract_address, - call: ExportedFunction::Constructor, - input: vec![], - result: if after { Some(vec![]) } else { None }, - } - } - - fn call_frame(contract_address: &H160, args: Vec, after: bool) -> DebugFrame { - DebugFrame { - contract_address: *contract_address, - call: ExportedFunction::Call, - input: args, - result: if after { Some(vec![]) } else { None }, - } - } - - ExtBuilder::default().existential_deposit(200).build().execute_with(|| { - let _ = Balances::deposit_creating(&ALICE, 1_000_000); - - assert_eq!(current_stack(), vec![]); - - let addr_caller = deploy(wasm_caller); - let addr_callee = deploy(wasm_callee); - - assert_eq!( - current_stack(), - vec![ - constructor_frame(&addr_caller, false), - constructor_frame(&addr_caller, true), - constructor_frame(&addr_callee, false), - constructor_frame(&addr_callee, true), - ] - ); - - let main_args = (100u32, &addr_callee.clone()).encode(); - let inner_args = (100u32).encode(); - - assert_ok!(Contracts::call( - RuntimeOrigin::signed(ALICE), - addr_caller, - 0, - GAS_LIMIT, - deposit_limit::(), - main_args.clone() - )); - - let stack_top = current_stack()[4..].to_vec(); - assert_eq!( - stack_top, - vec![ - call_frame(&addr_caller, main_args.clone(), false), - call_frame(&addr_callee, inner_args.clone(), false), - call_frame(&addr_callee, inner_args, true), - call_frame(&addr_caller, main_args, true), - ] - ); - }); -} - #[test] fn call_interception_works() { let (wasm, _) = compile_module("dummy").unwrap(); @@ -204,8 +64,6 @@ fn call_interception_works() { vec![], // some salt to ensure that the address of this contract is unique among all tests Some([0x41; 32]), - DebugInfo::Skip, - CollectEvents::Skip, ) .result .unwrap() diff --git a/substrate/frame/revive/src/wasm/mod.rs b/substrate/frame/revive/src/wasm/mod.rs index 3bd4bde5679f..b45d7026ba91 100644 --- a/substrate/frame/revive/src/wasm/mod.rs +++ b/substrate/frame/revive/src/wasm/mod.rs @@ -288,7 +288,7 @@ impl WasmBlob { } let engine = polkavm::Engine::new(&config).expect( "on-chain (no_std) use of interpreter is hard coded. - interpreter is available on all plattforms; qed", + interpreter is available on all platforms; qed", ); let mut module_config = polkavm::ModuleConfig::new(); diff --git a/substrate/frame/revive/src/wasm/runtime.rs b/substrate/frame/revive/src/wasm/runtime.rs index 52f79f2eb55a..6edc2cbe7bf6 100644 --- a/substrate/frame/revive/src/wasm/runtime.rs +++ b/substrate/frame/revive/src/wasm/runtime.rs @@ -339,8 +339,6 @@ pub enum RuntimeCosts { Terminate(u32), /// 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` per byte of passed message. - DebugMessage(u32), /// Weight of calling `seal_set_storage` for the given storage item sizes. SetStorage { old_bytes: u32, new_bytes: u32 }, /// Weight of calling `seal_clear_storage` per cleared byte. @@ -489,7 +487,6 @@ impl Token for RuntimeCosts { WeightToFee => T::WeightInfo::seal_weight_to_fee(), Terminate(locked_dependencies) => T::WeightInfo::seal_terminate(locked_dependencies), DepositEvent { num_topic, len } => T::WeightInfo::seal_deposit_event(num_topic, len), - DebugMessage(len) => T::WeightInfo::seal_debug_message(len), SetStorage { new_bytes, old_bytes } => { cost_storage!(write, seal_set_storage, new_bytes, old_bytes) }, @@ -669,10 +666,7 @@ impl<'a, E: Ext, M: ?Sized + Memory> Runtime<'a, E, M> { match result { Ok(_) => Ok(ReturnErrorCode::Success), Err(e) => { - if self.ext.debug_buffer_enabled() { - self.ext.append_debug_buffer("call failed with: "); - self.ext.append_debug_buffer(e.into()); - }; + log::debug!(target: LOG_TARGET, "call failed with: {e:?}"); Ok(ErrorReturnCode::get()) }, } @@ -1834,27 +1828,6 @@ pub mod env { self.contains_storage(memory, flags, key_ptr, key_len) } - /// Emit a custom debug message. - /// See [`pallet_revive_uapi::HostFn::debug_message`]. - fn debug_message( - &mut self, - memory: &mut M, - str_ptr: u32, - str_len: u32, - ) -> Result { - let str_len = str_len.min(limits::DEBUG_BUFFER_BYTES); - self.charge_gas(RuntimeCosts::DebugMessage(str_len))?; - if self.ext.append_debug_buffer("") { - let data = memory.read(str_ptr, str_len)?; - if let Some(msg) = core::str::from_utf8(&data).ok() { - self.ext.append_debug_buffer(msg); - } - Ok(ReturnErrorCode::Success) - } else { - Ok(ReturnErrorCode::LoggingDisabled) - } - } - /// Recovers the ECDSA public key from the given message hash and signature. /// See [`pallet_revive_uapi::HostFn::ecdsa_recover`]. fn ecdsa_recover( @@ -2164,10 +2137,7 @@ pub mod env { Ok(ReturnErrorCode::Success) }, Err(e) => { - if self.ext.append_debug_buffer("") { - self.ext.append_debug_buffer("seal0::xcm_send failed with: "); - self.ext.append_debug_buffer(e.into()); - }; + log::debug!(target: LOG_TARGET, "seal0::xcm_send failed with: {e:?}"); Ok(ReturnErrorCode::XcmSendFailed) }, } diff --git a/substrate/frame/revive/src/weights.rs b/substrate/frame/revive/src/weights.rs index e35ba5ca0766..06495d5d21aa 100644 --- a/substrate/frame/revive/src/weights.rs +++ b/substrate/frame/revive/src/weights.rs @@ -96,7 +96,6 @@ pub trait WeightInfo { fn seal_return(n: u32, ) -> Weight; fn seal_terminate(n: u32, ) -> Weight; fn seal_deposit_event(t: u32, n: u32, ) -> Weight; - fn seal_debug_message(i: u32, ) -> Weight; fn get_storage_empty() -> Weight; fn get_storage_full() -> Weight; fn set_storage_empty() -> Weight; @@ -643,16 +642,6 @@ impl WeightInfo for SubstrateWeight { // Standard Error: 34 .saturating_add(Weight::from_parts(774, 0).saturating_mul(n.into())) } - /// The range of component `i` is `[0, 262144]`. - fn seal_debug_message(i: u32, ) -> Weight { - // Proof Size summary in bytes: - // Measured: `0` - // Estimated: `0` - // Minimum execution time: 340_000 picoseconds. - Weight::from_parts(306_527, 0) - // Standard Error: 1 - .saturating_add(Weight::from_parts(728, 0).saturating_mul(i.into())) - } /// Storage: `Skipped::Metadata` (r:0 w:0) /// Proof: `Skipped::Metadata` (`max_values`: None, `max_size`: None, mode: `Measured`) fn get_storage_empty() -> Weight { @@ -1539,16 +1528,6 @@ impl WeightInfo for () { // Standard Error: 34 .saturating_add(Weight::from_parts(774, 0).saturating_mul(n.into())) } - /// The range of component `i` is `[0, 262144]`. - fn seal_debug_message(i: u32, ) -> Weight { - // Proof Size summary in bytes: - // Measured: `0` - // Estimated: `0` - // Minimum execution time: 340_000 picoseconds. - Weight::from_parts(306_527, 0) - // Standard Error: 1 - .saturating_add(Weight::from_parts(728, 0).saturating_mul(i.into())) - } /// Storage: `Skipped::Metadata` (r:0 w:0) /// Proof: `Skipped::Metadata` (`max_values`: None, `max_size`: None, mode: `Measured`) fn get_storage_empty() -> Weight { diff --git a/substrate/frame/revive/uapi/src/host.rs b/substrate/frame/revive/uapi/src/host.rs index eced4843b552..75e1fcf5e61e 100644 --- a/substrate/frame/revive/uapi/src/host.rs +++ b/substrate/frame/revive/uapi/src/host.rs @@ -515,26 +515,6 @@ pub trait HostFn: private::Sealed { #[unstable_hostfn] fn contains_storage(flags: StorageFlags, key: &[u8]) -> Option; - /// Emit a custom debug message. - /// - /// No newlines are added to the supplied message. - /// Specifying invalid UTF-8 just drops the message with no trap. - /// - /// This is a no-op if debug message recording is disabled which is always the case - /// when the code is executing on-chain. The message is interpreted as UTF-8 and - /// appended to the debug buffer which is then supplied to the calling RPC client. - /// - /// # Note - /// - /// Even though no action is taken when debug message recording is disabled there is still - /// a non trivial overhead (and weight cost) associated with calling this function. Contract - /// languages should remove calls to this function (either at runtime or compile time) when - /// 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_hostfn] - fn debug_message(str: &[u8]) -> Result; - /// Recovers the ECDSA public key from the given message hash and signature. /// /// Writes the public key into the given output buffer. diff --git a/substrate/frame/revive/uapi/src/host/riscv64.rs b/substrate/frame/revive/uapi/src/host/riscv64.rs index 6fdda86892d5..60b0df8bb51e 100644 --- a/substrate/frame/revive/uapi/src/host/riscv64.rs +++ b/substrate/frame/revive/uapi/src/host/riscv64.rs @@ -109,7 +109,6 @@ mod sys { out_ptr: *mut u8, out_len_ptr: *mut u32, ) -> ReturnCode; - pub fn debug_message(str_ptr: *const u8, str_len: u32) -> ReturnCode; pub fn call_runtime(call_ptr: *const u8, call_len: u32) -> ReturnCode; pub fn ecdsa_recover( signature_ptr: *const u8, @@ -518,12 +517,6 @@ impl HostFn for HostFnImpl { ret_code.into() } - #[unstable_hostfn] - fn debug_message(str: &[u8]) -> Result { - let ret_code = unsafe { sys::debug_message(str.as_ptr(), str.len() as u32) }; - ret_code.into() - } - #[unstable_hostfn] fn ecdsa_recover( signature: &[u8; 65], diff --git a/substrate/frame/revive/uapi/src/lib.rs b/substrate/frame/revive/uapi/src/lib.rs index ef1798b4bf61..ce373f7ee443 100644 --- a/substrate/frame/revive/uapi/src/lib.rs +++ b/substrate/frame/revive/uapi/src/lib.rs @@ -86,9 +86,6 @@ define_error_codes! { /// Transfer failed for other not further specified reason. Most probably /// reserved or locked balance of the sender that was preventing the transfer. TransferFailed = 4, - /// The call to `debug_message` had no effect because debug message - /// recording was disabled. - LoggingDisabled = 5, /// The call dispatched by `call_runtime` was executed but returned an error. CallRuntimeFailed = 6, /// ECDSA public key recovery failed. Most probably wrong recovery id or signature. From fd2c61d46d03a07b96e8d19edf6a73df349481c0 Mon Sep 17 00:00:00 2001 From: pgherveou Date: Tue, 14 Jan 2025 22:54:47 +0100 Subject: [PATCH 2/5] !fixup d3baa2a09c601e3cd907bbb6d54ca71bf0a1af67 --- substrate/frame/revive/src/lib.rs | 3 +- .../frame/revive/src/tests/test_debug.rs | 140 +++++++++++++++++- 2 files changed, 140 insertions(+), 3 deletions(-) diff --git a/substrate/frame/revive/src/lib.rs b/substrate/frame/revive/src/lib.rs index 39dce27d91d8..3dbf4ab37626 100644 --- a/substrate/frame/revive/src/lib.rs +++ b/substrate/frame/revive/src/lib.rs @@ -1116,8 +1116,7 @@ where /// /// Identical to [`Self::instantiate`] or [`Self::instantiate_with_code`] but tailored towards /// being called by other code within the runtime as opposed to from an extrinsic. It returns - /// more information and allows the enablement of debugging features that are not suitable for - /// an extrinsic. + /// more information to the caller useful to estimate the cost of the operation. pub fn bare_instantiate( origin: OriginFor, value: BalanceOf, diff --git a/substrate/frame/revive/src/tests/test_debug.rs b/substrate/frame/revive/src/tests/test_debug.rs index 314011928f64..b1fdb2d47441 100644 --- a/substrate/frame/revive/src/tests/test_debug.rs +++ b/substrate/frame/revive/src/tests/test_debug.rs @@ -18,19 +18,59 @@ use super::*; use crate::{ - debug::{CallInterceptor, ExecResult, ExportedFunction}, + debug::{CallInterceptor, CallSpan, ExecResult, ExportedFunction, Tracing}, primitives::ExecReturnValue, test_utils::*, + DepositLimit, }; use frame_support::traits::Currency; use pretty_assertions::assert_eq; +use sp_core::H160; use std::cell::RefCell; +#[derive(Clone, PartialEq, Eq, Debug)] +struct DebugFrame { + contract_address: sp_core::H160, + call: ExportedFunction, + input: Vec, + result: Option>, +} + thread_local! { + static DEBUG_EXECUTION_TRACE: RefCell> = RefCell::new(Vec::new()); static INTERCEPTED_ADDRESS: RefCell> = RefCell::new(None); } pub struct TestDebug; +pub struct TestCallSpan { + contract_address: sp_core::H160, + call: ExportedFunction, + input: Vec, +} + +impl Tracing for TestDebug { + type CallSpan = TestCallSpan; + + fn new_call_span( + contract_address: &crate::H160, + entry_point: ExportedFunction, + input_data: &[u8], + ) -> TestCallSpan { + DEBUG_EXECUTION_TRACE.with(|d| { + d.borrow_mut().push(DebugFrame { + contract_address: *contract_address, + call: entry_point, + input: input_data.to_vec(), + result: None, + }) + }); + TestCallSpan { + contract_address: *contract_address, + call: entry_point, + input: input_data.to_vec(), + } + } +} impl CallInterceptor for TestDebug { fn intercept_call( @@ -48,6 +88,104 @@ impl CallInterceptor for TestDebug { } } +impl CallSpan for TestCallSpan { + fn after_call(self, output: &ExecReturnValue) { + DEBUG_EXECUTION_TRACE.with(|d| { + d.borrow_mut().push(DebugFrame { + contract_address: self.contract_address, + call: self.call, + input: self.input, + result: Some(output.data.clone()), + }) + }); + } +} + +#[test] +fn debugging_works() { + let (wasm_caller, _) = compile_module("call").unwrap(); + let (wasm_callee, _) = compile_module("store_call").unwrap(); + + fn current_stack() -> Vec { + DEBUG_EXECUTION_TRACE.with(|stack| stack.borrow().clone()) + } + + fn deploy(wasm: Vec) -> H160 { + Contracts::bare_instantiate( + RuntimeOrigin::signed(ALICE), + 0, + GAS_LIMIT, + DepositLimit::Balance(deposit_limit::()), + Code::Upload(wasm), + vec![], + Some([0u8; 32]), + ) + .result + .unwrap() + .addr + } + + fn constructor_frame(contract_address: &H160, after: bool) -> DebugFrame { + DebugFrame { + contract_address: *contract_address, + call: ExportedFunction::Constructor, + input: vec![], + result: if after { Some(vec![]) } else { None }, + } + } + + fn call_frame(contract_address: &H160, args: Vec, after: bool) -> DebugFrame { + DebugFrame { + contract_address: *contract_address, + call: ExportedFunction::Call, + input: args, + result: if after { Some(vec![]) } else { None }, + } + } + + ExtBuilder::default().existential_deposit(200).build().execute_with(|| { + let _ = Balances::deposit_creating(&ALICE, 1_000_000); + + assert_eq!(current_stack(), vec![]); + + let addr_caller = deploy(wasm_caller); + let addr_callee = deploy(wasm_callee); + + assert_eq!( + current_stack(), + vec![ + constructor_frame(&addr_caller, false), + constructor_frame(&addr_caller, true), + constructor_frame(&addr_callee, false), + constructor_frame(&addr_callee, true), + ] + ); + + let main_args = (100u32, &addr_callee.clone()).encode(); + let inner_args = (100u32).encode(); + + assert_ok!(Contracts::call( + RuntimeOrigin::signed(ALICE), + addr_caller, + 0, + GAS_LIMIT, + deposit_limit::(), + main_args.clone() + )); + + let stack_top = current_stack()[4..].to_vec(); + assert_eq!( + stack_top, + vec![ + call_frame(&addr_caller, main_args.clone(), false), + call_frame(&addr_callee, inner_args.clone(), false), + call_frame(&addr_callee, inner_args, true), + call_frame(&addr_caller, main_args, true), + ] + ); + }); +} + #[test] fn call_interception_works() { let (wasm, _) = compile_module("dummy").unwrap(); From 17cf00d87b9c960d69aa291d43729c6ba3eed94d Mon Sep 17 00:00:00 2001 From: pgherveou Date: Tue, 14 Jan 2025 23:24:17 +0100 Subject: [PATCH 3/5] Remove revive events --- substrate/frame/revive/src/exec.rs | 103 +------ substrate/frame/revive/src/lib.rs | 72 ----- substrate/frame/revive/src/storage/meter.rs | 16 +- substrate/frame/revive/src/tests.rs | 323 +------------------- substrate/frame/revive/src/wasm/mod.rs | 18 +- 5 files changed, 32 insertions(+), 500 deletions(-) diff --git a/substrate/frame/revive/src/exec.rs b/substrate/frame/revive/src/exec.rs index cf94a94ca476..92f087dad02a 100644 --- a/substrate/frame/revive/src/exec.rs +++ b/substrate/frame/revive/src/exec.rs @@ -1102,34 +1102,11 @@ where .enforce_subcall_limit(contract) .map_err(|e| ExecError { error: e, origin: ErrorOrigin::Callee })?; - let account_id = T::AddressMapper::to_address(&frame.account_id); - match (entry_point, delegated_code_hash) { - (ExportedFunction::Constructor, _) => { - // It is not allowed to terminate a contract inside its constructor. - if matches!(frame.contract_info, CachedContract::Terminated) { - return Err(Error::::TerminatedInConstructor.into()); - } - - let caller = T::AddressMapper::to_address(self.caller().account_id()?); - // Deposit an instantiation event. - Contracts::::deposit_event(Event::Instantiated { - deployer: caller, - contract: account_id, - }); - }, - (ExportedFunction::Call, Some(code_hash)) => { - Contracts::::deposit_event(Event::DelegateCalled { - contract: account_id, - code_hash, - }); - }, - (ExportedFunction::Call, None) => { - let caller = self.caller(); - Contracts::::deposit_event(Event::Called { - caller: caller.clone(), - contract: account_id, - }); - }, + // It is not allowed to terminate a contract inside its constructor. + if entry_point == ExportedFunction::Constructor && + matches!(frame.contract_info, CachedContract::Terminated) + { + return Err(Error::::TerminatedInConstructor.into()); } Ok(output) @@ -1536,10 +1513,6 @@ where .charge_deposit(frame.account_id.clone(), StorageDeposit::Refund(*deposit)); } - Contracts::::deposit_event(Event::Terminated { - contract: account_address, - beneficiary: *beneficiary, - }); Ok(()) } @@ -1792,11 +1765,6 @@ where Self::increment_refcount(hash)?; Self::decrement_refcount(prev_hash); - Contracts::::deposit_event(Event::ContractCodeUpdated { - contract: T::AddressMapper::to_address(&frame.account_id), - new_code_hash: hash, - old_code_hash: prev_hash, - }); Ok(()) } @@ -2943,13 +2911,6 @@ mod tests { ContractInfo::::load_code_hash(&instantiated_contract_id).unwrap(), dummy_ch ); - assert_eq!( - &events(), - &[Event::Instantiated { - deployer: ALICE_ADDR, - contract: instantiated_contract_address - }] - ); }); } @@ -3065,19 +3026,6 @@ mod tests { ContractInfo::::load_code_hash(&instantiated_contract_id).unwrap(), dummy_ch ); - assert_eq!( - &events(), - &[ - Event::Instantiated { - deployer: BOB_ADDR, - contract: instantiated_contract_address - }, - Event::Called { - caller: Origin::from_account_id(ALICE), - contract: BOB_ADDR - }, - ] - ); }); } @@ -3129,13 +3077,6 @@ mod tests { ), Ok(_) ); - - // The contract wasn't instantiated so we don't expect to see an instantiation - // event here. - assert_eq!( - &events(), - &[Event::Called { caller: Origin::from_account_id(ALICE), contract: BOB_ADDR },] - ); }); } @@ -3475,24 +3416,14 @@ mod tests { let remark_hash = ::Hashing::hash(b"Hello World"); assert_eq!( System::events(), - vec![ - EventRecord { - phase: Phase::Initialization, - event: MetaEvent::System(frame_system::Event::Remarked { - sender: BOB_FALLBACK, - hash: remark_hash - }), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: MetaEvent::Contracts(crate::Event::Called { - caller: Origin::from_account_id(ALICE), - contract: BOB_ADDR, - }), - topics: vec![], - }, - ] + vec![EventRecord { + phase: Phase::Initialization, + event: MetaEvent::System(frame_system::Event::Remarked { + sender: BOB_FALLBACK, + hash: remark_hash + }), + topics: vec![], + },] ); }); } @@ -3581,14 +3512,6 @@ mod tests { },), topics: vec![], }, - EventRecord { - phase: Phase::Initialization, - event: MetaEvent::Contracts(crate::Event::Called { - caller: Origin::from_account_id(ALICE), - contract: BOB_ADDR, - }), - topics: vec![], - }, ] ); }); diff --git a/substrate/frame/revive/src/lib.rs b/substrate/frame/revive/src/lib.rs index 3dbf4ab37626..9c2ab1b76f77 100644 --- a/substrate/frame/revive/src/lib.rs +++ b/substrate/frame/revive/src/lib.rs @@ -379,25 +379,6 @@ pub mod pallet { #[pallet::event] pub enum Event { - /// Contract deployed by address at the specified address. - Instantiated { deployer: H160, contract: H160 }, - - /// Contract has been removed. - /// - /// # Note - /// - /// The only way for a contract to be removed and emitting this event is by calling - /// `seal_terminate`. - Terminated { - /// The contract that was terminated. - contract: H160, - /// The account that received the contracts remaining balance - beneficiary: H160, - }, - - /// Code with the specified hash has been stored. - CodeStored { code_hash: H256, deposit_held: BalanceOf, uploader: H160 }, - /// A custom event emitted by the contract. ContractEmitted { /// The contract that emitted the event. @@ -409,54 +390,6 @@ pub mod pallet { /// Number of topics is capped by [`limits::NUM_EVENT_TOPICS`]. topics: Vec, }, - - /// A code with the specified hash was removed. - CodeRemoved { code_hash: H256, deposit_released: BalanceOf, remover: H160 }, - - /// A contract's code was updated. - ContractCodeUpdated { - /// The contract that has been updated. - contract: H160, - /// New code hash that was set for the contract. - new_code_hash: H256, - /// Previous code hash of the contract. - old_code_hash: H256, - }, - - /// A contract was called either by a plain account or another contract. - /// - /// # Note - /// - /// Please keep in mind that like all events this is only emitted for successful - /// calls. This is because on failure all storage changes including events are - /// rolled back. - Called { - /// The caller of the `contract`. - caller: Origin, - /// The contract that was called. - contract: H160, - }, - - /// A contract delegate called a code hash. - /// - /// # Note - /// - /// Please keep in mind that like all events this is only emitted for successful - /// calls. This is because on failure all storage changes including events are - /// rolled back. - DelegateCalled { - /// The contract that performed the delegate call and hence in whose context - /// the `code_hash` is executed. - contract: H160, - /// The code hash that was delegate called. - code_hash: H256, - }, - - /// Some funds have been transferred and held as storage deposit. - StorageDepositTransferredAndHeld { from: H160, to: H160, amount: BalanceOf }, - - /// Some storage deposit funds have been transferred and released. - StorageDepositTransferredAndReleased { from: H160, to: H160, amount: BalanceOf }, } #[pallet::error] @@ -985,11 +918,6 @@ pub mod pallet { }; >>::increment_refcount(code_hash)?; >>::decrement_refcount(contract.code_hash); - Self::deposit_event(Event::ContractCodeUpdated { - contract: dest, - new_code_hash: code_hash, - old_code_hash: contract.code_hash, - }); contract.code_hash = code_hash; Ok(()) }) diff --git a/substrate/frame/revive/src/storage/meter.rs b/substrate/frame/revive/src/storage/meter.rs index 6eddf048be98..16d0441ee062 100644 --- a/substrate/frame/revive/src/storage/meter.rs +++ b/substrate/frame/revive/src/storage/meter.rs @@ -18,8 +18,8 @@ //! This module contains functions to meter the storage deposit. use crate::{ - address::AddressMapper, storage::ContractInfo, AccountIdOf, BalanceOf, CodeInfo, Config, Error, - Event, HoldReason, Inspect, Origin, Pallet, StorageDeposit as Deposit, System, LOG_TARGET, + storage::ContractInfo, AccountIdOf, BalanceOf, CodeInfo, Config, Error, HoldReason, Inspect, + Origin, Pallet, StorageDeposit as Deposit, System, LOG_TARGET, }; use alloc::vec::Vec; use core::{fmt::Debug, marker::PhantomData}; @@ -548,12 +548,6 @@ impl Ext for ReservingExt { Preservation::Preserve, Fortitude::Polite, )?; - - Pallet::::deposit_event(Event::StorageDepositTransferredAndHeld { - from: T::AddressMapper::to_address(origin), - to: T::AddressMapper::to_address(contract), - amount: *amount, - }); }, Deposit::Refund(amount) => { let transferred = T::Currency::transfer_on_hold( @@ -566,12 +560,6 @@ impl Ext for ReservingExt { Fortitude::Polite, )?; - Pallet::::deposit_event(Event::StorageDepositTransferredAndReleased { - from: T::AddressMapper::to_address(contract), - to: T::AddressMapper::to_address(origin), - amount: transferred, - }); - if transferred < *amount { // This should never happen, if it does it means that there is a bug in the // runtime logic. In the rare case this happens we try to refund as much as we diff --git a/substrate/frame/revive/src/tests.rs b/substrate/frame/revive/src/tests.rs index 1a2e0dfdcac1..fd4e3cfd21c6 100644 --- a/substrate/frame/revive/src/tests.rs +++ b/substrate/frame/revive/src/tests.rs @@ -713,25 +713,6 @@ fn instantiate_and_call_and_deposit_event() { }), topics: vec![], }, - EventRecord { - phase: Phase::Initialization, - event: RuntimeEvent::Contracts(crate::Event::Instantiated { - deployer: ALICE_ADDR, - contract: addr - }), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: RuntimeEvent::Contracts( - pallet_revive::Event::StorageDepositTransferredAndHeld { - from: ALICE_ADDR, - to: addr, - amount: test_utils::contract_info_storage_deposit(&addr), - } - ), - topics: vec![], - }, ] ); }); @@ -1078,14 +1059,6 @@ fn deploy_and_call_other_contract() { }), topics: vec![], }, - EventRecord { - phase: Phase::Initialization, - event: RuntimeEvent::Contracts(crate::Event::Instantiated { - deployer: caller_addr, - contract: callee_addr, - }), - topics: vec![], - }, EventRecord { phase: Phase::Initialization, event: RuntimeEvent::Balances(pallet_balances::Event::Transfer { @@ -1095,33 +1068,6 @@ fn deploy_and_call_other_contract() { }), topics: vec![], }, - EventRecord { - phase: Phase::Initialization, - event: RuntimeEvent::Contracts(crate::Event::Called { - caller: Origin::from_account_id(caller_account.clone()), - contract: callee_addr, - }), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: RuntimeEvent::Contracts(crate::Event::Called { - caller: Origin::from_account_id(ALICE), - contract: caller_addr, - }), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: RuntimeEvent::Contracts( - pallet_revive::Event::StorageDepositTransferredAndHeld { - from: ALICE_ADDR, - to: callee_addr, - amount: test_utils::contract_info_storage_deposit(&callee_addr), - } - ), - topics: vec![], - }, ] ); }); @@ -1373,8 +1319,6 @@ fn self_destruct_works() { // Check that the BOB contract has been instantiated. let _ = get_contract(&contract.addr); - let info_deposit = test_utils::contract_info_storage_deposit(&contract.addr); - // Drop all previous events initialize_block(2); @@ -1404,33 +1348,6 @@ fn self_destruct_works() { pretty_assertions::assert_eq!( System::events(), vec![ - EventRecord { - phase: Phase::Initialization, - event: RuntimeEvent::Contracts(crate::Event::Terminated { - contract: contract.addr, - beneficiary: DJANGO_ADDR, - }), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: RuntimeEvent::Contracts(crate::Event::Called { - caller: Origin::from_account_id(ALICE), - contract: contract.addr, - }), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: RuntimeEvent::Contracts( - pallet_revive::Event::StorageDepositTransferredAndReleased { - from: contract.addr, - to: ALICE_ADDR, - amount: info_deposit, - } - ), - topics: vec![], - }, EventRecord { phase: Phase::Initialization, event: RuntimeEvent::System(frame_system::Event::KilledAccount { @@ -2512,23 +2429,9 @@ fn upload_code_works() { initialize_block(2); assert!(!PristineCode::::contains_key(&code_hash)); - assert_ok!(Contracts::upload_code(RuntimeOrigin::signed(ALICE), wasm, 1_000,)); // Ensure the contract was stored and get expected deposit amount to be reserved. - let deposit_expected = expected_deposit(ensure_stored(code_hash)); - - assert_eq!( - System::events(), - vec![EventRecord { - phase: Phase::Initialization, - event: RuntimeEvent::Contracts(crate::Event::CodeStored { - code_hash, - deposit_held: deposit_expected, - uploader: ALICE_ADDR - }), - topics: vec![], - },] - ); + expected_deposit(ensure_stored(code_hash)); }); } @@ -2586,32 +2489,8 @@ fn remove_code_works() { assert_ok!(Contracts::upload_code(RuntimeOrigin::signed(ALICE), wasm, 1_000,)); // Ensure the contract was stored and get expected deposit amount to be reserved. - let deposit_expected = expected_deposit(ensure_stored(code_hash)); - + expected_deposit(ensure_stored(code_hash)); assert_ok!(Contracts::remove_code(RuntimeOrigin::signed(ALICE), code_hash)); - assert_eq!( - System::events(), - vec![ - EventRecord { - phase: Phase::Initialization, - event: RuntimeEvent::Contracts(crate::Event::CodeStored { - code_hash, - deposit_held: deposit_expected, - uploader: ALICE_ADDR - }), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: RuntimeEvent::Contracts(crate::Event::CodeRemoved { - code_hash, - deposit_released: deposit_expected, - remover: ALICE_ADDR - }), - topics: vec![], - }, - ] - ); }); } @@ -2627,25 +2506,12 @@ fn remove_code_wrong_origin() { assert_ok!(Contracts::upload_code(RuntimeOrigin::signed(ALICE), wasm, 1_000,)); // Ensure the contract was stored and get expected deposit amount to be reserved. - let deposit_expected = expected_deposit(ensure_stored(code_hash)); + expected_deposit(ensure_stored(code_hash)); assert_noop!( Contracts::remove_code(RuntimeOrigin::signed(BOB), code_hash), sp_runtime::traits::BadOrigin, ); - - assert_eq!( - System::events(), - vec![EventRecord { - phase: Phase::Initialization, - event: RuntimeEvent::Contracts(crate::Event::CodeStored { - code_hash, - deposit_held: deposit_expected, - uploader: ALICE_ADDR - }), - topics: vec![], - },] - ); }); } @@ -2704,7 +2570,7 @@ fn instantiate_with_zero_balance_works() { builder::bare_instantiate(Code::Upload(wasm)).build_and_unwrap_contract(); // Ensure the contract was stored and get expected deposit amount to be reserved. - let deposit_expected = expected_deposit(ensure_stored(code_hash)); + expected_deposit(ensure_stored(code_hash)); // Make sure the account exists even though no free balance was send assert_eq!(::Currency::free_balance(&account_id), min_balance); @@ -2716,15 +2582,6 @@ fn instantiate_with_zero_balance_works() { assert_eq!( System::events(), vec![ - EventRecord { - phase: Phase::Initialization, - event: RuntimeEvent::Contracts(crate::Event::CodeStored { - code_hash, - deposit_held: deposit_expected, - uploader: ALICE_ADDR - }), - topics: vec![], - }, EventRecord { phase: Phase::Initialization, event: RuntimeEvent::System(frame_system::Event::NewAccount { @@ -2749,25 +2606,6 @@ fn instantiate_with_zero_balance_works() { }), topics: vec![], }, - EventRecord { - phase: Phase::Initialization, - event: RuntimeEvent::Contracts(crate::Event::Instantiated { - deployer: ALICE_ADDR, - contract: addr, - }), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: RuntimeEvent::Contracts( - pallet_revive::Event::StorageDepositTransferredAndHeld { - from: ALICE_ADDR, - to: addr, - amount: test_utils::contract_info_storage_deposit(&addr), - } - ), - topics: vec![], - }, ] ); }); @@ -2790,7 +2628,7 @@ fn instantiate_with_below_existential_deposit_works() { .build_and_unwrap_contract(); // Ensure the contract was stored and get expected deposit amount to be reserved. - let deposit_expected = expected_deposit(ensure_stored(code_hash)); + expected_deposit(ensure_stored(code_hash)); // Make sure the account exists even though not enough free balance was send assert_eq!(::Currency::free_balance(&account_id), min_balance + value); assert_eq!( @@ -2801,15 +2639,6 @@ fn instantiate_with_below_existential_deposit_works() { assert_eq!( System::events(), vec![ - EventRecord { - phase: Phase::Initialization, - event: RuntimeEvent::Contracts(crate::Event::CodeStored { - code_hash, - deposit_held: deposit_expected, - uploader: ALICE_ADDR - }), - topics: vec![], - }, EventRecord { phase: Phase::Initialization, event: RuntimeEvent::System(frame_system::Event::NewAccount { @@ -2843,25 +2672,6 @@ fn instantiate_with_below_existential_deposit_works() { }), topics: vec![], }, - EventRecord { - phase: Phase::Initialization, - event: RuntimeEvent::Contracts(crate::Event::Instantiated { - deployer: ALICE_ADDR, - contract: addr, - }), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: RuntimeEvent::Contracts( - pallet_revive::Event::StorageDepositTransferredAndHeld { - from: ALICE_ADDR, - to: addr, - amount: test_utils::contract_info_storage_deposit(&addr), - } - ), - topics: vec![], - }, ] ); }); @@ -2903,74 +2713,15 @@ fn storage_deposit_works() { assert_eq!( System::events(), - vec![ - EventRecord { - phase: Phase::Initialization, - event: RuntimeEvent::Balances(pallet_balances::Event::Transfer { - from: ALICE, - to: account_id.clone(), - amount: 42, - }), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: RuntimeEvent::Contracts(crate::Event::Called { - caller: Origin::from_account_id(ALICE), - contract: addr, - }), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: RuntimeEvent::Contracts( - pallet_revive::Event::StorageDepositTransferredAndHeld { - from: ALICE_ADDR, - to: addr, - amount: charged0, - } - ), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: RuntimeEvent::Contracts(crate::Event::Called { - caller: Origin::from_account_id(ALICE), - contract: addr, - }), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: RuntimeEvent::Contracts( - pallet_revive::Event::StorageDepositTransferredAndHeld { - from: ALICE_ADDR, - to: addr, - amount: charged1, - } - ), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: RuntimeEvent::Contracts(crate::Event::Called { - caller: Origin::from_account_id(ALICE), - contract: addr, - }), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: RuntimeEvent::Contracts( - pallet_revive::Event::StorageDepositTransferredAndReleased { - from: addr, - to: ALICE_ADDR, - amount: refunded0, - } - ), - topics: vec![], - }, - ] + vec![EventRecord { + phase: Phase::Initialization, + event: RuntimeEvent::Balances(pallet_balances::Event::Transfer { + from: ALICE, + to: account_id.clone(), + amount: 42, + }), + topics: vec![], + },] ); }); } @@ -3063,18 +2814,6 @@ fn set_code_extrinsic() { assert_eq!(get_contract(&addr).code_hash, new_code_hash); assert_refcount!(&code_hash, 0); assert_refcount!(&new_code_hash, 1); - assert_eq!( - System::events(), - vec![EventRecord { - phase: Phase::Initialization, - event: RuntimeEvent::Contracts(pallet_revive::Event::ContractCodeUpdated { - contract: addr, - new_code_hash, - old_code_hash: code_hash, - }), - topics: vec![], - },] - ); }); } @@ -3180,7 +2919,7 @@ fn contract_reverted() { #[test] fn set_code_hash() { - let (wasm, code_hash) = compile_module("set_code_hash").unwrap(); + let (wasm, _) = compile_module("set_code_hash").unwrap(); let (new_wasm, new_code_hash) = compile_module("new_set_code_hash_contract").unwrap(); ExtBuilder::default().existential_deposit(100).build().execute_with(|| { @@ -3208,38 +2947,6 @@ fn set_code_hash() { // Second calls new contract code that returns 2 let result = builder::bare_call(contract_addr).build_and_unwrap_result(); assert_return_code!(result, 2); - - // Checking for the last event only - assert_eq!( - &System::events(), - &[ - EventRecord { - phase: Phase::Initialization, - event: RuntimeEvent::Contracts(crate::Event::ContractCodeUpdated { - contract: contract_addr, - new_code_hash, - old_code_hash: code_hash, - }), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: RuntimeEvent::Contracts(crate::Event::Called { - caller: Origin::from_account_id(ALICE), - contract: contract_addr, - }), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: RuntimeEvent::Contracts(crate::Event::Called { - caller: Origin::from_account_id(ALICE), - contract: contract_addr, - }), - topics: vec![], - }, - ], - ); }); } diff --git a/substrate/frame/revive/src/wasm/mod.rs b/substrate/frame/revive/src/wasm/mod.rs index b45d7026ba91..527cf1630954 100644 --- a/substrate/frame/revive/src/wasm/mod.rs +++ b/substrate/frame/revive/src/wasm/mod.rs @@ -29,14 +29,13 @@ pub use crate::wasm::runtime::{ReturnData, TrapReason}; pub use crate::wasm::runtime::{Memory, Runtime, RuntimeCosts}; use crate::{ - address::AddressMapper, exec::{ExecResult, Executable, ExportedFunction, Ext}, gas::{GasMeter, Token}, limits, storage::meter::Diff, weights::WeightInfo, - AccountIdOf, BadOrigin, BalanceOf, CodeInfoOf, CodeVec, Config, Error, Event, ExecError, - HoldReason, Pallet, PristineCode, Weight, LOG_TARGET, + AccountIdOf, BadOrigin, BalanceOf, CodeInfoOf, CodeVec, Config, Error, ExecError, HoldReason, + PristineCode, Weight, LOG_TARGET, }; use alloc::vec::Vec; use codec::{Decode, Encode, MaxEncodedLen}; @@ -157,16 +156,9 @@ where code_info.deposit, BestEffort, ); - let deposit_released = code_info.deposit; - let remover = T::AddressMapper::to_address(&code_info.owner); *existing = None; >::remove(&code_hash); - >::deposit_event(Event::CodeRemoved { - code_hash, - deposit_released, - remover, - }); Ok(()) } else { Err(>::CodeNotFound.into()) @@ -202,12 +194,6 @@ where self.code_info.refcount = 0; >::insert(code_hash, &self.code); *stored_code_info = Some(self.code_info.clone()); - let uploader = T::AddressMapper::to_address(&self.code_info.owner); - >::deposit_event(Event::CodeStored { - code_hash, - deposit_held: deposit, - uploader, - }); Ok(deposit) }, } From de46b985456681bfef359939724b9798b9296e57 Mon Sep 17 00:00:00 2001 From: command-bot <> Date: Tue, 14 Jan 2025 22:29:42 +0000 Subject: [PATCH 4/5] Update from pgherveou running command 'prdoc --audience runtime_dev --bump minor' --- prdoc/pr_7164.prdoc | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 prdoc/pr_7164.prdoc diff --git a/prdoc/pr_7164.prdoc b/prdoc/pr_7164.prdoc new file mode 100644 index 000000000000..ff921d0f15c5 --- /dev/null +++ b/prdoc/pr_7164.prdoc @@ -0,0 +1,8 @@ +title: '[pallet-revive] Remove revive events' +doc: +- audience: Runtime Dev + description: Remove all pallet::events except for the `ContractEmitted` event that + is emitted by contracts +crates: +- name: pallet-revive + bump: minor From 4c3acdeb2daaafa5806cdaf1b36c82ed7fb79129 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Wed, 15 Jan 2025 19:20:44 +0100 Subject: [PATCH 5/5] Update prdoc/pr_7164.prdoc --- prdoc/pr_7164.prdoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prdoc/pr_7164.prdoc b/prdoc/pr_7164.prdoc index ff921d0f15c5..cb0410a9de79 100644 --- a/prdoc/pr_7164.prdoc +++ b/prdoc/pr_7164.prdoc @@ -5,4 +5,4 @@ doc: is emitted by contracts crates: - name: pallet-revive - bump: minor + bump: major