Skip to content

Commit

Permalink
im-online removal final cleanup (#3902)
Browse files Browse the repository at this point in the history 
Rejoice! Rejoice! The story is nearly over.

This PR removes stale migrations, auxiliary structures, and package
dependencies, thus making Rococo and Westend totally free from any
`im-online`-related stuff.

`im-online` still stays a part of the Substrate node and its runtime:
https://github.com/paritytech/polkadot-sdk/blob/0d9324847391e902bb42f84f0e76096b1f764efe/substrate/bin/node/runtime/src/lib.rs#L2276-L2277
I'm not sure if it makes sense to remove it from there considering that
we're not removing `im-online` from FRAME. Please share your opinion.
  • Loading branch information
@s0me0ne-unkn0wn @Ank4n
s0me0ne-unkn0wn authored and Ank4n committed Apr 9, 2024
1 parent be8bd2d commit 5e3d97e
Show file tree
Hide file tree
Showing 7 changed files with 2 additions and 291 deletions.
4 changes: 0 additions & 4 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions cumulus/test/service/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ pallet-timestamp = { path = "../../../substrate/frame/timestamp" }
futures = "0.3.28"
portpicker = "0.1.1"
rococo-parachain-runtime = { path = "../../parachains/runtimes/testing/rococo-parachain" }
pallet-im-online = { path = "../../../substrate/frame/im-online" }
sp-consensus-grandpa = { path = "../../../substrate/primitives/consensus/grandpa" }
sp-authority-discovery = { path = "../../../substrate/primitives/authority-discovery" }
cumulus-test-client = { path = "../client" }
Expand All @@ -106,7 +105,6 @@ runtime-benchmarks = [
"cumulus-primitives-core/runtime-benchmarks",
"cumulus-test-client/runtime-benchmarks",
"frame-system/runtime-benchmarks",
"pallet-im-online/runtime-benchmarks",
"pallet-timestamp/runtime-benchmarks",
"parachains-common/runtime-benchmarks",
"polkadot-cli/runtime-benchmarks",
Expand Down
3 changes: 0 additions & 3 deletions polkadot/node/service/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ sp-version = { path = "../../../substrate/primitives/version" }

# Substrate Pallets
pallet-babe = { path = "../../../substrate/frame/babe" }
pallet-im-online = { path = "../../../substrate/frame/im-online" }
pallet-staking = { path = "../../../substrate/frame/staking" }
pallet-transaction-payment-rpc-runtime-api = { path = "../../../substrate/frame/transaction-payment/rpc/runtime-api" }
frame-system = { path = "../../../substrate/frame/system" }
Expand Down Expand Up @@ -197,7 +196,6 @@ runtime-benchmarks = [
"frame-support/runtime-benchmarks",
"frame-system/runtime-benchmarks",
"pallet-babe/runtime-benchmarks",
"pallet-im-online/runtime-benchmarks",
"pallet-staking/runtime-benchmarks",
"polkadot-parachain-primitives/runtime-benchmarks",
"polkadot-primitives/runtime-benchmarks",
Expand All @@ -213,7 +211,6 @@ try-runtime = [
"frame-support/try-runtime",
"frame-system/try-runtime",
"pallet-babe/try-runtime",
"pallet-im-online/try-runtime",
"pallet-staking/try-runtime",
"pallet-transaction-payment/try-runtime",
"polkadot-runtime-parachains/try-runtime",
Expand Down
4 changes: 0 additions & 4 deletions polkadot/runtime/rococo/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ pallet-asset-rate = { path = "../../../substrate/frame/asset-rate", default-feat
frame-executive = { path = "../../../substrate/frame/executive", default-features = false }
pallet-grandpa = { path = "../../../substrate/frame/grandpa", default-features = false }
pallet-identity = { path = "../../../substrate/frame/identity", default-features = false }
pallet-im-online = { path = "../../../substrate/frame/im-online", default-features = false }
pallet-indices = { path = "../../../substrate/frame/indices", default-features = false }
pallet-membership = { path = "../../../substrate/frame/membership", default-features = false }
pallet-message-queue = { path = "../../../substrate/frame/message-queue", default-features = false }
Expand Down Expand Up @@ -153,7 +152,6 @@ std = [
"pallet-elections-phragmen/std",
"pallet-grandpa/std",
"pallet-identity/std",
"pallet-im-online/std",
"pallet-indices/std",
"pallet-membership/std",
"pallet-message-queue/std",
Expand Down Expand Up @@ -228,7 +226,6 @@ runtime-benchmarks = [
"pallet-elections-phragmen/runtime-benchmarks",
"pallet-grandpa/runtime-benchmarks",
"pallet-identity/runtime-benchmarks",
"pallet-im-online/runtime-benchmarks",
"pallet-indices/runtime-benchmarks",
"pallet-membership/runtime-benchmarks",
"pallet-message-queue/runtime-benchmarks",
Expand Down Expand Up @@ -284,7 +281,6 @@ try-runtime = [
"pallet-elections-phragmen/try-runtime",
"pallet-grandpa/try-runtime",
"pallet-identity/try-runtime",
"pallet-im-online/try-runtime",
"pallet-indices/try-runtime",
"pallet-membership/try-runtime",
"pallet-message-queue/try-runtime",
Expand Down
137 changes: 1 addition & 136 deletions polkadot/runtime/rococo/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,7 @@ use sp_runtime::{
IdentityLookup, Keccak256, OpaqueKeys, SaturatedConversion, Verify,
},
transaction_validity::{TransactionPriority, TransactionSource, TransactionValidity},
ApplyExtrinsicResult, BoundToRuntimeAppPublic, FixedU128, KeyTypeId, Perbill, Percent, Permill,
RuntimeAppPublic, RuntimeDebug,
ApplyExtrinsicResult, FixedU128, KeyTypeId, Perbill, Percent, Permill, RuntimeDebug,
};
use sp_staking::SessionIndex;
#[cfg(any(feature = "std", test))]
Expand Down Expand Up @@ -347,46 +346,6 @@ impl pallet_authorship::Config for Runtime {
type EventHandler = ();
}

#[derive(Clone, Debug, PartialEq, Eq, Encode, Decode)]
pub struct OldSessionKeys {
pub grandpa: <Grandpa as BoundToRuntimeAppPublic>::Public,
pub babe: <Babe as BoundToRuntimeAppPublic>::Public,
pub im_online: pallet_im_online::sr25519::AuthorityId,
pub para_validator: <Initializer as BoundToRuntimeAppPublic>::Public,
pub para_assignment: <ParaSessionInfo as BoundToRuntimeAppPublic>::Public,
pub authority_discovery: <AuthorityDiscovery as BoundToRuntimeAppPublic>::Public,
pub beefy: <Beefy as BoundToRuntimeAppPublic>::Public,
}

impl OpaqueKeys for OldSessionKeys {
type KeyTypeIdProviders = ();
fn key_ids() -> &'static [KeyTypeId] {
&[
<<Grandpa as BoundToRuntimeAppPublic>::Public>::ID,
<<Babe as BoundToRuntimeAppPublic>::Public>::ID,
sp_core::crypto::key_types::IM_ONLINE,
<<Initializer as BoundToRuntimeAppPublic>::Public>::ID,
<<ParaSessionInfo as BoundToRuntimeAppPublic>::Public>::ID,
<<AuthorityDiscovery as BoundToRuntimeAppPublic>::Public>::ID,
<<Beefy as BoundToRuntimeAppPublic>::Public>::ID,
]
}
fn get_raw(&self, i: KeyTypeId) -> &[u8] {
match i {
<<Grandpa as BoundToRuntimeAppPublic>::Public>::ID => self.grandpa.as_ref(),
<<Babe as BoundToRuntimeAppPublic>::Public>::ID => self.babe.as_ref(),
sp_core::crypto::key_types::IM_ONLINE => self.im_online.as_ref(),
<<Initializer as BoundToRuntimeAppPublic>::Public>::ID => self.para_validator.as_ref(),
<<ParaSessionInfo as BoundToRuntimeAppPublic>::Public>::ID =>
self.para_assignment.as_ref(),
<<AuthorityDiscovery as BoundToRuntimeAppPublic>::Public>::ID =>
self.authority_discovery.as_ref(),
<<Beefy as BoundToRuntimeAppPublic>::Public>::ID => self.beefy.as_ref(),
_ => &[],
}
}
}

impl_opaque_keys! {
pub struct SessionKeys {
pub grandpa: Grandpa,
Expand All @@ -398,18 +357,6 @@ impl_opaque_keys! {
}
}

// remove this when removing `OldSessionKeys`
fn transform_session_keys(_val: AccountId, old: OldSessionKeys) -> SessionKeys {
SessionKeys {
grandpa: old.grandpa,
babe: old.babe,
para_validator: old.para_validator,
para_assignment: old.para_assignment,
authority_discovery: old.authority_discovery,
beefy: old.beefy,
}
}

/// Special `ValidatorIdOf` implementation that is just returning the input as result.
pub struct ValidatorIdOf;
impl sp_runtime::traits::Convert<AccountId, Option<AccountId>> for ValidatorIdOf {
Expand Down Expand Up @@ -1486,8 +1433,6 @@ pub mod migrations {

use frame_support::traits::LockIdentifier;
use frame_system::pallet_prelude::BlockNumberFor;
#[cfg(feature = "try-runtime")]
use sp_core::crypto::ByteArray;

pub struct GetLegacyLeaseImpl;
impl coretime::migration::GetLegacyLease<BlockNumber> for GetLegacyLeaseImpl {
Expand All @@ -1514,7 +1459,6 @@ pub mod migrations {
pub const PhragmenElectionPalletName: &'static str = "PhragmenElection";
pub const TechnicalMembershipPalletName: &'static str = "TechnicalMembership";
pub const TipsPalletName: &'static str = "Tips";
pub const ImOnlinePalletName: &'static str = "ImOnline";
pub const PhragmenElectionPalletId: LockIdentifier = *b"phrelect";
}

Expand Down Expand Up @@ -1551,79 +1495,6 @@ pub mod migrations {
type PalletName = TipsPalletName;
}

/// Upgrade Session keys to exclude `ImOnline` key.
/// When this is removed, should also remove `OldSessionKeys`.
pub struct UpgradeSessionKeys;
const UPGRADE_SESSION_KEYS_FROM_SPEC: u32 = 104000;

impl frame_support::traits::OnRuntimeUpgrade for UpgradeSessionKeys {
#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<sp_std::vec::Vec<u8>, sp_runtime::TryRuntimeError> {
if System::last_runtime_upgrade_spec_version() > UPGRADE_SESSION_KEYS_FROM_SPEC {
log::warn!(target: "runtime::session_keys", "Skipping session keys migration pre-upgrade check due to spec version (already applied?)");
return Ok(Vec::new());
}

log::info!(target: "runtime::session_keys", "Collecting pre-upgrade session keys state");
let key_ids = SessionKeys::key_ids();
frame_support::ensure!(
key_ids.into_iter().find(|&k| *k == sp_core::crypto::key_types::IM_ONLINE) == None,
"New session keys contain the ImOnline key that should have been removed",
);
let storage_key = pallet_session::QueuedKeys::<Runtime>::hashed_key();
let mut state: Vec<u8> = Vec::new();
frame_support::storage::unhashed::get::<Vec<(ValidatorId, OldSessionKeys)>>(
&storage_key,
)
.ok_or::<sp_runtime::TryRuntimeError>("Queued keys are not available".into())?
.into_iter()
.for_each(|(id, keys)| {
state.extend_from_slice(id.as_slice());
for key_id in key_ids {
state.extend_from_slice(keys.get_raw(*key_id));
}
});
frame_support::ensure!(state.len() > 0, "Queued keys are not empty before upgrade");
Ok(state)
}

fn on_runtime_upgrade() -> Weight {
if System::last_runtime_upgrade_spec_version() > UPGRADE_SESSION_KEYS_FROM_SPEC {
log::info!("Skipping session keys upgrade: already applied");
return <Runtime as frame_system::Config>::DbWeight::get().reads(1);
}
log::trace!("Upgrading session keys");
Session::upgrade_keys::<OldSessionKeys, _>(transform_session_keys);
Perbill::from_percent(50) * BlockWeights::get().max_block
}

#[cfg(feature = "try-runtime")]
fn post_upgrade(
old_state: sp_std::vec::Vec<u8>,
) -> Result<(), sp_runtime::TryRuntimeError> {
if System::last_runtime_upgrade_spec_version() > UPGRADE_SESSION_KEYS_FROM_SPEC {
log::warn!(target: "runtime::session_keys", "Skipping session keys migration post-upgrade check due to spec version (already applied?)");
return Ok(());
}

let key_ids = SessionKeys::key_ids();
let mut new_state: Vec<u8> = Vec::new();
pallet_session::QueuedKeys::<Runtime>::get().into_iter().for_each(|(id, keys)| {
new_state.extend_from_slice(id.as_slice());
for key_id in key_ids {
new_state.extend_from_slice(keys.get_raw(*key_id));
}
});
frame_support::ensure!(new_state.len() > 0, "Queued keys are not empty after upgrade");
frame_support::ensure!(
old_state == new_state,
"Pre-upgrade and post-upgrade keys do not match!"
);
log::info!(target: "runtime::session_keys", "Session keys migrated successfully");
Ok(())
}
}

// We don't have a limit in the Relay Chain.
const IDENTITY_MIGRATION_KEY_LIMIT: u64 = u64::MAX;

Expand Down Expand Up @@ -1657,12 +1528,6 @@ pub mod migrations {
pallet_grandpa::migrations::MigrateV4ToV5<Runtime>,
parachains_configuration::migration::v10::MigrateToV10<Runtime>,

// Upgrade `SessionKeys` to exclude `ImOnline`
UpgradeSessionKeys,

// Remove `im-online` pallet on-chain storage
frame_support::migrations::RemovePallet<ImOnlinePalletName, <Runtime as frame_system::Config>::DbWeight>,

// Migrate Identity pallet for Usernames
pallet_identity::migration::versioned::V0ToV1<Runtime, IDENTITY_MIGRATION_KEY_LIMIT>,
parachains_configuration::migration::v11::MigrateToV11<Runtime>,
Expand Down
4 changes: 0 additions & 4 deletions polkadot/runtime/westend/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ pallet-election-provider-multi-phase = { path = "../../../substrate/frame/electi
pallet-fast-unstake = { path = "../../../substrate/frame/fast-unstake", default-features = false }
pallet-grandpa = { path = "../../../substrate/frame/grandpa", default-features = false }
pallet-identity = { path = "../../../substrate/frame/identity", default-features = false }
pallet-im-online = { path = "../../../substrate/frame/im-online", default-features = false }
pallet-indices = { path = "../../../substrate/frame/indices", default-features = false }
pallet-membership = { path = "../../../substrate/frame/membership", default-features = false }
pallet-message-queue = { path = "../../../substrate/frame/message-queue", default-features = false }
Expand Down Expand Up @@ -169,7 +168,6 @@ std = [
"pallet-fast-unstake/std",
"pallet-grandpa/std",
"pallet-identity/std",
"pallet-im-online/std",
"pallet-indices/std",
"pallet-membership/std",
"pallet-message-queue/std",
Expand Down Expand Up @@ -254,7 +252,6 @@ runtime-benchmarks = [
"pallet-fast-unstake/runtime-benchmarks",
"pallet-grandpa/runtime-benchmarks",
"pallet-identity/runtime-benchmarks",
"pallet-im-online/runtime-benchmarks",
"pallet-indices/runtime-benchmarks",
"pallet-membership/runtime-benchmarks",
"pallet-message-queue/runtime-benchmarks",
Expand Down Expand Up @@ -314,7 +311,6 @@ try-runtime = [
"pallet-fast-unstake/try-runtime",
"pallet-grandpa/try-runtime",
"pallet-identity/try-runtime",
"pallet-im-online/try-runtime",
"pallet-indices/try-runtime",
"pallet-membership/try-runtime",
"pallet-message-queue/try-runtime",
Expand Down
Loading

0 comments on commit 5e3d97e

Please sign in to comment.