Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Remove state migration from westend runtime. #6737

Merged
merged 10 commits into from
Feb 22, 2023

Conversation

cheme
Copy link
Contributor

@cheme cheme commented Feb 17, 2023

State migration did pass on westend and can be remove from the runtime.

Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pallet stored data? We should write some cleanup function.

@cheme
Copy link
Contributor Author

cheme commented Feb 17, 2023

Oh, did not think about it 👍 . The data stored is small though (just the current position) so could make sense to keep it just to know it was migrated. But I think I will clean it up.

@cheme
Copy link
Contributor Author

cheme commented Feb 17, 2023

I did write a clean up function, but did not find a good way to also remove the dependency from the runtime.

@cheme
Copy link
Contributor Author

cheme commented Feb 17, 2023

I am hesitant to just hardcode the trie storage key of the storage value items (and remove directly with the storage api) and remove the module.
But then it is really not auditable code.

Comment on lines 1927 to 1929
MigrationProcess::<Runtime>::kill();
AutoLimits::<Runtime>::kill();
SignedMigrationMaxLimits::<Runtime>::kill();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use storage_alias to have the definition of them without including the pallet.

Comment on lines 1903 to 1918
struct Pallet<T>(sp_std::marker::PhantomData<T>);

impl<T> frame_support::traits::PalletInfoAccess for Pallet<T> {
fn index() -> usize {
35
}
fn name() -> &'static str {
"StateTrieMigration"
}
fn module_name() -> &'static str {
"pallet_state_trie_migration"
}
fn crate_version() -> frame_support::traits::CrateVersion {
frame_support::traits::CrateVersion { major: 4u16, minor: 0u8, patch: 0u8 }
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
struct Pallet<T>(sp_std::marker::PhantomData<T>);
impl<T> frame_support::traits::PalletInfoAccess for Pallet<T> {
fn index() -> usize {
35
}
fn name() -> &'static str {
"StateTrieMigration"
}
fn module_name() -> &'static str {
"pallet_state_trie_migration"
}
fn crate_version() -> frame_support::traits::CrateVersion {
frame_support::traits::CrateVersion { major: 4u16, minor: 0u8, patch: 0u8 }
}
}

}

#[storage_alias]
type AutoLimits<T> = StorageValue<Pallet<T>, Option<MigrationLimits>, ValueQuery>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
type AutoLimits<T> = StorageValue<Pallet<T>, Option<MigrationLimits>, ValueQuery>;
type AutoLimits = StorageValue<StateTrieMigration, Option<MigrationLimits>, ValueQuery>;

Comment on lines 1927 to 1930
type MigrationProcess<T> = StorageValue<Pallet<T>, u32, ValueQuery>;

#[storage_alias]
type SignedMigrationMaxLimits<T> = StorageValue<Pallet<T>, MigrationLimits, OptionQuery>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
type MigrationProcess<T> = StorageValue<Pallet<T>, u32, ValueQuery>;
#[storage_alias]
type SignedMigrationMaxLimits<T> = StorageValue<Pallet<T>, MigrationLimits, OptionQuery>;
type MigrationProcess = StorageValue<StateTrieMigration, u32, ValueQuery>;
#[storage_alias]
type SignedMigrationMaxLimits = StorageValue<StateTrieMigration, MigrationLimits, OptionQuery>;

@cheme cheme added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. labels Feb 17, 2023
// one to avoid the trait constraint on T.
// Since we only use `kill` it is fine.
#[storage_alias]
type MigrationProcess = StorageValue<StateTrieMigration, u32, ValueQuery>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can also just delete the pallet prefix instead of defining each alias but this also works obviously.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the tip, did not think about this approach, with 3 value only I will still keep this version.

@cheme
Copy link
Contributor Author

cheme commented Feb 22, 2023

bot merge

@paritytech-processbot paritytech-processbot bot merged commit deae047 into paritytech:master Feb 22, 2023
@ggwpez ggwpez added the E0-runtime_migration PR introduces code that might require downstream chains to run a runtime upgrade. label Feb 22, 2023
ordian added a commit that referenced this pull request Mar 16, 2023
* master: (98 commits)
  Ensure max_weight is assigned properly in AllowTopPaidExecutionFrom (#6787)
  Explicitly Handling ProvisionableData Cases (#6757)
  Companion for Substrate#12520 (#6730)
  Revert back to bare metal runners for weights generation (#6762)
  Improve XCM fuzzer (#6190)
  Corrected weight trader comment (#6752)
  clean up executed migrations (#6763)
  Remove state migration from westend runtime. (#6737)
  polkadot companion #12608 (Pools claim permissions) (#6753)
  Add Turboflakes bootnodes to Polkadot, Kusama and Westend (#6628)
  Companion for substrate#13284 (#6653)
  Companion for Substrate #13410: Introduce EnsureOrigin to democracy.propose (#6750)
  `BlockId` removal: `BlockBuilderProvider::new_block_at` (#6734)
  Companion PR for PR#13119 (#6683)
  Companion for Substrate#13411: frame/beefy: prune entries in set id session mapping (#6743)
  `BlockId` removal: refactor of runtime API (#6721)
  Fix auction bench (#6747)
  Use PVF code paired with executor params wherever possible (#6742)
  Retire `OldV1SessionInfo` (#6744)
  Companion for substrate #13121 - BEEFY Equivocations support (#6593)
  ...
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. E0-runtime_migration PR introduces code that might require downstream chains to run a runtime upgrade.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants