Revert "FRAME: Create TransactionExtension as a replacement for SignedExtension (#2280)" (#3665)

This PR reverts #2280 which introduced `TransactionExtension` to replace
`SignedExtension`.

As a result of the discussion
[here](https://github.com/paritytech/polkadot-sdk/pull/3623#issuecomment-1986789700),
the changes will be reverted for now with plans to reintroduce the
concept in the future.

---------

Signed-off-by: georgepisaltu <george.pisaltu@parity.io>

substrate/frame/transaction-payment/skip-feeless-payment/src/lib.rs

@@ -21,7 +21,7 @@
 //!
 //! ## Overview
 //!
-//! It does this by wrapping an existing [`TransactionExtension`] implementation (e.g.
+//! It does this by wrapping an existing [`SignedExtension`] implementation (e.g.
 //! [`pallet-transaction-payment`]) and checking if the dispatchable is feeless before applying the
 //! wrapped extension. If the dispatchable is indeed feeless, the extension is skipped and a custom
 //! event is emitted instead. Otherwise, the extension is applied as usual.
@@ -31,8 +31,7 @@
 //!
 //! This pallet wraps an existing transaction payment pallet. This means you should both pallets
 //! in your `construct_runtime` macro and include this pallet's
-//! [`TransactionExtension`] ([`SkipCheckIfFeeless`]) that would accept the existing one as an
-//! argument.
+//! [`SignedExtension`] ([`SkipCheckIfFeeless`]) that would accept the existing one as an argument.
 
 #![cfg_attr(not(feature = "std"), no_std)]
 
@@ -43,10 +42,7 @@ use frame_support::{
 };
 use scale_info::{StaticTypeInfo, TypeInfo};
 use sp_runtime::{
-	traits::{
-		DispatchInfoOf, OriginOf, PostDispatchInfoOf, TransactionExtension,
-		TransactionExtensionBase, ValidateResult,
-	},
+	traits::{DispatchInfoOf, PostDispatchInfoOf, SignedExtension},
 	transaction_validity::TransactionValidityError,
 };
 
@@ -74,11 +70,11 @@ pub mod pallet {
 	#[pallet::generate_deposit(pub(super) fn deposit_event)]
 	pub enum Event<T: Config> {
 		/// A transaction fee was skipped.
-		FeeSkipped { origin: <T::RuntimeOrigin as OriginTrait>::PalletsOrigin },
+		FeeSkipped { who: T::AccountId },
 	}
 }
 
-/// A [`TransactionExtension`] that skips the wrapped extension if the dispatchable is feeless.
+/// A [`SignedExtension`] that skips the wrapped extension if the dispatchable is feeless.
 #[derive(Encode, Decode, Clone, Eq, PartialEq)]
 pub struct SkipCheckIfFeeless<T, S>(pub S, sp_std::marker::PhantomData<T>);
 
@@ -107,96 +103,53 @@ impl<T, S> From<S> for SkipCheckIfFeeless<T, S> {
 	}
 }
 
-pub enum Intermediate<T, O> {
-	/// The wrapped extension should be applied.
-	Apply(T),
-	/// The wrapped extension should be skipped.
-	Skip(O),
-}
-use Intermediate::*;
-
-impl<T: Config + Send + Sync, S: TransactionExtensionBase> TransactionExtensionBase
+impl<T: Config + Send + Sync, S: SignedExtension<AccountId = T::AccountId>> SignedExtension
 	for SkipCheckIfFeeless<T, S>
+where
+	S::Call: CheckIfFeeless<Origin = frame_system::pallet_prelude::OriginFor<T>>,
 {
+	type AccountId = T::AccountId;
+	type Call = S::Call;
+	type AdditionalSigned = S::AdditionalSigned;
+	type Pre = (Self::AccountId, Option<<S as SignedExtension>::Pre>);
 	// From the outside this extension should be "invisible", because it just extends the wrapped
 	// extension with an extra check in `pre_dispatch` and `post_dispatch`. Thus, we should forward
 	// the identifier of the wrapped extension to let wallets see this extension as it would only be
 	// the wrapped extension itself.
 	const IDENTIFIER: &'static str = S::IDENTIFIER;
-	type Implicit = S::Implicit;
 
-	fn implicit(&self) -> Result<Self::Implicit, TransactionValidityError> {
-		self.0.implicit()
+	fn additional_signed(&self) -> Result<Self::AdditionalSigned, TransactionValidityError> {
+		self.0.additional_signed()
 	}
 
-	fn weight(&self) -> frame_support::weights::Weight {
-		self.0.weight()
-	}
-}
-
-impl<T: Config + Send + Sync, Context, S: TransactionExtension<T::RuntimeCall, Context>>
-	TransactionExtension<T::RuntimeCall, Context> for SkipCheckIfFeeless<T, S>
-where
-	T::RuntimeCall: CheckIfFeeless<Origin = frame_system::pallet_prelude::OriginFor<T>>,
-{
-	type Val = Intermediate<S::Val, <OriginOf<T::RuntimeCall> as OriginTrait>::PalletsOrigin>;
-	type Pre = Intermediate<S::Pre, <OriginOf<T::RuntimeCall> as OriginTrait>::PalletsOrigin>;
-
-	fn validate(
-		&self,
-		origin: OriginOf<T::RuntimeCall>,
-		call: &T::RuntimeCall,
-		info: &DispatchInfoOf<T::RuntimeCall>,
-		len: usize,
-		context: &mut Context,
-		self_implicit: S::Implicit,
-		inherited_implication: &impl Encode,
-	) -> ValidateResult<Self::Val, T::RuntimeCall> {
-		if call.is_feeless(&origin) {
-			Ok((Default::default(), Skip(origin.caller().clone()), origin))
-		} else {
-			let (x, y, z) = self.0.validate(
-				origin,
-				call,
-				info,
-				len,
-				context,
-				self_implicit,
-				inherited_implication,
-			)?;
-			Ok((x, Apply(y), z))
-		}
-	}
-
-	fn prepare(
+	fn pre_dispatch(
 		self,
-		val: Self::Val,
-		origin: &OriginOf<T::RuntimeCall>,
-		call: &T::RuntimeCall,
-		info: &DispatchInfoOf<T::RuntimeCall>,
+		who: &Self::AccountId,
+		call: &Self::Call,
+		info: &DispatchInfoOf<Self::Call>,
 		len: usize,
-		context: &Context,
 	) -> Result<Self::Pre, TransactionValidityError> {
-		match val {
-			Apply(val) => self.0.prepare(val, origin, call, info, len, context).map(Apply),
-			Skip(origin) => Ok(Skip(origin)),
+		if call.is_feeless(&<T as frame_system::Config>::RuntimeOrigin::signed(who.clone())) {
+			Ok((who.clone(), None))
+		} else {
+			Ok((who.clone(), Some(self.0.pre_dispatch(who, call, info, len)?)))
 		}
 	}
 
 	fn post_dispatch(
-		pre: Self::Pre,
-		info: &DispatchInfoOf<T::RuntimeCall>,
-		post_info: &PostDispatchInfoOf<T::RuntimeCall>,
+		pre: Option<Self::Pre>,
+		info: &DispatchInfoOf<Self::Call>,
+		post_info: &PostDispatchInfoOf<Self::Call>,
 		len: usize,
 		result: &DispatchResult,
-		context: &Context,
 	) -> Result<(), TransactionValidityError> {
-		match pre {
-			Apply(pre) => S::post_dispatch(pre, info, post_info, len, result, context),
-			Skip(origin) => {
-				Pallet::<T>::deposit_event(Event::<T>::FeeSkipped { origin });
-				Ok(())
-			},
+		if let Some(pre) = pre {
+			if let Some(pre) = pre.1 {
+				S::post_dispatch(Some(pre), info, post_info, len, result)?;
+			} else {
+				Pallet::<T>::deposit_event(Event::<T>::FeeSkipped { who: pre.0 });
+			}
 		}
+		Ok(())
 	}
 }

substrate/frame/transaction-payment/skip-feeless-payment/src/mock.rs

@@ -18,12 +18,9 @@ use crate as pallet_skip_feeless_payment;
 
 use frame_support::{derive_impl, parameter_types};
 use frame_system as system;
-use sp_runtime::{
-	impl_tx_ext_default,
-	traits::{OriginOf, TransactionExtension},
-};
 
 type Block = frame_system::mocking::MockBlock<Runtime>;
+type AccountId = u64;
 
 #[derive_impl(frame_system::config_preludes::TestDefaultConfig as frame_system::DefaultConfig)]
 impl frame_system::Config for Runtime {
@@ -41,22 +38,21 @@ parameter_types! {
 #[derive(Clone, Eq, PartialEq, Debug, Encode, Decode, TypeInfo)]
 pub struct DummyExtension;
 
-impl TransactionExtensionBase for DummyExtension {
-	const IDENTIFIER: &'static str = "DummyExtension";
-	type Implicit = ();
-}
-impl<C> TransactionExtension<RuntimeCall, C> for DummyExtension {
-	type Val = ();
+impl SignedExtension for DummyExtension {
+	type AccountId = AccountId;
+	type Call = RuntimeCall;
+	type AdditionalSigned = ();
 	type Pre = ();
-	impl_tx_ext_default!(RuntimeCall; C; validate);
-	fn prepare(
+	const IDENTIFIER: &'static str = "DummyExtension";
+	fn additional_signed(&self) -> sp_std::result::Result<(), TransactionValidityError> {
+		Ok(())
+	}
+	fn pre_dispatch(
 		self,
-		_val: Self::Val,
-		_origin: &OriginOf<RuntimeCall>,
-		_call: &RuntimeCall,
-		_info: &DispatchInfoOf<RuntimeCall>,
+		_who: &Self::AccountId,
+		_call: &Self::Call,
+		_info: &DispatchInfoOf<Self::Call>,
 		_len: usize,
-		_context: &C,
 	) -> Result<Self::Pre, TransactionValidityError> {
 		PreDispatchCount::mutate(|c| *c += 1);
 		Ok(())

substrate/frame/transaction-payment/skip-feeless-payment/src/tests.rs

@@ -16,19 +16,18 @@
 use super::*;
 use crate::mock::{pallet_dummy::Call, DummyExtension, PreDispatchCount, Runtime, RuntimeCall};
 use frame_support::dispatch::DispatchInfo;
-use sp_runtime::traits::DispatchTransaction;
 
 #[test]
 fn skip_feeless_payment_works() {
 	let call = RuntimeCall::DummyPallet(Call::<Runtime>::aux { data: 1 });
 	SkipCheckIfFeeless::<Runtime, DummyExtension>::from(DummyExtension)
-		.validate_and_prepare(Some(0).into(), &call, &DispatchInfo::default(), 0)
+		.pre_dispatch(&0, &call, &DispatchInfo::default(), 0)
 		.unwrap();
 	assert_eq!(PreDispatchCount::get(), 1);
 
 	let call = RuntimeCall::DummyPallet(Call::<Runtime>::aux { data: 0 });
 	SkipCheckIfFeeless::<Runtime, DummyExtension>::from(DummyExtension)
-		.validate_and_prepare(Some(0).into(), &call, &DispatchInfo::default(), 0)
+		.pre_dispatch(&0, &call, &DispatchInfo::default(), 0)
 		.unwrap();
 	assert_eq!(PreDispatchCount::get(), 1);
 }