From 7b0c96b01ffd7cde4caeb2b0a892afa41d833b2a Mon Sep 17 00:00:00 2001 From: Rodrigo Quelhas Date: Tue, 3 Mar 2026 07:17:45 +0000 Subject: [PATCH 1/3] Migrate pallet-im-online to TransactionExtension API --- prdoc/pr_11234.prdoc | 19 +++ .../bin/node/cli/tests/submit_transaction.rs | 11 +- substrate/frame/im-online/src/benchmarking.rs | 41 +++--- substrate/frame/im-online/src/lib.rs | 124 +++++++++--------- substrate/frame/im-online/src/mock.rs | 29 +++- substrate/frame/im-online/src/tests.rs | 36 +++-- substrate/frame/im-online/src/weights.rs | 75 +++++++---- .../frame/offences/benchmarking/src/mock.rs | 25 +++- 8 files changed, 224 insertions(+), 136 deletions(-) create mode 100644 prdoc/pr_11234.prdoc diff --git a/prdoc/pr_11234.prdoc b/prdoc/pr_11234.prdoc new file mode 100644 index 0000000000000..9a316624c4770 --- /dev/null +++ b/prdoc/pr_11234.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>` instead of `CreateBare>`. + 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 diff --git a/substrate/bin/node/cli/tests/submit_transaction.rs b/substrate/bin/node/cli/tests/submit_transaction.rs index a71adf3d24754..5607c4f2400ff 100644 --- a/substrate/bin/node/cli/tests/submit_transaction.rs +++ b/substrate/bin/node/cli/tests/submit_transaction.rs @@ -17,20 +17,21 @@ // along with this program. If not, see . 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 = , + >>::create_authorized_transaction(call.into()); SubmitTransaction::>::submit_transaction(xt) .unwrap(); diff --git a/substrate/frame/im-online/src/benchmarking.rs b/substrate/frame/im-online/src/benchmarking.rs index 06b7231b1f485..bca49b7c68215 100644 --- a/substrate/frame/im-online/src/benchmarking.rs +++ b/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( Ok((input_heartbeat, signature)) } -#[benchmarks] +#[benchmarks(where ::RuntimeCall: From>)] mod benchmarks { use super::*; - #[benchmark(extra)] + #[benchmark] fn heartbeat(k: Linear<1, { ::MaxKeys::get() }>) -> Result<(), BenchmarkError> { let (input_heartbeat, signature) = create_heartbeat::(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, { ::MaxKeys::get() }>, ) -> Result<(), BenchmarkError> { let (input_heartbeat, signature) = create_heartbeat::(k)?; - let call = Call::heartbeat { heartbeat: input_heartbeat, signature }; + let call: ::RuntimeCall = + Call::heartbeat { heartbeat: input_heartbeat, signature }.into(); #[block] { - Pallet::::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, { ::MaxKeys::get() }>, ) -> Result<(), BenchmarkError> { let (input_heartbeat, signature) = create_heartbeat::(k)?; let call = Call::heartbeat { heartbeat: input_heartbeat, signature }; + let runtime_call: ::RuntimeCall = call.clone().into(); let call_enc = call.encode(); #[block] { - Pallet::::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")?; 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(()) diff --git a/substrate/frame/im-online/src/lib.rs b/substrate/frame/im-online/src/lib.rs index dc18f394a767c..3d1632094b601 100644 --- a/substrate/frame/im-online/src/lib.rs +++ b/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(_); #[pallet::config] - pub trait Config: CreateBare> + frame_system::Config { + /// # Requirements + /// + /// This pallet requires `frame_system::AuthorizeCall` to be included in the runtime's + /// transaction extension pipeline. + pub trait Config: CreateAuthorizedTransaction> + frame_system::Config { /// The identifier type for an authority. type AuthorityId: Member + Parameter @@ -384,20 +389,68 @@ 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(::WeightInfo::validate_unsigned_and_then_heartbeat( + #[pallet::weight(::WeightInfo::heartbeat( + heartbeat.validators_len, + ))] + #[pallet::weight_of_authorize(::WeightInfo::authorize_heartbeat( heartbeat.validators_len, ))] + #[pallet::authorize(|_source: TransactionSource, + heartbeat: &Heartbeat>, + signature: &::Signature, + | -> TransactionValidityWithRefund { + if Pallet::::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::::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::::try_into( + T::NextSessionRotation::average_session_length() / 2u32.into(), + ) + .unwrap_or(64_u64), + ) + .propagate(true) + .build() + .map(|v| (v, Weight::zero())) + })] pub fn heartbeat( origin: OriginFor, heartbeat: Heartbeat>, - // since signature verification is done in `validate_unsigned` + // since signature verification is done in `authorize` // we can skip doing it here again. _signature: ::Signature, ) -> DispatchResult { - ensure_none(origin)?; + ensure_authorized(origin)?; let current_session = T::ValidatorSet::session_index(); let exists = @@ -446,59 +499,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 ValidateUnsigned for Pallet { - type Call = Call; - - fn validate_unsigned(_source: TransactionSource, call: &Self::Call) -> TransactionValidity { - if let Call::heartbeat { heartbeat, signature } = call { - if >::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::::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::::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 +643,7 @@ impl Pallet { call, ); - let xt = T::create_bare(call.into()); + let xt = T::create_authorized_transaction(call.into()); SubmitTransaction::>::submit_transaction(xt) .map_err(|_| OffchainErr::SubmitTransaction)?; diff --git a/substrate/frame/im-online/src/mock.rs b/substrate/frame/im-online/src/mock.rs index 06446cbac1439..aec8cc9c754ad 100644 --- a/substrate/frame/im-online/src/mock.rs +++ b/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; +type TxExtension = frame_system::AuthorizeCall; +/// An extrinsic type used for tests. +pub type Extrinsic = sp_runtime::testing::TestXt; +type Block = sp_runtime::generic::Block, Extrinsic>; frame_support::construct_runtime!( pub enum Runtime { @@ -73,8 +80,6 @@ impl pallet_session::historical::SessionManager for TestSessionManager fn start_session(_: SessionIndex) {} } -/// An extrinsic type used for tests. -pub type Extrinsic = sp_runtime::testing::TestXt; type IdentificationTuple = (u64, u64); type Offence = crate::UnresponsivenessOffence; @@ -206,12 +211,22 @@ where type Extrinsic = Extrinsic; } -impl frame_system::offchain::CreateBare for Runtime +impl frame_system::offchain::CreateTransaction for Runtime +where + RuntimeCall: From, +{ + type Extension = TxExtension; + fn create_transaction(call: RuntimeCall, extension: Self::Extension) -> Extrinsic { + Extrinsic::new_transaction(call, extension) + } +} + +impl frame_system::offchain::CreateAuthorizedTransaction for Runtime where RuntimeCall: From, { - fn create_bare(call: Self::RuntimeCall) -> Self::Extrinsic { - Extrinsic::new_bare(call) + fn create_extension() -> Self::Extension { + TxExtension::new() } } diff --git a/substrate/frame/im-online/src/tests.rs b/substrate/frame/im-online/src/tests.rs index 5a5c69b43b122..254b6ce116db5 100644 --- a/substrate/frame/im-online/src/tests.rs +++ b/substrate/frame/im-online/src/tests.rs @@ -21,12 +21,15 @@ use super::*; use crate::mock::*; -use frame_support::{assert_noop, dispatch}; +use frame_support::{assert_noop, dispatch, traits::Authorize}; use sp_core::offchain::{ testing::{TestOffchainExt, TestTransactionPoolExt}, OffchainDbExt, OffchainWorkerExt, TransactionPoolExt, }; -use sp_runtime::testing::UintAuthorityId; +use sp_runtime::{ + testing::UintAuthorityId, + transaction_validity::{InvalidTransaction, TransactionSource, TransactionValidityError}, +}; #[test] fn test_unresponsiveness_slash_fraction() { @@ -120,17 +123,24 @@ fn heartbeat( }; let signature = id.sign(&heartbeat.encode()).unwrap(); - ImOnline::pre_dispatch(&crate::Call::heartbeat { - heartbeat: heartbeat.clone(), - signature: signature.clone(), - }) - .map_err(|e| match e { - TransactionValidityError::Invalid(InvalidTransaction::Custom(INVALID_VALIDATORS_LEN)) => { - "invalid validators len" - }, - e @ _ => <&'static str>::from(e), - })?; - ImOnline::heartbeat(RuntimeOrigin::none(), heartbeat, signature) + let call: RuntimeCall = + crate::Call::heartbeat { heartbeat: heartbeat.clone(), signature: signature.clone() } + .into(); + + call.authorize(TransactionSource::External) + .expect("heartbeat call should have authorize logic") + .map_err(|e| match e { + TransactionValidityError::Invalid(InvalidTransaction::Custom( + INVALID_VALIDATORS_LEN, + )) => "invalid validators len", + e @ _ => <&'static str>::from(e), + })?; + + ImOnline::heartbeat( + RuntimeOrigin::from(frame_system::RawOrigin::Authorized), + heartbeat, + signature, + ) } #[test] diff --git a/substrate/frame/im-online/src/weights.rs b/substrate/frame/im-online/src/weights.rs index 8906f7cd633d4..f8b2c29cdc836 100644 --- a/substrate/frame/im-online/src/weights.rs +++ b/substrate/frame/im-online/src/weights.rs @@ -72,56 +72,77 @@ use core::marker::PhantomData; /// Weight functions needed for `pallet_im_online`. pub trait WeightInfo { - fn validate_unsigned_and_then_heartbeat(k: u32, ) -> Weight; + fn heartbeat(k: u32, ) -> Weight; + fn authorize_heartbeat(k: u32, ) -> Weight; } /// Weights for `pallet_im_online` using the Substrate node and recommended hardware. pub struct SubstrateWeight(PhantomData); impl WeightInfo for SubstrateWeight { + /// Storage: `Session::CurrentIndex` (r:1 w:0) + /// Proof: `Session::CurrentIndex` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + /// Storage: `ImOnline::ReceivedHeartbeats` (r:1 w:1) + /// Proof: `ImOnline::ReceivedHeartbeats` (`max_values`: None, `max_size`: Some(25), added: 2500, mode: `MaxEncodedLen`) + /// Storage: `ImOnline::Keys` (r:1 w:0) + /// Proof: `ImOnline::Keys` (`max_values`: Some(1), `max_size`: Some(320002), added: 320497, mode: `MaxEncodedLen`) + /// The range of component `k` is `[1, 10000]`. + fn heartbeat(_k: u32, ) -> Weight { + // Proof Size summary in bytes: + // Measured: `4 + k * (32 ±0)` + // Estimated: `321494` + // Minimum execution time: 11_000_000 picoseconds. + Weight::from_parts(220_000_000, 321494) + .saturating_add(T::DbWeight::get().reads(3_u64)) + .saturating_add(T::DbWeight::get().writes(1_u64)) + } /// Storage: `Session::Validators` (r:1 w:0) /// Proof: `Session::Validators` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) /// Storage: `Session::CurrentIndex` (r:1 w:0) /// Proof: `Session::CurrentIndex` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) /// Storage: `ImOnline::Keys` (r:1 w:0) /// Proof: `ImOnline::Keys` (`max_values`: Some(1), `max_size`: Some(320002), added: 320497, mode: `MaxEncodedLen`) - /// Storage: `ImOnline::ReceivedHeartbeats` (r:1 w:1) - /// Proof: `ImOnline::ReceivedHeartbeats` (`max_values`: None, `max_size`: Some(25), added: 2500, mode: `MaxEncodedLen`) - /// The range of component `k` is `[1, 1000]`. - fn validate_unsigned_and_then_heartbeat(k: u32, ) -> Weight { + /// The range of component `k` is `[1, 10000]`. + fn authorize_heartbeat(_k: u32, ) -> Weight { // Proof Size summary in bytes: - // Measured: `6 + k * (32 ±0)` - // Estimated: `321487 + k * (32 ±0)` - // Minimum execution time: 55_456_000 picoseconds. - Weight::from_parts(71_841_451, 321487) - // Standard Error: 561 - .saturating_add(Weight::from_parts(43_801, 0).saturating_mul(k.into())) - .saturating_add(T::DbWeight::get().reads(4_u64)) - .saturating_add(T::DbWeight::get().writes(1_u64)) - .saturating_add(Weight::from_parts(0, 32).saturating_mul(k.into())) + // Measured: `4 + k * (32 ±0)` + // Estimated: `321494` + // Minimum execution time: 46_000_000 picoseconds. + Weight::from_parts(135_000_000, 321494) + .saturating_add(T::DbWeight::get().reads(3_u64)) } } // For backwards compatibility and tests. impl WeightInfo for () { + /// Storage: `Session::CurrentIndex` (r:1 w:0) + /// Proof: `Session::CurrentIndex` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + /// Storage: `ImOnline::ReceivedHeartbeats` (r:1 w:1) + /// Proof: `ImOnline::ReceivedHeartbeats` (`max_values`: None, `max_size`: Some(25), added: 2500, mode: `MaxEncodedLen`) + /// Storage: `ImOnline::Keys` (r:1 w:0) + /// Proof: `ImOnline::Keys` (`max_values`: Some(1), `max_size`: Some(320002), added: 320497, mode: `MaxEncodedLen`) + /// The range of component `k` is `[1, 10000]`. + fn heartbeat(_k: u32, ) -> Weight { + // Proof Size summary in bytes: + // Measured: `4 + k * (32 ±0)` + // Estimated: `321494` + // Minimum execution time: 11_000_000 picoseconds. + Weight::from_parts(220_000_000, 321494) + .saturating_add(RocksDbWeight::get().reads(3_u64)) + .saturating_add(RocksDbWeight::get().writes(1_u64)) + } /// Storage: `Session::Validators` (r:1 w:0) /// Proof: `Session::Validators` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) /// Storage: `Session::CurrentIndex` (r:1 w:0) /// Proof: `Session::CurrentIndex` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) /// Storage: `ImOnline::Keys` (r:1 w:0) /// Proof: `ImOnline::Keys` (`max_values`: Some(1), `max_size`: Some(320002), added: 320497, mode: `MaxEncodedLen`) - /// Storage: `ImOnline::ReceivedHeartbeats` (r:1 w:1) - /// Proof: `ImOnline::ReceivedHeartbeats` (`max_values`: None, `max_size`: Some(25), added: 2500, mode: `MaxEncodedLen`) - /// The range of component `k` is `[1, 1000]`. - fn validate_unsigned_and_then_heartbeat(k: u32, ) -> Weight { + /// The range of component `k` is `[1, 10000]`. + fn authorize_heartbeat(_k: u32, ) -> Weight { // Proof Size summary in bytes: - // Measured: `6 + k * (32 ±0)` - // Estimated: `321487 + k * (32 ±0)` - // Minimum execution time: 55_456_000 picoseconds. - Weight::from_parts(71_841_451, 321487) - // Standard Error: 561 - .saturating_add(Weight::from_parts(43_801, 0).saturating_mul(k.into())) - .saturating_add(RocksDbWeight::get().reads(4_u64)) - .saturating_add(RocksDbWeight::get().writes(1_u64)) - .saturating_add(Weight::from_parts(0, 32).saturating_mul(k.into())) + // Measured: `4 + k * (32 ±0)` + // Estimated: `321494` + // Minimum execution time: 46_000_000 picoseconds. + Weight::from_parts(135_000_000, 321494) + .saturating_add(RocksDbWeight::get().reads(3_u64)) } } diff --git a/substrate/frame/offences/benchmarking/src/mock.rs b/substrate/frame/offences/benchmarking/src/mock.rs index 22818db1cca4b..0b86c4664d1b6 100644 --- a/substrate/frame/offences/benchmarking/src/mock.rs +++ b/substrate/frame/offences/benchmarking/src/mock.rs @@ -180,19 +180,34 @@ where type RuntimeCall = RuntimeCall; } -impl frame_system::offchain::CreateBare for Test +impl frame_system::offchain::CreateTransaction for Test where RuntimeCall: From, { - fn create_bare(call: Self::RuntimeCall) -> Self::Extrinsic { - UncheckedExtrinsic::new_bare(call) + type Extension = (frame_system::AuthorizeCall,); + fn create_transaction(call: RuntimeCall, extension: Self::Extension) -> UncheckedExtrinsic { + UncheckedExtrinsic::new_transaction(call, extension) + } +} + +impl frame_system::offchain::CreateAuthorizedTransaction for Test +where + RuntimeCall: From, +{ + fn create_extension() -> Self::Extension { + (frame_system::AuthorizeCall::::new(),) } } impl crate::Config for Test {} pub type Block = sp_runtime::generic::Block; -pub type UncheckedExtrinsic = sp_runtime::generic::UncheckedExtrinsic; +pub type UncheckedExtrinsic = sp_runtime::generic::UncheckedExtrinsic< + u32, + RuntimeCall, + u64, + (frame_system::AuthorizeCall,), +>; frame_support::construct_runtime!( pub enum Test @@ -201,7 +216,7 @@ frame_support::construct_runtime!( Balances: pallet_balances, Staking: pallet_staking, Session: pallet_session, - ImOnline: pallet_im_online::{Pallet, Call, Storage, Event, ValidateUnsigned, Config}, + ImOnline: pallet_im_online::{Pallet, Call, Storage, Event, Config}, Offences: pallet_offences::{Pallet, Storage, Event}, Historical: pallet_session_historical::{Pallet, Event}, } From 65abd3a87ebb802af320a3b6f5d9634caae29a57 Mon Sep 17 00:00:00 2001 From: Rodrigo Quelhas Date: Tue, 3 Mar 2026 09:37:12 +0000 Subject: [PATCH 2/3] fix prdoc name --- prdoc/{pr_11234.prdoc => pr_11235.prdoc} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename prdoc/{pr_11234.prdoc => pr_11235.prdoc} (100%) diff --git a/prdoc/pr_11234.prdoc b/prdoc/pr_11235.prdoc similarity index 100% rename from prdoc/pr_11234.prdoc rename to prdoc/pr_11235.prdoc From 1ed6f34c5f1c6d8eb9ca49fbabe2457ad9caf4df Mon Sep 17 00:00:00 2001 From: Rodrigo Quelhas Date: Sat, 28 Mar 2026 08:31:43 +0000 Subject: [PATCH 3/3] move heartbeat authorization logic to a separate method --- substrate/frame/im-online/src/lib.rs | 101 ++++++++++++++------------- 1 file changed, 54 insertions(+), 47 deletions(-) diff --git a/substrate/frame/im-online/src/lib.rs b/substrate/frame/im-online/src/lib.rs index 3d1632094b601..0dbcbfa1063c1 100644 --- a/substrate/frame/im-online/src/lib.rs +++ b/substrate/frame/im-online/src/lib.rs @@ -396,53 +396,7 @@ pub mod pallet { #[pallet::weight_of_authorize(::WeightInfo::authorize_heartbeat( heartbeat.validators_len, ))] - #[pallet::authorize(|_source: TransactionSource, - heartbeat: &Heartbeat>, - signature: &::Signature, - | -> TransactionValidityWithRefund { - if Pallet::::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::::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::::try_into( - T::NextSessionRotation::average_session_length() / 2u32.into(), - ) - .unwrap_or(64_u64), - ) - .propagate(true) - .build() - .map(|v| (v, Weight::zero())) - })] + #[pallet::authorize(Self::authorize_heartbeat_call)] pub fn heartbeat( origin: OriginFor, heartbeat: Heartbeat>, @@ -729,6 +683,59 @@ impl Pallet { } } + /// 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>, + signature: &::Signature, + ) -> TransactionValidityWithRefund { + if Pallet::::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::::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::::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) { let bounded_keys = WeakBoundedVec::<_, T::MaxKeys>::try_from(keys)