Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Implementer's guide: downward messages and HRMP, take 2 #1503

Merged
merged 54 commits into from
Aug 6, 2020

Conversation

pepyakin
Copy link
Contributor

@pepyakin pepyakin commented Jul 30, 2020

Closes #1139
Subsumes #1399

This is a refined version of #1399 that uses channels for HRMP. Changes from the previous approach:

  1. HRMP uses channels now. The channels behavior mimics that of XCMP. It's important to note that the messages are removed eagerly when a channel is closed or the para being offboarded.
  2. Now we somewhat consider offboarding.
  3. DMQ now is reserved exclusively for VMP. HRMP messages are not routed through DMQ.
  4. To specify what messages were drained from the HRMP channels we introduce a hrmp_watermark into the candidate.

@pepyakin pepyakin added A3-in_progress Pull request is in progress. No review needed at this stage. I7-documentation Documentation needs fixing, improving or augmenting. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Jul 30, 2020
@pepyakin pepyakin marked this pull request as ready for review August 4, 2020 12:30
@github-actions github-actions bot added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Aug 4, 2020
/// - `prev_head`: is the previous value of `mqc_head`.
/// - `B`: is the [relay-chain] block number in which a message was appended
/// - `H(M)`: is the hash of the message being appended.
/// This value is initialized to a special value that consists of all zeroes which indicates
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this comment about the value of the field when no messages are pending?

Copy link
Contributor Author

@pepyakin pepyakin Aug 4, 2020

Choose a reason for hiding this comment

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

No, this comment refers to the initial state assigned to this field when a new fresh channel is created. One can answer a question if there are pending messages in the channel only having:

  • the mqc head
  • the receiver's watermark
  • the preimage of the latest link (to extract the block number of the latest message)

FWIW, this is similar to what we have in XCMP

@pepyakin
Copy link
Contributor Author

pepyakin commented Aug 4, 2020

Some tabs sneaked in *.md again. Changed the configuration of my editor, so I hope this won't happen again.

Copy link
Contributor

@rphmeier rphmeier left a comment

Choose a reason for hiding this comment

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

UPD(pepyakin): see #1539 where this comment is expanded

We currently maintain an invariant that everything that is backed in the relay chain satisfies the checks on outgoing HRMP messages and watermark progression.

However, it is not clear to me how the validators are meant to check those things off-chain, because there is currently no runtime API that exposes these things. I think we want to expose this data from the runtime API, but we don't need to keep it available for secondary checkers because of the invariant I mentioned at the beginning.

I'm thinking of redefining LocalValidationData:

struct LocalValidationData {
    /// Validation data that needs to be persisted for secondary checkers.
    ///
    /// These validation data are generally derived from some relay-chain state to form inputs to 
    /// the validation function, and as such need to be persisted by the availability system
    ///  to avoid dependence on availability of the relay-chain state.
    persisted: PersistedLocalValidationData,
    /// These validation data are derived from some relay-chain state to check outputs of the validation
    /// function. Since the commitments of the validation function are checked by the relay-chain,
    /// secondary checkers can rely on the invariant that the relay-chain only includes para-blocks
    /// for which these checks have already been done. As such, there is no need for this information
    /// to be persisted by the availability system. Nevertheless, we expose it so the backing validators
    /// can validate the outputs of a candidate before voting to submit it to the relay-chain and so
    /// collators can collate candidates that satisfy the criteria implied by this field.
    non_persisted: NonPersistedLocalValidationData,
}

We could even merge GlobalValidationData into this, but that can be a later PR.

@rphmeier
Copy link
Contributor

rphmeier commented Aug 5, 2020

We can defer the ValidationData refactoring to another PR (#1539). This LGTM

@rphmeier rphmeier merged commit 864fff1 into master Aug 6, 2020
@rphmeier rphmeier deleted the ser-ch-based-hrmp branch August 6, 2020 15:29
ordian added a commit that referenced this pull request Aug 6, 2020
* master:
  implement provisioner (#1473)
  Implementer's guide: downward messages and HRMP, take 2 (#1503)
  Revert "Ignore checks for companion PRs (#1455)" (#1549)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. I7-documentation Documentation needs fixing, improving or augmenting.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Guide: Downwards Messages
3 participants