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

Rename OnDemandAssignmentProvider to OnDemand on Kusama #417

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

tdimitrov
Copy link
Contributor

Extracted from #401

TODOs:

  • Test the migration
  • Ensure the migration is executed only once

@tdimitrov tdimitrov mentioned this pull request Aug 2, 2024
2 tasks
@s0me0ne-unkn0wn
Copy link
Contributor

try-runtime will enforce migration idempotency (if you add a try-runtime test, of course).

For non-storage-versioned migrations, I used this trick to make a migration idempotent: https://github.com/polkadot-fellows/runtimes/pull/178/files#diff-efa4caeb17487ecb13d8f5eb7863c3241d84afa2e73fbf25909a2ca89df0f362R1816-R1819

That needs a lot of care, but until we have paritytech/polkadot-sdk#2421 we don't have a better option AFAIK

@eskimor
Copy link
Contributor

eskimor commented Aug 8, 2024

@tdimitrov Just realized: We also should do this renaming on our testnets in polkadot-sdk.

@tdimitrov
Copy link
Contributor Author

Renaming a pallet turns out to be a tedious process:

  1. The rename triggers a hook which believes a new pallet is added and sets its onchain version to the one in the code automatically.
  2. After the version is updated the other storage migration we have (MigrateV0ToV1) will no longer be executed as it checks the onchain version which gets set to 1.

I suggest to hold off the rename until MigrateV0ToV1 is executed and optionally move the migration in polkadot-sdk so that we can rely on the version of the pallet to figure out if the migration should be applied or not instead of on a hacks like checking storage keys.

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.

3 participants