diff --git a/frame/contracts/src/exec.rs b/frame/contracts/src/exec.rs index c28490dfacccf..7ef1aec2dfc60 100644 --- a/frame/contracts/src/exec.rs +++ b/frame/contracts/src/exec.rs @@ -1094,10 +1094,7 @@ mod tests { use pallet_contracts_primitives::ReturnFlags; use pretty_assertions::assert_eq; use sp_core::Bytes; - use sp_runtime::{ - traits::{BadOrigin, Hash}, - DispatchError, - }; + use sp_runtime::{traits::Hash, DispatchError}; use std::{cell::RefCell, collections::HashMap, rc::Rc}; type System = frame_system::Pallet; @@ -2114,7 +2111,10 @@ mod tests { let forbidden_call = Call::Balances(BalanceCall::transfer { dest: CHARLIE, value: 22 }); // simple cases: direct call - assert_err!(ctx.ext.call_runtime(forbidden_call.clone()), BadOrigin); + assert_err!( + ctx.ext.call_runtime(forbidden_call.clone()), + frame_system::Error::::CallFiltered + ); // as part of a patch: return is OK (but it interrupted the batch) assert_ok!(ctx.ext.call_runtime(Call::Utility(UtilCall::batch { @@ -2159,7 +2159,7 @@ mod tests { phase: Phase::Initialization, event: MetaEvent::Utility(pallet_utility::Event::BatchInterrupted( 1, - BadOrigin.into() + frame_system::Error::::CallFiltered.into() ),), topics: vec![], }, diff --git a/frame/multisig/src/tests.rs b/frame/multisig/src/tests.rs index d46c22ec73d09..c5607c80abce4 100644 --- a/frame/multisig/src/tests.rs +++ b/frame/multisig/src/tests.rs @@ -846,7 +846,7 @@ fn multisig_filters() { let call = Box::new(Call::System(frame_system::Call::set_code { code: vec![] })); assert_noop!( Multisig::as_multi_threshold_1(Origin::signed(1), vec![2], call.clone()), - DispatchError::BadOrigin, + DispatchError::from(frame_system::Error::::CallFiltered), ); }); } diff --git a/frame/proxy/src/tests.rs b/frame/proxy/src/tests.rs index d319ebb1a5ab0..20efd085fe882 100644 --- a/frame/proxy/src/tests.rs +++ b/frame/proxy/src/tests.rs @@ -174,6 +174,8 @@ use frame_system::Call as SystemCall; use pallet_balances::{Call as BalancesCall, Error as BalancesError, Event as BalancesEvent}; use pallet_utility::{Call as UtilityCall, Event as UtilityEvent}; +type SystemError = frame_system::Error; + pub fn new_test_ext() -> sp_io::TestExternalities { let mut t = frame_system::GenesisConfig::default().build_storage::().unwrap(); pallet_balances::GenesisConfig:: { @@ -333,7 +335,9 @@ fn filtering_works() { assert_ok!(Proxy::proxy(Origin::signed(3), 1, None, call.clone())); System::assert_last_event(ProxyEvent::ProxyExecuted(Ok(())).into()); assert_ok!(Proxy::proxy(Origin::signed(4), 1, None, call.clone())); - System::assert_last_event(ProxyEvent::ProxyExecuted(Err(DispatchError::BadOrigin)).into()); + System::assert_last_event( + ProxyEvent::ProxyExecuted(Err(SystemError::CallFiltered.into())).into(), + ); let derivative_id = Utility::derivative_account_id(1, 0); assert!(Balances::mutate_account(&derivative_id, |a| a.free = 1000).is_ok()); @@ -344,9 +348,13 @@ fn filtering_works() { assert_ok!(Proxy::proxy(Origin::signed(2), 1, None, call.clone())); System::assert_last_event(ProxyEvent::ProxyExecuted(Ok(())).into()); assert_ok!(Proxy::proxy(Origin::signed(3), 1, None, call.clone())); - System::assert_last_event(ProxyEvent::ProxyExecuted(Err(DispatchError::BadOrigin)).into()); + System::assert_last_event( + ProxyEvent::ProxyExecuted(Err(SystemError::CallFiltered.into())).into(), + ); assert_ok!(Proxy::proxy(Origin::signed(4), 1, None, call.clone())); - System::assert_last_event(ProxyEvent::ProxyExecuted(Err(DispatchError::BadOrigin)).into()); + System::assert_last_event( + ProxyEvent::ProxyExecuted(Err(SystemError::CallFiltered.into())).into(), + ); let call = Box::new(Call::Utility(UtilityCall::batch { calls: vec![*inner] })); assert_ok!(Proxy::proxy(Origin::signed(2), 1, None, call.clone())); @@ -355,10 +363,12 @@ fn filtering_works() { ProxyEvent::ProxyExecuted(Ok(())).into(), ]); assert_ok!(Proxy::proxy(Origin::signed(3), 1, None, call.clone())); - System::assert_last_event(ProxyEvent::ProxyExecuted(Err(DispatchError::BadOrigin)).into()); + System::assert_last_event( + ProxyEvent::ProxyExecuted(Err(SystemError::CallFiltered.into())).into(), + ); assert_ok!(Proxy::proxy(Origin::signed(4), 1, None, call.clone())); expect_events(vec![ - UtilityEvent::BatchInterrupted(0, DispatchError::BadOrigin).into(), + UtilityEvent::BatchInterrupted(0, SystemError::CallFiltered.into()).into(), ProxyEvent::ProxyExecuted(Ok(())).into(), ]); @@ -371,18 +381,24 @@ fn filtering_works() { ProxyEvent::ProxyExecuted(Ok(())).into(), ]); assert_ok!(Proxy::proxy(Origin::signed(3), 1, None, call.clone())); - System::assert_last_event(ProxyEvent::ProxyExecuted(Err(DispatchError::BadOrigin)).into()); + System::assert_last_event( + ProxyEvent::ProxyExecuted(Err(SystemError::CallFiltered.into())).into(), + ); assert_ok!(Proxy::proxy(Origin::signed(4), 1, None, call.clone())); expect_events(vec![ - UtilityEvent::BatchInterrupted(0, DispatchError::BadOrigin).into(), + UtilityEvent::BatchInterrupted(0, SystemError::CallFiltered.into()).into(), ProxyEvent::ProxyExecuted(Ok(())).into(), ]); let call = Box::new(Call::Proxy(ProxyCall::remove_proxies {})); assert_ok!(Proxy::proxy(Origin::signed(3), 1, None, call.clone())); - System::assert_last_event(ProxyEvent::ProxyExecuted(Err(DispatchError::BadOrigin)).into()); + System::assert_last_event( + ProxyEvent::ProxyExecuted(Err(SystemError::CallFiltered.into())).into(), + ); assert_ok!(Proxy::proxy(Origin::signed(4), 1, None, call.clone())); - System::assert_last_event(ProxyEvent::ProxyExecuted(Err(DispatchError::BadOrigin)).into()); + System::assert_last_event( + ProxyEvent::ProxyExecuted(Err(SystemError::CallFiltered.into())).into(), + ); assert_ok!(Proxy::proxy(Origin::signed(2), 1, None, call.clone())); expect_events(vec![ BalancesEvent::::Unreserved(1, 5).into(), @@ -462,13 +478,17 @@ fn proxying_works() { let call = Box::new(Call::System(SystemCall::set_code { code: vec![] })); assert_ok!(Proxy::proxy(Origin::signed(3), 1, None, call.clone())); - System::assert_last_event(ProxyEvent::ProxyExecuted(Err(DispatchError::BadOrigin)).into()); + System::assert_last_event( + ProxyEvent::ProxyExecuted(Err(SystemError::CallFiltered.into())).into(), + ); let call = Box::new(Call::Balances(BalancesCall::transfer_keep_alive { dest: 6, value: 1 })); assert_ok!(Call::Proxy(super::Call::new_call_variant_proxy(1, None, call.clone())) .dispatch(Origin::signed(2))); - System::assert_last_event(ProxyEvent::ProxyExecuted(Err(DispatchError::BadOrigin)).into()); + System::assert_last_event( + ProxyEvent::ProxyExecuted(Err(SystemError::CallFiltered.into())).into(), + ); assert_ok!(Proxy::proxy(Origin::signed(3), 1, None, call.clone())); System::assert_last_event(ProxyEvent::ProxyExecuted(Ok(())).into()); assert_eq!(Balances::free_balance(6), 2); diff --git a/frame/support/procedural/src/construct_runtime/expand/call.rs b/frame/support/procedural/src/construct_runtime/expand/call.rs index 2532a680e21be..5658ec045433a 100644 --- a/frame/support/procedural/src/construct_runtime/expand/call.rs +++ b/frame/support/procedural/src/construct_runtime/expand/call.rs @@ -22,6 +22,7 @@ use syn::Ident; pub fn expand_outer_dispatch( runtime: &Ident, + system_pallet: &Pallet, pallet_decls: &[Pallet], scrate: &TokenStream, ) -> TokenStream { @@ -29,6 +30,7 @@ pub fn expand_outer_dispatch( let mut variant_patterns = Vec::new(); let mut query_call_part_macros = Vec::new(); let mut pallet_names = Vec::new(); + let system_path = &system_pallet.path; let pallets_with_call = pallet_decls.iter().filter(|decl| decl.exists_part("Call")); @@ -106,7 +108,9 @@ pub fn expand_outer_dispatch( type PostInfo = #scrate::weights::PostDispatchInfo; fn dispatch(self, origin: Origin) -> #scrate::dispatch::DispatchResultWithPostInfo { if !::filter_call(&origin, &self) { - return #scrate::sp_std::result::Result::Err(#scrate::dispatch::DispatchError::BadOrigin.into()); + return #scrate::sp_std::result::Result::Err( + #system_path::Error::<#runtime>::CallFiltered.into() + ); } #scrate::traits::UnfilteredDispatchable::dispatch_bypass_filter(self, origin) diff --git a/frame/support/procedural/src/construct_runtime/expand/origin.rs b/frame/support/procedural/src/construct_runtime/expand/origin.rs index 57adf86a9fe18..eb0212c3efee3 100644 --- a/frame/support/procedural/src/construct_runtime/expand/origin.rs +++ b/frame/support/procedural/src/construct_runtime/expand/origin.rs @@ -18,23 +18,14 @@ use crate::construct_runtime::{Pallet, SYSTEM_PALLET_NAME}; use proc_macro2::TokenStream; use quote::quote; -use syn::{token, Generics, Ident}; +use syn::{Generics, Ident}; pub fn expand_outer_origin( runtime: &Ident, + system_pallet: &Pallet, pallets: &[Pallet], - pallets_token: token::Brace, scrate: &TokenStream, ) -> syn::Result { - let system_pallet = - pallets.iter().find(|decl| decl.name == SYSTEM_PALLET_NAME).ok_or_else(|| { - syn::Error::new( - pallets_token.span, - "`System` pallet declaration is missing. \ - Please add this line: `System: frame_system::{Pallet, Call, Storage, Config, Event},`", - ) - })?; - let mut caller_variants = TokenStream::new(); let mut pallet_conversions = TokenStream::new(); let mut query_origin_part_macros = Vec::new(); diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index 4315d4278183a..f54fa79ce609b 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -214,17 +214,26 @@ fn construct_runtime_final_expansion( pallets_token, } = definition; + let system_pallet = + pallets.iter().find(|decl| decl.name == SYSTEM_PALLET_NAME).ok_or_else(|| { + syn::Error::new( + pallets_token.span, + "`System` pallet declaration is missing. \ + Please add this line: `System: frame_system::{Pallet, Call, Storage, Config, Event},`", + ) + })?; + let hidden_crate_name = "construct_runtime"; let scrate = generate_crate_access(&hidden_crate_name, "frame-support"); let scrate_decl = generate_hidden_includes(&hidden_crate_name, "frame-support"); let outer_event = expand::expand_outer_event(&name, &pallets, &scrate)?; - let outer_origin = expand::expand_outer_origin(&name, &pallets, pallets_token, &scrate)?; + let outer_origin = expand::expand_outer_origin(&name, &system_pallet, &pallets, &scrate)?; let all_pallets = decl_all_pallets(&name, pallets.iter()); let pallet_to_index = decl_pallet_runtime_setup(&name, &pallets, &scrate); - let dispatch = expand::expand_outer_dispatch(&name, &pallets, &scrate); + let dispatch = expand::expand_outer_dispatch(&name, &system_pallet, &pallets, &scrate); let metadata = expand::expand_runtime_metadata(&name, &pallets, &scrate, &unchecked_extrinsic); let outer_config = expand::expand_outer_config(&name, &pallets, &scrate); let inherent = diff --git a/frame/support/test/tests/system.rs b/frame/support/test/tests/system.rs index 4acc248d25f20..9def12131dd19 100644 --- a/frame/support/test/tests/system.rs +++ b/frame/support/test/tests/system.rs @@ -63,7 +63,9 @@ frame_support::decl_error! { TestError, /// Error documentation /// with multiple lines - AnotherError + AnotherError, + // Required by construct_runtime + CallFiltered, } } diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index 2e7f26eef16f4..41e1738c034f1 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -560,6 +560,8 @@ pub mod pallet { NonDefaultComposite, /// There is a non-zero reference count preventing the account from being purged. NonZeroRefCount, + /// The origin filter prevent the call to be dispatched. + CallFiltered, } /// Exposed trait-generic origin type. diff --git a/frame/utility/src/tests.rs b/frame/utility/src/tests.rs index bbfbb417e23d1..cb7a3d9a21e29 100644 --- a/frame/utility/src/tests.rs +++ b/frame/utility/src/tests.rs @@ -288,7 +288,7 @@ fn as_derivative_filters() { value: 1 })), ), - DispatchError::BadOrigin + DispatchError::from(frame_system::Error::::CallFiltered), ); }); } @@ -338,7 +338,8 @@ fn batch_with_signed_filters() { vec![Call::Balances(pallet_balances::Call::transfer_keep_alive { dest: 2, value: 1 })] ),); System::assert_last_event( - utility::Event::BatchInterrupted(0, DispatchError::BadOrigin).into(), + utility::Event::BatchInterrupted(0, frame_system::Error::::CallFiltered.into()) + .into(), ); }); } @@ -573,7 +574,7 @@ fn batch_all_does_not_nest() { actual_weight: Some(::WeightInfo::batch_all(1) + info.weight), pays_fee: Pays::Yes }, - error: DispatchError::BadOrigin, + error: frame_system::Error::::CallFiltered.into(), } ); @@ -585,7 +586,8 @@ fn batch_all_does_not_nest() { // and balances. assert_ok!(Utility::batch_all(Origin::signed(1), vec![batch_nested])); System::assert_has_event( - utility::Event::BatchInterrupted(0, DispatchError::BadOrigin).into(), + utility::Event::BatchInterrupted(0, frame_system::Error::::CallFiltered.into()) + .into(), ); assert_eq!(Balances::free_balance(1), 10); assert_eq!(Balances::free_balance(2), 10);