Validate unsigned -> pallet::authorize

pallets/transaction-storage/src/lib.rs

@@ -144,28 +144,6 @@ impl TransactionInfo {
 	}
 }
 
-/// Context of a `check_signed`/`check_unsigned` call.
-#[derive(Clone, Copy)]
-enum CheckContext {
-	/// `validate_signed` or `validate_unsigned`.
-	Validate,
-	/// `pre_dispatch_signed` or `pre_dispatch`.
-	PreDispatch,
-}
-
-impl CheckContext {
-	/// Should authorization be consumed in this context? If not, we merely check that
-	/// authorization exists.
-	fn consume_authorization(self) -> bool {
-		matches!(self, CheckContext::PreDispatch)
-	}
-
-	/// Should `check_signed`/`check_unsigned` return a `ValidTransaction`?
-	fn want_valid_transaction(self) -> bool {
-		matches!(self, CheckContext::Validate)
-	}
-}
-
 #[polkadot_sdk_frame::pallet]
 pub mod pallet {
 	use super::*;
@@ -344,7 +322,20 @@ pub mod pallet {
 		#[pallet::call_index(0)]
 		#[pallet::weight(T::WeightInfo::store(data.len() as u32))]
 		#[pallet::feeless_if(|origin: &OriginFor<T>, data: &Vec<u8>| -> bool { /*TODO: add here correct validation */ true })]
-		pub fn store(_origin: OriginFor<T>, data: Vec<u8>) -> DispatchResult {
+		#[pallet::authorize(|_source, data| {
+			Pallet::<T>::to_validity_with_refund(Pallet::<T>::check_unsigned_store(
+				data.as_slice(),
+				false,
+			))
+		})]
+		#[pallet::weight_of_authorize(Weight::zero())]
+		pub fn store(origin: OriginFor<T>, data: Vec<u8>) -> DispatchResult {
+			let is_authorized = matches!(origin.into(), Ok(frame_system::RawOrigin::Authorized));
+			if is_authorized {
+				Self::check_unsigned_store(data.as_slice(), true)
+					.map_err(Self::dispatch_error_from_validity)?;
+			}
+
 			// In the case of a regular unsigned transaction, this should have been checked by
 			// pre_dispatch. In the case of a regular signed transaction, this should have been
 			// checked by pre_dispatch_signed.
@@ -395,11 +386,25 @@ pub mod pallet {
 		/// O(1).
 		#[pallet::call_index(1)]
 		#[pallet::weight(T::WeightInfo::renew())]
+		#[pallet::authorize(|_source, block, index| {
+			Pallet::<T>::to_validity_with_refund(Pallet::<T>::check_unsigned_renew(
+				block,
+				index,
+				false,
+			))
+		})]
+		#[pallet::weight_of_authorize(Weight::zero())]
 		pub fn renew(
-			_origin: OriginFor<T>,
+			origin: OriginFor<T>,
 			block: BlockNumberFor<T>,
 			index: u32,
 		) -> DispatchResultWithPostInfo {
+			let is_authorized = matches!(origin.into(), Ok(frame_system::RawOrigin::Authorized));
+			if is_authorized {
+				Self::check_unsigned_renew(&block, &index, true)
+					.map_err(Self::dispatch_error_from_validity)?;
+			}
+
 			let info = Self::transaction_info(block, index).ok_or(Error::<T>::RenewedNotFound)?;
 
 			// In the case of a regular unsigned transaction, this should have been checked by
@@ -531,6 +536,12 @@ pub mod pallet {
 		/// when successful.
 		#[pallet::call_index(5)]
 		#[pallet::weight(T::WeightInfo::remove_expired_account_authorization())]
+		#[pallet::authorize(|_source, who| {
+			Pallet::<T>::to_validity_with_refund(
+				Pallet::<T>::check_unsigned_remove_expired_account(who),
+			)
+		})]
+		#[pallet::weight_of_authorize(Weight::zero())]
 		pub fn remove_expired_account_authorization(
 			_origin: OriginFor<T>,
 			who: T::AccountId,
@@ -551,6 +562,12 @@ pub mod pallet {
 		/// when successful.
 		#[pallet::call_index(6)]
 		#[pallet::weight(T::WeightInfo::remove_expired_preimage_authorization())]
+		#[pallet::authorize(|_source, hash| {
+			Pallet::<T>::to_validity_with_refund(
+				Pallet::<T>::check_unsigned_remove_expired_preimage_authorization(hash),
+			)
+		})]
+		#[pallet::weight_of_authorize(Weight::zero())]
 		pub fn remove_expired_preimage_authorization(
 			_origin: OriginFor<T>,
 			content_hash: ContentHash,
@@ -719,25 +736,19 @@ pub mod pallet {
 		}
 	}
 
-	#[pallet::validate_unsigned]
-	impl<T: Config> ValidateUnsigned for Pallet<T> {
-		type Call = Call<T>;
-
-		fn validate_unsigned(_source: TransactionSource, call: &Self::Call) -> TransactionValidity {
-			Self::check_unsigned(call, CheckContext::Validate)?.ok_or(IMPOSSIBLE.into())
+	impl<T: Config> Pallet<T> {
+		fn to_validity_with_refund(
+			result: Result<ValidTransaction, TransactionValidityError>,
+		) -> Result<(ValidTransaction, Weight), TransactionValidityError> {
+			let validity = result?;
+			Ok((validity, Weight::zero()))
 		}
 
-		fn pre_dispatch(call: &Self::Call) -> Result<(), TransactionValidityError> {
-			// Allow inherents here.
-			if Self::is_inherent(call) {
-				return Ok(());
-			}
-
-			Self::check_unsigned(call, CheckContext::PreDispatch).map(|_| ())
+		fn dispatch_error_from_validity(error: TransactionValidityError) -> DispatchError {
+			let message: &'static str = error.into();
+			DispatchError::Other(message)
 		}
-	}
 
-	impl<T: Config> Pallet<T> {
 		/// Returns `true` if the system is beyond the given expiration point.
 		fn expired(expiration: BlockNumberFor<T>) -> bool {
 			let now = frame_system::Pallet::<T>::block_number();
@@ -869,7 +880,7 @@ pub mod pallet {
 		/// This is equivalent to `validate_unsigned` but for signed transactions. It should be
 		/// called from a `SignedExtension` implementation.
 		pub fn validate_signed(who: &T::AccountId, call: &Call<T>) -> TransactionValidity {
-			Self::check_signed(who, call, CheckContext::Validate)?.ok_or(IMPOSSIBLE.into())
+			Self::check_signed(who, call, false)
 		}
 
 		/// Check the validity of the given call, signed by the given account, and consume
@@ -881,7 +892,7 @@ pub mod pallet {
 			who: &T::AccountId,
 			call: &Call<T>,
 		) -> Result<(), TransactionValidityError> {
-			Self::check_signed(who, call, CheckContext::PreDispatch).map(|_| ())
+			Self::check_signed(who, call, true).map(|_| ())
 		}
 
 		/// Get RetentionPeriod storage information from the outside of this pallet.
@@ -986,8 +997,8 @@ pub mod pallet {
 		fn check_store_renew_unsigned(
 			size: usize,
 			hash: impl FnOnce() -> ContentHash,
-			context: CheckContext,
-		) -> Result<Option<ValidTransaction>, TransactionValidityError> {
+			consume: bool,
+		) -> Result<ValidTransaction, TransactionValidityError> {
 			if !Self::data_size_ok(size) {
 				return Err(BAD_DATA_SIZE.into());
 			}
@@ -998,72 +1009,70 @@ pub mod pallet {
 
 			let hash = hash();
 
-			Self::check_authorization(
-				AuthorizationScope::Preimage(hash),
-				size as u32,
-				context.consume_authorization(),
-			)?;
+			Self::check_authorization(AuthorizationScope::Preimage(hash), size as u32, consume)?;
 
-			Ok(context.want_valid_transaction().then(|| {
-				ValidTransaction::with_tag_prefix("TransactionStorageStoreRenew")
-					.and_provides(hash)
-					.priority(T::StoreRenewPriority::get())
-					.longevity(T::StoreRenewLongevity::get())
-					.into()
-			}))
+			Ok(ValidTransaction::with_tag_prefix("TransactionStorageStoreRenew")
+				.and_provides(hash)
+				.priority(T::StoreRenewPriority::get())
+				.longevity(T::StoreRenewLongevity::get())
+				.into())
 		}
 
-		fn check_unsigned(
-			call: &Call<T>,
-			context: CheckContext,
-		) -> Result<Option<ValidTransaction>, TransactionValidityError> {
-			match call {
-				Call::<T>::store { data } => Self::check_store_renew_unsigned(
-					data.len(),
-					|| sp_io::hashing::blake2_256(data),
-					context,
-				),
-				Call::<T>::renew { block, index } => {
-					let info = Self::transaction_info(*block, *index).ok_or(RENEWED_NOT_FOUND)?;
-					Self::check_store_renew_unsigned(
-						info.size as usize,
-						|| info.content_hash.into(),
-						context,
-					)
-				},
-				Call::<T>::remove_expired_account_authorization { who } => {
-					Self::check_authorization_expired(AuthorizationScope::Account(who.clone()))?;
-					Ok(context.want_valid_transaction().then(|| {
-						ValidTransaction::with_tag_prefix(
-							"TransactionStorageRemoveExpiredAccountAuthorization",
-						)
-						.and_provides(who)
-						.priority(T::RemoveExpiredAuthorizationPriority::get())
-						.longevity(T::RemoveExpiredAuthorizationLongevity::get())
-						.into()
-					}))
-				},
-				Call::<T>::remove_expired_preimage_authorization { content_hash } => {
-					Self::check_authorization_expired(AuthorizationScope::Preimage(*content_hash))?;
-					Ok(context.want_valid_transaction().then(|| {
-						ValidTransaction::with_tag_prefix(
-							"TransactionStorageRemoveExpiredPreimageAuthorization",
-						)
-						.and_provides(content_hash)
-						.priority(T::RemoveExpiredAuthorizationPriority::get())
-						.longevity(T::RemoveExpiredAuthorizationLongevity::get())
-						.into()
-					}))
-				},
-				_ => Err(InvalidTransaction::Call.into()),
-			}
+		fn check_unsigned_store(
+			data: &[u8],
+			consume: bool,
+		) -> Result<ValidTransaction, TransactionValidityError> {
+			Self::check_store_renew_unsigned(
+				data.len(),
+				|| sp_io::hashing::blake2_256(data),
+				consume,
+			)
+		}
+
+		fn check_unsigned_renew(
+			block: &BlockNumberFor<T>,
+			index: &u32,
+			consume: bool,
+		) -> Result<ValidTransaction, TransactionValidityError> {
+			let info = Self::transaction_info(*block, *index).ok_or(RENEWED_NOT_FOUND)?;
+			Self::check_store_renew_unsigned(
+				info.size as usize,
+				|| info.content_hash.into(),
+				consume,
+			)
+		}
+
+		fn check_unsigned_remove_expired_account(
+			who: &T::AccountId,
+		) -> Result<ValidTransaction, TransactionValidityError> {
+			Self::check_authorization_expired(AuthorizationScope::Account(who.clone()))?;
+			Ok(ValidTransaction::with_tag_prefix(
+				"TransactionStorageRemoveExpiredAccountAuthorization",
+			)
+			.and_provides(who)
+			.priority(T::RemoveExpiredAuthorizationPriority::get())
+			.longevity(T::RemoveExpiredAuthorizationLongevity::get())
+			.into())
+		}
+
+		fn check_unsigned_remove_expired_preimage_authorization(
+			hash: &ContentHash,
+		) -> Result<ValidTransaction, TransactionValidityError> {
+			Self::check_authorization_expired(AuthorizationScope::Preimage(*hash))?;
+			Ok(ValidTransaction::with_tag_prefix(
+				"TransactionStorageRemoveExpiredPreimageAuthorization",
+			)
+			.and_provides(hash)
+			.priority(T::RemoveExpiredAuthorizationPriority::get())
+			.longevity(T::RemoveExpiredAuthorizationLongevity::get())
+			.into())
 		}
 
 		fn check_signed(
 			who: &T::AccountId,
 			call: &Call<T>,
-			context: CheckContext,
-		) -> Result<Option<ValidTransaction>, TransactionValidityError> {
+			consume: bool,
+		) -> Result<ValidTransaction, TransactionValidityError> {
 			let size = match call {
 				Call::<T>::store { data } => data.len(),
 				Call::<T>::renew { block, index } => {
@@ -1084,14 +1093,14 @@ pub mod pallet {
 			Self::check_authorization(
 				AuthorizationScope::Account(who.clone()),
 				size as u32,
-				context.consume_authorization(),
+				consume,
 			)?;
 
-			Ok(context.want_valid_transaction().then(|| ValidTransaction {
+			Ok(ValidTransaction {
 				priority: T::StoreRenewPriority::get(),
 				longevity: T::StoreRenewLongevity::get(),
 				..Default::default()
-			}))
+			})
 		}
 
 		pub(crate) fn verify_chunk_proof(

pallets/transaction-storage/src/tests.rs

@@ -26,8 +26,9 @@ use super::{
 	BAD_DATA_SIZE, DEFAULT_MAX_TRANSACTION_SIZE,
 };
 use polkadot_sdk_frame::{
-	prelude::{frame_system::RawOrigin, *},
+	prelude::{frame_system::RawOrigin, TransactionSource, *},
 	testing_prelude::*,
+	traits::Authorize,
 };
 use sp_transaction_storage_proof::{random_chunk, registration::build_proof, CHUNK_SIZE};
 
@@ -104,19 +105,34 @@ fn uses_preimage_authorization() {
 			AuthorizationExtent { transactions: 1, bytes: 2002 }
 		);
 		let call = Call::store { data: vec![1; 2000] };
-		assert_noop!(TransactionStorage::pre_dispatch(&call), InvalidTransaction::Payment);
+		assert_noop!(
+			call.authorize(TransactionSource::External).unwrap(),
+			InvalidTransaction::Payment
+		);
 		let call = Call::store { data };
-		assert_ok!(TransactionStorage::pre_dispatch(&call));
+		assert_ok!(call.authorize(TransactionSource::External).unwrap());
+		assert_eq!(
+			TransactionStorage::preimage_authorization_extent(hash),
+			AuthorizationExtent { transactions: 1, bytes: 2002 }
+		);
+		assert_ok!(Into::<RuntimeCall>::into(call).dispatch(RawOrigin::Authorized.into()));
 		assert_eq!(
 			TransactionStorage::preimage_authorization_extent(hash),
 			AuthorizationExtent { transactions: 0, bytes: 0 }
 		);
-		assert_ok!(Into::<RuntimeCall>::into(call).dispatch(RuntimeOrigin::none()));
 		run_to_block(3, || None);
 		let call = Call::renew { block: 1, index: 0 };
-		assert_noop!(TransactionStorage::pre_dispatch(&call), InvalidTransaction::Payment);
+		assert_noop!(
+			call.authorize(TransactionSource::External).unwrap(),
+			InvalidTransaction::Payment
+		);
 		assert_ok!(TransactionStorage::authorize_preimage(RuntimeOrigin::root(), hash, 2000));
-		assert_ok!(TransactionStorage::pre_dispatch(&call));
+		assert_ok!(call.authorize(TransactionSource::External).unwrap());
+		assert_eq!(
+			TransactionStorage::preimage_authorization_extent(hash),
+			AuthorizationExtent { transactions: 1, bytes: 2000 }
+		);
+		assert_ok!(Into::<RuntimeCall>::into(call).dispatch(RawOrigin::Authorized.into()));
 		assert_eq!(
 			TransactionStorage::preimage_authorization_extent(hash),
 			AuthorizationExtent { transactions: 0, bytes: 0 }
@@ -292,9 +308,12 @@ fn expired_authorization_clears() {
 		// Can't remove too early
 		run_to_block(10, || None);
 		let remove_call = Call::remove_expired_account_authorization { who };
-		assert_noop!(TransactionStorage::pre_dispatch(&remove_call), AUTHORIZATION_NOT_EXPIRED);
 		assert_noop!(
-			Into::<RuntimeCall>::into(remove_call.clone()).dispatch(RuntimeOrigin::none()),
+			remove_call.authorize(TransactionSource::External).unwrap(),
+			AUTHORIZATION_NOT_EXPIRED
+		);
+		assert_noop!(
+			Into::<RuntimeCall>::into(remove_call.clone()).dispatch(RawOrigin::Authorized.into()),
 			Error::AuthorizationNotExpired,
 		);
 
@@ -308,8 +327,8 @@ fn expired_authorization_clears() {
 			InvalidTransaction::Payment,
 		);
 		// Anyone can remove it
-		assert_ok!(TransactionStorage::pre_dispatch(&remove_call));
-		assert_ok!(Into::<RuntimeCall>::into(remove_call).dispatch(RuntimeOrigin::none()));
+		assert_ok!(remove_call.authorize(TransactionSource::External).unwrap());
+		assert_ok!(Into::<RuntimeCall>::into(remove_call).dispatch(RawOrigin::Authorized.into()));
 		System::assert_has_event(RuntimeEvent::TransactionStorage(
 			Event::ExpiredAccountAuthorizationRemoved { who },
 		));