From 47200ffd7fee9865ac44641a96918dae81bbad3c Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Mon, 3 Apr 2023 11:17:41 +0200 Subject: [PATCH 01/28] contracts: add events to ContractResult --- frame/contracts/primitives/src/lib.rs | 9 +++++---- frame/contracts/src/lib.rs | 8 ++++++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/frame/contracts/primitives/src/lib.rs b/frame/contracts/primitives/src/lib.rs index e73ceb031e184..8aae016fb32c2 100644 --- a/frame/contracts/primitives/src/lib.rs +++ b/frame/contracts/primitives/src/lib.rs @@ -33,7 +33,7 @@ use sp_weights::Weight; /// /// It contains the execution result together with some auxiliary information. #[derive(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. @@ -71,15 +71,16 @@ pub struct ContractResult { pub debug_message: Vec, /// The execution result of the wasm code. pub result: R, + pub events: Option>, } /// Result type of a `bare_call` call. -pub type ContractExecResult = - ContractResult, Balance>; +pub type ContractExecResult = + ContractResult, Balance, Event>; /// Result type of a `bare_instantiate` call. pub type ContractInstantiateResult = - ContractResult, DispatchError>, Balance>; + ContractResult, DispatchError>, Balance, ()>; /// Result type of a `bare_code_upload` call. pub type CodeUploadResult = diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index cd4b7daa6da0f..4dc3eb36851a5 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -118,7 +118,7 @@ use frame_support::{ weights::{OldWeight, Weight}, BoundedVec, WeakBoundedVec, }; -use frame_system::Pallet as System; +use frame_system::{EventRecord, Pallet as System}; use pallet_contracts_primitives::{ Code, CodeUploadResult, CodeUploadReturnValue, ContractAccessError, ContractExecResult, ContractInstantiateResult, ExecReturnValue, GetStorageResult, InstantiateReturnValue, @@ -1167,7 +1167,7 @@ impl Pallet { data: Vec, debug: bool, determinism: Determinism, - ) -> ContractExecResult> { + ) -> ContractExecResult, EventRecord<::RuntimeEvent, T>> { let mut debug_message = if debug { Some(DebugBufferVec::::default()) } else { None }; let common = CommonInput { origin, @@ -1184,6 +1184,9 @@ impl Pallet { gas_required: output.gas_meter.gas_required(), storage_deposit: output.storage_deposit, debug_message: debug_message.unwrap_or_default().to_vec(), + events: Some(System::events()), /* TODO: is it okay to call System::events() here? + * The function says it should only be used in tests. + * But what about offchain calls? */ } } @@ -1228,6 +1231,7 @@ impl Pallet { gas_required: output.gas_meter.gas_required(), storage_deposit: output.storage_deposit, debug_message: debug_message.unwrap_or_default().to_vec(), + events: None, } } From a534758d90e9f074bc73b790f9438802948f6ebe Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Mon, 3 Apr 2023 17:04:38 +0200 Subject: [PATCH 02/28] contracts: add encoded events to ContractResult --- frame/contracts/primitives/src/lib.rs | 10 +++++----- frame/contracts/src/lib.rs | 13 ++++++++----- frame/contracts/src/tests.rs | 2 +- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/frame/contracts/primitives/src/lib.rs b/frame/contracts/primitives/src/lib.rs index 8aae016fb32c2..fde319256b2f1 100644 --- a/frame/contracts/primitives/src/lib.rs +++ b/frame/contracts/primitives/src/lib.rs @@ -33,7 +33,7 @@ use sp_weights::Weight; /// /// It contains the execution result together with some auxiliary information. #[derive(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. @@ -71,16 +71,16 @@ pub struct ContractResult { pub debug_message: Vec, /// The execution result of the wasm code. pub result: R, - pub events: Option>, + pub events: Option>>, } /// Result type of a `bare_call` call. -pub type ContractExecResult = - ContractResult, Balance, Event>; +pub type ContractExecResult = + ContractResult, Balance>; /// Result type of a `bare_instantiate` call. pub type ContractInstantiateResult = - ContractResult, DispatchError>, Balance, ()>; + ContractResult, DispatchError>, Balance>; /// Result type of a `bare_code_upload` call. pub type CodeUploadResult = diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 4dc3eb36851a5..885ccc8a49771 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -118,7 +118,7 @@ use frame_support::{ weights::{OldWeight, Weight}, BoundedVec, WeakBoundedVec, }; -use frame_system::{EventRecord, Pallet as System}; +use frame_system::Pallet as System; use pallet_contracts_primitives::{ Code, CodeUploadResult, CodeUploadReturnValue, ContractAccessError, ContractExecResult, ContractInstantiateResult, ExecReturnValue, GetStorageResult, InstantiateReturnValue, @@ -1167,7 +1167,7 @@ impl Pallet { data: Vec, debug: bool, determinism: Determinism, - ) -> ContractExecResult, EventRecord<::RuntimeEvent, T>> { + ) -> ContractExecResult> { let mut debug_message = if debug { Some(DebugBufferVec::::default()) } else { None }; let common = CommonInput { origin, @@ -1178,15 +1178,18 @@ impl Pallet { debug_message: debug_message.as_mut(), }; let output = CallInput:: { dest, determinism }.run_guarded(common); + // We are good to call System::events() from the runtime API (i.e offchain). + // Even though it says it should only be used in tests, it is actually not allowed to be + // read on-chain cause it will put all the Events emitted in the block so far into the PoV. + let events: Vec> = + System::::events().iter().map(|e| e.clone().event.encode()).collect(); // todo: should determinism::Relaxed be checked here? ContractExecResult { result: output.result.map_err(|r| r.error), gas_consumed: output.gas_meter.gas_consumed(), gas_required: output.gas_meter.gas_required(), storage_deposit: output.storage_deposit, debug_message: debug_message.unwrap_or_default().to_vec(), - events: Some(System::events()), /* TODO: is it okay to call System::events() here? - * The function says it should only be used in tests. - * But what about offchain calls? */ + events: Some(events), } } diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index 146e5fd24ad07..a901c26f088c9 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -458,7 +458,7 @@ fn compile_module(fixture_name: &str) -> wat::Result<(Vec, Date: Mon, 3 Apr 2023 18:24:54 +0200 Subject: [PATCH 03/28] contracts: add generic Event to ContractResult --- frame/contracts/primitives/src/lib.rs | 10 +++++----- frame/contracts/src/lib.rs | 10 +++++----- frame/contracts/src/tests.rs | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/frame/contracts/primitives/src/lib.rs b/frame/contracts/primitives/src/lib.rs index fde319256b2f1..8aae016fb32c2 100644 --- a/frame/contracts/primitives/src/lib.rs +++ b/frame/contracts/primitives/src/lib.rs @@ -33,7 +33,7 @@ use sp_weights::Weight; /// /// It contains the execution result together with some auxiliary information. #[derive(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. @@ -71,16 +71,16 @@ pub struct ContractResult { pub debug_message: Vec, /// The execution result of the wasm code. pub result: R, - pub events: Option>>, + pub events: Option>, } /// Result type of a `bare_call` call. -pub type ContractExecResult = - ContractResult, Balance>; +pub type ContractExecResult = + ContractResult, Balance, Event>; /// Result type of a `bare_instantiate` call. pub type ContractInstantiateResult = - ContractResult, DispatchError>, Balance>; + ContractResult, DispatchError>, Balance, ()>; /// Result type of a `bare_code_upload` call. pub type CodeUploadResult = diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 885ccc8a49771..97e074d82bba8 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -1167,7 +1167,7 @@ impl Pallet { data: Vec, debug: bool, determinism: Determinism, - ) -> ContractExecResult> { + ) -> ContractExecResult, ::RuntimeEvent> { let mut debug_message = if debug { Some(DebugBufferVec::::default()) } else { None }; let common = CommonInput { origin, @@ -1181,8 +1181,7 @@ impl Pallet { // We are good to call System::events() from the runtime API (i.e offchain). // Even though it says it should only be used in tests, it is actually not allowed to be // read on-chain cause it will put all the Events emitted in the block so far into the PoV. - let events: Vec> = - System::::events().iter().map(|e| e.clone().event.encode()).collect(); // todo: should determinism::Relaxed be checked here? + let events = System::::events().iter().map(|e| e.clone().event).collect::>(); // todo: should determinism::Relaxed be checked here? ContractExecResult { result: output.result.map_err(|r| r.error), gas_consumed: output.gas_meter.gas_consumed(), @@ -1342,11 +1341,12 @@ impl Pallet { sp_api::decl_runtime_apis! { /// The API used to dry-run contract interactions. #[api_version(2)] - pub trait ContractsApi where + pub trait ContractsApi where AccountId: Codec, Balance: Codec, BlockNumber: Codec, Hash: Codec, + Event: Codec, { /// Perform a call from a specified account to a given contract. /// @@ -1358,7 +1358,7 @@ sp_api::decl_runtime_apis! { gas_limit: Option, storage_deposit_limit: Option, input_data: Vec, - ) -> ContractExecResult; + ) -> ContractExecResult; /// Instantiate a new contract. /// diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index a901c26f088c9..146e5fd24ad07 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -458,7 +458,7 @@ fn compile_module(fixture_name: &str) -> wat::Result<(Vec, Date: Mon, 3 Apr 2023 19:29:32 +0200 Subject: [PATCH 04/28] contracts: test bare_call events --- frame/contracts/src/lib.rs | 13 +++++++++--- frame/contracts/src/tests.rs | 39 ++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 997ee18f112c2..eee9a2c6c4f80 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -118,7 +118,7 @@ use frame_support::{ weights::{OldWeight, Weight}, BoundedVec, WeakBoundedVec, }; -use frame_system::Pallet as System; +use frame_system::{EventRecord, Pallet as System}; use pallet_contracts_primitives::{ Code, CodeUploadResult, CodeUploadReturnValue, ContractAccessError, ContractExecResult, ContractInstantiateResult, ExecReturnValue, GetStorageResult, InstantiateReturnValue, @@ -149,6 +149,8 @@ type CodeVec = BoundedVec::MaxCodeLen>; type RelaxedCodeVec = WeakBoundedVec::MaxCodeLen>; type AccountIdLookupOf = <::Lookup as StaticLookup>::Source; type DebugBufferVec = BoundedVec::MaxDebugBufferLen>; +type EventRecordOf = + EventRecord<::RuntimeEvent, ::Hash>; /// Used as a sentinel value when reading and writing contract memory. /// @@ -1120,7 +1122,7 @@ impl Pallet { data: Vec, debug: bool, determinism: Determinism, - ) -> ContractExecResult, ::RuntimeEvent> { + ) -> ContractExecResult, EventRecordOf> { let mut debug_message = if debug { Some(DebugBufferVec::::default()) } else { None }; let common = CommonInput { origin, @@ -1134,7 +1136,12 @@ impl Pallet { // We are good to call System::events() from the runtime API (i.e offchain). // Even though it says it should only be used in tests, it is actually not allowed to be // read on-chain cause it will put all the Events emitted in the block so far into the PoV. - let events = System::::events().iter().map(|e| e.clone().event).collect::>(); // todo: should determinism::Relaxed be checked here? + // let events = System::::events().iter().map(|e| e.clone().event).collect::>(); + // // todo: should determinism::Relaxed be checked here? + let events = System::::events(); + // .into_iter() + // .map(|e| e.event) + // .collect::>(); ContractExecResult { result: output.result.map_err(|r| r.error), gas_consumed: output.gas_meter.gas_consumed(), diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index bae6c1946e183..7e98787633a71 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -2900,6 +2900,45 @@ fn ecdsa_recover() { }) } +#[test] +fn result_returns_events() { + let (wasm, _code_hash) = compile_module::("transfer_return_code").unwrap(); + ExtBuilder::default().existential_deposit(50).build().execute_with(|| { + let min_balance = ::Currency::minimum_balance(); + let _ = Balances::deposit_creating(&ALICE, 1000 * min_balance); + + let addr = Contracts::bare_instantiate( + ALICE, + min_balance * 100, + GAS_LIMIT, + None, + Code::Upload(wasm), + vec![], + vec![], + false, + ) + .result + .unwrap() + .account_id; + + let result = Contracts::bare_call( + ALICE, + addr.clone(), + 0, + GAS_LIMIT, + None, + vec![], + false, + Determinism::Enforced, + ); + + let events = result.events.unwrap(); + assert_return_code!(&result.result.unwrap(), RuntimeReturnCode::Success); + assert!(!events.is_empty()); + assert_eq!(events, System::events()); + }); +} + #[test] fn upload_code_works() { let (wasm, code_hash) = compile_module::("dummy").unwrap(); From 7ca1b4b9ac8beddddb43737247f8b817f0bcebda Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Tue, 4 Apr 2023 09:42:47 +0200 Subject: [PATCH 05/28] contracts: update bare_call test name --- frame/contracts/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index 7e98787633a71..a83121d9f35e0 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -2901,7 +2901,7 @@ fn ecdsa_recover() { } #[test] -fn result_returns_events() { +fn bare_call_result_returns_events() { let (wasm, _code_hash) = compile_module::("transfer_return_code").unwrap(); ExtBuilder::default().existential_deposit(50).build().execute_with(|| { let min_balance = ::Currency::minimum_balance(); From 5ad08687bd182510b9a2ae606807da84ac0b96b6 Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Tue, 4 Apr 2023 10:28:11 +0200 Subject: [PATCH 06/28] contracts: add better comments to dry run events --- frame/contracts/primitives/src/lib.rs | 2 ++ frame/contracts/src/lib.rs | 16 ++++++---------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/frame/contracts/primitives/src/lib.rs b/frame/contracts/primitives/src/lib.rs index 8aae016fb32c2..3714390004bc3 100644 --- a/frame/contracts/primitives/src/lib.rs +++ b/frame/contracts/primitives/src/lib.rs @@ -71,6 +71,8 @@ pub struct ContractResult { pub debug_message: Vec, /// The execution result of the wasm code. pub result: R, + /// The events that were emitted during execution. During on-chain execution these events are + /// None to avoid putting all the Events emitted in the block so far into the PoV. pub events: Option>, } diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index eee9a2c6c4f80..d2e7a5eaa70b2 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -1133,15 +1133,11 @@ impl Pallet { debug_message: debug_message.as_mut(), }; let output = CallInput:: { dest, determinism }.run_guarded(common); - // We are good to call System::events() from the runtime API (i.e offchain). - // Even though it says it should only be used in tests, it is actually not allowed to be - // read on-chain cause it will put all the Events emitted in the block so far into the PoV. - // let events = System::::events().iter().map(|e| e.clone().event).collect::>(); - // // todo: should determinism::Relaxed be checked here? - let events = System::::events(); - // .into_iter() - // .map(|e| e.event) - // .collect::>(); + // We are good to call System::events() here. Even though its documentation says it should + // only be used in tests, it is actually not allowed to be read on-chain cause it will put + // all the Events emitted in the block so far into the PoV. + let events = System::::events(); // TODO: do we want to conditionally set None here? + ContractExecResult { result: output.result.map_err(|r| r.error), gas_consumed: output.gas_meter.gas_consumed(), @@ -1193,7 +1189,7 @@ impl Pallet { gas_required: output.gas_meter.gas_required(), storage_deposit: output.storage_deposit, debug_message: debug_message.unwrap_or_default().to_vec(), - events: None, + events: None, // TODO: do we want to return events here? } } From dbd8735a75f00aef276900531ca173461b948fca Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Tue, 4 Apr 2023 13:49:20 +0200 Subject: [PATCH 07/28] contracts: fix pallet contracts primitives implementation --- bin/node/runtime/src/lib.rs | 9 +++++++-- frame/contracts/src/lib.rs | 14 +++++++------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 433ca8e8b839e..d8cfcf3b59054 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -1862,6 +1862,11 @@ type Migrations = ( pallet_contracts::Migration, ); +type EventRecord = frame_system::EventRecord< + ::RuntimeEvent, + ::Hash, +>; + /// MMR helper types. mod mmr { use super::Runtime; @@ -2122,7 +2127,7 @@ impl_runtime_apis! { } } - impl pallet_contracts::ContractsApi for Runtime + impl pallet_contracts::ContractsApi for Runtime { fn call( origin: AccountId, @@ -2131,7 +2136,7 @@ impl_runtime_apis! { gas_limit: Option, storage_deposit_limit: Option, input_data: Vec, - ) -> pallet_contracts_primitives::ContractExecResult { + ) -> pallet_contracts_primitives::ContractExecResult { let gas_limit = gas_limit.unwrap_or(RuntimeBlockWeights::get().max_block); Contracts::bare_call( origin, diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index d2e7a5eaa70b2..1123a12ff10b9 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -1133,10 +1133,10 @@ impl Pallet { debug_message: debug_message.as_mut(), }; let output = CallInput:: { dest, determinism }.run_guarded(common); - // We are good to call System::events() here. Even though its documentation says it should - // only be used in tests, it is actually not allowed to be read on-chain cause it will put - // all the Events emitted in the block so far into the PoV. - let events = System::::events(); // TODO: do we want to conditionally set None here? + // We are good to call System::read_events_no_consensus() from the runtime API. + // `read_events_no_consensus` is actually not allowed to be called on-chain cause it will + // put all the Events emitted in the block so far into the PoV. + let events = System::::read_events_no_consensus().map(|e| *e).collect(); // TODO: do we want to conditionally set None here? ContractExecResult { result: output.result.map_err(|r| r.error), @@ -1297,12 +1297,12 @@ impl Pallet { sp_api::decl_runtime_apis! { /// The API used to dry-run contract interactions. #[api_version(2)] - pub trait ContractsApi where + pub trait ContractsApi where AccountId: Codec, Balance: Codec, BlockNumber: Codec, Hash: Codec, - Event: Codec, + EventRecord: Codec, { /// Perform a call from a specified account to a given contract. /// @@ -1314,7 +1314,7 @@ sp_api::decl_runtime_apis! { gas_limit: Option, storage_deposit_limit: Option, input_data: Vec, - ) -> ContractExecResult; + ) -> ContractExecResult; /// Instantiate a new contract. /// From 171840079702afd91a613cf9dea74dc681a10c91 Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Tue, 4 Apr 2023 17:01:44 +0200 Subject: [PATCH 08/28] contracts: add EventRecord generic to ContractInstantiateResult --- bin/node/runtime/src/lib.rs | 2 +- frame/contracts/primitives/src/lib.rs | 12 ++++++------ frame/contracts/src/lib.rs | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index d8cfcf3b59054..257b2aef26f8d 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -2158,7 +2158,7 @@ impl_runtime_apis! { code: pallet_contracts_primitives::Code, data: Vec, salt: Vec, - ) -> pallet_contracts_primitives::ContractInstantiateResult + ) -> pallet_contracts_primitives::ContractInstantiateResult { let gas_limit = gas_limit.unwrap_or(RuntimeBlockWeights::get().max_block); Contracts::bare_instantiate( diff --git a/frame/contracts/primitives/src/lib.rs b/frame/contracts/primitives/src/lib.rs index 3714390004bc3..3f17872d11c79 100644 --- a/frame/contracts/primitives/src/lib.rs +++ b/frame/contracts/primitives/src/lib.rs @@ -33,7 +33,7 @@ use sp_weights::Weight; /// /// It contains the execution result together with some auxiliary information. #[derive(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. @@ -73,16 +73,16 @@ pub struct ContractResult { pub result: R, /// The events that were emitted during execution. During on-chain execution these events are /// None to avoid putting all the Events emitted in the block so far into the PoV. - pub events: Option>, + pub events: Option>, } /// Result type of a `bare_call` call. -pub type ContractExecResult = - ContractResult, Balance, Event>; +pub type ContractExecResult = + ContractResult, Balance, EventRecord>; /// Result type of a `bare_instantiate` call. -pub type ContractInstantiateResult = - ContractResult, DispatchError>, Balance, ()>; +pub type ContractInstantiateResult = + ContractResult, DispatchError>, Balance, EventRecord>; /// Result type of a `bare_code_upload` call. pub type CodeUploadResult = diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 1123a12ff10b9..54dd5b4b439a8 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -1169,7 +1169,7 @@ impl Pallet { data: Vec, salt: Vec, debug: bool, - ) -> ContractInstantiateResult> { + ) -> ContractInstantiateResult, EventRecordOf> { let mut debug_message = if debug { Some(DebugBufferVec::::default()) } else { None }; let common = CommonInput { origin, @@ -1327,7 +1327,7 @@ sp_api::decl_runtime_apis! { code: Code, data: Vec, salt: Vec, - ) -> ContractInstantiateResult; + ) -> ContractInstantiateResult; /// Upload new code without instantiating a contract from it. From 782aba78ab9c4828e20226b442d3dd19afb92a6b Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Wed, 5 Apr 2023 17:42:56 +0200 Subject: [PATCH 09/28] contracts: make event collection optional --- bin/node/runtime/src/lib.rs | 4 +- frame/contracts/src/lib.rs | 22 +++-- frame/contracts/src/tests.rs | 162 ++++++++++++++++++++++++++++++++++- 3 files changed, 180 insertions(+), 8 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 257b2aef26f8d..f96a58331a1d5 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -2147,6 +2147,7 @@ impl_runtime_apis! { input_data, true, pallet_contracts::Determinism::Enforced, + true, ) } @@ -2169,7 +2170,8 @@ impl_runtime_apis! { code, data, salt, - true + true, + true, ) } diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 54dd5b4b439a8..6ffa6210988c5 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -1111,6 +1111,8 @@ impl Pallet { /// `debug` should only ever be set to `true` when executing as an RPC because /// it adds allocations and could be abused to drive the runtime into an OOM panic. /// If set to `true` it returns additional human readable debugging information. + /// `collect_events` should only be set to `true` when called off-chain as it enables collecting + /// all the Events emitted in the block so far and put them in them into the PoV. /// /// It returns the execution result and the amount of used weight. pub fn bare_call( @@ -1122,6 +1124,7 @@ impl Pallet { data: Vec, debug: bool, determinism: Determinism, + collect_events: bool, ) -> ContractExecResult, EventRecordOf> { let mut debug_message = if debug { Some(DebugBufferVec::::default()) } else { None }; let common = CommonInput { @@ -1133,10 +1136,11 @@ impl Pallet { debug_message: debug_message.as_mut(), }; let output = CallInput:: { dest, determinism }.run_guarded(common); - // We are good to call System::read_events_no_consensus() from the runtime API. - // `read_events_no_consensus` is actually not allowed to be called on-chain cause it will - // put all the Events emitted in the block so far into the PoV. - let events = System::::read_events_no_consensus().map(|e| *e).collect(); // TODO: do we want to conditionally set None here? + let events = if collect_events { + Some(System::::read_events_no_consensus().map(|e| *e).collect()) + } else { + None + }; ContractExecResult { result: output.result.map_err(|r| r.error), @@ -1144,7 +1148,7 @@ impl Pallet { gas_required: output.gas_meter.gas_required(), storage_deposit: output.storage_deposit, debug_message: debug_message.unwrap_or_default().to_vec(), - events: Some(events), + events, } } @@ -1169,6 +1173,7 @@ impl Pallet { data: Vec, salt: Vec, debug: bool, + collect_events: bool, ) -> ContractInstantiateResult, EventRecordOf> { let mut debug_message = if debug { Some(DebugBufferVec::::default()) } else { None }; let common = CommonInput { @@ -1180,6 +1185,11 @@ impl Pallet { debug_message: debug_message.as_mut(), }; let output = InstantiateInput:: { code, salt }.run_guarded(common); + let events = if collect_events { + Some(System::::read_events_no_consensus().map(|e| *e).collect()) + } else { + None + }; ContractInstantiateResult { result: output .result @@ -1189,7 +1199,7 @@ impl Pallet { gas_required: output.gas_meter.gas_required(), storage_deposit: output.storage_deposit, debug_message: debug_message.unwrap_or_default().to_vec(), - events: None, // TODO: do we want to return events here? + events, } } diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index a83121d9f35e0..193ab440319a1 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -550,6 +550,7 @@ fn instantiate_and_call_and_deposit_event() { vec![], vec![], false, + false, ) .result .unwrap() @@ -656,6 +657,7 @@ fn deposit_event_max_value_limit() { vec![], vec![], false, + false, ) .result .unwrap() @@ -702,6 +704,7 @@ fn run_out_of_gas() { vec![], vec![], false, + false, ) .result .unwrap() @@ -744,6 +747,7 @@ fn instantiate_unique_trie_id() { vec![], vec![], false, + false, ) .result .unwrap() @@ -806,6 +810,7 @@ fn storage_max_value_limit() { vec![], vec![], false, + false, ) .result .unwrap() @@ -856,6 +861,7 @@ fn deploy_and_call_other_contract() { vec![], vec![], false, + false, ) .result .unwrap() @@ -1002,6 +1008,7 @@ fn delegate_call() { vec![], vec![], false, + false, ) .result .unwrap() @@ -1041,6 +1048,7 @@ fn transfer_allow_death_cannot_kill_account() { vec![], vec![], false, + false, ) .result .unwrap() @@ -1081,6 +1089,7 @@ fn cannot_self_destruct_through_draning() { vec![], vec![], false, + false, ) .result .unwrap() @@ -1125,6 +1134,7 @@ fn cannot_self_destruct_through_storage_refund_after_price_change() { vec![], vec![], false, + false, ) .result .unwrap() @@ -1183,6 +1193,7 @@ fn cannot_self_destruct_while_live() { vec![], vec![], false, + false, ) .result .unwrap() @@ -1227,6 +1238,7 @@ fn self_destruct_works() { vec![], vec![], false, + false, ) .result .unwrap() @@ -1334,6 +1346,7 @@ fn destroy_contract_and_transfer_funds() { callee_code_hash.as_ref().to_vec(), vec![], false, + false, ) .result .unwrap() @@ -1398,6 +1411,7 @@ fn crypto_hashes() { vec![], vec![], false, + false, ) .result .unwrap() @@ -1432,6 +1446,7 @@ fn crypto_hashes() { params, false, Determinism::Enforced, + false, ) .result .unwrap(); @@ -1458,6 +1473,7 @@ fn transfer_return_code() { vec![], vec![], false, + false, ) .result .unwrap() @@ -1474,6 +1490,7 @@ fn transfer_return_code() { vec![], false, Determinism::Enforced, + false, ) .result .unwrap(); @@ -1499,6 +1516,7 @@ fn call_return_code() { vec![0], vec![], false, + false, ) .result .unwrap() @@ -1515,6 +1533,7 @@ fn call_return_code() { AsRef::<[u8]>::as_ref(&DJANGO).to_vec(), false, Determinism::Enforced, + false, ) .result .unwrap(); @@ -1529,6 +1548,7 @@ fn call_return_code() { vec![0], vec![], false, + false, ) .result .unwrap() @@ -1549,6 +1569,7 @@ fn call_return_code() { .collect(), false, Determinism::Enforced, + false, ) .result .unwrap(); @@ -1569,6 +1590,7 @@ fn call_return_code() { .collect(), false, Determinism::Enforced, + false, ) .result .unwrap(); @@ -1588,6 +1610,7 @@ fn call_return_code() { .collect(), false, Determinism::Enforced, + false, ) .result .unwrap(); @@ -1624,6 +1647,7 @@ fn instantiate_return_code() { vec![], vec![], false, + false, ) .result .unwrap() @@ -1640,6 +1664,7 @@ fn instantiate_return_code() { callee_hash.clone(), false, Determinism::Enforced, + false, ) .result .unwrap(); @@ -1656,6 +1681,7 @@ fn instantiate_return_code() { vec![0; 33], false, Determinism::Enforced, + false, ) .result .unwrap(); @@ -1671,6 +1697,7 @@ fn instantiate_return_code() { callee_hash.iter().chain(&1u32.to_le_bytes()).cloned().collect(), false, Determinism::Enforced, + false, ) .result .unwrap(); @@ -1686,6 +1713,7 @@ fn instantiate_return_code() { callee_hash.iter().chain(&2u32.to_le_bytes()).cloned().collect(), false, Determinism::Enforced, + false, ) .result .unwrap(); @@ -1730,6 +1758,7 @@ fn disabled_chain_extension_errors_on_call() { vec![], vec![], false, + false, ) .result .unwrap() @@ -1757,6 +1786,7 @@ fn chain_extension_works() { vec![], vec![], false, + false, ) .result .unwrap() @@ -1773,6 +1803,7 @@ fn chain_extension_works() { input.clone(), false, Determinism::Enforced, + false, ); assert_eq!(TestExtension::last_seen_buffer(), input); assert_eq!(result.result.unwrap().data, input); @@ -1787,6 +1818,7 @@ fn chain_extension_works() { ExtensionInput { extension_id: 0, func_id: 1, extra: &[] }.into(), false, Determinism::Enforced, + false, ) .result .unwrap(); @@ -1803,6 +1835,7 @@ fn chain_extension_works() { ExtensionInput { extension_id: 0, func_id: 2, extra: &[0] }.into(), false, Determinism::Enforced, + false, ); assert_ok!(result.result); let gas_consumed = result.gas_consumed; @@ -1815,6 +1848,7 @@ fn chain_extension_works() { ExtensionInput { extension_id: 0, func_id: 2, extra: &[42] }.into(), false, Determinism::Enforced, + false, ); assert_ok!(result.result); assert_eq!(result.gas_consumed.ref_time(), gas_consumed.ref_time() + 42); @@ -1827,6 +1861,7 @@ fn chain_extension_works() { ExtensionInput { extension_id: 0, func_id: 2, extra: &[95] }.into(), false, Determinism::Enforced, + false, ); assert_ok!(result.result); assert_eq!(result.gas_consumed.ref_time(), gas_consumed.ref_time() + 95); @@ -1841,6 +1876,7 @@ fn chain_extension_works() { ExtensionInput { extension_id: 0, func_id: 3, extra: &[] }.into(), false, Determinism::Enforced, + false, ) .result .unwrap(); @@ -1859,6 +1895,7 @@ fn chain_extension_works() { ExtensionInput { extension_id: 1, func_id: 0, extra: &[] }.into(), false, Determinism::Enforced, + false, ) .result .unwrap(); @@ -1896,6 +1933,7 @@ fn chain_extension_temp_storage_works() { vec![], vec![], false, + false, ) .result .unwrap() @@ -1919,7 +1957,8 @@ fn chain_extension_temp_storage_works() { None, input.clone(), false, - Determinism::Enforced + Determinism::Enforced, + false, ) .result ); @@ -1942,6 +1981,7 @@ fn lazy_removal_works() { vec![], vec![], false, + false, ) .result .unwrap() @@ -1995,6 +2035,7 @@ fn lazy_batch_removal_works() { vec![], vec![i], false, + false, ) .result .unwrap() @@ -2060,6 +2101,7 @@ fn lazy_removal_partial_remove_works() { vec![], vec![], false, + false, ) .result .unwrap() @@ -2141,6 +2183,7 @@ fn lazy_removal_does_no_run_on_low_remaining_weight() { vec![], vec![], false, + false, ) .result .unwrap() @@ -2212,6 +2255,7 @@ fn lazy_removal_does_not_use_all_weight() { vec![], vec![], false, + false, ) .result .unwrap() @@ -2302,6 +2346,7 @@ fn deletion_queue_ring_buffer_overflow() { vec![], vec![i], false, + false, ) .result .unwrap() @@ -2359,6 +2404,7 @@ fn refcounter() { vec![], vec![0], false, + false, ) .result .unwrap() @@ -2372,6 +2418,7 @@ fn refcounter() { vec![], vec![1], false, + false, ) .result .unwrap() @@ -2388,6 +2435,7 @@ fn refcounter() { vec![], vec![2], false, + false, ) .result .unwrap() @@ -2454,6 +2502,7 @@ fn reinstrument_does_charge() { zero.clone(), vec![], false, + false, ) .result .unwrap() @@ -2470,6 +2519,7 @@ fn reinstrument_does_charge() { zero.clone(), false, Determinism::Enforced, + false, ); assert!(!result0.result.unwrap().did_revert()); @@ -2482,6 +2532,7 @@ fn reinstrument_does_charge() { zero.clone(), false, Determinism::Enforced, + false, ); assert!(!result1.result.unwrap().did_revert()); @@ -2504,6 +2555,7 @@ fn reinstrument_does_charge() { zero.clone(), false, Determinism::Enforced, + false, ); assert!(!result2.result.unwrap().did_revert()); assert!(result2.gas_consumed.ref_time() > result1.gas_consumed.ref_time()); @@ -2530,6 +2582,7 @@ fn debug_message_works() { vec![], vec![], false, + false, ) .result .unwrap() @@ -2543,6 +2596,7 @@ fn debug_message_works() { vec![], true, Determinism::Enforced, + false, ); assert_matches!(result.result, Ok(_)); @@ -2565,6 +2619,7 @@ fn debug_message_logging_disabled() { vec![], vec![], false, + false, ) .result .unwrap() @@ -2579,6 +2634,7 @@ fn debug_message_logging_disabled() { vec![], false, Determinism::Enforced, + false, ); assert_matches!(result.result, Ok(_)); // the dispatchables always run without debugging @@ -2602,6 +2658,7 @@ fn debug_message_invalid_utf8() { vec![], vec![], false, + false, ) .result .unwrap() @@ -2615,6 +2672,7 @@ fn debug_message_invalid_utf8() { vec![], true, Determinism::Enforced, + false, ); assert_ok!(result.result); assert!(result.debug_message.is_empty()); @@ -2639,6 +2697,7 @@ fn gas_estimation_nested_call_fixed_limit() { vec![], vec![0], false, + false, ) .result .unwrap() @@ -2653,6 +2712,7 @@ fn gas_estimation_nested_call_fixed_limit() { vec![], vec![1], false, + false, ) .result .unwrap() @@ -2674,6 +2734,7 @@ fn gas_estimation_nested_call_fixed_limit() { input.clone(), false, Determinism::Enforced, + false, ); assert_ok!(&result.result); @@ -2691,6 +2752,7 @@ fn gas_estimation_nested_call_fixed_limit() { input, false, Determinism::Enforced, + false, ) .result ); @@ -2716,6 +2778,7 @@ fn gas_estimation_call_runtime() { vec![], vec![0], false, + false, ) .result .unwrap() @@ -2730,6 +2793,7 @@ fn gas_estimation_call_runtime() { vec![], vec![1], false, + false, ) .result .unwrap() @@ -2750,6 +2814,7 @@ fn gas_estimation_call_runtime() { call.encode(), false, Determinism::Enforced, + false, ); // contract encodes the result of the dispatch runtime let outcome = u32::decode(&mut result.result.unwrap().data.as_ref()).unwrap(); @@ -2767,6 +2832,7 @@ fn gas_estimation_call_runtime() { call.encode(), false, Determinism::Enforced, + false, ) .result ); @@ -2791,6 +2857,7 @@ fn gas_call_runtime_reentrancy_guarded() { vec![], vec![0], false, + false, ) .result .unwrap() @@ -2805,6 +2872,7 @@ fn gas_call_runtime_reentrancy_guarded() { vec![], vec![1], false, + false, ) .result .unwrap() @@ -2830,6 +2898,7 @@ fn gas_call_runtime_reentrancy_guarded() { call.encode(), false, Determinism::Enforced, + false, ) .result .unwrap(); @@ -2855,6 +2924,7 @@ fn ecdsa_recover() { vec![], vec![], false, + false, ) .result .unwrap() @@ -2892,6 +2962,7 @@ fn ecdsa_recover() { params, false, Determinism::Enforced, + false, ) .result .unwrap(); @@ -2900,6 +2971,56 @@ fn ecdsa_recover() { }) } +#[test] +fn bare_instantiate_result_returns_events() { + let (wasm, _code_hash) = compile_module::("transfer_return_code").unwrap(); + ExtBuilder::default().existential_deposit(50).build().execute_with(|| { + let min_balance = ::Currency::minimum_balance(); + let _ = Balances::deposit_creating(&ALICE, 1000 * min_balance); + + let result = Contracts::bare_instantiate( + ALICE, + min_balance * 100, + GAS_LIMIT, + None, + Code::Upload(wasm), + vec![], + vec![], + false, + true, + ); + + let events = result.events.unwrap(); + assert!(!events.is_empty()); + assert_eq!(events, System::events()); + }); +} + +#[test] +fn bare_instantiate_result_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 = ::Currency::minimum_balance(); + let _ = Balances::deposit_creating(&ALICE, 1000 * min_balance); + + let result = Contracts::bare_instantiate( + ALICE, + min_balance * 100, + GAS_LIMIT, + None, + Code::Upload(wasm), + vec![], + vec![], + false, + false, + ); + + let events = result.events; + assert!(!System::events().is_empty()); + assert!(events.is_none()); + }); +} + #[test] fn bare_call_result_returns_events() { let (wasm, _code_hash) = compile_module::("transfer_return_code").unwrap(); @@ -2916,6 +3037,7 @@ fn bare_call_result_returns_events() { vec![], vec![], false, + false, ) .result .unwrap() @@ -2930,6 +3052,7 @@ fn bare_call_result_returns_events() { vec![], false, Determinism::Enforced, + true, ); let events = result.events.unwrap(); @@ -3193,6 +3316,7 @@ fn instantiate_with_zero_balance_works() { vec![], vec![], false, + false, ) .result .unwrap() @@ -3303,6 +3427,7 @@ fn instantiate_with_below_existential_deposit_works() { vec![], vec![], false, + false, ) .result .unwrap() @@ -3418,6 +3543,7 @@ fn storage_deposit_works() { vec![], vec![], false, + false, ) .result .unwrap() @@ -3557,6 +3683,7 @@ fn set_code_extrinsic() { vec![], vec![], false, + false, ) .result .unwrap() @@ -3641,6 +3768,7 @@ fn slash_cannot_kill_account() { vec![], vec![], false, + false, ) .result .unwrap() @@ -3730,6 +3858,7 @@ fn contract_reverted() { input.clone(), vec![], false, + false, ) .result .unwrap(); @@ -3747,6 +3876,7 @@ fn contract_reverted() { ReturnFlags::empty().bits().encode(), vec![], false, + false, ) .result .unwrap() @@ -3775,6 +3905,7 @@ fn contract_reverted() { input, false, Determinism::Enforced, + false, ) .result .unwrap(); @@ -3807,6 +3938,7 @@ fn code_rejected_error_works() { vec![], vec![], true, + false, ); assert_err!(result.result, >::CodeRejected); assert_eq!( @@ -3834,6 +3966,7 @@ fn code_rejected_error_works() { vec![], vec![], true, + false, ); assert_err!(result.result, >::CodeRejected); assert_eq!( @@ -3861,6 +3994,7 @@ fn set_code_hash() { vec![], vec![], false, + false, ) .result .unwrap() @@ -3885,6 +4019,7 @@ fn set_code_hash() { new_code_hash.as_ref().to_vec(), true, Determinism::Enforced, + false, ) .result .unwrap(); @@ -3900,6 +4035,7 @@ fn set_code_hash() { vec![], true, Determinism::Enforced, + false, ) .result .unwrap(); @@ -3956,6 +4092,7 @@ fn storage_deposit_limit_is_enforced() { vec![], vec![], false, + false, ) .result .unwrap() @@ -3998,6 +4135,7 @@ fn storage_deposit_limit_is_enforced_late() { vec![], vec![], false, + false, ) .result .unwrap() @@ -4011,6 +4149,7 @@ fn storage_deposit_limit_is_enforced_late() { vec![], vec![], false, + false, ) .result .unwrap() @@ -4143,6 +4282,7 @@ fn deposit_limit_honors_liquidity_restrictions() { vec![], vec![], false, + false, ) .result .unwrap() @@ -4187,6 +4327,7 @@ fn deposit_limit_honors_existential_deposit() { vec![], vec![], false, + false, ) .result .unwrap() @@ -4230,6 +4371,7 @@ fn deposit_limit_honors_min_leftover() { vec![], vec![], false, + false, ) .result .unwrap() @@ -4285,6 +4427,7 @@ fn cannot_instantiate_indeterministic_code() { vec![], vec![], false, + false, ) .result, >::CodeRejected, @@ -4330,6 +4473,7 @@ fn cannot_instantiate_indeterministic_code() { vec![], vec![], false, + false, ) .result, >::Indeterministic, @@ -4345,6 +4489,7 @@ fn cannot_instantiate_indeterministic_code() { vec![], vec![], false, + false, ) .result .unwrap() @@ -4361,6 +4506,7 @@ fn cannot_instantiate_indeterministic_code() { code_hash.encode(), false, Determinism::Enforced, + false, ) .result, >::Indeterministic, @@ -4377,6 +4523,7 @@ fn cannot_instantiate_indeterministic_code() { code_hash.encode(), false, Determinism::Relaxed, + false, ) .result, >::Indeterministic, @@ -4409,6 +4556,7 @@ fn cannot_set_code_indeterministic_code() { vec![], vec![], false, + false, ) .result .unwrap() @@ -4425,6 +4573,7 @@ fn cannot_set_code_indeterministic_code() { code_hash.encode(), false, Determinism::Relaxed, + false, ) .result, >::Indeterministic, @@ -4457,6 +4606,7 @@ fn delegate_call_indeterministic_code() { vec![], vec![], false, + false, ) .result .unwrap() @@ -4473,6 +4623,7 @@ fn delegate_call_indeterministic_code() { code_hash.encode(), false, Determinism::Enforced, + false, ) .result, >::Indeterministic, @@ -4489,6 +4640,7 @@ fn delegate_call_indeterministic_code() { code_hash.encode(), false, Determinism::Relaxed, + false, ) .result ); @@ -4511,6 +4663,7 @@ fn reentrance_count_works_with_call() { vec![], vec![], false, + false, ) .result .unwrap() @@ -4528,6 +4681,7 @@ fn reentrance_count_works_with_call() { input, true, Determinism::Enforced, + false, ) .result .unwrap(); @@ -4550,6 +4704,7 @@ fn reentrance_count_works_with_delegated_call() { vec![], vec![], false, + false, ) .result .unwrap() @@ -4567,6 +4722,7 @@ fn reentrance_count_works_with_delegated_call() { input, true, Determinism::Enforced, + false, ) .result .unwrap(); @@ -4591,6 +4747,7 @@ fn account_reentrance_count_works() { vec![], vec![], false, + false, ) .result .unwrap() @@ -4605,6 +4762,7 @@ fn account_reentrance_count_works() { vec![], vec![], false, + false, ) .result .unwrap() @@ -4619,6 +4777,7 @@ fn account_reentrance_count_works() { contract_addr.encode(), true, Determinism::Enforced, + false, ) .result .unwrap(); @@ -4632,6 +4791,7 @@ fn account_reentrance_count_works() { another_contract_addr.encode(), true, Determinism::Enforced, + false, ) .result .unwrap(); From 74d4b708eaa0a97554977b1bcefda3ed5a9a9ca8 Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Wed, 5 Apr 2023 17:54:20 +0200 Subject: [PATCH 10/28] contracts: impreved notes on `collect_events` --- frame/contracts/src/lib.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 6ffa6210988c5..aa02dc76fa4dd 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -1111,8 +1111,9 @@ impl Pallet { /// `debug` should only ever be set to `true` when executing as an RPC because /// it adds allocations and could be abused to drive the runtime into an OOM panic. /// If set to `true` it returns additional human readable debugging information. - /// `collect_events` should only be set to `true` when called off-chain as it enables collecting - /// all the Events emitted in the block so far and put them in them into the PoV. + /// `collect_events` should only be set to `true` when called off-chain as it would enable + /// collecting all the Events emitted in the block so far and put them in them into the PoV. If + /// `collect_events` is `true` the result return `Some(events)`, otherwise it returns `None` /// /// It returns the execution result and the amount of used weight. pub fn bare_call( @@ -1164,6 +1165,9 @@ impl Pallet { /// `debug` should only ever be set to `true` when executing as an RPC because /// it adds allocations and could be abused to drive the runtime into an OOM panic. /// If set to `true` it returns additional human readable debugging information. + /// `collect_events` should only be set to `true` when called off-chain as it would enable + /// collecting all the Events emitted in the block so far and put them in them into the PoV. If + /// `collect_events` is `true` the result return `Some(events)`, otherwise it returns `None` pub fn bare_instantiate( origin: T::AccountId, value: BalanceOf, From 45e6eab1756a8df425ae74978701fd2ab7310272 Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Thu, 6 Apr 2023 10:43:45 +0200 Subject: [PATCH 11/28] contracts: update benchmarking calls --- frame/contracts/src/benchmarking/mod.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/frame/contracts/src/benchmarking/mod.rs b/frame/contracts/src/benchmarking/mod.rs index 1bb6f3395fcef..20392256378c0 100644 --- a/frame/contracts/src/benchmarking/mod.rs +++ b/frame/contracts/src/benchmarking/mod.rs @@ -930,6 +930,7 @@ benchmarks! { vec![], true, Determinism::Enforced, + false, ) .result?; } @@ -979,6 +980,7 @@ benchmarks! { vec![], true, Determinism::Enforced, + false, ) .result?; } @@ -3054,6 +3056,7 @@ benchmarks! { data, false, Determinism::Enforced, + false, ) .result?; } @@ -3103,6 +3106,7 @@ benchmarks! { data, false, Determinism::Enforced, + false, ) .result?; } From 4426bbe9747eb3256a65b8dc7bcebec99fd0ede7 Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Wed, 12 Apr 2023 16:27:45 +0200 Subject: [PATCH 12/28] contracts: change bare_call and bare_instantiate to accept enums instead of bools --- frame/contracts/src/lib.rs | 69 ++++-- frame/contracts/src/tests.rs | 452 +++++++++++++++++------------------ 2 files changed, 274 insertions(+), 247 deletions(-) diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index aa02dc76fa4dd..a91b7a21b2946 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -932,6 +932,27 @@ struct InstantiateInput { salt: Vec, } +pub enum CollectEvents { + /// # Note + /// + /// Events should only be collected when called off-chain, as this would + /// collect all the Events emitted in the block so far and put them in them into the PoV. + /// + /// **Never** use this mode for on-chain execution. + UnsafeCollect, + /// Events should not be collected. + Skip, +} + +pub enum DebugInfo { + /// # Note + /// + /// This should only ever be set to `UnsafeYes` when executing as an RPC because + /// it adds allocations and could be abused to drive the runtime into an OOM panic. + UnsafeDebug, + Skip, +} + /// Return type of private helper functions. struct InternalOutput { /// The gas meter that was used to execute the call. @@ -1108,14 +1129,11 @@ impl Pallet { /// /// # Note /// - /// `debug` should only ever be set to `true` when executing as an RPC because - /// it adds allocations and could be abused to drive the runtime into an OOM panic. - /// If set to `true` it returns additional human readable debugging information. - /// `collect_events` should only be set to `true` when called off-chain as it would enable - /// collecting all the Events emitted in the block so far and put them in them into the PoV. If - /// `collect_events` is `true` the result return `Some(events)`, otherwise it returns `None` + /// If `debug` is set to `DebugInfo::UnsafeDebug` it returns additional human readable debugging + /// information. /// - /// It returns the execution result and the amount of used weight. + /// If `collect_events` is set to `CollectEvents::UnsafeCollect` it collects all the Events + /// emitted in the block so far. pub fn bare_call( origin: T::AccountId, dest: T::AccountId, @@ -1123,11 +1141,15 @@ impl Pallet { gas_limit: Weight, storage_deposit_limit: Option>, data: Vec, - debug: bool, + debug: DebugInfo, determinism: Determinism, - collect_events: bool, + collect_events: CollectEvents, ) -> ContractExecResult, EventRecordOf> { - let mut debug_message = if debug { Some(DebugBufferVec::::default()) } else { None }; + let mut debug_message = if matches!(debug, DebugInfo::UnsafeDebug) { + Some(DebugBufferVec::::default()) + } else { + None + }; let common = CommonInput { origin, value, @@ -1137,7 +1159,8 @@ impl Pallet { debug_message: debug_message.as_mut(), }; let output = CallInput:: { dest, determinism }.run_guarded(common); - let events = if collect_events { + // collect events if CollectEvents is UnsafeCollect + let events = if matches!(collect_events, CollectEvents::UnsafeCollect) { Some(System::::read_events_no_consensus().map(|e| *e).collect()) } else { None @@ -1162,12 +1185,11 @@ impl Pallet { /// /// # Note /// - /// `debug` should only ever be set to `true` when executing as an RPC because - /// it adds allocations and could be abused to drive the runtime into an OOM panic. - /// If set to `true` it returns additional human readable debugging information. - /// `collect_events` should only be set to `true` when called off-chain as it would enable - /// collecting all the Events emitted in the block so far and put them in them into the PoV. If - /// `collect_events` is `true` the result return `Some(events)`, otherwise it returns `None` + /// If `debug` is set to `DebugInfo::UnsafeDebug` it returns additional human readable debugging + /// information. + /// + /// If `collect_events` is set to `CollectEvents::UnsafeCollect` it collects all the Events + /// emitted in the block so far. pub fn bare_instantiate( origin: T::AccountId, value: BalanceOf, @@ -1176,10 +1198,14 @@ impl Pallet { code: Code>, data: Vec, salt: Vec, - debug: bool, - collect_events: bool, + debug: DebugInfo, + collect_events: CollectEvents, ) -> ContractInstantiateResult, EventRecordOf> { - let mut debug_message = if debug { Some(DebugBufferVec::::default()) } else { None }; + let mut debug_message = if matches!(debug, DebugInfo::UnsafeDebug) { + Some(DebugBufferVec::::default()) + } else { + None + }; let common = CommonInput { origin, value, @@ -1189,7 +1215,8 @@ impl Pallet { debug_message: debug_message.as_mut(), }; let output = InstantiateInput:: { code, salt }.run_guarded(common); - let events = if collect_events { + // collect events if CollectEvents is UnsafeCollect + let events = if matches!(collect_events, CollectEvents::UnsafeCollect) { Some(System::::read_events_no_consensus().map(|e| *e).collect()) } else { None diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index 193ab440319a1..ab46b4990c5b7 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -26,8 +26,8 @@ use crate::{ tests::test_utils::{get_contract, get_contract_checked}, wasm::{Determinism, PrefabWasmModule, ReturnCode as RuntimeReturnCode}, weights::WeightInfo, - BalanceOf, Code, CodeStorage, Config, ContractInfo, ContractInfoOf, DefaultAddressGenerator, - DeletionQueueCounter, Error, Pallet, Schedule, + BalanceOf, Code, CodeStorage, CollectEvents, Config, ContractInfo, ContractInfoOf, DebugInfo, + DefaultAddressGenerator, DeletionQueueCounter, Error, Pallet, Schedule, }; use assert_matches::assert_matches; use codec::Encode; @@ -549,8 +549,8 @@ fn instantiate_and_call_and_deposit_event() { Code::Existing(code_hash), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -656,8 +656,8 @@ fn deposit_event_max_value_limit() { Code::Upload(wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -703,8 +703,8 @@ fn run_out_of_gas() { Code::Upload(wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -746,8 +746,8 @@ fn instantiate_unique_trie_id() { Code::Existing(code_hash), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -809,8 +809,8 @@ fn storage_max_value_limit() { Code::Upload(wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -860,8 +860,8 @@ fn deploy_and_call_other_contract() { Code::Upload(caller_wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -1007,8 +1007,8 @@ fn delegate_call() { Code::Upload(caller_wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -1047,8 +1047,8 @@ fn transfer_allow_death_cannot_kill_account() { Code::Upload(wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -1088,8 +1088,8 @@ fn cannot_self_destruct_through_draning() { Code::Upload(wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -1133,8 +1133,8 @@ fn cannot_self_destruct_through_storage_refund_after_price_change() { Code::Upload(wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -1192,8 +1192,8 @@ fn cannot_self_destruct_while_live() { Code::Upload(wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -1237,8 +1237,8 @@ fn self_destruct_works() { Code::Upload(wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -1345,8 +1345,8 @@ fn destroy_contract_and_transfer_funds() { Code::Upload(caller_wasm), callee_code_hash.as_ref().to_vec(), vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -1410,8 +1410,8 @@ fn crypto_hashes() { Code::Upload(wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -1444,9 +1444,9 @@ fn crypto_hashes() { GAS_LIMIT, None, params, - false, + DebugInfo::Skip, Determinism::Enforced, - false, + CollectEvents::Skip, ) .result .unwrap(); @@ -1472,8 +1472,8 @@ fn transfer_return_code() { Code::Upload(wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -1488,9 +1488,9 @@ fn transfer_return_code() { GAS_LIMIT, None, vec![], - false, + DebugInfo::Skip, Determinism::Enforced, - false, + CollectEvents::Skip, ) .result .unwrap(); @@ -1515,8 +1515,8 @@ fn call_return_code() { Code::Upload(caller_code), vec![0], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -1531,9 +1531,9 @@ fn call_return_code() { GAS_LIMIT, None, AsRef::<[u8]>::as_ref(&DJANGO).to_vec(), - false, + DebugInfo::Skip, Determinism::Enforced, - false, + CollectEvents::Skip, ) .result .unwrap(); @@ -1547,8 +1547,8 @@ fn call_return_code() { Code::Upload(callee_code), vec![0], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -1567,9 +1567,9 @@ fn call_return_code() { .chain(&0u32.to_le_bytes()) .cloned() .collect(), - false, + DebugInfo::Skip, Determinism::Enforced, - false, + CollectEvents::Skip, ) .result .unwrap(); @@ -1588,9 +1588,9 @@ fn call_return_code() { .chain(&1u32.to_le_bytes()) .cloned() .collect(), - false, + DebugInfo::Skip, Determinism::Enforced, - false, + CollectEvents::Skip, ) .result .unwrap(); @@ -1608,9 +1608,9 @@ fn call_return_code() { .chain(&2u32.to_le_bytes()) .cloned() .collect(), - false, + DebugInfo::Skip, Determinism::Enforced, - false, + CollectEvents::Skip, ) .result .unwrap(); @@ -1646,8 +1646,8 @@ fn instantiate_return_code() { Code::Upload(caller_code), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -1662,9 +1662,9 @@ fn instantiate_return_code() { GAS_LIMIT, None, callee_hash.clone(), - false, + DebugInfo::Skip, Determinism::Enforced, - false, + CollectEvents::Skip, ) .result .unwrap(); @@ -1679,9 +1679,9 @@ fn instantiate_return_code() { GAS_LIMIT, None, vec![0; 33], - false, + DebugInfo::Skip, Determinism::Enforced, - false, + CollectEvents::Skip, ) .result .unwrap(); @@ -1695,9 +1695,9 @@ fn instantiate_return_code() { GAS_LIMIT, None, callee_hash.iter().chain(&1u32.to_le_bytes()).cloned().collect(), - false, + DebugInfo::Skip, Determinism::Enforced, - false, + CollectEvents::Skip, ) .result .unwrap(); @@ -1711,9 +1711,9 @@ fn instantiate_return_code() { GAS_LIMIT, None, callee_hash.iter().chain(&2u32.to_le_bytes()).cloned().collect(), - false, + DebugInfo::Skip, Determinism::Enforced, - false, + CollectEvents::Skip, ) .result .unwrap(); @@ -1757,8 +1757,8 @@ fn disabled_chain_extension_errors_on_call() { Code::Upload(code), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -1785,8 +1785,8 @@ fn chain_extension_works() { Code::Upload(code), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -1801,9 +1801,9 @@ fn chain_extension_works() { GAS_LIMIT, None, input.clone(), - false, + DebugInfo::Skip, Determinism::Enforced, - false, + CollectEvents::Skip, ); assert_eq!(TestExtension::last_seen_buffer(), input); assert_eq!(result.result.unwrap().data, input); @@ -1816,9 +1816,9 @@ fn chain_extension_works() { GAS_LIMIT, None, ExtensionInput { extension_id: 0, func_id: 1, extra: &[] }.into(), - false, + DebugInfo::Skip, Determinism::Enforced, - false, + CollectEvents::Skip, ) .result .unwrap(); @@ -1833,9 +1833,9 @@ fn chain_extension_works() { GAS_LIMIT, None, ExtensionInput { extension_id: 0, func_id: 2, extra: &[0] }.into(), - false, + DebugInfo::Skip, Determinism::Enforced, - false, + CollectEvents::Skip, ); assert_ok!(result.result); let gas_consumed = result.gas_consumed; @@ -1846,9 +1846,9 @@ fn chain_extension_works() { GAS_LIMIT, None, ExtensionInput { extension_id: 0, func_id: 2, extra: &[42] }.into(), - false, + DebugInfo::Skip, Determinism::Enforced, - false, + CollectEvents::Skip, ); assert_ok!(result.result); assert_eq!(result.gas_consumed.ref_time(), gas_consumed.ref_time() + 42); @@ -1859,9 +1859,9 @@ fn chain_extension_works() { GAS_LIMIT, None, ExtensionInput { extension_id: 0, func_id: 2, extra: &[95] }.into(), - false, + DebugInfo::Skip, Determinism::Enforced, - false, + CollectEvents::Skip, ); assert_ok!(result.result); assert_eq!(result.gas_consumed.ref_time(), gas_consumed.ref_time() + 95); @@ -1874,9 +1874,9 @@ fn chain_extension_works() { GAS_LIMIT, None, ExtensionInput { extension_id: 0, func_id: 3, extra: &[] }.into(), - false, + DebugInfo::Skip, Determinism::Enforced, - false, + CollectEvents::Skip, ) .result .unwrap(); @@ -1893,9 +1893,9 @@ fn chain_extension_works() { GAS_LIMIT, None, ExtensionInput { extension_id: 1, func_id: 0, extra: &[] }.into(), - false, + DebugInfo::Skip, Determinism::Enforced, - false, + CollectEvents::Skip, ) .result .unwrap(); @@ -1932,8 +1932,8 @@ fn chain_extension_temp_storage_works() { Code::Upload(code), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -1956,9 +1956,9 @@ fn chain_extension_temp_storage_works() { GAS_LIMIT, None, input.clone(), - false, + DebugInfo::Skip, Determinism::Enforced, - false, + CollectEvents::Skip, ) .result ); @@ -1980,8 +1980,8 @@ fn lazy_removal_works() { Code::Upload(code), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -2034,8 +2034,8 @@ fn lazy_batch_removal_works() { Code::Upload(code.clone()), vec![], vec![i], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -2100,8 +2100,8 @@ fn lazy_removal_partial_remove_works() { Code::Upload(code), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -2182,8 +2182,8 @@ fn lazy_removal_does_no_run_on_low_remaining_weight() { Code::Upload(code), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -2254,8 +2254,8 @@ fn lazy_removal_does_not_use_all_weight() { Code::Upload(code), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -2345,8 +2345,8 @@ fn deletion_queue_ring_buffer_overflow() { Code::Upload(code.clone()), vec![], vec![i], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -2403,8 +2403,8 @@ fn refcounter() { Code::Upload(wasm.clone()), vec![], vec![0], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -2417,8 +2417,8 @@ fn refcounter() { Code::Upload(wasm.clone()), vec![], vec![1], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -2434,8 +2434,8 @@ fn refcounter() { Code::Existing(code_hash), vec![], vec![2], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -2501,8 +2501,8 @@ fn reinstrument_does_charge() { Code::Upload(wasm), zero.clone(), vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -2517,9 +2517,9 @@ fn reinstrument_does_charge() { GAS_LIMIT, None, zero.clone(), - false, + DebugInfo::Skip, Determinism::Enforced, - false, + CollectEvents::Skip, ); assert!(!result0.result.unwrap().did_revert()); @@ -2530,9 +2530,9 @@ fn reinstrument_does_charge() { GAS_LIMIT, None, zero.clone(), - false, + DebugInfo::Skip, Determinism::Enforced, - false, + CollectEvents::Skip, ); assert!(!result1.result.unwrap().did_revert()); @@ -2553,9 +2553,9 @@ fn reinstrument_does_charge() { GAS_LIMIT, None, zero.clone(), - false, + DebugInfo::Skip, Determinism::Enforced, - false, + CollectEvents::Skip, ); assert!(!result2.result.unwrap().did_revert()); assert!(result2.gas_consumed.ref_time() > result1.gas_consumed.ref_time()); @@ -2581,8 +2581,8 @@ fn debug_message_works() { Code::Upload(wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -2594,9 +2594,9 @@ fn debug_message_works() { GAS_LIMIT, None, vec![], - true, + DebugInfo::UnsafeDebug, Determinism::Enforced, - false, + CollectEvents::Skip, ); assert_matches!(result.result, Ok(_)); @@ -2618,8 +2618,8 @@ fn debug_message_logging_disabled() { Code::Upload(wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -2632,9 +2632,9 @@ fn debug_message_logging_disabled() { GAS_LIMIT, None, vec![], - false, + DebugInfo::Skip, Determinism::Enforced, - false, + CollectEvents::Skip, ); assert_matches!(result.result, Ok(_)); // the dispatchables always run without debugging @@ -2657,8 +2657,8 @@ fn debug_message_invalid_utf8() { Code::Upload(wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -2670,9 +2670,9 @@ fn debug_message_invalid_utf8() { GAS_LIMIT, None, vec![], - true, + DebugInfo::UnsafeDebug, Determinism::Enforced, - false, + CollectEvents::Skip, ); assert_ok!(result.result); assert!(result.debug_message.is_empty()); @@ -2696,8 +2696,8 @@ fn gas_estimation_nested_call_fixed_limit() { Code::Upload(caller_code), vec![], vec![0], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -2711,8 +2711,8 @@ fn gas_estimation_nested_call_fixed_limit() { Code::Upload(callee_code), vec![], vec![1], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -2732,9 +2732,9 @@ fn gas_estimation_nested_call_fixed_limit() { GAS_LIMIT, None, input.clone(), - false, + DebugInfo::Skip, Determinism::Enforced, - false, + CollectEvents::Skip, ); assert_ok!(&result.result); @@ -2750,9 +2750,9 @@ fn gas_estimation_nested_call_fixed_limit() { result.gas_required, Some(result.storage_deposit.charge_or_zero()), input, - false, + DebugInfo::Skip, Determinism::Enforced, - false, + CollectEvents::Skip, ) .result ); @@ -2777,8 +2777,8 @@ fn gas_estimation_call_runtime() { Code::Upload(caller_code), vec![], vec![0], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -2792,8 +2792,8 @@ fn gas_estimation_call_runtime() { Code::Upload(callee_code), vec![], vec![1], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -2812,9 +2812,9 @@ fn gas_estimation_call_runtime() { GAS_LIMIT, None, call.encode(), - false, + DebugInfo::Skip, Determinism::Enforced, - false, + CollectEvents::Skip, ); // contract encodes the result of the dispatch runtime let outcome = u32::decode(&mut result.result.unwrap().data.as_ref()).unwrap(); @@ -2830,9 +2830,9 @@ fn gas_estimation_call_runtime() { result.gas_required, None, call.encode(), - false, + DebugInfo::Skip, Determinism::Enforced, - false, + CollectEvents::Skip, ) .result ); @@ -2856,8 +2856,8 @@ fn gas_call_runtime_reentrancy_guarded() { Code::Upload(caller_code), vec![], vec![0], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -2871,8 +2871,8 @@ fn gas_call_runtime_reentrancy_guarded() { Code::Upload(callee_code), vec![], vec![1], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -2896,9 +2896,9 @@ fn gas_call_runtime_reentrancy_guarded() { GAS_LIMIT, None, call.encode(), - false, + DebugInfo::Skip, Determinism::Enforced, - false, + CollectEvents::Skip, ) .result .unwrap(); @@ -2923,8 +2923,8 @@ fn ecdsa_recover() { Code::Upload(wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -2960,9 +2960,9 @@ fn ecdsa_recover() { GAS_LIMIT, None, params, - false, + DebugInfo::Skip, Determinism::Enforced, - false, + CollectEvents::Skip, ) .result .unwrap(); @@ -2986,8 +2986,8 @@ fn bare_instantiate_result_returns_events() { Code::Upload(wasm), vec![], vec![], - false, - true, + DebugInfo::Skip, + CollectEvents::UnsafeCollect, ); let events = result.events.unwrap(); @@ -3011,8 +3011,8 @@ fn bare_instantiate_result_does_not_return_events() { Code::Upload(wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ); let events = result.events; @@ -3036,8 +3036,8 @@ fn bare_call_result_returns_events() { Code::Upload(wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -3050,9 +3050,9 @@ fn bare_call_result_returns_events() { GAS_LIMIT, None, vec![], - false, + DebugInfo::Skip, Determinism::Enforced, - true, + CollectEvents::UnsafeCollect, ); let events = result.events.unwrap(); @@ -3315,8 +3315,8 @@ fn instantiate_with_zero_balance_works() { Code::Upload(wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -3426,8 +3426,8 @@ fn instantiate_with_below_existential_deposit_works() { Code::Upload(wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -3542,8 +3542,8 @@ fn storage_deposit_works() { Code::Upload(wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -3682,8 +3682,8 @@ fn set_code_extrinsic() { Code::Upload(wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -3767,8 +3767,8 @@ fn slash_cannot_kill_account() { Code::Upload(wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -3857,8 +3857,8 @@ fn contract_reverted() { Code::Existing(code_hash), input.clone(), vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap(); @@ -3875,8 +3875,8 @@ fn contract_reverted() { Code::Existing(code_hash), ReturnFlags::empty().bits().encode(), vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -3903,9 +3903,9 @@ fn contract_reverted() { GAS_LIMIT, None, input, - false, + DebugInfo::Skip, Determinism::Enforced, - false, + CollectEvents::Skip, ) .result .unwrap(); @@ -3937,8 +3937,8 @@ fn code_rejected_error_works() { Code::Upload(wasm), vec![], vec![], - true, - false, + DebugInfo::UnsafeDebug, + CollectEvents::Skip, ); assert_err!(result.result, >::CodeRejected); assert_eq!( @@ -3965,8 +3965,8 @@ fn code_rejected_error_works() { Code::Upload(wasm), vec![], vec![], - true, - false, + DebugInfo::UnsafeDebug, + CollectEvents::Skip, ); assert_err!(result.result, >::CodeRejected); assert_eq!( @@ -3993,8 +3993,8 @@ fn set_code_hash() { Code::Upload(wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -4017,9 +4017,9 @@ fn set_code_hash() { GAS_LIMIT, None, new_code_hash.as_ref().to_vec(), - true, + DebugInfo::UnsafeDebug, Determinism::Enforced, - false, + CollectEvents::Skip, ) .result .unwrap(); @@ -4033,9 +4033,9 @@ fn set_code_hash() { GAS_LIMIT, None, vec![], - true, + DebugInfo::UnsafeDebug, Determinism::Enforced, - false, + CollectEvents::Skip, ) .result .unwrap(); @@ -4091,8 +4091,8 @@ fn storage_deposit_limit_is_enforced() { Code::Upload(wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -4134,8 +4134,8 @@ fn storage_deposit_limit_is_enforced_late() { Code::Upload(wasm_caller), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -4148,8 +4148,8 @@ fn storage_deposit_limit_is_enforced_late() { Code::Upload(wasm_callee), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -4281,8 +4281,8 @@ fn deposit_limit_honors_liquidity_restrictions() { Code::Upload(wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -4326,8 +4326,8 @@ fn deposit_limit_honors_existential_deposit() { Code::Upload(wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -4370,8 +4370,8 @@ fn deposit_limit_honors_min_leftover() { Code::Upload(wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -4426,8 +4426,8 @@ fn cannot_instantiate_indeterministic_code() { Code::Upload(wasm.clone()), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result, >::CodeRejected, @@ -4472,8 +4472,8 @@ fn cannot_instantiate_indeterministic_code() { Code::Existing(code_hash), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result, >::Indeterministic, @@ -4488,8 +4488,8 @@ fn cannot_instantiate_indeterministic_code() { Code::Upload(caller_wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -4504,9 +4504,9 @@ fn cannot_instantiate_indeterministic_code() { GAS_LIMIT, None, code_hash.encode(), - false, + DebugInfo::Skip, Determinism::Enforced, - false, + CollectEvents::Skip, ) .result, >::Indeterministic, @@ -4521,9 +4521,9 @@ fn cannot_instantiate_indeterministic_code() { GAS_LIMIT, None, code_hash.encode(), - false, + DebugInfo::Skip, Determinism::Relaxed, - false, + CollectEvents::Skip, ) .result, >::Indeterministic, @@ -4555,8 +4555,8 @@ fn cannot_set_code_indeterministic_code() { Code::Upload(caller_wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -4571,9 +4571,9 @@ fn cannot_set_code_indeterministic_code() { GAS_LIMIT, None, code_hash.encode(), - false, + DebugInfo::Skip, Determinism::Relaxed, - false, + CollectEvents::Skip, ) .result, >::Indeterministic, @@ -4605,8 +4605,8 @@ fn delegate_call_indeterministic_code() { Code::Upload(caller_wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -4621,9 +4621,9 @@ fn delegate_call_indeterministic_code() { GAS_LIMIT, None, code_hash.encode(), - false, + DebugInfo::Skip, Determinism::Enforced, - false, + CollectEvents::Skip, ) .result, >::Indeterministic, @@ -4638,9 +4638,9 @@ fn delegate_call_indeterministic_code() { GAS_LIMIT, None, code_hash.encode(), - false, + DebugInfo::Skip, Determinism::Relaxed, - false, + CollectEvents::Skip, ) .result ); @@ -4662,8 +4662,8 @@ fn reentrance_count_works_with_call() { Code::Upload(wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -4679,9 +4679,9 @@ fn reentrance_count_works_with_call() { GAS_LIMIT, None, input, - true, + DebugInfo::UnsafeDebug, Determinism::Enforced, - false, + CollectEvents::Skip, ) .result .unwrap(); @@ -4703,8 +4703,8 @@ fn reentrance_count_works_with_delegated_call() { Code::Upload(wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -4720,9 +4720,9 @@ fn reentrance_count_works_with_delegated_call() { GAS_LIMIT, None, input, - true, + DebugInfo::UnsafeDebug, Determinism::Enforced, - false, + CollectEvents::Skip, ) .result .unwrap(); @@ -4746,8 +4746,8 @@ fn account_reentrance_count_works() { Code::Upload(wasm), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -4761,8 +4761,8 @@ fn account_reentrance_count_works() { Code::Upload(wasm_reentrance_count), vec![], vec![], - false, - false, + DebugInfo::Skip, + CollectEvents::Skip, ) .result .unwrap() @@ -4775,9 +4775,9 @@ fn account_reentrance_count_works() { GAS_LIMIT, None, contract_addr.encode(), - true, + DebugInfo::UnsafeDebug, Determinism::Enforced, - false, + CollectEvents::Skip, ) .result .unwrap(); @@ -4789,9 +4789,9 @@ fn account_reentrance_count_works() { GAS_LIMIT, None, another_contract_addr.encode(), - true, + DebugInfo::UnsafeDebug, Determinism::Enforced, - false, + CollectEvents::Skip, ) .result .unwrap(); From 0de45b61d15c9048f69840c14b19d964149cbe75 Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Mon, 17 Apr 2023 14:59:24 +0200 Subject: [PATCH 13/28] contracts: add partial eq to new enums --- frame/contracts/src/lib.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index a91b7a21b2946..7d403767f0d9f 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -932,6 +932,7 @@ struct InstantiateInput { salt: Vec, } +#[derive(PartialEq)] pub enum CollectEvents { /// # Note /// @@ -944,10 +945,11 @@ pub enum CollectEvents { Skip, } +#[derive(PartialEq)] pub enum DebugInfo { /// # Note /// - /// This should only ever be set to `UnsafeYes` when executing as an RPC because + /// 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, @@ -1201,7 +1203,7 @@ impl Pallet { debug: DebugInfo, collect_events: CollectEvents, ) -> ContractInstantiateResult, EventRecordOf> { - let mut debug_message = if matches!(debug, DebugInfo::UnsafeDebug) { + let mut debug_message = if debug == DebugInfo::UnsafeDebug { Some(DebugBufferVec::::default()) } else { None @@ -1216,7 +1218,7 @@ impl Pallet { }; let output = InstantiateInput:: { code, salt }.run_guarded(common); // collect events if CollectEvents is UnsafeCollect - let events = if matches!(collect_events, CollectEvents::UnsafeCollect) { + let events = if collect_events == CollectEvents::UnsafeCollect { Some(System::::read_events_no_consensus().map(|e| *e).collect()) } else { None From 8d36e914eded82f731000ac867b3d1294b3b502c Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Mon, 17 Apr 2023 15:46:42 +0200 Subject: [PATCH 14/28] contracts: improve comments --- frame/contracts/primitives/src/lib.rs | 4 ++-- frame/contracts/src/lib.rs | 9 +++++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/frame/contracts/primitives/src/lib.rs b/frame/contracts/primitives/src/lib.rs index 3f17872d11c79..783ce493d5ed1 100644 --- a/frame/contracts/primitives/src/lib.rs +++ b/frame/contracts/primitives/src/lib.rs @@ -71,8 +71,8 @@ pub struct ContractResult { pub debug_message: Vec, /// The execution result of the wasm code. pub result: R, - /// The events that were emitted during execution. During on-chain execution these events are - /// None to avoid putting all the Events emitted in the block so far into the PoV. + /// The events that were emitted during execution. It is an options as event collection is + /// optional. pub events: Option>, } diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 7d403767f0d9f..bc4560e2fd06b 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -932,8 +932,11 @@ struct InstantiateInput { salt: Vec, } +/// Determines whether events should be collected during execution. #[derive(PartialEq)] pub enum CollectEvents { + /// Collect events. + /// /// # Note /// /// Events should only be collected when called off-chain, as this would @@ -941,17 +944,19 @@ pub enum CollectEvents { /// /// **Never** use this mode for on-chain execution. UnsafeCollect, - /// Events should not be collected. + /// Skip event collection. Skip, } #[derive(PartialEq)] 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, } @@ -1135,7 +1140,7 @@ impl Pallet { /// information. /// /// If `collect_events` is set to `CollectEvents::UnsafeCollect` it collects all the Events - /// emitted in the block so far. + /// emitted in the block so far and the ones emitted during the execution of this contract. pub fn bare_call( origin: T::AccountId, dest: T::AccountId, From 02c41d32fb16d6e62b33bdb0eaddf04873b7c3b6 Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Mon, 17 Apr 2023 15:49:33 +0200 Subject: [PATCH 15/28] contracts: improve comments --- frame/contracts/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index bc4560e2fd06b..9a1d99ed4409b 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -948,6 +948,7 @@ pub enum CollectEvents { Skip, } +/// Determines whether debug messages will be collected. #[derive(PartialEq)] pub enum DebugInfo { /// Collect debug messages. From b9cb4efef74217a735d749013c6f5dad383d4b89 Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Tue, 18 Apr 2023 11:05:07 +0200 Subject: [PATCH 16/28] contracts: fix bare_call benchmarking --- frame/contracts/src/benchmarking/mod.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/frame/contracts/src/benchmarking/mod.rs b/frame/contracts/src/benchmarking/mod.rs index 52889c5cd3268..881f54e167afb 100644 --- a/frame/contracts/src/benchmarking/mod.rs +++ b/frame/contracts/src/benchmarking/mod.rs @@ -928,9 +928,9 @@ benchmarks! { Weight::MAX, None, vec![], - true, + DebugInfo::UnsafeDebug, Determinism::Enforced, - false, + CollectEvents::Skip, ) .result?; } @@ -978,9 +978,9 @@ benchmarks! { Weight::MAX, None, vec![], - true, + DebugInfo::UnsafeDebug, Determinism::Enforced, - false, + CollectEvents::Skip, ) .result?; } @@ -3159,9 +3159,9 @@ benchmarks! { Weight::MAX, None, data, - false, + DebugInfo::Skip, Determinism::Enforced, - false, + CollectEvents::Skip, ) .result?; } @@ -3209,9 +3209,9 @@ benchmarks! { Weight::MAX, None, data, - false, + DebugInfo::Skip, Determinism::Enforced, - false, + CollectEvents::Skip, ) .result?; } From a8b0c8b9d438af904af54ca8d2b5e89129854762 Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Tue, 18 Apr 2023 11:48:07 +0200 Subject: [PATCH 17/28] contracts: fix bare call and instantiate in impl_runtime_apis --- bin/node/runtime/src/lib.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index f96a58331a1d5..56b49f8bfa401 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -2145,9 +2145,9 @@ impl_runtime_apis! { gas_limit, storage_deposit_limit, input_data, - true, + pallet_contracts::DebugInfo::UnsafeDebug, pallet_contracts::Determinism::Enforced, - true, + pallet_contracts::CollectEvents::UnsafeCollect, ) } @@ -2170,8 +2170,8 @@ impl_runtime_apis! { code, data, salt, - true, - true, + pallet_contracts::DebugInfo::UnsafeDebug, + pallet_contracts::CollectEvents::UnsafeCollect, ) } From 160cfb359f97131365d1f7d46d998b432573a47e Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Thu, 20 Apr 2023 13:05:42 +0200 Subject: [PATCH 18/28] contracts: add api versioning to new ContractsApi functions --- bin/node/runtime/src/lib.rs | 81 ++++++++++++++++++++++++--- frame/contracts/primitives/src/lib.rs | 16 ++++-- frame/contracts/src/lib.rs | 43 +++++++++++--- 3 files changed, 121 insertions(+), 19 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 56b49f8bfa401..bb49126dda475 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -52,6 +52,10 @@ use frame_system::{ }; pub use node_primitives::{AccountId, Signature}; use node_primitives::{AccountIndex, Balance, BlockNumber, Hash, Index, Moment}; +use pallet_contracts_primitives::{ + Code, CodeUploadResult, ContractExecResult, ContractExecResultWEvents, + ContractInstantiateResult, ContractInstantiateResultWEvents, GetStorageResult, +}; use pallet_election_provider_multi_phase::SolutionAccuracyOf; use pallet_im_online::sr25519::AuthorityId as ImOnlineId; use pallet_nfts::PalletFeatures; @@ -2127,16 +2131,17 @@ impl_runtime_apis! { } } + #[api_version(3)] impl pallet_contracts::ContractsApi for Runtime { - fn call( + fn call_v3( origin: AccountId, dest: AccountId, value: Balance, gas_limit: Option, storage_deposit_limit: Option, input_data: Vec, - ) -> pallet_contracts_primitives::ContractExecResult { + ) -> ContractExecResultWEvents { let gas_limit = gas_limit.unwrap_or(RuntimeBlockWeights::get().max_block); Contracts::bare_call( origin, @@ -2151,15 +2156,15 @@ impl_runtime_apis! { ) } - fn instantiate( + fn instantiate_v3( origin: AccountId, value: Balance, gas_limit: Option, storage_deposit_limit: Option, - code: pallet_contracts_primitives::Code, + code: Code, data: Vec, salt: Vec, - ) -> pallet_contracts_primitives::ContractInstantiateResult + ) -> ContractInstantiateResultWEvents { let gas_limit = gas_limit.unwrap_or(RuntimeBlockWeights::get().max_block); Contracts::bare_instantiate( @@ -2175,12 +2180,74 @@ impl_runtime_apis! { ) } + fn call( + origin: AccountId, + dest: AccountId, + value: Balance, + gas_limit: Option, + storage_deposit_limit: Option, + input_data: Vec, + ) -> ContractExecResult { + let gas_limit = gas_limit.unwrap_or(RuntimeBlockWeights::get().max_block); + let result = Contracts::bare_call( + origin, + dest, + value, + gas_limit, + storage_deposit_limit, + input_data, + pallet_contracts::DebugInfo::UnsafeDebug, + pallet_contracts::Determinism::Enforced, + pallet_contracts::CollectEvents::Skip, + ); + ContractExecResult { + gas_consumed: result.gas_consumed, + gas_required: result.gas_required, + storage_deposit: result.storage_deposit, + debug_message: result.debug_message, + result: result.result, + events: None, + } + } + + fn instantiate( + origin: AccountId, + value: Balance, + gas_limit: Option, + storage_deposit_limit: Option, + code: Code, + data: Vec, + salt: Vec, + ) -> ContractInstantiateResult + { + let gas_limit = gas_limit.unwrap_or(RuntimeBlockWeights::get().max_block); + let result = Contracts::bare_instantiate( + origin, + value, + gas_limit, + storage_deposit_limit, + code, + data, + salt, + pallet_contracts::DebugInfo::UnsafeDebug, + pallet_contracts::CollectEvents::Skip, + ); + ContractInstantiateResult { + gas_consumed: result.gas_consumed, + gas_required: result.gas_required, + storage_deposit: result.storage_deposit, + debug_message: result.debug_message, + result: result.result, + events: None, + } + } + fn upload_code( origin: AccountId, code: Vec, storage_deposit_limit: Option, determinism: pallet_contracts::Determinism, - ) -> pallet_contracts_primitives::CodeUploadResult + ) -> CodeUploadResult { Contracts::bare_upload_code( origin, @@ -2193,7 +2260,7 @@ impl_runtime_apis! { fn get_storage( address: AccountId, key: Vec, - ) -> pallet_contracts_primitives::GetStorageResult { + ) -> GetStorageResult { Contracts::get_storage( address, key diff --git a/frame/contracts/primitives/src/lib.rs b/frame/contracts/primitives/src/lib.rs index 783ce493d5ed1..e1031b1ccf736 100644 --- a/frame/contracts/primitives/src/lib.rs +++ b/frame/contracts/primitives/src/lib.rs @@ -76,14 +76,22 @@ pub struct ContractResult { pub events: Option>, } -/// Result type of a `bare_call` call. -pub type ContractExecResult = +/// Result type of a `bare_call` call as well as `ContractsApi::call_v3`. +pub type ContractExecResultWEvents = ContractResult, Balance, EventRecord>; -/// Result type of a `bare_instantiate` call. -pub type ContractInstantiateResult = +/// Result type of the backwards compatible `ContractsApi::call` call. +pub type ContractExecResult = + ContractResult, Balance, ()>; + +/// Result type of a `bare_instantiate` call as well as `ContractsApi::instantiate`. +pub type ContractInstantiateResultWEvents = ContractResult, DispatchError>, Balance, EventRecord>; +/// Result type of the backwards compatible `ContractsApi::instantiate` call. +pub type ContractInstantiateResult = + ContractResult, DispatchError>, Balance, ()>; + /// Result type of a `bare_code_upload` call. pub type CodeUploadResult = Result, DispatchError>; diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 24a5e71dbb6b9..34f854045762f 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -121,8 +121,8 @@ use frame_support::{ use frame_system::{EventRecord, Pallet as System}; use pallet_contracts_primitives::{ Code, CodeUploadResult, CodeUploadReturnValue, ContractAccessError, ContractExecResult, - ContractInstantiateResult, ExecReturnValue, GetStorageResult, InstantiateReturnValue, - StorageDeposit, + ContractExecResultWEvents, ContractInstantiateResult, ContractInstantiateResultWEvents, + ExecReturnValue, GetStorageResult, InstantiateReturnValue, StorageDeposit, }; use scale_info::TypeInfo; use smallvec::Array; @@ -1158,7 +1158,7 @@ impl Pallet { debug: DebugInfo, determinism: Determinism, collect_events: CollectEvents, - ) -> ContractExecResult, EventRecordOf> { + ) -> ContractExecResultWEvents, EventRecordOf> { let mut debug_message = if matches!(debug, DebugInfo::UnsafeDebug) { Some(DebugBufferVec::::default()) } else { @@ -1180,7 +1180,7 @@ impl Pallet { None }; - ContractExecResult { + ContractExecResultWEvents { result: output.result.map_err(|r| r.error), gas_consumed: output.gas_meter.gas_consumed(), gas_required: output.gas_meter.gas_required(), @@ -1214,7 +1214,7 @@ impl Pallet { salt: Vec, debug: DebugInfo, collect_events: CollectEvents, - ) -> ContractInstantiateResult, EventRecordOf> { + ) -> ContractInstantiateResultWEvents, EventRecordOf> { let mut debug_message = if debug == DebugInfo::UnsafeDebug { Some(DebugBufferVec::::default()) } else { @@ -1235,7 +1235,7 @@ impl Pallet { } else { None }; - ContractInstantiateResult { + ContractInstantiateResultWEvents { result: output .result .map(|(account_id, result)| InstantiateReturnValue { result, account_id }) @@ -1369,7 +1369,20 @@ sp_api::decl_runtime_apis! { gas_limit: Option, storage_deposit_limit: Option, input_data: Vec, - ) -> ContractExecResult; + ) -> ContractExecResult; + + /// Perform a call from a specified account to a given contract. + /// + /// See [`crate::Pallet::bare_call`]. + #[api_version(3)] + fn call_v3( + origin: AccountId, + dest: AccountId, + value: Balance, + gas_limit: Option, + storage_deposit_limit: Option, + input_data: Vec, + ) -> ContractExecResultWEvents; /// Instantiate a new contract. /// @@ -1382,7 +1395,21 @@ sp_api::decl_runtime_apis! { code: Code, data: Vec, salt: Vec, - ) -> ContractInstantiateResult; + ) -> ContractInstantiateResult; + + /// Instantiate a new contract. + /// + /// See `[crate::Pallet::bare_instantiate]`. + #[api_version(3)] + fn instantiate_v3( + origin: AccountId, + value: Balance, + gas_limit: Option, + storage_deposit_limit: Option, + code: Code, + data: Vec, + salt: Vec, + ) -> ContractInstantiateResultWEvents; /// Upload new code without instantiating a contract from it. From dacdc53c3d49f885c01b7fa29ae709e9dabc391b Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Thu, 20 Apr 2023 15:44:51 +0200 Subject: [PATCH 19/28] contracts: modify api versioning to new ContractsApi functions --- frame/contracts/src/lib.rs | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 34f854045762f..f377223ca341a 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -1359,18 +1359,6 @@ sp_api::decl_runtime_apis! { Hash: Codec, EventRecord: Codec, { - /// Perform a call from a specified account to a given contract. - /// - /// See [`crate::Pallet::bare_call`]. - fn call( - origin: AccountId, - dest: AccountId, - value: Balance, - gas_limit: Option, - storage_deposit_limit: Option, - input_data: Vec, - ) -> ContractExecResult; - /// Perform a call from a specified account to a given contract. /// /// See [`crate::Pallet::bare_call`]. @@ -1387,7 +1375,8 @@ sp_api::decl_runtime_apis! { /// Instantiate a new contract. /// /// See `[crate::Pallet::bare_instantiate]`. - fn instantiate( + #[api_version(3)] + fn instantiate_v3( origin: AccountId, value: Balance, gas_limit: Option, @@ -1395,13 +1384,24 @@ sp_api::decl_runtime_apis! { code: Code, data: Vec, salt: Vec, - ) -> ContractInstantiateResult; + ) -> ContractInstantiateResultWEvents; + + /// Perform a call from a specified account to a given contract. + /// + /// See [`crate::Pallet::bare_call`]. + fn call( + origin: AccountId, + dest: AccountId, + value: Balance, + gas_limit: Option, + storage_deposit_limit: Option, + input_data: Vec, + ) -> ContractExecResult; /// Instantiate a new contract. /// /// See `[crate::Pallet::bare_instantiate]`. - #[api_version(3)] - fn instantiate_v3( + fn instantiate( origin: AccountId, value: Balance, gas_limit: Option, @@ -1409,8 +1409,7 @@ sp_api::decl_runtime_apis! { code: Code, data: Vec, salt: Vec, - ) -> ContractInstantiateResultWEvents; - + ) -> ContractInstantiateResult; /// Upload new code without instantiating a contract from it. /// From 337e54e350d454245386c82d3c7f17c234e9cd86 Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Fri, 28 Apr 2023 17:16:10 +0200 Subject: [PATCH 20/28] contracts: create new contracts api trait --- bin/node/runtime/src/lib.rs | 107 +++++++++++++++----------- frame/contracts/primitives/src/lib.rs | 8 +- frame/contracts/src/lib.rs | 52 ++++++++----- 3 files changed, 100 insertions(+), 67 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 8618834e13f60..c6de5461ec704 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -143,7 +143,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { // and set impl_version to 0. If only runtime // implementation changes and behavior does not, then leave spec_version as // is and increment impl_version. - spec_version: 268, + spec_version: 269, impl_version: 0, apis: RUNTIME_API_VERSIONS, transaction_version: 2, @@ -2150,19 +2150,18 @@ impl_runtime_apis! { } } - #[api_version(3)] impl pallet_contracts::ContractsApi for Runtime { - fn call_v3( + fn call( origin: AccountId, dest: AccountId, value: Balance, gas_limit: Option, storage_deposit_limit: Option, input_data: Vec, - ) -> ContractExecResultWEvents { + ) -> ContractExecResult { let gas_limit = gas_limit.unwrap_or(RuntimeBlockWeights::get().max_block); - Contracts::bare_call( + let result = Contracts::bare_call( origin, dest, value, @@ -2171,11 +2170,19 @@ impl_runtime_apis! { input_data, pallet_contracts::DebugInfo::UnsafeDebug, pallet_contracts::Determinism::Enforced, - pallet_contracts::CollectEvents::UnsafeCollect, - ) + pallet_contracts::CollectEvents::Skip, + ); + ContractExecResult { + gas_consumed: result.gas_consumed, + gas_required: result.gas_required, + storage_deposit: result.storage_deposit, + debug_message: result.debug_message, + result: result.result, + events: None, + } } - fn instantiate_v3( + fn instantiate( origin: AccountId, value: Balance, gas_limit: Option, @@ -2183,10 +2190,10 @@ impl_runtime_apis! { code: Code, data: Vec, salt: Vec, - ) -> ContractInstantiateResultWEvents + ) -> ContractInstantiateResult { let gas_limit = gas_limit.unwrap_or(RuntimeBlockWeights::get().max_block); - Contracts::bare_instantiate( + let result = Contracts::bare_instantiate( origin, value, gas_limit, @@ -2195,10 +2202,46 @@ impl_runtime_apis! { data, salt, pallet_contracts::DebugInfo::UnsafeDebug, - pallet_contracts::CollectEvents::UnsafeCollect, + pallet_contracts::CollectEvents::Skip, + ); + ContractInstantiateResult { + gas_consumed: result.gas_consumed, + gas_required: result.gas_required, + storage_deposit: result.storage_deposit, + debug_message: result.debug_message, + result: result.result, + events: None, + } + } + + fn upload_code( + origin: AccountId, + code: Vec, + storage_deposit_limit: Option, + determinism: pallet_contracts::Determinism, + ) -> CodeUploadResult + { + Contracts::bare_upload_code( + origin, + code, + storage_deposit_limit, + determinism, ) } + fn get_storage( + address: AccountId, + key: Vec, + ) -> GetStorageResult { + Contracts::get_storage( + address, + key + ) + } + } + + impl pallet_contracts::ContractsApiV3 for Runtime + { fn call( origin: AccountId, dest: AccountId, @@ -2206,9 +2249,9 @@ impl_runtime_apis! { gas_limit: Option, storage_deposit_limit: Option, input_data: Vec, - ) -> ContractExecResult { + ) -> ContractExecResultWEvents { let gas_limit = gas_limit.unwrap_or(RuntimeBlockWeights::get().max_block); - let result = Contracts::bare_call( + Contracts::bare_call( origin, dest, value, @@ -2217,16 +2260,8 @@ impl_runtime_apis! { input_data, pallet_contracts::DebugInfo::UnsafeDebug, pallet_contracts::Determinism::Enforced, - pallet_contracts::CollectEvents::Skip, - ); - ContractExecResult { - gas_consumed: result.gas_consumed, - gas_required: result.gas_required, - storage_deposit: result.storage_deposit, - debug_message: result.debug_message, - result: result.result, - events: None, - } + pallet_contracts::CollectEvents::UnsafeCollect, + ) } fn instantiate( @@ -2237,10 +2272,10 @@ impl_runtime_apis! { code: Code, data: Vec, salt: Vec, - ) -> ContractInstantiateResult + ) -> ContractInstantiateResultWEvents { let gas_limit = gas_limit.unwrap_or(RuntimeBlockWeights::get().max_block); - let result = Contracts::bare_instantiate( + Contracts::bare_instantiate( origin, value, gas_limit, @@ -2249,16 +2284,8 @@ impl_runtime_apis! { data, salt, pallet_contracts::DebugInfo::UnsafeDebug, - pallet_contracts::CollectEvents::Skip, - ); - ContractInstantiateResult { - gas_consumed: result.gas_consumed, - gas_required: result.gas_required, - storage_deposit: result.storage_deposit, - debug_message: result.debug_message, - result: result.result, - events: None, - } + pallet_contracts::CollectEvents::UnsafeCollect, + ) } fn upload_code( @@ -2275,16 +2302,6 @@ impl_runtime_apis! { determinism, ) } - - fn get_storage( - address: AccountId, - key: Vec, - ) -> GetStorageResult { - Contracts::get_storage( - address, - key - ) - } } impl pallet_transaction_payment_rpc_runtime_api::TransactionPaymentApi< diff --git a/frame/contracts/primitives/src/lib.rs b/frame/contracts/primitives/src/lib.rs index e1031b1ccf736..fee351e8f22de 100644 --- a/frame/contracts/primitives/src/lib.rs +++ b/frame/contracts/primitives/src/lib.rs @@ -76,19 +76,19 @@ pub struct ContractResult { pub events: Option>, } -/// Result type of a `bare_call` call as well as `ContractsApi::call_v3`. +/// Result type of a `bare_call` call as well as `ContractsApiV3::call`. pub type ContractExecResultWEvents = ContractResult, Balance, EventRecord>; -/// Result type of the backwards compatible `ContractsApi::call` call. +/// Result type of the `ContractsApi::call` call. pub type ContractExecResult = ContractResult, Balance, ()>; -/// Result type of a `bare_instantiate` call as well as `ContractsApi::instantiate`. +/// Result type of a `bare_instantiate` call as well as `ContractsApiV3::instantiate`. pub type ContractInstantiateResultWEvents = ContractResult, DispatchError>, Balance, EventRecord>; -/// Result type of the backwards compatible `ContractsApi::instantiate` call. +/// Result type of the `ContractsApi::instantiate` call. pub type ContractInstantiateResult = ContractResult, DispatchError>, Balance, ()>; diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index d683354e6a96e..13982155a4fb6 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -1372,21 +1372,19 @@ sp_api::decl_runtime_apis! { /// Perform a call from a specified account to a given contract. /// /// See [`crate::Pallet::bare_call`]. - #[api_version(3)] - fn call_v3( + fn call( origin: AccountId, dest: AccountId, value: Balance, gas_limit: Option, storage_deposit_limit: Option, input_data: Vec, - ) -> ContractExecResultWEvents; + ) -> ContractExecResult; /// Instantiate a new contract. /// /// See `[crate::Pallet::bare_instantiate]`. - #[api_version(3)] - fn instantiate_v3( + fn instantiate( origin: AccountId, value: Balance, gas_limit: Option, @@ -1394,8 +1392,36 @@ sp_api::decl_runtime_apis! { code: Code, data: Vec, salt: Vec, - ) -> ContractInstantiateResultWEvents; + ) -> ContractInstantiateResult; + + /// Upload new code without instantiating a contract from it. + /// + /// See [`crate::Pallet::bare_upload_code`]. + fn upload_code( + origin: AccountId, + code: Vec, + storage_deposit_limit: Option, + determinism: Determinism, + ) -> CodeUploadResult; + /// Query a given storage key in a given contract. + /// + /// Returns `Ok(Some(Vec))` if the storage value exists under the given key in the + /// specified account and `Ok(None)` if it doesn't. If the account specified by the address + /// doesn't exist, or doesn't have a contract then `Err` is returned. + fn get_storage( + address: AccountId, + key: Vec, + ) -> GetStorageResult; + } + + pub trait ContractsApiV3 where + AccountId: Codec, + Balance: Codec, + BlockNumber: Codec, + Hash: Codec, + EventRecord: Codec, + { /// Perform a call from a specified account to a given contract. /// /// See [`crate::Pallet::bare_call`]. @@ -1406,7 +1432,7 @@ sp_api::decl_runtime_apis! { gas_limit: Option, storage_deposit_limit: Option, input_data: Vec, - ) -> ContractExecResult; + ) -> ContractExecResultWEvents; /// Instantiate a new contract. /// @@ -1419,7 +1445,7 @@ sp_api::decl_runtime_apis! { code: Code, data: Vec, salt: Vec, - ) -> ContractInstantiateResult; + ) -> ContractInstantiateResultWEvents; /// Upload new code without instantiating a contract from it. /// @@ -1430,15 +1456,5 @@ sp_api::decl_runtime_apis! { storage_deposit_limit: Option, determinism: Determinism, ) -> CodeUploadResult; - - /// Query a given storage key in a given contract. - /// - /// Returns `Ok(Some(Vec))` if the storage value exists under the given key in the - /// specified account and `Ok(None)` if it doesn't. If the account specified by the address - /// doesn't exist, or doesn't have a contract then `Err` is returned. - fn get_storage( - address: AccountId, - key: Vec, - ) -> GetStorageResult; } } From b9838b1e209acd466d6980ec3a424f15b56b900d Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Tue, 2 May 2023 11:03:12 +0200 Subject: [PATCH 21/28] contracts: clean up code --- bin/node/runtime/src/lib.rs | 4 ++-- frame/contracts/src/lib.rs | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index c6de5461ec704..6e56b2d9563b8 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -143,7 +143,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { // and set impl_version to 0. If only runtime // implementation changes and behavior does not, then leave spec_version as // is and increment impl_version. - spec_version: 269, + spec_version: 268, impl_version: 0, apis: RUNTIME_API_VERSIONS, transaction_version: 2, @@ -2150,7 +2150,7 @@ impl_runtime_apis! { } } - impl pallet_contracts::ContractsApi for Runtime + impl pallet_contracts::ContractsApi for Runtime { fn call( origin: AccountId, diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 13982155a4fb6..3614560d0ab56 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -1183,7 +1183,6 @@ impl Pallet { debug_message: debug_message.as_mut(), }; let output = CallInput:: { dest, determinism }.run_guarded(common); - // collect events if CollectEvents is UnsafeCollect let events = if matches!(collect_events, CollectEvents::UnsafeCollect) { Some(System::::read_events_no_consensus().map(|e| *e).collect()) } else { @@ -1362,12 +1361,11 @@ impl Pallet { sp_api::decl_runtime_apis! { /// The API used to dry-run contract interactions. #[api_version(2)] - pub trait ContractsApi where + pub trait ContractsApi where AccountId: Codec, Balance: Codec, BlockNumber: Codec, Hash: Codec, - EventRecord: Codec, { /// Perform a call from a specified account to a given contract. /// From fca001fcc7da5c354f59cdef242c18b6a4b71260 Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Tue, 2 May 2023 14:41:18 +0200 Subject: [PATCH 22/28] contracts: remove the contract results with events --- bin/node/runtime/src/lib.rs | 11 +++++------ frame/contracts/primitives/src/lib.rs | 16 ++++------------ frame/contracts/src/lib.rs | 20 ++++++++++---------- 3 files changed, 19 insertions(+), 28 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 6e56b2d9563b8..404a8cbeb5781 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -53,8 +53,7 @@ use frame_system::{ pub use node_primitives::{AccountId, Signature}; use node_primitives::{AccountIndex, Balance, BlockNumber, Hash, Index, Moment}; use pallet_contracts_primitives::{ - Code, CodeUploadResult, ContractExecResult, ContractExecResultWEvents, - ContractInstantiateResult, ContractInstantiateResultWEvents, GetStorageResult, + Code, CodeUploadResult, ContractExecResult, ContractInstantiateResult, GetStorageResult, }; use pallet_election_provider_multi_phase::SolutionAccuracyOf; use pallet_im_online::sr25519::AuthorityId as ImOnlineId; @@ -2159,7 +2158,7 @@ impl_runtime_apis! { gas_limit: Option, storage_deposit_limit: Option, input_data: Vec, - ) -> ContractExecResult { + ) -> ContractExecResult { let gas_limit = gas_limit.unwrap_or(RuntimeBlockWeights::get().max_block); let result = Contracts::bare_call( origin, @@ -2190,7 +2189,7 @@ impl_runtime_apis! { code: Code, data: Vec, salt: Vec, - ) -> ContractInstantiateResult + ) -> ContractInstantiateResult { let gas_limit = gas_limit.unwrap_or(RuntimeBlockWeights::get().max_block); let result = Contracts::bare_instantiate( @@ -2249,7 +2248,7 @@ impl_runtime_apis! { gas_limit: Option, storage_deposit_limit: Option, input_data: Vec, - ) -> ContractExecResultWEvents { + ) -> ContractExecResult { let gas_limit = gas_limit.unwrap_or(RuntimeBlockWeights::get().max_block); Contracts::bare_call( origin, @@ -2272,7 +2271,7 @@ impl_runtime_apis! { code: Code, data: Vec, salt: Vec, - ) -> ContractInstantiateResultWEvents + ) -> ContractInstantiateResult { let gas_limit = gas_limit.unwrap_or(RuntimeBlockWeights::get().max_block); Contracts::bare_instantiate( diff --git a/frame/contracts/primitives/src/lib.rs b/frame/contracts/primitives/src/lib.rs index fee351e8f22de..ed260322ba034 100644 --- a/frame/contracts/primitives/src/lib.rs +++ b/frame/contracts/primitives/src/lib.rs @@ -76,22 +76,14 @@ pub struct ContractResult { pub events: Option>, } -/// Result type of a `bare_call` call as well as `ContractsApiV3::call`. -pub type ContractExecResultWEvents = +/// Result type of a `bare_call` call as well as `ContractsApi::call` and `ContractsApiV3::call`. +pub type ContractExecResult = ContractResult, Balance, EventRecord>; -/// Result type of the `ContractsApi::call` call. -pub type ContractExecResult = - ContractResult, Balance, ()>; - -/// Result type of a `bare_instantiate` call as well as `ContractsApiV3::instantiate`. -pub type ContractInstantiateResultWEvents = +/// Result type of a `bare_instantiate` call as well as `ContractsApi::call` and `ContractsApiV3::instantiate`. +pub type ContractInstantiateResult = ContractResult, DispatchError>, Balance, EventRecord>; -/// Result type of the `ContractsApi::instantiate` call. -pub type ContractInstantiateResult = - ContractResult, DispatchError>, Balance, ()>; - /// Result type of a `bare_code_upload` call. pub type CodeUploadResult = Result, DispatchError>; diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 3614560d0ab56..19e88ac173a2a 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -120,8 +120,8 @@ use frame_support::{ use frame_system::{EventRecord, Pallet as System}; use pallet_contracts_primitives::{ Code, CodeUploadResult, CodeUploadReturnValue, ContractAccessError, ContractExecResult, - ContractExecResultWEvents, ContractInstantiateResult, ContractInstantiateResultWEvents, - ExecReturnValue, GetStorageResult, InstantiateReturnValue, StorageDeposit, + ContractInstantiateResult, ExecReturnValue, GetStorageResult, InstantiateReturnValue, + StorageDeposit, }; use scale_info::TypeInfo; use smallvec::Array; @@ -1168,7 +1168,7 @@ impl Pallet { debug: DebugInfo, determinism: Determinism, collect_events: CollectEvents, - ) -> ContractExecResultWEvents, EventRecordOf> { + ) -> ContractExecResult, EventRecordOf> { let mut debug_message = if matches!(debug, DebugInfo::UnsafeDebug) { Some(DebugBufferVec::::default()) } else { @@ -1189,7 +1189,7 @@ impl Pallet { None }; - ContractExecResultWEvents { + ContractExecResult { result: output.result.map_err(|r| r.error), gas_consumed: output.gas_meter.gas_consumed(), gas_required: output.gas_meter.gas_required(), @@ -1223,7 +1223,7 @@ impl Pallet { salt: Vec, debug: DebugInfo, collect_events: CollectEvents, - ) -> ContractInstantiateResultWEvents, EventRecordOf> { + ) -> ContractInstantiateResult, EventRecordOf> { let mut debug_message = if debug == DebugInfo::UnsafeDebug { Some(DebugBufferVec::::default()) } else { @@ -1244,7 +1244,7 @@ impl Pallet { } else { None }; - ContractInstantiateResultWEvents { + ContractInstantiateResult { result: output .result .map(|(account_id, result)| InstantiateReturnValue { result, account_id }) @@ -1377,7 +1377,7 @@ sp_api::decl_runtime_apis! { gas_limit: Option, storage_deposit_limit: Option, input_data: Vec, - ) -> ContractExecResult; + ) -> ContractExecResult; /// Instantiate a new contract. /// @@ -1390,7 +1390,7 @@ sp_api::decl_runtime_apis! { code: Code, data: Vec, salt: Vec, - ) -> ContractInstantiateResult; + ) -> ContractInstantiateResult; /// Upload new code without instantiating a contract from it. /// @@ -1430,7 +1430,7 @@ sp_api::decl_runtime_apis! { gas_limit: Option, storage_deposit_limit: Option, input_data: Vec, - ) -> ContractExecResultWEvents; + ) -> ContractExecResult; /// Instantiate a new contract. /// @@ -1443,7 +1443,7 @@ sp_api::decl_runtime_apis! { code: Code, data: Vec, salt: Vec, - ) -> ContractInstantiateResultWEvents; + ) -> ContractInstantiateResult; /// Upload new code without instantiating a contract from it. /// From a8da1fff5d34593f63e7eb1c3b7f11786b25ea26 Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Tue, 2 May 2023 14:53:36 +0200 Subject: [PATCH 23/28] contracts: undo contracts api v3 --- bin/node/runtime/src/lib.rs | 188 ++++++++++++++------------ frame/contracts/primitives/src/lib.rs | 4 +- frame/contracts/src/lib.rs | 91 +++++++------ 3 files changed, 147 insertions(+), 136 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 404a8cbeb5781..19223fcfb16de 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -2149,7 +2149,97 @@ impl_runtime_apis! { } } - impl pallet_contracts::ContractsApi for Runtime + // impl pallet_contracts::ContractsApi for Runtime + // { + // fn call( + // origin: AccountId, + // dest: AccountId, + // value: Balance, + // gas_limit: Option, + // storage_deposit_limit: Option, + // input_data: Vec, + // ) -> ContractExecResult { + // let gas_limit = gas_limit.unwrap_or(RuntimeBlockWeights::get().max_block); + // let result = Contracts::bare_call( + // origin, + // dest, + // value, + // gas_limit, + // storage_deposit_limit, + // input_data, + // pallet_contracts::DebugInfo::UnsafeDebug, + // pallet_contracts::Determinism::Enforced, + // pallet_contracts::CollectEvents::Skip, + // ); + // ContractExecResult { + // gas_consumed: result.gas_consumed, + // gas_required: result.gas_required, + // storage_deposit: result.storage_deposit, + // debug_message: result.debug_message, + // result: result.result, + // events: None, + // } + // } + + // fn instantiate( + // origin: AccountId, + // value: Balance, + // gas_limit: Option, + // storage_deposit_limit: Option, + // code: Code, + // data: Vec, + // salt: Vec, + // ) -> ContractInstantiateResult + // { + // let gas_limit = gas_limit.unwrap_or(RuntimeBlockWeights::get().max_block); + // let result = Contracts::bare_instantiate( + // origin, + // value, + // gas_limit, + // storage_deposit_limit, + // code, + // data, + // salt, + // pallet_contracts::DebugInfo::UnsafeDebug, + // pallet_contracts::CollectEvents::Skip, + // ); + // ContractInstantiateResult { + // gas_consumed: result.gas_consumed, + // gas_required: result.gas_required, + // storage_deposit: result.storage_deposit, + // debug_message: result.debug_message, + // result: result.result, + // events: None, + // } + // } + + // fn upload_code( + // origin: AccountId, + // code: Vec, + // storage_deposit_limit: Option, + // determinism: pallet_contracts::Determinism, + // ) -> CodeUploadResult + // { + // Contracts::bare_upload_code( + // origin, + // code, + // storage_deposit_limit, + // determinism, + // ) + // } + + // fn get_storage( + // address: AccountId, + // key: Vec, + // ) -> GetStorageResult { + // Contracts::get_storage( + // address, + // key + // ) + // } + // } + + impl pallet_contracts::ContractsApi for Runtime { fn call( origin: AccountId, @@ -2158,9 +2248,9 @@ impl_runtime_apis! { gas_limit: Option, storage_deposit_limit: Option, input_data: Vec, - ) -> ContractExecResult { + ) -> ContractExecResult { let gas_limit = gas_limit.unwrap_or(RuntimeBlockWeights::get().max_block); - let result = Contracts::bare_call( + Contracts::bare_call( origin, dest, value, @@ -2169,16 +2259,8 @@ impl_runtime_apis! { input_data, pallet_contracts::DebugInfo::UnsafeDebug, pallet_contracts::Determinism::Enforced, - pallet_contracts::CollectEvents::Skip, - ); - ContractExecResult { - gas_consumed: result.gas_consumed, - gas_required: result.gas_required, - storage_deposit: result.storage_deposit, - debug_message: result.debug_message, - result: result.result, - events: None, - } + pallet_contracts::CollectEvents::UnsafeCollect, + ) } fn instantiate( @@ -2189,10 +2271,10 @@ impl_runtime_apis! { code: Code, data: Vec, salt: Vec, - ) -> ContractInstantiateResult + ) -> ContractInstantiateResult { let gas_limit = gas_limit.unwrap_or(RuntimeBlockWeights::get().max_block); - let result = Contracts::bare_instantiate( + Contracts::bare_instantiate( origin, value, gas_limit, @@ -2201,16 +2283,8 @@ impl_runtime_apis! { data, salt, pallet_contracts::DebugInfo::UnsafeDebug, - pallet_contracts::CollectEvents::Skip, - ); - ContractInstantiateResult { - gas_consumed: result.gas_consumed, - gas_required: result.gas_required, - storage_deposit: result.storage_deposit, - debug_message: result.debug_message, - result: result.result, - events: None, - } + pallet_contracts::CollectEvents::UnsafeCollect, + ) } fn upload_code( @@ -2239,70 +2313,6 @@ impl_runtime_apis! { } } - impl pallet_contracts::ContractsApiV3 for Runtime - { - fn call( - origin: AccountId, - dest: AccountId, - value: Balance, - gas_limit: Option, - storage_deposit_limit: Option, - input_data: Vec, - ) -> ContractExecResult { - let gas_limit = gas_limit.unwrap_or(RuntimeBlockWeights::get().max_block); - Contracts::bare_call( - origin, - dest, - value, - gas_limit, - storage_deposit_limit, - input_data, - pallet_contracts::DebugInfo::UnsafeDebug, - pallet_contracts::Determinism::Enforced, - pallet_contracts::CollectEvents::UnsafeCollect, - ) - } - - fn instantiate( - origin: AccountId, - value: Balance, - gas_limit: Option, - storage_deposit_limit: Option, - code: Code, - data: Vec, - salt: Vec, - ) -> ContractInstantiateResult - { - let gas_limit = gas_limit.unwrap_or(RuntimeBlockWeights::get().max_block); - Contracts::bare_instantiate( - origin, - value, - gas_limit, - storage_deposit_limit, - code, - data, - salt, - pallet_contracts::DebugInfo::UnsafeDebug, - pallet_contracts::CollectEvents::UnsafeCollect, - ) - } - - fn upload_code( - origin: AccountId, - code: Vec, - storage_deposit_limit: Option, - determinism: pallet_contracts::Determinism, - ) -> CodeUploadResult - { - Contracts::bare_upload_code( - origin, - code, - storage_deposit_limit, - determinism, - ) - } - } - impl pallet_transaction_payment_rpc_runtime_api::TransactionPaymentApi< Block, Balance, diff --git a/frame/contracts/primitives/src/lib.rs b/frame/contracts/primitives/src/lib.rs index ed260322ba034..e64d75a1d711e 100644 --- a/frame/contracts/primitives/src/lib.rs +++ b/frame/contracts/primitives/src/lib.rs @@ -76,11 +76,11 @@ pub struct ContractResult { pub events: Option>, } -/// Result type of a `bare_call` call as well as `ContractsApi::call` and `ContractsApiV3::call`. +/// Result type of a `bare_call` call as well as `ContractsApi::call`. pub type ContractExecResult = ContractResult, Balance, EventRecord>; -/// Result type of a `bare_instantiate` call as well as `ContractsApi::call` and `ContractsApiV3::instantiate`. +/// Result type of a `bare_instantiate` call as well as `ContractsApi::call`. pub type ContractInstantiateResult = ContractResult, DispatchError>, Balance, EventRecord>; diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 19e88ac173a2a..a3ef3900095d5 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -1361,11 +1361,12 @@ impl Pallet { sp_api::decl_runtime_apis! { /// The API used to dry-run contract interactions. #[api_version(2)] - pub trait ContractsApi where + pub trait ContractsApi where AccountId: Codec, Balance: Codec, BlockNumber: Codec, Hash: Codec, + EventRecord: Codec, { /// Perform a call from a specified account to a given contract. /// @@ -1377,7 +1378,7 @@ sp_api::decl_runtime_apis! { gas_limit: Option, storage_deposit_limit: Option, input_data: Vec, - ) -> ContractExecResult; + ) -> ContractExecResult; /// Instantiate a new contract. /// @@ -1390,7 +1391,7 @@ sp_api::decl_runtime_apis! { code: Code, data: Vec, salt: Vec, - ) -> ContractInstantiateResult; + ) -> ContractInstantiateResult; /// Upload new code without instantiating a contract from it. /// @@ -1413,46 +1414,46 @@ sp_api::decl_runtime_apis! { ) -> GetStorageResult; } - pub trait ContractsApiV3 where - AccountId: Codec, - Balance: Codec, - BlockNumber: Codec, - Hash: Codec, - EventRecord: Codec, - { - /// Perform a call from a specified account to a given contract. - /// - /// See [`crate::Pallet::bare_call`]. - fn call( - origin: AccountId, - dest: AccountId, - value: Balance, - gas_limit: Option, - storage_deposit_limit: Option, - input_data: Vec, - ) -> ContractExecResult; - - /// Instantiate a new contract. - /// - /// See `[crate::Pallet::bare_instantiate]`. - fn instantiate( - origin: AccountId, - value: Balance, - gas_limit: Option, - storage_deposit_limit: Option, - code: Code, - data: Vec, - salt: Vec, - ) -> ContractInstantiateResult; - - /// Upload new code without instantiating a contract from it. - /// - /// See [`crate::Pallet::bare_upload_code`]. - fn upload_code( - origin: AccountId, - code: Vec, - storage_deposit_limit: Option, - determinism: Determinism, - ) -> CodeUploadResult; - } + // pub trait ContractsApiV3 where + // AccountId: Codec, + // Balance: Codec, + // BlockNumber: Codec, + // Hash: Codec, + // EventRecord: Codec, + // { + // /// Perform a call from a specified account to a given contract. + // /// + // /// See [`crate::Pallet::bare_call`]. + // fn call( + // origin: AccountId, + // dest: AccountId, + // value: Balance, + // gas_limit: Option, + // storage_deposit_limit: Option, + // input_data: Vec, + // ) -> ContractExecResult; + + // /// Instantiate a new contract. + // /// + // /// See `[crate::Pallet::bare_instantiate]`. + // fn instantiate( + // origin: AccountId, + // value: Balance, + // gas_limit: Option, + // storage_deposit_limit: Option, + // code: Code, + // data: Vec, + // salt: Vec, + // ) -> ContractInstantiateResult; + + // /// Upload new code without instantiating a contract from it. + // /// + // /// See [`crate::Pallet::bare_upload_code`]. + // fn upload_code( + // origin: AccountId, + // code: Vec, + // storage_deposit_limit: Option, + // determinism: Determinism, + // ) -> CodeUploadResult; + // } } From af6587ab46ec9aa3bc9e19bda9d399457607c0a7 Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Tue, 2 May 2023 15:08:59 +0200 Subject: [PATCH 24/28] contracts: remove commented out code --- bin/node/runtime/src/lib.rs | 90 ------------------------------------- frame/contracts/src/lib.rs | 43 ------------------ 2 files changed, 133 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 19223fcfb16de..24ae597698136 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -2149,96 +2149,6 @@ impl_runtime_apis! { } } - // impl pallet_contracts::ContractsApi for Runtime - // { - // fn call( - // origin: AccountId, - // dest: AccountId, - // value: Balance, - // gas_limit: Option, - // storage_deposit_limit: Option, - // input_data: Vec, - // ) -> ContractExecResult { - // let gas_limit = gas_limit.unwrap_or(RuntimeBlockWeights::get().max_block); - // let result = Contracts::bare_call( - // origin, - // dest, - // value, - // gas_limit, - // storage_deposit_limit, - // input_data, - // pallet_contracts::DebugInfo::UnsafeDebug, - // pallet_contracts::Determinism::Enforced, - // pallet_contracts::CollectEvents::Skip, - // ); - // ContractExecResult { - // gas_consumed: result.gas_consumed, - // gas_required: result.gas_required, - // storage_deposit: result.storage_deposit, - // debug_message: result.debug_message, - // result: result.result, - // events: None, - // } - // } - - // fn instantiate( - // origin: AccountId, - // value: Balance, - // gas_limit: Option, - // storage_deposit_limit: Option, - // code: Code, - // data: Vec, - // salt: Vec, - // ) -> ContractInstantiateResult - // { - // let gas_limit = gas_limit.unwrap_or(RuntimeBlockWeights::get().max_block); - // let result = Contracts::bare_instantiate( - // origin, - // value, - // gas_limit, - // storage_deposit_limit, - // code, - // data, - // salt, - // pallet_contracts::DebugInfo::UnsafeDebug, - // pallet_contracts::CollectEvents::Skip, - // ); - // ContractInstantiateResult { - // gas_consumed: result.gas_consumed, - // gas_required: result.gas_required, - // storage_deposit: result.storage_deposit, - // debug_message: result.debug_message, - // result: result.result, - // events: None, - // } - // } - - // fn upload_code( - // origin: AccountId, - // code: Vec, - // storage_deposit_limit: Option, - // determinism: pallet_contracts::Determinism, - // ) -> CodeUploadResult - // { - // Contracts::bare_upload_code( - // origin, - // code, - // storage_deposit_limit, - // determinism, - // ) - // } - - // fn get_storage( - // address: AccountId, - // key: Vec, - // ) -> GetStorageResult { - // Contracts::get_storage( - // address, - // key - // ) - // } - // } - impl pallet_contracts::ContractsApi for Runtime { fn call( diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index a3ef3900095d5..12b1c68d487ac 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -1413,47 +1413,4 @@ sp_api::decl_runtime_apis! { key: Vec, ) -> GetStorageResult; } - - // pub trait ContractsApiV3 where - // AccountId: Codec, - // Balance: Codec, - // BlockNumber: Codec, - // Hash: Codec, - // EventRecord: Codec, - // { - // /// Perform a call from a specified account to a given contract. - // /// - // /// See [`crate::Pallet::bare_call`]. - // fn call( - // origin: AccountId, - // dest: AccountId, - // value: Balance, - // gas_limit: Option, - // storage_deposit_limit: Option, - // input_data: Vec, - // ) -> ContractExecResult; - - // /// Instantiate a new contract. - // /// - // /// See `[crate::Pallet::bare_instantiate]`. - // fn instantiate( - // origin: AccountId, - // value: Balance, - // gas_limit: Option, - // storage_deposit_limit: Option, - // code: Code, - // data: Vec, - // salt: Vec, - // ) -> ContractInstantiateResult; - - // /// Upload new code without instantiating a contract from it. - // /// - // /// See [`crate::Pallet::bare_upload_code`]. - // fn upload_code( - // origin: AccountId, - // code: Vec, - // storage_deposit_limit: Option, - // determinism: Determinism, - // ) -> CodeUploadResult; - // } } From 9919fd1e832d1b00083661393f41eb42ce617b9c Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Tue, 2 May 2023 15:15:15 +0200 Subject: [PATCH 25/28] contracts: add new test bare call result does not return events --- frame/contracts/src/tests.rs | 41 ++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index 0acff48c88b59..1f729b3343750 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -3080,6 +3080,47 @@ fn bare_call_result_returns_events() { }); } +#[test] +fn bare_call_result_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 = ::Currency::minimum_balance(); + let _ = Balances::deposit_creating(&ALICE, 1000 * min_balance); + + let addr = Contracts::bare_instantiate( + ALICE, + min_balance * 100, + GAS_LIMIT, + None, + Code::Upload(wasm), + vec![], + vec![], + DebugInfo::Skip, + CollectEvents::Skip, + ) + .result + .unwrap() + .account_id; + + let result = Contracts::bare_call( + ALICE, + addr.clone(), + 0, + GAS_LIMIT, + None, + vec![], + DebugInfo::Skip, + Determinism::Enforced, + CollectEvents::Skip, + ); + + 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(); From 1e09bdbc2373e5200b6887bf18aeea097fef21f0 Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Wed, 3 May 2023 10:32:17 +0200 Subject: [PATCH 26/28] contracts: add code review improvements --- bin/node/runtime/src/lib.rs | 2 +- frame/contracts/primitives/src/lib.rs | 2 +- frame/contracts/src/benchmarking/mod.rs | 8 +- frame/contracts/src/lib.rs | 4 +- frame/contracts/src/tests.rs | 102 ++++++++++++------------ 5 files changed, 59 insertions(+), 59 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 24ae597698136..c3dd783870f17 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -2168,8 +2168,8 @@ impl_runtime_apis! { storage_deposit_limit, input_data, pallet_contracts::DebugInfo::UnsafeDebug, - pallet_contracts::Determinism::Enforced, pallet_contracts::CollectEvents::UnsafeCollect, + pallet_contracts::Determinism::Enforced, ) } diff --git a/frame/contracts/primitives/src/lib.rs b/frame/contracts/primitives/src/lib.rs index e64d75a1d711e..e1bd5d1e1cabc 100644 --- a/frame/contracts/primitives/src/lib.rs +++ b/frame/contracts/primitives/src/lib.rs @@ -71,7 +71,7 @@ pub struct ContractResult { pub debug_message: Vec, /// The execution result of the wasm code. pub result: R, - /// The events that were emitted during execution. It is an options as event collection is + /// The events that were emitted during execution. It is an option as event collection is /// optional. pub events: Option>, } diff --git a/frame/contracts/src/benchmarking/mod.rs b/frame/contracts/src/benchmarking/mod.rs index d727a8be1595c..fa9417a59042d 100644 --- a/frame/contracts/src/benchmarking/mod.rs +++ b/frame/contracts/src/benchmarking/mod.rs @@ -952,8 +952,8 @@ benchmarks! { None, vec![], DebugInfo::UnsafeDebug, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ) .result?; } @@ -1002,8 +1002,8 @@ benchmarks! { None, vec![], DebugInfo::UnsafeDebug, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ) .result?; } @@ -3195,8 +3195,8 @@ benchmarks! { None, data, DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ) .result?; } @@ -3245,8 +3245,8 @@ benchmarks! { None, data, DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ) .result?; } diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index f42953e26cff0..d6fc3a45031b6 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -980,7 +980,7 @@ pub enum CollectEvents { /// /// # Note /// - /// Events should only be collected when called off-chain, as this would + /// 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 in them into the PoV. /// /// **Never** use this mode for on-chain execution. @@ -1231,8 +1231,8 @@ impl Pallet { storage_deposit_limit: Option>, data: Vec, debug: DebugInfo, - determinism: Determinism, collect_events: CollectEvents, + determinism: Determinism, ) -> ContractExecResult, EventRecordOf> { let mut debug_message = if matches!(debug, DebugInfo::UnsafeDebug) { Some(DebugBufferVec::::default()) diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index b28ee1a9ac148..c32999d0ade3a 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -1473,8 +1473,8 @@ fn crypto_hashes() { None, params, DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ) .result .unwrap(); @@ -1517,8 +1517,8 @@ fn transfer_return_code() { None, vec![], DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ) .result .unwrap(); @@ -1560,8 +1560,8 @@ fn call_return_code() { None, AsRef::<[u8]>::as_ref(&DJANGO).to_vec(), DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ) .result .unwrap(); @@ -1596,8 +1596,8 @@ fn call_return_code() { .cloned() .collect(), DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ) .result .unwrap(); @@ -1617,8 +1617,8 @@ fn call_return_code() { .cloned() .collect(), DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ) .result .unwrap(); @@ -1637,8 +1637,8 @@ fn call_return_code() { .cloned() .collect(), DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ) .result .unwrap(); @@ -1691,8 +1691,8 @@ fn instantiate_return_code() { None, callee_hash.clone(), DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ) .result .unwrap(); @@ -1708,8 +1708,8 @@ fn instantiate_return_code() { None, vec![0; 33], DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ) .result .unwrap(); @@ -1724,8 +1724,8 @@ fn instantiate_return_code() { None, callee_hash.iter().chain(&1u32.to_le_bytes()).cloned().collect(), DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ) .result .unwrap(); @@ -1740,8 +1740,8 @@ fn instantiate_return_code() { None, callee_hash.iter().chain(&2u32.to_le_bytes()).cloned().collect(), DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ) .result .unwrap(); @@ -1830,8 +1830,8 @@ fn chain_extension_works() { None, input.clone(), DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ); assert_eq!(TestExtension::last_seen_buffer(), input); assert_eq!(result.result.unwrap().data, input); @@ -1845,8 +1845,8 @@ fn chain_extension_works() { None, ExtensionInput { extension_id: 0, func_id: 1, extra: &[] }.into(), DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ) .result .unwrap(); @@ -1862,8 +1862,8 @@ fn chain_extension_works() { None, ExtensionInput { extension_id: 0, func_id: 2, extra: &[0] }.into(), DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ); assert_ok!(result.result); let gas_consumed = result.gas_consumed; @@ -1875,8 +1875,8 @@ fn chain_extension_works() { None, ExtensionInput { extension_id: 0, func_id: 2, extra: &[42] }.into(), DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ); assert_ok!(result.result); assert_eq!(result.gas_consumed.ref_time(), gas_consumed.ref_time() + 42); @@ -1888,8 +1888,8 @@ fn chain_extension_works() { None, ExtensionInput { extension_id: 0, func_id: 2, extra: &[95] }.into(), DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ); assert_ok!(result.result); assert_eq!(result.gas_consumed.ref_time(), gas_consumed.ref_time() + 95); @@ -1903,8 +1903,8 @@ fn chain_extension_works() { None, ExtensionInput { extension_id: 0, func_id: 3, extra: &[] }.into(), DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ) .result .unwrap(); @@ -1922,8 +1922,8 @@ fn chain_extension_works() { None, ExtensionInput { extension_id: 1, func_id: 0, extra: &[] }.into(), DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ) .result .unwrap(); @@ -1985,8 +1985,8 @@ fn chain_extension_temp_storage_works() { None, input.clone(), DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ) .result ); @@ -2546,8 +2546,8 @@ fn reinstrument_does_charge() { None, zero.clone(), DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ); assert!(!result0.result.unwrap().did_revert()); @@ -2559,8 +2559,8 @@ fn reinstrument_does_charge() { None, zero.clone(), DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ); assert!(!result1.result.unwrap().did_revert()); @@ -2582,8 +2582,8 @@ fn reinstrument_does_charge() { None, zero.clone(), DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ); assert!(!result2.result.unwrap().did_revert()); assert!(result2.gas_consumed.ref_time() > result1.gas_consumed.ref_time()); @@ -2623,8 +2623,8 @@ fn debug_message_works() { None, vec![], DebugInfo::UnsafeDebug, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ); assert_matches!(result.result, Ok(_)); @@ -2661,8 +2661,8 @@ fn debug_message_logging_disabled() { None, vec![], DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ); assert_matches!(result.result, Ok(_)); // the dispatchables always run without debugging @@ -2699,8 +2699,8 @@ fn debug_message_invalid_utf8() { None, vec![], DebugInfo::UnsafeDebug, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ); assert_ok!(result.result); assert!(result.debug_message.is_empty()); @@ -2761,8 +2761,8 @@ fn gas_estimation_nested_call_fixed_limit() { None, input.clone(), DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ); assert_ok!(&result.result); @@ -2779,8 +2779,8 @@ fn gas_estimation_nested_call_fixed_limit() { Some(result.storage_deposit.charge_or_zero()), input.clone(), DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ) .result ); @@ -2794,8 +2794,8 @@ fn gas_estimation_nested_call_fixed_limit() { Some(result.storage_deposit.charge_or_zero()), input, DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ) .result; assert_err!(result, >::OutOfGas); @@ -2856,8 +2856,8 @@ fn gas_estimation_call_runtime() { None, call.encode(), DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ); // contract encodes the result of the dispatch runtime let outcome = u32::decode(&mut result.result.unwrap().data.as_ref()).unwrap(); @@ -2874,8 +2874,8 @@ fn gas_estimation_call_runtime() { None, call.encode(), DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ) .result ); @@ -2940,8 +2940,8 @@ fn call_runtime_reentrancy_guarded() { None, call.encode(), DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ) .result .unwrap(); @@ -3004,8 +3004,8 @@ fn ecdsa_recover() { None, params, DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ) .result .unwrap(); @@ -3015,7 +3015,7 @@ fn ecdsa_recover() { } #[test] -fn bare_instantiate_result_returns_events() { +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 = ::Currency::minimum_balance(); @@ -3040,7 +3040,7 @@ fn bare_instantiate_result_returns_events() { } #[test] -fn bare_instantiate_result_does_not_return_events() { +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 = ::Currency::minimum_balance(); @@ -3065,7 +3065,7 @@ fn bare_instantiate_result_does_not_return_events() { } #[test] -fn bare_call_result_returns_events() { +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 = ::Currency::minimum_balance(); @@ -3094,8 +3094,8 @@ fn bare_call_result_returns_events() { None, vec![], DebugInfo::Skip, - Determinism::Enforced, CollectEvents::UnsafeCollect, + Determinism::Enforced, ); let events = result.events.unwrap(); @@ -3106,7 +3106,7 @@ fn bare_call_result_returns_events() { } #[test] -fn bare_call_result_does_not_return_events() { +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 = ::Currency::minimum_balance(); @@ -3135,8 +3135,8 @@ fn bare_call_result_does_not_return_events() { None, vec![], DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ); let events = result.events; @@ -3199,8 +3199,8 @@ fn sr25519_verify() { None, params, DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ) .result .unwrap() @@ -4193,8 +4193,8 @@ fn contract_reverted() { None, input, DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ) .result .unwrap(); @@ -4307,8 +4307,8 @@ fn set_code_hash() { None, new_code_hash.as_ref().to_vec(), DebugInfo::UnsafeDebug, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ) .result .unwrap(); @@ -4323,8 +4323,8 @@ fn set_code_hash() { None, vec![], DebugInfo::UnsafeDebug, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ) .result .unwrap(); @@ -4717,8 +4717,8 @@ fn deposit_limit_in_nested_instantiate() { Some(codec::Compact(callee_info_len + 2 + ED + 4).into()), (1u32, &code_hash_callee, callee_info_len + 2 + ED + 3).encode(), DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ); let returned = result.result.unwrap(); @@ -4984,8 +4984,8 @@ fn cannot_instantiate_indeterministic_code() { None, code_hash.encode(), DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ) .result, >::Indeterministic, @@ -5001,8 +5001,8 @@ fn cannot_instantiate_indeterministic_code() { None, code_hash.encode(), DebugInfo::Skip, - Determinism::Relaxed, CollectEvents::Skip, + Determinism::Relaxed, ) .result, >::Indeterministic, @@ -5051,8 +5051,8 @@ fn cannot_set_code_indeterministic_code() { None, code_hash.encode(), DebugInfo::Skip, - Determinism::Relaxed, CollectEvents::Skip, + Determinism::Relaxed, ) .result, >::Indeterministic, @@ -5101,8 +5101,8 @@ fn delegate_call_indeterministic_code() { None, code_hash.encode(), DebugInfo::Skip, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ) .result, >::Indeterministic, @@ -5118,8 +5118,8 @@ fn delegate_call_indeterministic_code() { None, code_hash.encode(), DebugInfo::Skip, - Determinism::Relaxed, CollectEvents::Skip, + Determinism::Relaxed, ) .result ); @@ -5159,8 +5159,8 @@ fn reentrance_count_works_with_call() { None, input, DebugInfo::UnsafeDebug, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ) .result .unwrap(); @@ -5200,8 +5200,8 @@ fn reentrance_count_works_with_delegated_call() { None, input, DebugInfo::UnsafeDebug, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ) .result .unwrap(); @@ -5255,8 +5255,8 @@ fn account_reentrance_count_works() { None, contract_addr.encode(), DebugInfo::UnsafeDebug, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ) .result .unwrap(); @@ -5269,8 +5269,8 @@ fn account_reentrance_count_works() { None, another_contract_addr.encode(), DebugInfo::UnsafeDebug, - Determinism::Enforced, CollectEvents::Skip, + Determinism::Enforced, ) .result .unwrap(); From eef0f979c2f0a2e5632d491e8aa261fa43933ce2 Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Wed, 3 May 2023 11:21:44 +0200 Subject: [PATCH 27/28] contracts: remove type imports --- bin/node/runtime/src/lib.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index c3dd783870f17..60d9549135b4f 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -52,9 +52,6 @@ use frame_system::{ }; pub use node_primitives::{AccountId, Signature}; use node_primitives::{AccountIndex, Balance, BlockNumber, Hash, Index, Moment}; -use pallet_contracts_primitives::{ - Code, CodeUploadResult, ContractExecResult, ContractInstantiateResult, GetStorageResult, -}; use pallet_election_provider_multi_phase::SolutionAccuracyOf; use pallet_im_online::sr25519::AuthorityId as ImOnlineId; use pallet_nfts::PalletFeatures; @@ -2158,7 +2155,7 @@ impl_runtime_apis! { gas_limit: Option, storage_deposit_limit: Option, input_data: Vec, - ) -> ContractExecResult { + ) -> pallet_contracts_primitives::ContractExecResult { let gas_limit = gas_limit.unwrap_or(RuntimeBlockWeights::get().max_block); Contracts::bare_call( origin, @@ -2178,10 +2175,10 @@ impl_runtime_apis! { value: Balance, gas_limit: Option, storage_deposit_limit: Option, - code: Code, + code: pallet_contracts_primitives::Code, data: Vec, salt: Vec, - ) -> ContractInstantiateResult + ) -> pallet_contracts_primitives::ContractInstantiateResult { let gas_limit = gas_limit.unwrap_or(RuntimeBlockWeights::get().max_block); Contracts::bare_instantiate( @@ -2202,7 +2199,7 @@ impl_runtime_apis! { code: Vec, storage_deposit_limit: Option, determinism: pallet_contracts::Determinism, - ) -> CodeUploadResult + ) -> pallet_contracts_primitives::CodeUploadResult { Contracts::bare_upload_code( origin, @@ -2215,7 +2212,7 @@ impl_runtime_apis! { fn get_storage( address: AccountId, key: Vec, - ) -> GetStorageResult { + ) -> pallet_contracts_primitives::GetStorageResult { Contracts::get_storage( address, key From 3023dfd782d04b89bff3d7ba80d5a8c9332f32a2 Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Wed, 3 May 2023 11:42:57 +0200 Subject: [PATCH 28/28] contracts: minor code review improvements --- frame/contracts/primitives/src/lib.rs | 11 +++++++++-- frame/contracts/src/lib.rs | 2 +- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/frame/contracts/primitives/src/lib.rs b/frame/contracts/primitives/src/lib.rs index e1bd5d1e1cabc..cc21e29e67b3d 100644 --- a/frame/contracts/primitives/src/lib.rs +++ b/frame/contracts/primitives/src/lib.rs @@ -29,9 +29,16 @@ use sp_runtime::{ use sp_std::prelude::*; use sp_weights::Weight; -/// Result type of a `bare_call` or `bare_instantiate` call. +/// Result type of a `bare_call` or `bare_instantiate` call as well as `ContractsApi::call` and +/// `ContractsApi::instantiate`. /// /// It contains the execution result together with some auxiliary information. +/// +/// #Note +/// +/// It has been extended to include `events` at the end of the struct while not bumping the +/// `ContractsApi` version. Therefore when SCALE decoding a `ContractResult` its trailing data +/// should be ignored to avoid any potential compatibility issues. #[derive(Eq, PartialEq, Encode, Decode, RuntimeDebug, TypeInfo)] pub struct ContractResult { /// How much weight was consumed during execution. @@ -80,7 +87,7 @@ pub struct ContractResult { pub type ContractExecResult = ContractResult, Balance, EventRecord>; -/// Result type of a `bare_instantiate` call as well as `ContractsApi::call`. +/// Result type of a `bare_instantiate` call as well as `ContractsApi::instantiate`. pub type ContractInstantiateResult = ContractResult, DispatchError>, Balance, EventRecord>; diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index d6fc3a45031b6..e0e8e2d15b06d 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -981,7 +981,7 @@ pub enum CollectEvents { /// # 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 in them into the PoV. + /// 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,