Use relay chain block number in the broker pallet instead of block number #5656
Conversation
davidk-pt
force-pushed
the
davidk/use-relay-blocknumber-in-broker
branch
from
8ae1688
to
c928b8a
Compare
davidk-pt
force-pushed
the
davidk/use-relay-blocknumber-in-broker
branch
from
3b7ba18
to
60c5662
Compare
|
this will break all the UI and indexers and I am not sure if there exists a good solution... |
|
The CI pipeline was cancelled due to failure one of the required jobs. |
Yeah, only solution that comes to mind is something like polkadot-api/polkadot-api#689 |
|
a backward compatible approach will be use relaychain block number / 2 + offset so we don’t need complicated migration and existing dapps will still be functional without changes |
Why would you use the RC block number? The offset should account for the difference in block height, but the coefficient (here, 1/2) should be w/r/t the block time. |
|
the idea is simulate 12s block time regardless local block time so the meaning of the block numbers in storage stays the same and keep UI compatible |
|
I still don't understand from that description. So you are saying that if a parachain goes 24 second between two blocks, that those would be |
| if let Some(config_record) = v3::Configuration::<T>::get() { | ||
| (config_record.interlude_length, config_record.leadin_length) | ||
| } else { | ||
| ((0 as u32).into(), (0 as u32).into()) |
There was a problem hiding this comment.
Is there any correct scenario in which this Configuration does not exist? should we not err, or panic here?
There was a problem hiding this comment.
Looking deeper into the code it seems that it is indeed possible, if configure call on the pallet is not called yet then Configuration will be none
There was a problem hiding this comment.
I see, then what you have done could make sense!
but it is also worth taking it into account: this pallet is only used in coretime chain, what is the live state of the coretime chain right now?
| log::info!(target: LOG_TARGET, "Configuration Pre-Migration: Interlude Length {:?}->{:?} Leadin Length {:?}->{:?}", interlude_length, updated_interlude_length, configuration_leadin_length, updated_leadin_length); | ||
|
|
||
| let (sale_start, sale_info_leadin_length) = | ||
| if let Some(sale_info_record) = v3::SaleInfo::<T>::get() { |
There was a problem hiding this comment.
In #[try-runtime] feature, you might as well feel free to panic.
There was a problem hiding this comment.
SaleInfo might not exist if start_sales pallet call is not called yet
Co-authored-by: Kian Paimani <[email protected]>
Co-authored-by: Kian Paimani <[email protected]>
This reverts commit 25532a2.
This reverts commit 5131629.
This reverts commit e44ad5e.
After more thorough code inspection I have left the graceful handling of |
Indeed, I was leaning a bit toward the fact that the broker pallet not really a general pallet, but rather made ONLY for the polkadot relay chain or coretime chain. But your poitn is also valid; thanks for clarifying! |
cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs
Outdated
Show resolved
Hide resolved
davidk-pt
force-pushed
the
davidk/use-relay-blocknumber-in-broker
branch
from
76a02f6
to
48d96c8
Compare
Based on #3331
Related to #3268
Implements migrations with customizable block number to relay height number translation function.
Adds block to relay height migration code for rococo and westend.