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

Add block provider to core-fellowship #6978

Open
wants to merge 35 commits into
base: master
Choose a base branch
from

Conversation

PolkadotDom
Copy link
Contributor

@PolkadotDom PolkadotDom commented Dec 21, 2024

Following #3617, core fellowship and related code is to be made async backing friendly. This PR adds the block number provider config parameter to pallet-core-fellowship.

In addition it

  • Adds the migration code for those teams who want to transition and need it
  • Applies the migration on Westend
  • Shuffles some pre-existing core-fellowship migration code for ergonomics
  • Adds a bound to the BlockNumberProvider sp_runtime trait
  • Fixes a couple spelling & syntax issues

TODO:
Once Westend is updated I will write the migration for polkadot collectives.

Notes:
@xlc This will be my first migration and overall first PR with a bit more frame complexity, so please review carefully!
I've tested the migration for Westend using try-runtime. Unsure what the standard process is outside of that if any.
Also, the migration assumes consistent block times for westend and westend collectives, which is not the case, but I imagine this is not so much a concern and will largely be ameliorated when writing for polkadot collectives. Lastly, not confident in the runtime spec_version bump, that was a bit opaque to me, if you want to take a look.

@gupnik Tracking list

Copy link
Contributor

Sorry, only members of the organization paritytech members can run commands.

Copy link
Contributor

@gui1117 gui1117 left a comment

Choose a reason for hiding this comment

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

I still have to review the migration

substrate/primitives/runtime/Cargo.toml Outdated Show resolved Hide resolved
prdoc/pr_6978.prdoc Show resolved Hide resolved
substrate/frame/core-fellowship/src/migration.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@gui1117 gui1117 left a comment

Choose a reason for hiding this comment

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

Looks good to me, once the comments are resolved

@paritytech-review-bot paritytech-review-bot bot requested a review from a team January 7, 2025 19:59
@PolkadotDom PolkadotDom requested a review from gui1117 January 8, 2025 14:57
@PolkadotDom
Copy link
Contributor Author

PolkadotDom commented Jan 9, 2025

EDIT: Okay should be good to re-review now 🙏

@gui1117 gui1117 added the T2-pallets This PR/Issue is related to a particular pallet. label Jan 25, 2025
Copy link
Contributor

@gui1117 gui1117 left a comment

Choose a reason for hiding this comment

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

Looks good to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T2-pallets This PR/Issue is related to a particular pallet.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants