Migrate pallet-im-online to TransactionExtension API

prdoc/pr_11235.prdoc

@@ -0,0 +1,19 @@
+title: Migrate `pallet-im-online` to use `TransactionExtension` API
+
+doc:
+- audience: Runtime Dev
+  description: |-
+    Migrates `pallet-im-online` from the deprecated `ValidateUnsigned` trait to the modern
+    `TransactionExtension` API using the `#[pallet::authorize]` attribute.
+
+    Runtime integrators: the pallet's `Config` trait now requires
+    `CreateAuthorizedTransaction<Call<Self>>` instead of `CreateBare<Call<Self>>`.
+    The runtime must include `frame_system::AuthorizeCall` in its transaction extension pipeline.
+
+crates:
+- name: pallet-im-online
+  bump: major
+- name: pallet-offences-benchmarking
+  bump: patch
+- name: kitchensink-runtime
+  bump: patch

substrate/bin/node/cli/tests/submit_transaction.rs

@@ -17,20 +17,21 @@
 // along with this program. If not, see <https://www.gnu.org/licenses/>.
 
 use codec::Decode;
-use frame_system::offchain::{SendSignedTransaction, Signer, SubmitTransaction};
+use frame_system::offchain::{
+	CreateAuthorizedTransaction, SendSignedTransaction, Signer, SubmitTransaction,
+};
 use kitchensink_runtime::{Executive, ExistentialDeposit, Indices, Runtime, UncheckedExtrinsic};
 use polkadot_sdk::*;
 use sp_application_crypto::AppCrypto;
 use sp_core::offchain::{testing::TestTransactionPoolExt, TransactionPoolExt};
 use sp_keyring::sr25519::Keyring::Alice;
 use sp_keystore::{testing::MemoryKeystore, Keystore, KeystoreExt};
-use sp_runtime::generic;
 
 pub mod common;
 use self::common::*;
 
 #[test]
-fn should_submit_unsigned_transaction() {
+fn should_submit_authorized_transaction() {
 	let mut t = new_test_ext(compact_code_unwrap());
 	let (pool, state) = TestTransactionPoolExt::new();
 	t.register_extension(TransactionPoolExt::new(pool));
@@ -46,7 +47,9 @@ fn should_submit_unsigned_transaction() {
 		};
 
 		let call = pallet_im_online::Call::heartbeat { heartbeat: heartbeat_data, signature };
-		let xt = generic::UncheckedExtrinsic::new_bare(call.into()).into();
+		let xt = <Runtime as CreateAuthorizedTransaction<
+			pallet_im_online::Call<Runtime>,
+		>>::create_authorized_transaction(call.into());
 		SubmitTransaction::<Runtime, pallet_im_online::Call<Runtime>>::submit_transaction(xt)
 			.unwrap();
 

substrate/frame/im-online/src/benchmarking.rs

@@ -20,12 +20,12 @@
 #![cfg(feature = "runtime-benchmarks")]
 
 use frame_benchmarking::v2::*;
-use frame_support::{traits::UnfilteredDispatchable, WeakBoundedVec};
-use frame_system::RawOrigin;
-use sp_runtime::{
-	traits::{ValidateUnsigned, Zero},
-	transaction_validity::TransactionSource,
+use frame_support::{
+	traits::{Authorize, UnfilteredDispatchable},
+	WeakBoundedVec,
 };
+use frame_system::RawOrigin;
+use sp_runtime::{traits::Zero, transaction_validity::TransactionSource};
 
 use crate::*;
 
@@ -60,51 +60,56 @@ pub fn create_heartbeat<T: Config>(
 	Ok((input_heartbeat, signature))
 }
 
-#[benchmarks]
+#[benchmarks(where <T as frame_system::Config>::RuntimeCall: From<Call<T>>)]
 mod benchmarks {
 	use super::*;
 
-	#[benchmark(extra)]
+	#[benchmark]
 	fn heartbeat(k: Linear<1, { <T as Config>::MaxKeys::get() }>) -> Result<(), BenchmarkError> {
 		let (input_heartbeat, signature) = create_heartbeat::<T>(k)?;
 
 		#[extrinsic_call]
-		_(RawOrigin::None, input_heartbeat, signature);
+		_(RawOrigin::Authorized, input_heartbeat, signature);
 
 		Ok(())
 	}
 
-	#[benchmark(extra)]
-	fn validate_unsigned(
+	#[benchmark]
+	fn authorize_heartbeat(
 		k: Linear<1, { <T as Config>::MaxKeys::get() }>,
 	) -> Result<(), BenchmarkError> {
 		let (input_heartbeat, signature) = create_heartbeat::<T>(k)?;
-		let call = Call::heartbeat { heartbeat: input_heartbeat, signature };
+		let call: <T as frame_system::Config>::RuntimeCall =
+			Call::heartbeat { heartbeat: input_heartbeat, signature }.into();
 
 		#[block]
 		{
-			Pallet::<T>::validate_unsigned(TransactionSource::InBlock, &call)
-				.map_err(<&str>::from)?;
+			call.authorize(TransactionSource::InBlock)
+				.expect("heartbeat call should have authorize logic")
+				.map_err(|_| "authorize failed")?;
 		}
 
 		Ok(())
 	}
 
-	#[benchmark]
-	fn validate_unsigned_and_then_heartbeat(
+	#[benchmark(extra)]
+	fn authorize_and_then_heartbeat(
 		k: Linear<1, { <T as Config>::MaxKeys::get() }>,
 	) -> Result<(), BenchmarkError> {
 		let (input_heartbeat, signature) = create_heartbeat::<T>(k)?;
 		let call = Call::heartbeat { heartbeat: input_heartbeat, signature };
+		let runtime_call: <T as frame_system::Config>::RuntimeCall = call.clone().into();
 		let call_enc = call.encode();
 
 		#[block]
 		{
-			Pallet::<T>::validate_unsigned(TransactionSource::InBlock, &call)
-				.map_err(<&str>::from)?;
+			runtime_call
+				.authorize(TransactionSource::InBlock)
+				.expect("heartbeat call should have authorize logic")
+				.map_err(|_| "authorize failed")?;
 			<Call<T> as Decode>::decode(&mut &*call_enc)
 				.expect("call is encoded above, encoding must be correct")
-				.dispatch_bypass_filter(RawOrigin::None.into())?;
+				.dispatch_bypass_filter(RawOrigin::Authorized.into())?;
 		}
 
 		Ok(())

substrate/frame/im-online/src/lib.rs

@@ -27,7 +27,7 @@
 //!
 //! The heartbeat is a signed transaction, which was signed using the session key
 //! and includes the recent best block number of the local validators chain.
-//! It is submitted as an Unsigned Transaction via off-chain workers.
+//! It is submitted as an Authorized Transaction via off-chain workers.
 //!
 //! - [`Config`]
 //! - [`Call`]
@@ -95,7 +95,7 @@ use frame_support::{
 	BoundedSlice, WeakBoundedVec,
 };
 use frame_system::{
-	offchain::{CreateBare, SubmitTransaction},
+	offchain::{CreateAuthorizedTransaction, SubmitTransaction},
 	pallet_prelude::*,
 };
 pub use pallet::*;
@@ -104,6 +104,7 @@ use sp_application_crypto::RuntimeAppPublic;
 use sp_runtime::{
 	offchain::storage::{MutateStorageError, StorageRetrievalError, StorageValueRef},
 	traits::{AtLeast32BitUnsigned, Convert, Saturating, TrailingZeroInput},
+	transaction_validity::TransactionValidityWithRefund,
 	Debug, PerThing, Perbill, Permill, SaturatedConversion,
 };
 use sp_staking::{
@@ -261,7 +262,11 @@ pub mod pallet {
 	pub struct Pallet<T>(_);
 
 	#[pallet::config]
-	pub trait Config: CreateBare<Call<Self>> + frame_system::Config {
+	/// # Requirements
+	///
+	/// This pallet requires `frame_system::AuthorizeCall` to be included in the runtime's
+	/// transaction extension pipeline.
+	pub trait Config: CreateAuthorizedTransaction<Call<Self>> + frame_system::Config {
 		/// The identifier type for an authority.
 		type AuthorityId: Member
 			+ Parameter
@@ -384,20 +389,22 @@ pub mod pallet {
 		/// ## Complexity:
 		/// - `O(K)` where K is length of `Keys` (heartbeat.validators_len)
 		///   - `O(K)`: decoding of length `K`
-		// NOTE: the weight includes the cost of validate_unsigned as it is part of the cost to
-		// import block with such an extrinsic.
 		#[pallet::call_index(0)]
-		#[pallet::weight(<T as Config>::WeightInfo::validate_unsigned_and_then_heartbeat(
+		#[pallet::weight(<T as Config>::WeightInfo::heartbeat(
+			heartbeat.validators_len,
+		))]
+		#[pallet::weight_of_authorize(<T as Config>::WeightInfo::authorize_heartbeat(
 			heartbeat.validators_len,
 		))]
+		#[pallet::authorize(Self::authorize_heartbeat_call)]
 		pub fn heartbeat(
 			origin: OriginFor<T>,
 			heartbeat: Heartbeat<BlockNumberFor<T>>,
-			// since signature verification is done in `validate_unsigned`
+			// since signature verification is done in `authorize`
 			// we can skip doing it here again.
 			_signature: <T::AuthorityId as RuntimeAppPublic>::Signature,
 		) -> DispatchResult {
-			ensure_none(origin)?;
+			ensure_authorized(origin)?;
 
 			let current_session = T::ValidatorSet::session_index();
 			let exists =
@@ -446,59 +453,6 @@ pub mod pallet {
 	/// Invalid transaction custom error. Returned when validators_len field in heartbeat is
 	/// incorrect.
 	pub(crate) const INVALID_VALIDATORS_LEN: u8 = 10;
-
-	#[pallet::validate_unsigned]
-	impl<T: Config> ValidateUnsigned for Pallet<T> {
-		type Call = Call<T>;
-
-		fn validate_unsigned(_source: TransactionSource, call: &Self::Call) -> TransactionValidity {
-			if let Call::heartbeat { heartbeat, signature } = call {
-				if <Pallet<T>>::is_online(heartbeat.authority_index) {
-					// we already received a heartbeat for this authority
-					return InvalidTransaction::Stale.into();
-				}
-
-				// check if session index from heartbeat is recent
-				let current_session = T::ValidatorSet::session_index();
-				if heartbeat.session_index != current_session {
-					return InvalidTransaction::Stale.into();
-				}
-
-				// verify that the incoming (unverified) pubkey is actually an authority id
-				let keys = Keys::<T>::get();
-				if keys.len() as u32 != heartbeat.validators_len {
-					return InvalidTransaction::Custom(INVALID_VALIDATORS_LEN).into();
-				}
-				let authority_id = match keys.get(heartbeat.authority_index as usize) {
-					Some(id) => id,
-					None => return InvalidTransaction::BadProof.into(),
-				};
-
-				// check signature (this is expensive so we do it last).
-				let signature_valid = heartbeat.using_encoded(|encoded_heartbeat| {
-					authority_id.verify(&encoded_heartbeat, signature)
-				});
-
-				if !signature_valid {
-					return InvalidTransaction::BadProof.into();
-				}
-
-				ValidTransaction::with_tag_prefix("ImOnline")
-					.priority(T::UnsignedPriority::get())
-					.and_provides((current_session, authority_id))
-					.longevity(
-						TryInto::<u64>::try_into(
-							T::NextSessionRotation::average_session_length() / 2u32.into(),
-						)
-						.unwrap_or(64_u64),
-					)
-					.propagate(true)
-					.build()
-			} else {
-				InvalidTransaction::Call.into()
-			}
-		}
-	}
 }
 
 /// Keep track of number of authored blocks per authority, uncles are counted as
@@ -643,7 +597,7 @@ impl<T: Config> Pallet<T> {
 				call,
 			);
 
-			let xt = T::create_bare(call.into());
+			let xt = T::create_authorized_transaction(call.into());
 			SubmitTransaction::<T, Call<T>>::submit_transaction(xt)
 				.map_err(|_| OffchainErr::SubmitTransaction)?;
 
@@ -729,6 +683,59 @@ impl<T: Config> Pallet<T> {
 		}
 	}
 
+	/// Authorization callback for the `heartbeat` call.
+	///
+	/// Validates that the heartbeat is from a recognized authority in the current session
+	/// and that the signature is correct. Cheap checks (staleness, session index, authority
+	/// membership) run first; the expensive signature verification runs last.
+	fn authorize_heartbeat_call(
+		_source: TransactionSource,
+		heartbeat: &Heartbeat<BlockNumberFor<T>>,
+		signature: &<T::AuthorityId as RuntimeAppPublic>::Signature,
+	) -> TransactionValidityWithRefund {
+		if Pallet::<T>::is_online(heartbeat.authority_index) {
+			// we already received a heartbeat for this authority
+			return Err(InvalidTransaction::Stale.into());
+		}
+
+		// check if session index from heartbeat is recent
+		let current_session = T::ValidatorSet::session_index();
+		if heartbeat.session_index != current_session {
+			return Err(InvalidTransaction::Stale.into());
+		}
+
+		// verify that the incoming (unverified) pubkey is actually an authority id
+		let keys = Keys::<T>::get();
+		if keys.len() as u32 != heartbeat.validators_len {
+			return Err(InvalidTransaction::Custom(INVALID_VALIDATORS_LEN).into());
+		}
+		let authority_id = match keys.get(heartbeat.authority_index as usize) {
+			Some(id) => id,
+			None => return Err(InvalidTransaction::BadProof.into()),
+		};
+
+		// check signature (this is expensive so we do it last).
+		let signature_valid = heartbeat
+			.using_encoded(|encoded_heartbeat| authority_id.verify(&encoded_heartbeat, signature));
+
+		if !signature_valid {
+			return Err(InvalidTransaction::BadProof.into());
+		}
+
+		ValidTransaction::with_tag_prefix("ImOnline")
+			.priority(T::UnsignedPriority::get())
+			.and_provides((current_session, authority_id))
+			.longevity(
+				TryInto::<u64>::try_into(
+					T::NextSessionRotation::average_session_length() / 2u32.into(),
+				)
+				.unwrap_or(64_u64),
+			)
+			.propagate(true)
+			.build()
+			.map(|v| (v, Weight::zero()))
+	}
+
 	#[cfg(test)]
 	fn set_keys(keys: Vec<T::AuthorityId>) {
 		let bounded_keys = WeakBoundedVec::<_, T::MaxKeys>::try_from(keys)

substrate/frame/im-online/src/mock.rs

@@ -25,7 +25,11 @@ use frame_support::{
 	weights::Weight,
 };
 use pallet_session::historical as pallet_session_historical;
-use sp_runtime::{testing::UintAuthorityId, traits::ConvertInto, BuildStorage, Permill};
+use sp_runtime::{
+	testing::UintAuthorityId,
+	traits::{BlakeTwo256, ConvertInto},
+	BuildStorage, Permill,
+};
 use sp_staking::{
 	offence::{OffenceError, ReportOffence},
 	SessionIndex,
@@ -34,7 +38,10 @@ use sp_staking::{
 use crate as imonline;
 use crate::Config;
 
-type Block = frame_system::mocking::MockBlock<Runtime>;
+type TxExtension = frame_system::AuthorizeCall<Runtime>;
+/// An extrinsic type used for tests.
+pub type Extrinsic = sp_runtime::testing::TestXt<RuntimeCall, TxExtension>;
+type Block = sp_runtime::generic::Block<sp_runtime::generic::Header<u64, BlakeTwo256>, Extrinsic>;
 
 frame_support::construct_runtime!(
 	pub enum Runtime {
@@ -73,8 +80,6 @@ impl pallet_session::historical::SessionManager<u64, u64> for TestSessionManager
 	fn start_session(_: SessionIndex) {}
 }
 
-/// An extrinsic type used for tests.
-pub type Extrinsic = sp_runtime::testing::TestXt<RuntimeCall, ()>;
 type IdentificationTuple = (u64, u64);
 type Offence = crate::UnresponsivenessOffence<IdentificationTuple>;
 
@@ -206,12 +211,22 @@ where
 	type Extrinsic = Extrinsic;
 }
 
-impl<LocalCall> frame_system::offchain::CreateBare<LocalCall> for Runtime
+impl<LocalCall> frame_system::offchain::CreateTransaction<LocalCall> for Runtime
+where
+	RuntimeCall: From<LocalCall>,
+{
+	type Extension = TxExtension;
+	fn create_transaction(call: RuntimeCall, extension: Self::Extension) -> Extrinsic {
+		Extrinsic::new_transaction(call, extension)
+	}
+}
+
+impl<LocalCall> frame_system::offchain::CreateAuthorizedTransaction<LocalCall> for Runtime
 where
 	RuntimeCall: From<LocalCall>,
 {
-	fn create_bare(call: Self::RuntimeCall) -> Self::Extrinsic {
-		Extrinsic::new_bare(call)
+	fn create_extension() -> Self::Extension {
+		TxExtension::new()
 	}
 }