Adds BlockNumberProvider in multisig, proxy and nft pallets
#5723
Conversation
| @@ -392,7 +397,7 @@ pub mod pallet { | |||
| let announcement = Announcement { | |||
| real: real.clone(), | |||
| call_hash, | |||
| height: system::Pallet::<T>::block_number(), | |||
| height: T::BlockNumberProvider::current_block_number(), | |||
There was a problem hiding this comment.
The migration would need to convert all old block numbers into new ones. We could provide a migration for teams upgrading these pallets that assumes the old times were 12 seconds and the new ones 6 seconds since it's the relay chain?
There was a problem hiding this comment.
Couldn't you just define three consts in the migration type (e.g. here)? They would be old_block_time (12s), new_block_time (6s), and offset (to account for the different block heights).
There was a problem hiding this comment.
#5656 (comment)
^^ This would be a better strategy than migration.
| @@ -168,6 +168,9 @@ pub mod pallet { | |||
|
|
|||
| /// Weight information for extrinsics in this pallet. | |||
| type WeightInfo: WeightInfo; | |||
|
|
|||
| /// Provider for the block number. Normally this is the `frame_system` pallet. | |||
There was a problem hiding this comment.
It is quite unfortunate that we cannot add this to parachain_system and be done with it. The underlying issue is that all of our pallets rely on frame_system, not parachain_system. In other words, I pallets are written mostly with the intention to be used as a solochain, which in retrospect is not how it should have been.
TLDR, adding the type to all pallets is as good as we can do this.
wdyt about marking frame_system::block_number as deprecated and explicitly asking everyone to decide on their block number source?
| doc: | ||
| - audience: Runtime Dev | ||
| description: | | ||
| This would allow these pallets to use the relay chain block number when deployed on a parachain. |
There was a problem hiding this comment.
What this PR in fact is doing is adding the ability for pallets to specify their source of block number.
Then, you should describe in which case this change is deployed backwards compatible: RC -> RC, or Para -> Para.
Then, you should describe if otherwise (RC -> Para, Para -> RC), what migratiion should be used.
@davidk-pt is trying to introduce such a migration in #5656, I suggest you two to work together, first polish the type BlockNumberProvider, then work on a reusable migration, then move individual pallets.
Step in #3268
This would allow these pallets to use the relay chain block number when deployed on a parachain. We would need migrations if the deployed pallets are upgraded, but not sure how to plan that yet.