Conversation
…vered, then we must select para header that may be proved using this relay header
|
Merging unreviewed, noted in #1407 |
| /// It is a "mild" error, which may appear when e.g. on-demand parachains relay is used. | ||
| /// This variant must be treated as "we don't want to update parachain head value at the | ||
| /// target chain at this moment". | ||
| Unavailable, |
There was a problem hiding this comment.
Maybe NotRequested or NotNeeded would be more accurate.
There was a problem hiding this comment.
I've been looking at it as "The source client refuses to report parachain head hash at this moment." primarily. But yeah - your option also seems sensible to me :)
|
|
||
| /// Parachain head hash, available at the source (relay) chain. | ||
| #[derive(Clone, Copy, Debug)] | ||
| pub enum ParaHashAtSource { |
There was a problem hiding this comment.
Rather related to #1484 . I would make this structure more generic, for example:
pub enum ExtOption<T> { // needs better name
None,
Some(T),
Unavailable,
}
And in parachain_head() instead of having
let mut para_hash_at_source = ParaHashAtSource::None;
let mut para_header_number_at_source = None;
...
I would use ExtOption<HeaderId> in order to manage both variables at once:
let para_header_id = ExtOption::<HeaderId>::None;
...
There was a problem hiding this comment.
Yeah, seems like a good idea. But we're using some non-trivial combintations there - e.g.
para_hash_at_source = ParaHashAtSource::Unavailable;
para_header_number_at_source = None;So it shall be done with care (if done at all). Can you, please, file an issue - I could try to handle it a bit later? Thanks!
There was a problem hiding this comment.
I was already experimenting with this. If it works out and if that's ok for you, I will publish a PR later.
* Parachains source cosmetic changes - Make `ParaHashAtSource` more generic - Modify `on_chain_parachain_header` to return `HeaderId` - Shortening variable names Signed-off-by: Serban Iorga <serban@parity.io> * Change ParachainsSource::max_head_id type Change ParachainsSource::max_head_id to Arc<Mutex<NoopOption>> Signed-off-by: Serban Iorga <serban@parity.io> * code review changes
|
The comments were addressed as part of #1531 . Marking the PR as reviewed. |
* insert zero epoch index into relay sproof * fix
…vered, then we must select para header that may be proved using this relay header (paritytech#1419)
* Parachains source cosmetic changes - Make `ParaHashAtSource` more generic - Modify `on_chain_parachain_header` to return `HeaderId` - Shortening variable names Signed-off-by: Serban Iorga <serban@parity.io> * Change ParachainsSource::max_head_id type Change ParachainsSource::max_head_id to Arc<Mutex<NoopOption>> Signed-off-by: Serban Iorga <serban@parity.io> * code review changes
…vered, then we must select para header that may be proved using this relay header (paritytech#1419)
* Parachains source cosmetic changes - Make `ParaHashAtSource` more generic - Modify `on_chain_parachain_header` to return `HeaderId` - Shortening variable names Signed-off-by: Serban Iorga <serban@parity.io> * Change ParachainsSource::max_head_id type Change ParachainsSource::max_head_id to Arc<Mutex<NoopOption>> Signed-off-by: Serban Iorga <serban@parity.io> * code review changes
That's the issue I've found when working on #1418:
The solution is to always explicitly read finalized parachain head from relayed relay chain block when switching from
RelayingRelayHeadertoRelayingParaHeaderstate. A good side effect of that is that now we don't need thisheaders_map_cache, originally introduced in the #1405