[Based on transaction extension 3685] make transaction extension pipeline not rejecting not signed origins

polkadot/runtime/common/src/claims.rs

@@ -657,7 +657,10 @@ where
 		(ValidTransaction, Self::Val, <T::RuntimeCall as Dispatchable>::RuntimeOrigin),
 		TransactionValidityError,
 	> {
-		let who = origin.as_system_origin_signer().ok_or(InvalidTransaction::BadSigner)?;
+		let Some(who) = origin.as_system_origin_signer()  else {
+			return Ok((ValidTransaction::default(), (), origin));
+		};
+
 		if let Some(local_call) = call.is_sub_type() {
 			if let Call::attest { statement: attested_statement } = local_call {
 				let signer = Preclaims::<T>::get(who)

substrate/frame/system/src/extensions/deny_none.rs

@@ -0,0 +1,224 @@
+// This file is part of Substrate.
+
+// Copyright (C) Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: Apache-2.0
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use crate::Config;
+use frame_support::{
+	dispatch::DispatchInfo,
+	pallet_prelude::{Decode, DispatchResult, Encode, TypeInfo, Weight},
+	traits::OriginTrait,
+	CloneNoBound, DefaultNoBound, EqNoBound, PartialEqNoBound, RuntimeDebugNoBound,
+};
+use sp_runtime::{
+	traits::{
+		transaction_extension::TransactionExtensionBase, Dispatchable, PostDispatchInfoOf,
+		TransactionExtension, ValidateResult,
+	},
+	transaction_validity::TransactionValidityError,
+};
+
+#[derive(
+	Encode,
+	Decode,
+	CloneNoBound,
+	EqNoBound,
+	PartialEqNoBound,
+	DefaultNoBound,
+	TypeInfo,
+	RuntimeDebugNoBound,
+)]
+#[scale_info(skip_type_params(T))]
+pub struct DenyNone<T>(core::marker::PhantomData<T>);
+
+impl<T> DenyNone<T> {
+	pub fn new() -> Self {
+		Default::default()
+	}
+}
+
+impl<T: Config + Send + Sync> TransactionExtensionBase for DenyNone<T> {
+	const IDENTIFIER: &'static str = "DenyNone";
+	type Implicit = ();
+}
+
+impl<T: Config + Send + Sync> TransactionExtension<T::RuntimeCall> for DenyNone<T>
+where
+	T::RuntimeCall: Dispatchable<Info = DispatchInfo>,
+{
+	type Val = ();
+	type Pre = ();
+
+	fn validate(
+		&self,
+		origin: T::RuntimeOrigin,
+		_call: &T::RuntimeCall,
+		_info: &DispatchInfo,
+		_len: usize,
+		_self_implicit: Self::Implicit,
+		_inherited_implication: &impl Encode,
+	) -> ValidateResult<Self::Val, T::RuntimeCall> {
+		if let Some(crate::RawOrigin::None) = origin.as_system_ref() {
+			// TODO TODO: find a better error variant
+			Err(TransactionValidityError::Invalid(crate::InvalidTransaction::Call))
+		} else {
+			Ok((Default::default(), (), origin))
+		}
+	}
+
+	fn prepare(
+		self,
+		_val: Self::Val,
+		_origin: &T::RuntimeOrigin,
+		_call: &T::RuntimeCall,
+		_info: &DispatchInfo,
+		_len: usize,
+	) -> Result<Self::Pre, TransactionValidityError> {
+		Ok(())
+	}
+
+	fn post_dispatch_details(
+		_pre: Self::Pre,
+		_info: &DispatchInfo,
+		_post_info: &PostDispatchInfoOf<T::RuntimeCall>,
+		_len: usize,
+		_result: &DispatchResult,
+	) -> Result<Weight, TransactionValidityError> {
+		Ok(Weight::zero())
+	}
+
+	fn weight(&self, _call: &T::RuntimeCall) -> Weight {
+		Weight::zero()
+	}
+}
+
+#[cfg(test)]
+mod tests {
+	use crate as frame_system;
+	use frame_support::{derive_impl, traits::OriginTrait};
+	use sp_runtime::{
+		traits::TransactionExtension as _,
+		transaction_validity::{InvalidTransaction, TransactionValidityError},
+		BuildStorage,
+	};
+
+	#[frame_support::pallet]
+	pub mod pallet1 {
+		use crate as frame_system;
+
+		#[pallet::pallet]
+		pub struct Pallet<T>(_);
+
+		#[pallet::config]
+		pub trait Config: frame_system::Config {}
+	}
+
+	#[frame_support::runtime]
+	mod runtime {
+		#[runtime::runtime]
+		#[runtime::derive(
+			RuntimeCall,
+			RuntimeEvent,
+			RuntimeError,
+			RuntimeOrigin,
+			RuntimeFreezeReason,
+			RuntimeHoldReason,
+			RuntimeSlashReason,
+			RuntimeLockId,
+			RuntimeTask
+		)]
+		pub struct Runtime;
+
+		#[runtime::pallet_index(0)]
+		pub type System = frame_system::Pallet<Runtime>;
+
+		#[runtime::pallet_index(1)]
+		pub type Pallet1 = pallet1::Pallet<Runtime>;
+	}
+
+	pub type TransactionExtension = (frame_system::DenyNone<Runtime>,);
+
+	pub type Header = sp_runtime::generic::Header<u32, sp_runtime::traits::BlakeTwo256>;
+	pub type Block = sp_runtime::generic::Block<Header, UncheckedExtrinsic>;
+	pub type UncheckedExtrinsic = sp_runtime::generic::UncheckedExtrinsic<
+		u64,
+		RuntimeCall,
+		sp_runtime::testing::MockU64Signature,
+		TransactionExtension,
+	>;
+
+	#[derive_impl(frame_system::config_preludes::TestDefaultConfig)]
+	impl frame_system::Config for Runtime {
+		type Block = Block;
+	}
+
+	impl pallet1::Config for Runtime {}
+
+	pub fn new_test_ext() -> sp_io::TestExternalities {
+		let t = RuntimeGenesisConfig { ..Default::default() }.build_storage().unwrap();
+		t.into()
+	}
+
+	#[test]
+	fn allowed_origin() {
+		new_test_ext().execute_with(|| {
+			let ext = frame_system::DenyNone::<Runtime>::new();
+
+			let filtered_call = RuntimeCall::System(frame_system::Call::remark { remark: vec![] });
+
+			let origin = {
+				let mut o: RuntimeOrigin = crate::Origin::<Runtime>::Signed(42).into();
+				let filter_clone = filtered_call.clone();
+				o.add_filter(move |call| filter_clone != *call);
+				o
+			};
+
+			let (_, (), new_origin) = ext
+				.validate(
+					origin,
+					&RuntimeCall::System(frame_system::Call::set_heap_pages { pages: 42 }),
+					&crate::DispatchInfo::default(),
+					Default::default(),
+					(),
+					&(),
+				)
+				.expect("valid");
+
+			assert!(!new_origin.filter_call(&filtered_call));
+		});
+	}
+
+	#[test]
+	fn denied_origin() {
+		new_test_ext().execute_with(|| {
+			let ext = frame_system::DenyNone::<Runtime>::new();
+
+			let origin: RuntimeOrigin = crate::Origin::<Runtime>::None.into();
+
+			let err = ext
+				.validate(
+					origin,
+					&RuntimeCall::System(frame_system::Call::set_heap_pages { pages: 42 }),
+					&crate::DispatchInfo::default(),
+					Default::default(),
+					(),
+					&(),
+				)
+				.expect_err("invalid");
+
+			assert_eq!(err, TransactionValidityError::Invalid(InvalidTransaction::Call));
+		});
+	}
+}

substrate/frame/system/src/extensions/mod.rs

@@ -22,6 +22,7 @@ pub mod check_nonce;
 pub mod check_spec_version;
 pub mod check_tx_version;
 pub mod check_weight;
+pub mod deny_none;
 pub mod weights;
 
 pub use weights::WeightInfo;

substrate/frame/system/src/lib.rs

@@ -169,7 +169,7 @@ pub use extensions::{
 	check_genesis::CheckGenesis, check_mortality::CheckMortality,
 	check_non_zero_sender::CheckNonZeroSender, check_nonce::CheckNonce,
 	check_spec_version::CheckSpecVersion, check_tx_version::CheckTxVersion,
-	check_weight::CheckWeight, WeightInfo as ExtensionsWeightInfo,
+	check_weight::CheckWeight, deny_none::DenyNone, WeightInfo as ExtensionsWeightInfo,
 };
 // Backward compatible re-export.
 pub use extensions::check_mortality::CheckMortality as CheckEra;

substrate/frame/transaction-payment/asset-conversion-tx-payment/src/lib.rs

@@ -260,31 +260,45 @@ where
 	type Implicit = ();
 }
 
+/// The info passed between the validate and prepare steps for the `ChargeAssetTxPayment` extension.
+pub enum Val<T: Config> {
+	Charge {
+		tip: BalanceOf<T>,
+		// who paid the fee
+		who: T::AccountId,
+		// transaction fee
+		fee: BalanceOf<T>,
+	},
+	NoCharge,
+}
+
+/// The info passed between the prepare and post-dispatch steps for the `ChargeAssetTxPayment`
+/// extension.
+pub enum Pre<T: Config> {
+	Charge {
+		tip: BalanceOf<T>,
+		// who paid the fee
+		who: T::AccountId,
+		// imbalance resulting from withdrawing the fee
+		initial_payment: InitialPayment<T>,
+		// weight used by the extension
+		weight: Weight,
+	},
+	NoCharge {
+		// weight used by the extension
+		weight: Weight,
+	},
+}
+
 impl<T: Config> TransactionExtension<T::RuntimeCall> for ChargeAssetTxPayment<T>
 where
 	T::RuntimeCall: Dispatchable<Info = DispatchInfo, PostInfo = PostDispatchInfo>,
 	BalanceOf<T>: Send + Sync + From<u64>,
 	T::AssetId: Send + Sync,
 	<T::RuntimeCall as Dispatchable>::RuntimeOrigin: AsSystemOriginSigner<T::AccountId> + Clone,
 {
-	type Val = (
-		// tip
-		BalanceOf<T>,
-		// who paid the fee
-		T::AccountId,
-		// transaction fee
-		BalanceOf<T>,
-	);
-	type Pre = (
-		// tip
-		BalanceOf<T>,
-		// who paid the fee
-		T::AccountId,
-		// imbalance resulting from withdrawing the fee
-		InitialPayment<T>,
-		// weight used by the extension
-		Weight,
-	);
+	type Val = Val<T>;
+	type Pre = Pre<T>;
 
 	fn weight(&self, _: &T::RuntimeCall) -> Weight {
 		if self.asset_id.is_some() {
@@ -303,13 +317,15 @@ where
 		_self_implicit: Self::Implicit,
 		_inherited_implication: &impl Encode,
 	) -> ValidateResult<Self::Val, T::RuntimeCall> {
-		let who = origin.as_system_origin_signer().ok_or(InvalidTransaction::BadSigner)?;
+		let Some(who) = origin.as_system_origin_signer() else {
+			return Ok((ValidTransaction::default(), Val::NoCharge, origin))
+		};
 		// Non-mutating call of `compute_fee` to calculate the fee used in the transaction priority.
 		let fee = pallet_transaction_payment::Pallet::<T>::compute_fee(len as u32, info, self.tip);
 		self.can_withdraw_fee(&who, call, info, fee)?;
 		let priority = ChargeTransactionPayment::<T>::get_priority(info, len, self.tip, fee);
 		let validity = ValidTransaction { priority, ..Default::default() };
-		let val = (self.tip, who.clone(), fee);
+		let val = Val::Charge { tip: self.tip, who: who.clone(), fee };
 		Ok((validity, val, origin))
 	}
 
@@ -321,10 +337,21 @@ where
 		info: &DispatchInfoOf<T::RuntimeCall>,
 		_len: usize,
 	) -> Result<Self::Pre, TransactionValidityError> {
-		let (_tip, who, fee) = val;
-		// Mutating call of `withdraw_fee` to actually charge for the transaction.
-		let (_fee, initial_payment) = self.withdraw_fee(&who, call, info, fee)?;
-		Ok((self.tip, who.clone(), initial_payment, self.weight(call)))
+		match val {
+			Val::Charge { tip, who, fee } => {
+				// Mutating call of `withdraw_fee` to actually charge for the transaction.
+				let (_fee, initial_payment) = self.withdraw_fee(&who, call, info, fee)?;
+				Ok(Pre::Charge {
+					tip,
+					who,
+					initial_payment,
+					weight: self.weight(call),
+				})
+			},
+			Val::NoCharge => {
+				Ok(Pre::NoCharge { weight: self.weight(call) })
+			},
+		}
 	}
 
 	fn post_dispatch_details(
@@ -334,7 +361,14 @@ where
 		len: usize,
 		_result: &DispatchResult,
 	) -> Result<Weight, TransactionValidityError> {
-		let (tip, who, initial_payment, extension_weight) = pre;
+		let (tip, who, initial_payment, extension_weight) = match pre {
+			Pre::Charge { tip, who, initial_payment, weight } => (tip, who, initial_payment, weight),
+			Pre::NoCharge { weight } => {
+				// No-op: Refund everything
+				return Ok(weight)
+			},
+		};
+
 		match initial_payment {
 			InitialPayment::Native(already_withdrawn) => {
 				// Take into account the weight used by this extension before calculating the