TransactionExtension follow up refactoring and fixes

docs/sdk/src/reference_docs/transaction_extensions.rs

@@ -13,7 +13,7 @@ pub mod transaction_extensions_example {
 	use sp_runtime::{
 		impl_tx_ext_default,
 		traits::{Dispatchable, TransactionExtension, TransactionExtensionBase},
-		TransactionValidityError,
+		transaction_validity::TransactionValidityError,
 	};
 
 	// This doesn't actually check anything, but simply allows

polkadot/runtime/common/src/claims.rs

@@ -174,13 +174,6 @@ pub mod pallet {
 	#[pallet::without_storage_info]
 	pub struct Pallet<T>(_);
 
-	#[cfg(feature = "runtime-benchmarks")]
-	/// Helper trait to benchmark the `PrevalidateAttests` transaction extension.
-	pub trait BenchmarkHelperTrait<RuntimeCall, DispatchInfo> {
-		/// `Call` to be used when benchmarking the transaction extension.
-		fn default_call_and_info() -> (RuntimeCall, DispatchInfo);
-	}
-
 	/// Configuration trait.
 	#[pallet::config]
 	pub trait Config: frame_system::Config {
@@ -191,12 +184,6 @@ pub mod pallet {
 		type Prefix: Get<&'static [u8]>;
 		type MoveClaimOrigin: EnsureOrigin<Self::RuntimeOrigin>;
 		type WeightInfo: WeightInfo;
-		#[cfg(feature = "runtime-benchmarks")]
-		/// Benchmark helper
-		type BenchmarkHelper: BenchmarkHelperTrait<
-			Self::RuntimeCall,
-			DispatchInfoOf<Self::RuntimeCall>,
-		>;
 	}
 
 	#[pallet::event]
@@ -815,19 +802,6 @@ pub(super) mod tests {
 		type Prefix = Prefix;
 		type MoveClaimOrigin = frame_system::EnsureSignedBy<Six, u64>;
 		type WeightInfo = TestWeightInfo;
-		#[cfg(feature = "runtime-benchmarks")]
-		type BenchmarkHelper = ();
-	}
-
-	#[cfg(feature = "runtime-benchmarks")]
-	impl BenchmarkHelperTrait<RuntimeCall, DispatchInfoOf<RuntimeCall>> for () {
-		fn default_call_and_info() -> (RuntimeCall, DispatchInfoOf<RuntimeCall>) {
-			let call = RuntimeCall::Claims(crate::claims::Call::attest {
-				statement: StatementKind::Regular.to_text().to_vec(),
-			});
-			let info = call.get_dispatch_info();
-			(call, info)
-		}
 	}
 
 	fn alice() -> libsecp256k1::SecretKey {
@@ -1486,7 +1460,10 @@ pub(super) mod benchmarking {
 	use super::*;
 	use crate::claims::Call;
 	use frame_benchmarking::{account, benchmarks};
-	use frame_support::traits::UnfilteredDispatchable;
+	use frame_support::{
+		dispatch::{DispatchInfo, GetDispatchInfo},
+		traits::UnfilteredDispatchable,
+	};
 	use frame_system::RawOrigin;
 	use secp_utils::*;
 	use sp_runtime::{
@@ -1528,7 +1505,8 @@ pub(super) mod benchmarking {
 	}
 
 	benchmarks! {
-		where_clause { where <T as frame_system::Config>::RuntimeCall: IsSubType<Call<T>>,
+		where_clause { where <T as frame_system::Config>::RuntimeCall: IsSubType<Call<T>> + From<Call<T>>,
+			<T as frame_system::Config>::RuntimeCall: Dispatchable<Info = DispatchInfo> + GetDispatchInfo,
 			<<T as frame_system::Config>::RuntimeCall as Dispatchable>::RuntimeOrigin: AsSystemOriginSigner<T::AccountId> + Clone,
 			<<T as frame_system::Config>::RuntimeCall as Dispatchable>::PostInfo: Default,
 		}
@@ -1720,7 +1698,11 @@ pub(super) mod benchmarking {
 			}
 
 			let ext = PrevalidateAttests::<T>::new();
-			let (call, info) = T::BenchmarkHelper::default_call_and_info();
+			let call = super::Call::attest {
+				statement: StatementKind::Regular.to_text().to_vec(),
+			};
+			let call: <T as frame_system::Config>::RuntimeCall = call.into();
+			let info = call.get_dispatch_info();
 			let attest_c = u32::MAX - c;
 			let secret_key = libsecp256k1::SecretKey::parse(&keccak_256(&attest_c.encode())).unwrap();
 			let eth_address = eth(&secret_key);

polkadot/runtime/rococo/src/lib.rs

@@ -636,32 +636,12 @@ parameter_types! {
 	pub Prefix: &'static [u8] = b"Pay ROCs to the Rococo account:";
 }
 
-#[cfg(feature = "runtime-benchmarks")]
-pub struct ClaimsHelper;
-
-#[cfg(feature = "runtime-benchmarks")]
-use frame_support::dispatch::DispatchInfo;
-
-#[cfg(feature = "runtime-benchmarks")]
-impl claims::BenchmarkHelperTrait<RuntimeCall, DispatchInfo> for ClaimsHelper {
-	fn default_call_and_info() -> (RuntimeCall, DispatchInfo) {
-		use frame_support::dispatch::GetDispatchInfo;
-		let call = RuntimeCall::Claims(claims::Call::attest {
-			statement: claims::StatementKind::Regular.to_text().to_vec(),
-		});
-		let info = call.get_dispatch_info();
-		(call, info)
-	}
-}
-
 impl claims::Config for Runtime {
 	type RuntimeEvent = RuntimeEvent;
 	type VestingSchedule = Vesting;
 	type Prefix = Prefix;
 	type MoveClaimOrigin = EnsureRoot<AccountId>;
 	type WeightInfo = weights::runtime_common_claims::WeightInfo<Runtime>;
-	#[cfg(feature = "runtime-benchmarks")]
-	type BenchmarkHelper = ClaimsHelper;
 }
 
 parameter_types! {

polkadot/runtime/test-runtime/src/lib.rs

@@ -444,32 +444,12 @@ parameter_types! {
 	pub Prefix: &'static [u8] = b"Pay KSMs to the Kusama account:";
 }
 
-#[cfg(feature = "runtime-benchmarks")]
-pub struct ClaimsHelper;
-
-#[cfg(feature = "runtime-benchmarks")]
-use frame_support::dispatch::DispatchInfo;
-
-#[cfg(feature = "runtime-benchmarks")]
-impl claims::BenchmarkHelperTrait<RuntimeCall, DispatchInfo> for ClaimsHelper {
-	fn default_call_and_info() -> (RuntimeCall, DispatchInfo) {
-		use frame_support::dispatch::GetDispatchInfo;
-		let call = RuntimeCall::Claims(claims::Call::attest {
-			statement: claims::StatementKind::Regular.to_text().to_vec(),
-		});
-		let info = call.get_dispatch_info();
-		(call, info)
-	}
-}
-
 impl claims::Config for Runtime {
 	type RuntimeEvent = RuntimeEvent;
 	type VestingSchedule = Vesting;
 	type Prefix = Prefix;
 	type MoveClaimOrigin = frame_system::EnsureRoot<AccountId>;
 	type WeightInfo = claims::TestWeightInfo;
-	#[cfg(feature = "runtime-benchmarks")]
-	type BenchmarkHelper = ClaimsHelper;
 }
 
 parameter_types! {

substrate/bin/node/testing/src/bench.rs

@@ -574,13 +574,7 @@ impl BenchKeyring {
 					genesis_hash,
 				);
 				let key = self.accounts.get(&signed).expect("Account id not found in keyring");
-				let signature = payload.using_encoded(|b| {
-					if b.len() > 256 {
-						key.sign(&blake2_256(b))
-					} else {
-						key.sign(b)
-					}
-				});
+				let signature = payload.using_encoded(|b| key.sign(&blake2_256(b)));
 				UncheckedExtrinsic {
 					preamble: Preamble::Signed(
 						sp_runtime::MultiAddress::Id(signed),

substrate/bin/node/testing/src/keyring.rs

@@ -100,16 +100,7 @@ pub fn sign(
 			let payload =
 				(xt.function, tx_ext.clone(), spec_version, tx_version, genesis_hash, genesis_hash);
 			let key = AccountKeyring::from_account_id(&signed).unwrap();
-			let signature =
-				payload
-					.using_encoded(|b| {
-						if b.len() > 256 {
-							key.sign(&blake2_256(b))
-						} else {
-							key.sign(b)
-						}
-					})
-					.into();
+			let signature = payload.using_encoded(|b| key.sign(&blake2_256(b))).into();
 			UncheckedExtrinsic {
 				preamble: sp_runtime::generic::Preamble::Signed(
 					sp_runtime::MultiAddress::Id(signed),

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

@@ -128,7 +128,7 @@ pub fn expand_runtime_metadata(
 								.map(|meta| #scrate::__private::metadata_ir::TransactionExtensionMetadataIR {
 									identifier: meta.identifier,
 									ty: meta.ty,
-									additional_signed: meta.additional_signed,
+									implicit: meta.additional_signed,
 								})
 								.collect(),
 					},

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

@@ -553,8 +553,8 @@ pub trait OriginTrait: Sized {
 		self.caller().as_system_ref()
 	}
 
-	/// Extract a reference to the sytsem signer, if that's what the caller is.
-	fn as_system_signer(&self) -> Option<&Self::AccountId> {
+	/// Extract a reference to the signer, if that's what the caller is.
+	fn as_signer(&self) -> Option<&Self::AccountId> {
 		self.caller().as_system_ref().and_then(|s| {
 			if let RawOrigin::Signed(ref who) = s {
 				Some(who)

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

@@ -813,7 +813,7 @@ fn test_metadata() {
 
 	let extrinsic = ExtrinsicMetadata {
 		ty: meta_type::<UncheckedExtrinsic>(),
-		version: 4,
+		version: 5,
 		signed_extensions: vec![SignedExtensionMetadata {
 			identifier: "UnitTransactionExtension",
 			ty: meta_type::<()>(),

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

@@ -74,7 +74,7 @@ impl<T: Config + Send + Sync, Context> TransactionExtension<T::RuntimeCall, Cont
 		_self_implicit: Self::Implicit,
 		_inherited_implication: &impl Encode,
 	) -> sp_runtime::traits::ValidateResult<Self::Val, T::RuntimeCall> {
-		if let Some(who) = origin.as_system_signer() {
+		if let Some(who) = origin.as_signer() {
 			if who.using_encoded(|d| d.iter().all(|x| *x == 0)) {
 				return Err(InvalidTransaction::BadSigner.into())
 			}
@@ -89,7 +89,7 @@ mod tests {
 	use super::*;
 	use crate::mock::{new_test_ext, Test, CALL};
 	use frame_support::{assert_ok, dispatch::DispatchInfo};
-	use sp_runtime::{traits::DispatchTransaction, TransactionValidityError};
+	use sp_runtime::{traits::DispatchTransaction, transaction_validity::TransactionValidityError};
 
 	#[test]
 	fn zero_account_ban_works() {

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

@@ -27,8 +27,8 @@ use sp_runtime::{
 		DispatchInfoOf, Dispatchable, PostDispatchInfoOf, TransactionExtension,
 		TransactionExtensionBase, ValidateResult,
 	},
-	transaction_validity::{InvalidTransaction, TransactionValidityError},
-	DispatchResult, ValidTransaction,
+	transaction_validity::{InvalidTransaction, TransactionValidityError, ValidTransaction},
+	DispatchResult,
 };
 use sp_weights::Weight;
 

substrate/primitives/metadata-ir/src/types.rs

@@ -167,7 +167,7 @@ pub struct ExtrinsicMetadataIR<T: Form = MetaForm> {
 	/// The type of the extrinsic's signature.
 	pub signature_ty: T::Type,
 	/// The type of the outermost Extra/Extensions enum.
-	// TODO: metadata-v16: rename this to `extension_ty`.
+	// TODO: metadata-v16: remove this.
 	pub extra_ty: T::Type,
 	/// The transaction extensions in the order they appear in the extrinsic.
 	pub extensions: Vec<TransactionExtensionMetadataIR<T>>,
@@ -196,8 +196,8 @@ pub struct TransactionExtensionMetadataIR<T: Form = MetaForm> {
 	pub identifier: T::String,
 	/// The type of the signed extension, with the data to be included in the extrinsic.
 	pub ty: T::Type,
-	/// The type of the additional signed data, with the data to be included in the signed payload.
-	pub additional_signed: T::Type,
+	/// The type of the implicit data, with the data to be included in the signed payload.
+	pub implicit: T::Type,
 }
 
 impl IntoPortable for TransactionExtensionMetadataIR {
@@ -207,7 +207,7 @@ impl IntoPortable for TransactionExtensionMetadataIR {
 		TransactionExtensionMetadataIR {
 			identifier: self.identifier.into_portable(registry),
 			ty: registry.register_type(&self.ty),
-			additional_signed: registry.register_type(&self.additional_signed),
+			implicit: registry.register_type(&self.implicit),
 		}
 	}
 }

substrate/primitives/metadata-ir/src/v14.rs

@@ -142,7 +142,7 @@ impl From<TransactionExtensionMetadataIR> for SignedExtensionMetadata {
 		SignedExtensionMetadata {
 			identifier: ir.identifier,
 			ty: ir.ty,
-			additional_signed: ir.additional_signed,
+			additional_signed: ir.implicit,
 		}
 	}
 }

substrate/primitives/metadata-ir/src/v15.rs

@@ -92,7 +92,7 @@ impl From<TransactionExtensionMetadataIR> for SignedExtensionMetadata {
 		SignedExtensionMetadata {
 			identifier: ir.identifier,
 			ty: ir.ty,
-			additional_signed: ir.additional_signed,
+			additional_signed: ir.implicit,
 		}
 	}
 }

substrate/primitives/runtime/src/generic/unchecked_extrinsic.rs

@@ -39,7 +39,7 @@ use sp_std::{fmt, prelude::*};
 /// This version needs to be bumped if the encoded representation changes.
 /// It ensures that if the representation is changed and the format is not known,
 /// the decoding fails.
-const EXTRINSIC_FORMAT_VERSION: u8 = 4;
+const EXTRINSIC_FORMAT_VERSION: u8 = 5;
 
 /// The `SignaturePayload` of `UncheckedExtrinsic`.
 type UncheckedSignaturePayload<Address, Signature, Extension> = (Address, Signature, Extension);
@@ -254,7 +254,7 @@ where
 		Ok(match self.preamble {
 			Preamble::Signed(signed, signature, tx_ext) => {
 				let signed = lookup.lookup(signed)?;
-				// CHECK! Should this not contain implicit?
+				// The `Implicit` is "implicitly" included in the payload.
 				let raw_payload = SignedPayload::new(self.function, tx_ext)?;
 				if !raw_payload.using_encoded(|payload| signature.verify(payload, &signed)) {
 					return Err(InvalidTransaction::BadProof.into())
@@ -432,17 +432,9 @@ where
 	Call: Encode + Dispatchable,
 	Extension: TransactionExtensionBase,
 {
-	/// Get an encoded version of this payload.
-	///
-	/// Payloads longer than 256 bytes are going to be `blake2_256`-hashed.
+	/// Get an encoded version of this `blake2_256`-hashed payload.
 	fn using_encoded<R, F: FnOnce(&[u8]) -> R>(&self, f: F) -> R {
-		self.0.using_encoded(|payload| {
-			if payload.len() > 256 {
-				f(&blake2_256(payload)[..])
-			} else {
-				f(payload)
-			}
-		})
+		self.0.using_encoded(|payload| f(&blake2_256(payload)[..]))
 	}
 }
 

substrate/primitives/runtime/src/lib.rs

@@ -125,11 +125,6 @@ pub use sp_arithmetic::{
 	Perquintill, Rational128, Rounding, UpperOf,
 };
 
-pub use transaction_validity::{
-	InvalidTransaction, TransactionSource, TransactionValidityError, UnknownTransaction,
-	ValidTransaction,
-};
-
 pub use either::Either;
 
 /// The number of bytes of the module-specific `error` field defined in [`ModuleError`].

substrate/primitives/runtime/src/traits/mod.rs

@@ -1659,11 +1659,11 @@ pub trait SignedExtension:
 	/// If you ever override this function, you need not perform the same validation as in
 	/// `validate_unsigned`.
 	fn pre_dispatch_unsigned(
-		_call: &Self::Call,
-		_info: &DispatchInfoOf<Self::Call>,
-		_len: usize,
+		call: &Self::Call,
+		info: &DispatchInfoOf<Self::Call>,
+		len: usize,
 	) -> Result<(), TransactionValidityError> {
-		Ok(())
+		Self::validate_unsigned(call, info, len).map(|_| ()).map_err(Into::into)
 	}
 }
 
@@ -1713,7 +1713,6 @@ pub trait GetNodeBlockType {
 /// function is called right before dispatching the call wrapped by an unsigned extrinsic. The
 /// [`validate_unsigned`](Self::validate_unsigned) function is mainly being used in the context of
 /// the transaction pool to check the validity of the call wrapped by an unsigned extrinsic.
-// TODO: Rename to ValidateBareTransaction (or just remove).
 pub trait ValidateUnsigned {
 	/// The call to validate
 	type Call;

substrate/primitives/runtime/src/traits/transaction_extension/as_transaction_extension.rs

@@ -25,7 +25,7 @@ use sp_core::RuntimeDebug;
 
 use crate::{
 	traits::{AsSystemOriginSigner, SignedExtension, ValidateResult},
-	InvalidTransaction,
+	transaction_validity::InvalidTransaction,
 };
 
 use super::*;

substrate/primitives/runtime/src/traits/transaction_extension/dispatch_transaction.rs

@@ -44,7 +44,7 @@ pub trait DispatchTransaction<Call: Dispatchable> {
 		info: &Self::Info,
 		len: usize,
 	) -> Result<(ValidTransaction, Self::Val, Self::Origin), TransactionValidityError>;
-	/// Prepare and validate a transaction, ready for dispatch.
+	/// Validate and prepare a transaction, ready for dispatch.
 	fn validate_and_prepare(
 		self,
 		origin: Self::Origin,