Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove state trie migration from kusama, add it to polkadot #170

Merged
merged 21 commits into from
Mar 17, 2024

Conversation

cheme
Copy link
Contributor

@cheme cheme commented Jan 30, 2024

This PR add the state trie migration to V1 to polkadot runtime.
The configuration is the same as the one that was run on kusama.
As in kusama this PR changes the state-trie-version to v1, so the migration declared in unreleased Have to run with the next upgrade. In case the migration is not include, the line state_version: 1 have to be roll back to state_version: 0 as warpsync will stop working until the end of the migration.

  • Does not require a CHANGELOG entry

@ggwpez
Copy link
Member

ggwpez commented Feb 5, 2024

We can dry-run this with chopsticks since it exposes a dev-rpc to mint new blocks.

@cheme
Copy link
Contributor Author

cheme commented Feb 6, 2024

We can dry-run this with chopsticks since it exposes a dev-rpc to mint new blocks.

Would be nice. For the kusama one my main worry was that the migration would consume too much weight on the block, is there a way to get the remaining weight in each block with chopstick (I do not know chopstick at all)?

@ggwpez
Copy link
Member

ggwpez commented Feb 13, 2024

No i dont think it can print anything. But you can subscribe on System::BlockWeights in PJS or so.

@cheme cheme mentioned this pull request Feb 14, 2024
11 tasks
@@ -1626,9 +1606,6 @@ construct_runtime! {
Auctions: auctions::{Pallet, Call, Storage, Event<T>} = 72,
Crowdloan: crowdloan::{Pallet, Call, Storage, Event<T>} = 73,

// State trie migration pallet, only temporary.
StateTrieMigration: pallet_state_trie_migration = 98,
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably need to write some migration to remove all the old pallet storage items? There already exists the RemovePallet migration that we can use for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes I will try to use the code we ran on westend

relay/polkadot/src/lib.rs Outdated Show resolved Hide resolved
@bkchr
Copy link
Contributor

bkchr commented Feb 14, 2024

For the kusama one my main worry was that the migration would consume too much weight on the block,

For the relay chain this isn't really a big problem, because we can also build a block that takes slightly longer.

relay/kusama/src/lib.rs Show resolved Hide resolved
@@ -1671,6 +1648,7 @@ pub mod migrations {

/// Unreleased migrations. Add new ones here:
pub type Unreleased = (
crate::clean_state_migration::CleanMigrate,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
crate::clean_state_migration::CleanMigrate,
frame_support::migrations::RemovePallet<StateTrieMigrationName, RocksDbWeight>,

@@ -2714,63 +2692,45 @@ mod remote_tests {
}
}

mod init_state_migration {
mod clean_state_migration {
Copy link
Contributor

Choose a reason for hiding this comment

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

Now you can remove all of this code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

whoa looks amazing, thx

@bkchr bkchr mentioned this pull request Feb 28, 2024
19 tasks
@ggwpez ggwpez changed the title Remove state trie migration to kusama, add it to polkadot Remove state trie migration from kusama, add it to polkadot Feb 29, 2024
@bkchr bkchr requested a review from ggwpez March 9, 2024 19:57
@bkchr
Copy link
Contributor

bkchr commented Mar 9, 2024

@kianenigma please also review this pr ;)

@bkchr
Copy link
Contributor

bkchr commented Mar 16, 2024

/merge

@fellowship-merge-bot fellowship-merge-bot bot enabled auto-merge (squash) March 16, 2024 11:06
@fellowship-merge-bot
Copy link
Contributor

Enabled auto-merge in Pull Request

Available commands
  • /merge: Enables auto-merge for Pull Request
  • /merge cancel: Cancels auto-merge for Pull Request
  • /merge help: Shows this menu

For more information see the documentation

@fellowship-merge-bot fellowship-merge-bot bot merged commit 360581f into polkadot-fellows:main Mar 17, 2024
29 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants