Root origin always bypass all filter, other origin cannot bypass BaseCallFilter even when constructed from Root origin (#9948)

* improve root filter bypass

* improve doc

* fmt

* refactor test for more understandable flow

* Update frame/support/procedural/src/construct_runtime/expand/origin.rs

Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>

* match

Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>

substrate/frame/support/procedural/src/construct_runtime/expand/origin.rs

@@ -82,8 +82,9 @@ pub fn expand_outer_origin(
 	Ok(quote! {
 		#( #query_origin_part_macros )*
 
-		// WARNING: All instance must hold the filter `frame_system::Config::BaseCallFilter`, except
-		// when caller is system Root. One can use `OriginTrait::reset_filter` to do so.
+		/// The runtime origin type represanting the origin of a call.
+		///
+		/// Origin is always created with the base filter configured in `frame_system::Config::BaseCallFilter`.
 		#[derive(Clone)]
 		pub struct Origin {
 			caller: OriginCaller,
@@ -140,7 +141,11 @@ pub fn expand_outer_origin(
 			}
 
 			fn filter_call(&self, call: &Self::Call) -> bool {
-				(self.filter)(call)
+				match self.caller {
+					// Root bypasses all filters
+					OriginCaller::system(#system_path::Origin::<#runtime>::Root) => true,
+					_ => (self.filter)(call),
+				}
 			}
 
 			fn caller(&self) -> &Self::PalletsOrigin {
@@ -157,15 +162,14 @@ pub fn expand_outer_origin(
 				}
 			}
 
-			/// Create with system none origin and `frame-system::Config::BaseCallFilter`.
 			fn none() -> Self {
 				#system_path::RawOrigin::None.into()
 			}
-			/// Create with system root origin and no filter.
+
 			fn root() -> Self {
 				#system_path::RawOrigin::Root.into()
 			}
-			/// Create with system signed origin and `frame-system::Config::BaseCallFilter`.
+
 			fn signed(by: <#runtime as #system_path::Config>::AccountId) -> Self {
 				#system_path::RawOrigin::Signed(by).into()
 			}
@@ -191,7 +195,7 @@ pub fn expand_outer_origin(
 			pub fn none() -> Self {
 				<Origin as #scrate::traits::OriginTrait>::none()
 			}
-			/// Create with system root origin and no filter.
+			/// Create with system root origin and `frame-system::Config::BaseCallFilter`.
 			pub fn root() -> Self {
 				<Origin as #scrate::traits::OriginTrait>::root()
 			}
@@ -221,9 +225,7 @@ pub fn expand_outer_origin(
 		}
 
 		impl From<#system_path::Origin<#runtime>> for Origin {
-			/// Convert to runtime origin:
-			/// * root origin is built with no filter
-			/// * others use `frame-system::Config::BaseCallFilter`
+			/// Convert to runtime origin, using as filter: `frame-system::Config::BaseCallFilter`.
 			fn from(x: #system_path::Origin<#runtime>) -> Self {
 				let o: OriginCaller = x.into();
 				o.into()
@@ -237,10 +239,7 @@ pub fn expand_outer_origin(
 					filter: #scrate::sp_std::rc::Rc::new(Box::new(|_| true)),
 				};
 
-				// Root has no filter
-				if !matches!(o.caller, OriginCaller::system(#system_path::Origin::<#runtime>::Root)) {
-					#scrate::traits::OriginTrait::reset_filter(&mut o);
-				}
+				#scrate::traits::OriginTrait::reset_filter(&mut o);
 
 				o
 			}

substrate/frame/support/src/traits/dispatch.rs

@@ -70,7 +70,10 @@ pub trait OriginTrait: Sized {
 	/// Replace the caller with caller from the other origin
 	fn set_caller_from(&mut self, other: impl Into<Self>);
 
-	/// Filter the call, if false then call is filtered out.
+	/// Filter the call if caller is not root, if false is returned then the call must be filtered
+	/// out.
+	///
+	/// For root origin caller, the filters are bypassed and true is returned.
 	fn filter_call(&self, call: &Self::Call) -> bool;
 
 	/// Get the caller.
@@ -82,12 +85,12 @@ pub trait OriginTrait: Sized {
 		f: impl FnOnce(Self::PalletsOrigin) -> Result<R, Self::PalletsOrigin>,
 	) -> Result<R, Self>;
 
-	/// Create with system none origin and `frame-system::Config::BaseCallFilter`.
+	/// Create with system none origin and `frame_system::Config::BaseCallFilter`.
 	fn none() -> Self;
 
-	/// Create with system root origin and no filter.
+	/// Create with system root origin and `frame_system::Config::BaseCallFilter`.
 	fn root() -> Self;
 
-	/// Create with system signed origin and `frame-system::Config::BaseCallFilter`.
+	/// Create with system signed origin and `frame_system::Config::BaseCallFilter`.
 	fn signed(by: Self::AccountId) -> Self;
 }

substrate/frame/support/test/tests/construct_runtime.rs

@@ -327,19 +327,31 @@ mod origin_test {
 		assert_eq!(Origin::from(super::nested::module3::Origin).filter_call(&rejected_call), false);
 
 		let mut origin = Origin::from(Some(0));
-
 		origin.add_filter(|c| matches!(c, Call::Module3(_)));
 		assert_eq!(origin.filter_call(&accepted_call), false);
 		assert_eq!(origin.filter_call(&rejected_call), false);
 
+		// Now test for root origin and filters:
+		let mut origin = Origin::from(Some(0));
 		origin.set_caller_from(Origin::root());
 		assert!(matches!(origin.caller, OriginCaller::system(super::system::RawOrigin::Root)));
-		assert_eq!(origin.filter_call(&accepted_call), false);
-		assert_eq!(origin.filter_call(&rejected_call), false);
 
-		origin.reset_filter();
+		// Root origin bypass all filter.
+		assert_eq!(origin.filter_call(&accepted_call), true);
+		assert_eq!(origin.filter_call(&rejected_call), true);
+
+		origin.set_caller_from(Origin::from(Some(0)));
+
+		// Back to another signed origin, the filtered are now effective again
 		assert_eq!(origin.filter_call(&accepted_call), true);
 		assert_eq!(origin.filter_call(&rejected_call), false);
+
+		origin.set_caller_from(Origin::root());
+		origin.reset_filter();
+
+		// Root origin bypass all filter, even when they are reset.
+		assert_eq!(origin.filter_call(&accepted_call), true);
+		assert_eq!(origin.filter_call(&rejected_call), true);
 	}
 }