From c973fe86f8c668462186c95655a58fda04508e9a Mon Sep 17 00:00:00 2001 From: PG Herveou Date: Tue, 30 Apr 2024 16:29:14 +0200 Subject: [PATCH] Contracts: revert reverted changes from 4266 (#4277) revert some reverted changes from #4266 --- .../src/parachain/contracts_config.rs | 14 ++------ .../frame/contracts/mock-network/src/tests.rs | 25 +------------- substrate/frame/contracts/src/lib.rs | 3 ++ substrate/frame/contracts/src/wasm/runtime.rs | 33 ++----------------- 4 files changed, 9 insertions(+), 66 deletions(-) diff --git a/substrate/frame/contracts/mock-network/src/parachain/contracts_config.rs b/substrate/frame/contracts/mock-network/src/parachain/contracts_config.rs index 20fdd9a243d1..bf3c00b3ff1f 100644 --- a/substrate/frame/contracts/mock-network/src/parachain/contracts_config.rs +++ b/substrate/frame/contracts/mock-network/src/parachain/contracts_config.rs @@ -14,8 +14,9 @@ // You should have received a copy of the GNU General Public License // along with Polkadot. If not, see . -use super::{Balances, Runtime, RuntimeCall, RuntimeEvent, RuntimeHoldReason}; -use frame_support::{derive_impl, parameter_types, traits::Contains}; +use super::{Balances, Runtime, RuntimeCall, RuntimeEvent}; +use crate::parachain::RuntimeHoldReason; +use frame_support::{derive_impl, parameter_types}; parameter_types! { pub Schedule: pallet_contracts::Schedule = Default::default(); @@ -28,14 +29,5 @@ impl pallet_contracts::Config for Runtime { type Currency = Balances; type Schedule = Schedule; type Time = super::Timestamp; - type CallFilter = CallFilter; type Xcm = pallet_xcm::Pallet; } - -/// In this mock, we only allow other contract calls via XCM. -pub struct CallFilter; -impl Contains for CallFilter { - fn contains(call: &RuntimeCall) -> bool { - matches!(call, RuntimeCall::Contracts(pallet_contracts::Call::call { .. })) - } -} diff --git a/substrate/frame/contracts/mock-network/src/tests.rs b/substrate/frame/contracts/mock-network/src/tests.rs index e7d1f6279aa3..48a94e172a02 100644 --- a/substrate/frame/contracts/mock-network/src/tests.rs +++ b/substrate/frame/contracts/mock-network/src/tests.rs @@ -22,10 +22,7 @@ use crate::{ relay_chain, MockNet, ParaA, ParachainBalances, Relay, ALICE, BOB, INITIAL_BALANCE, }; use codec::{Decode, Encode}; -use frame_support::{ - assert_err, - traits::{fungibles::Mutate, Currency}, -}; +use frame_support::traits::{fungibles::Mutate, Currency}; use pallet_contracts::{test_utils::builder::*, Code}; use pallet_contracts_fixtures::compile_module; use pallet_contracts_uapi::ReturnErrorCode; @@ -132,26 +129,6 @@ fn test_xcm_execute_incomplete() { }); } -#[test] -fn test_xcm_execute_filtered_call() { - MockNet::reset(); - - let contract_addr = instantiate_test_contract("xcm_execute"); - - ParaA::execute_with(|| { - // `remark` should be rejected, as it is not allowed by our CallFilter. - let call = parachain::RuntimeCall::System(frame_system::Call::remark { remark: vec![] }); - let message: Xcm = Xcm::builder_unsafe() - .transact(OriginKind::Native, Weight::MAX, call.encode()) - .build(); - let result = bare_call(contract_addr.clone()) - .data(VersionedXcm::V4(message).encode()) - .build() - .result; - assert_err!(result, frame_system::Error::::CallFiltered); - }); -} - #[test] fn test_xcm_execute_reentrant_call() { MockNet::reset(); diff --git a/substrate/frame/contracts/src/lib.rs b/substrate/frame/contracts/src/lib.rs index 0045d72141c9..3e87eb9f37ea 100644 --- a/substrate/frame/contracts/src/lib.rs +++ b/substrate/frame/contracts/src/lib.rs @@ -307,6 +307,9 @@ pub mod pallet { /// Therefore please make sure to be restrictive about which dispatchables are allowed /// in order to not introduce a new DoS vector like memory allocation patterns that can /// be exploited to drive the runtime into a panic. + /// + /// This filter does not apply to XCM transact calls. To impose restrictions on XCM transact + /// calls, you must configure them separately within the XCM pallet itself. #[pallet::no_default_bounds] type CallFilter: Contains<::RuntimeCall>; diff --git a/substrate/frame/contracts/src/wasm/runtime.rs b/substrate/frame/contracts/src/wasm/runtime.rs index 52ceda99edb7..3212aff31269 100644 --- a/substrate/frame/contracts/src/wasm/runtime.rs +++ b/substrate/frame/contracts/src/wasm/runtime.rs @@ -25,12 +25,8 @@ use crate::{ }; use codec::{Decode, DecodeLimit, Encode, MaxEncodedLen}; use frame_support::{ - dispatch::DispatchInfo, - ensure, - pallet_prelude::{DispatchResult, DispatchResultWithPostInfo}, - parameter_types, - traits::Get, - weights::Weight, + dispatch::DispatchInfo, ensure, pallet_prelude::DispatchResultWithPostInfo, parameter_types, + traits::Get, weights::Weight, }; use pallet_contracts_proc_macro::define_env; use pallet_contracts_uapi::{CallFlags, ReturnFlags}; @@ -41,7 +37,6 @@ use sp_runtime::{ }; use sp_std::{fmt, prelude::*}; use wasmi::{core::HostError, errors::LinkerError, Linker, Memory, Store}; -use xcm::VersionedXcm; type CallOf = ::RuntimeCall; @@ -378,29 +373,6 @@ fn already_charged(_: u32) -> Option { None } -/// Ensure that the XCM program is executable, by checking that it does not contain any [`Transact`] -/// instruction with a call that is not allowed by the CallFilter. -fn ensure_executable(message: &VersionedXcm>) -> DispatchResult { - use frame_support::traits::Contains; - use xcm::prelude::{Transact, Xcm}; - - let mut message: Xcm> = - message.clone().try_into().map_err(|_| Error::::XCMDecodeFailed)?; - - message.iter_mut().try_for_each(|inst| -> DispatchResult { - let Transact { ref mut call, .. } = inst else { return Ok(()) }; - let call = call.ensure_decoded().map_err(|_| Error::::XCMDecodeFailed)?; - - if !::CallFilter::contains(call) { - return Err(frame_system::Error::::CallFiltered.into()) - } - - Ok(()) - })?; - - Ok(()) -} - /// Can only be used for one call. pub struct Runtime<'a, E: Ext + 'a> { ext: &'a mut E, @@ -2117,7 +2089,6 @@ pub mod env { ctx.charge_gas(RuntimeCosts::CopyFromContract(msg_len))?; let message: VersionedXcm> = ctx.read_sandbox_memory_as_unbounded(memory, msg_ptr, msg_len)?; - ensure_executable::(&message)?; let execute_weight = <::Xcm as ExecuteController<_, _>>::WeightInfo::execute();