Add block provider to core-fellowship #6978
Conversation
This reverts commit cdd6c92.
…re-block-provider
…re-block-provider
|
Sorry, only members of the organization paritytech members can run commands. |
| @@ -1,6 +1,6 @@ | |||
| [package] | |||
| name = "collectives-westend-runtime" | |||
| version = "3.0.0" | |||
| version = "3.1.0" | |||
There was a problem hiding this comment.
I don't know exactly the process for versioning, but AFAIK we don't bump crate versions on master, instead we provide the prdoc, and from prdoc information crate versions will be changed on release
| @@ -1,6 +1,6 @@ | |||
| [package] | |||
| name = "sp-runtime" | |||
| version = "31.0.1" | |||
| version = "32.0.0" | |||
There was a problem hiding this comment.
similarly I don't think we need to bump the version in this PR.
| This PR adds a parameter 'BlockNumberProvider' to the pallet-core-fellowship | ||
| config such that a block provider can be set for use in the pallet. This would | ||
| usually be the local system pallet or the appropriate relay chain. Previously | ||
| it defaulted to the local system pallet. |
There was a problem hiding this comment.
westend is a testnet, should we notify Runtime User that westend collective fellowship pallet migrated from using parachain block number to relay chain block number?
(More generally I wonder how easy it is for tools to differentiate between both types. Some tools have to be careful and get the correct meaning for the block number.)
| local: CoreFellowshipLocalBlockNumber, | ||
| ) -> CoreFellowshipNewBlockNumber { | ||
| let block_number = System::block_number(); | ||
| let local_duration = block_number.saturating_sub(local); |
There was a problem hiding this comment.
if local represent a future block number, then the saturation will happen.
In the trait description it is not specified that local must be past.
I guess it works in our case because the pallet don't store any future block number.
Maybe we can handle the general case with a comparison if local<block_number ....
All similar migration will make use of a similar trait, and other pallet might store a future block number.
| where | ||
| BlockNumberConverter: ConvertBlockNumber<LocalBlockNumberFor<T>, NewBlockNumberFor<T, I>>, | ||
| { | ||
| fn on_runtime_upgrade() -> frame_support::weights::Weight { |
There was a problem hiding this comment.
we could count the number of member in pre_upgrade and then ensure the number of member is same in post_upgrade
And same query if there is a value in params in pre_upgrade and then ensure there is a value in params in post_upgrade.
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
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.