From ab14b9050a3062bb267082576d9e56127c1685ec Mon Sep 17 00:00:00 2001 From: Sergey Shulepov Date: Wed, 28 Oct 2020 17:29:14 +0100 Subject: [PATCH 01/27] HRMP: Update the impl guide --- .../src/runtime/inclusion.md | 7 ++--- .../implementers-guide/src/runtime/router.md | 28 +++++++++++++++---- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/roadmap/implementers-guide/src/runtime/inclusion.md b/roadmap/implementers-guide/src/runtime/inclusion.md index 46984e276d7d..aca0534f7c09 100644 --- a/roadmap/implementers-guide/src/runtime/inclusion.md +++ b/roadmap/implementers-guide/src/runtime/inclusion.md @@ -70,8 +70,7 @@ All failed checks should lead to an unrecoverable error making the block invalid 1. call `Router::check_upward_messages(para, commitments.upward_messages)` to check that the upward messages are valid. 1. call `Router::check_processed_downward_messages(para, commitments.processed_downward_messages)` to check that the DMQ is properly drained. 1. call `Router::check_hrmp_watermark(para, commitments.hrmp_watermark)` for each candidate to check rules of processing the HRMP watermark. - 1. check that in the commitments of each candidate the horizontal messages are sorted by ascending recipient ParaId and there is no two horizontal messages have the same recipient. - 1. using `Router::verify_outbound_hrmp(sender, commitments.horizontal_messages)` ensure that the each candidate send a valid set of horizontal messages + 1. using `Router::check_outbound_hrmp(sender, commitments.horizontal_messages)` ensure that the each candidate send a valid set of horizontal messages 1. create an entry in the `PendingAvailability` map for each backed candidate with a blank `availability_votes` bitfield. 1. create a corresponding entry in the `PendingAvailabilityCommitments` with the commitments. 1. Return a `Vec` of all scheduled cores of the list of passed assignments that a candidate was successfully backed for, sorted ascending by CoreIndex. @@ -79,9 +78,9 @@ All failed checks should lead to an unrecoverable error making the block invalid 1. If the receipt contains a code upgrade, Call `Paras::schedule_code_upgrade(para_id, code, relay_parent_number + config.validationl_upgrade_delay)`. > TODO: Note that this is safe as long as we never enact candidates where the relay parent is across a session boundary. In that case, which we should be careful to avoid with contextual execution, the configuration might have changed and the para may de-sync from the host's understanding of it. 1. call `Router::enact_upward_messages` for each backed candidate, using the [`UpwardMessage`s](../types/messages.md#upward-message) from the [`CandidateCommitments`](../types/candidate.md#candidate-commitments). - 1. call `Router::queue_outbound_hrmp` with the para id of the candidate and the list of horizontal messages taken from the commitment, - 1. call `Router::prune_hrmp` with the para id of the candiate and the candidate's `hrmp_watermark`. 1. call `Router::prune_dmq` with the para id of the candidate and the candidate's `processed_downward_messages`. + 1. call `Router::prune_hrmp` with the para id of the candiate and the candidate's `hrmp_watermark`. + 1. call `Router::queue_outbound_hrmp` with the para id of the candidate and the list of horizontal messages taken from the commitment, 1. Call `Paras::note_new_head` using the `HeadData` from the receipt and `relay_parent_number`. * `collect_pending`: diff --git a/roadmap/implementers-guide/src/runtime/router.md b/roadmap/implementers-guide/src/runtime/router.md index 4310aa711fc9..7dc927e7a02c 100644 --- a/roadmap/implementers-guide/src/runtime/router.md +++ b/roadmap/implementers-guide/src/runtime/router.md @@ -90,6 +90,8 @@ struct HrmpChannel { sender_deposit: Balance, /// The amount that the recipient supplied as a deposit when accepting opening this channel. recipient_deposit: Balance, + /// The maximum message size that could be put into the channel. + limit_message_size: u32, /// The maximum number of messages that can be pending in the channel at once. limit_used_places: u32, /// The maximum total size of the messages that can be pending in the channel at once. @@ -104,12 +106,12 @@ struct HrmpChannel { used_bytes: u32, /// A head of the Message Queue Chain for this channel. Each link in this chain has a form: /// `(prev_head, B, H(M))`, where - /// - `prev_head`: is the previous value of `mqc_head`. + /// - `prev_head`: is the previous value of `mqc_head` or zero if none. /// - `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 /// that no messages were previously added. - mqc_head: Hash, + mqc_head: Option, } ``` HRMP related storage layout @@ -144,14 +146,23 @@ HrmpCloseChannelRequests: map HrmpChannelId => Option<()>; HrmpCloseChannelRequestsList: Vec; /// The HRMP watermark associated with each para. +/// Invariant: every para should be onboarded. HrmpWatermarks: map ParaId => Option; /// HRMP channel data associated with each para. HrmpChannels: map HrmpChannelId => Option; -/// The indexes that map all senders to their recievers and vise versa. +/// Ingress/egress indexes allow to find all the senders and receivers given the opposite +/// side. I.e. +/// +/// (a) ingress index allows to find all the senders for a given recipient. +/// (b) egress index allows to find all the recipients for a given sender. +/// /// Invariants: -/// - for each ingress index entry for `P` each item `I` in the index should present in `HrmpChannels` as `(I, P)`. -/// - for each egress index entry for `P` each item `E` in the index should present in `HrmpChannels` as `(P, E)`. +/// - for each ingress index entry for `P` each item `I` in the index should present in `HrmpChannels` +/// as `(I, P)`. +/// - for each egress index entry for `P` each item `E` in the index should present in `HrmpChannels` +/// as `(P, E)`. /// - there should be no other dangling channels in `HrmpChannels`. +/// - the vectors are sorted. HrmpIngressChannelsIndex: map ParaId => Vec; HrmpEgressChannelsIndex: map ParaId => Vec; /// Storage for the messages for each channel. @@ -184,7 +195,8 @@ Candidate Acceptance Function: 1. `new_hrmp_watermark` should be either 1. equal to the context's block number 1. or in `HrmpChannelDigests` for `P` an entry with the block number should exist -* `verify_outbound_hrmp(sender: ParaId, Vec)`: +* `check_outbound_hrmp(sender: ParaId, Vec)`: + 1. Checks that horizontal messages are sorted by ascending recipient ParaId and there is no two horizontal messages have the same recipient. 1. For each horizontal message `M` with the channel `C` identified by `(sender, M.recipient)` check: 1. exists 1. `M`'s payload size doesn't exceed a preconfigured limit `C.limit_message_size` @@ -208,6 +220,10 @@ Candidate Enactment: 1. Decrement `C.used_places` 1. Decrement `C.used_bytes` by `M`'s payload size. 1. Set `HrmpWatermarks` for `P` to be equal to `new_hrmp_watermark` + > NOTE: That collecting digests can be inefficient and the time it takes grows very fast. Thanks to the aggresive + > parametrization this shouldn't be a big of a deal. + > If that becomes a problem consider introducing an extra dictionary which says at what block the given sender + > sent a message to the recipient. * `prune_dmq(P: ParaId, processed_downward_messages)`: 1. Remove the first `processed_downward_messages` from the `DownwardMessageQueues` of `P`. * `enact_upward_messages(P: ParaId, Vec)`: From c318044c9c34e1fa89acb2b1d403e78a7e268b23 Mon Sep 17 00:00:00 2001 From: Sergei Shulepov Date: Wed, 30 Sep 2020 14:45:00 +0200 Subject: [PATCH 02/27] HRMP: Incorporate the channel notifications into the guide --- .../implementers-guide/src/runtime/router.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/roadmap/implementers-guide/src/runtime/router.md b/roadmap/implementers-guide/src/runtime/router.md index 7dc927e7a02c..f559e7285e39 100644 --- a/roadmap/implementers-guide/src/runtime/router.md +++ b/roadmap/implementers-guide/src/runtime/router.md @@ -287,6 +287,12 @@ the parachain executed the message. 1. Set `limit_used_places` to `max_places` 1. Set `limit_message_size` to `max_message_size` 1. Set `limit_used_bytes` to `config.hrmp_channel_max_size` + 1. Send a downward message to `recipient` notifying about an inbound HRMP channel request. + - The DM is sent using `queue_downward_message`. + - The DM is represented by the `HrmpNewChannelOpenRequest` XCM message. + - `sender` is set to `origin`, + - `max_message_size` is set to `max_message_size`, + - `max_capacity` is set to `max_places`. * `hrmp_accept_open_channel(sender)`: 1. Check that there is an existing request between (`sender`, `origin`) in `HrmpOpenChannelRequests` 1. Check that it is not confirmed. @@ -299,12 +305,23 @@ the parachain executed the message. 1. Reserve the deposit for the `origin` according to `config.hrmp_recipient_deposit` 1. For the request in `HrmpOpenChannelRequests` identified by `(sender, P)`, set `confirmed` flag to `true`. 1. Increase `HrmpAcceptedChannelRequestCount` by 1 for `origin`. + 1. Send a downward message to `sender` notifying that the channel request was accepted. + - The DM is sent using `queue_downward_message`. + - The DM is represented by the `HrmpChannelAccepted` XCM message. + - `recipient` is set to `origin`. * `hrmp_close_channel(ch)`: 1. Check that `origin` is either `ch.sender` or `ch.recipient` 1. Check that `HrmpChannels` for `ch` exists. 1. Check that `ch` is not in the `HrmpCloseChannelRequests` set. 1. If not already there, insert a new entry `Some(())` to `HrmpCloseChannelRequests` for `ch` and append `ch` to `HrmpCloseChannelRequestsList`. + 1. Send a downward message to the opposite party notifying about the channel closing. + - The DM is sent using `queue_downward_message`. + - The DM is represented by the `HrmpChannelClosing` XCM message with: + - `initator` is set to `origin`, + - `sender` is set to `ch.sender`, + - `recipient` is set to `ch.recipient`. + - The opposite party is `ch.sender` if `origin` is `ch.recipient` and `ch.recipient` if `origin` is `ch.sender`. ## Session Change From 130accb384b4e5bfd854bff2283077e7f7c70533 Mon Sep 17 00:00:00 2001 From: Sergey Shulepov Date: Wed, 28 Oct 2020 19:15:24 +0100 Subject: [PATCH 03/27] HRMP: Renaming in the impl guide --- .../implementers-guide/src/runtime/router.md | 50 ++++++++++--------- .../implementers-guide/src/types/runtime.md | 4 +- 2 files changed, 28 insertions(+), 26 deletions(-) diff --git a/roadmap/implementers-guide/src/runtime/router.md b/roadmap/implementers-guide/src/runtime/router.md index f559e7285e39..d4dce49f3bf7 100644 --- a/roadmap/implementers-guide/src/runtime/router.md +++ b/roadmap/implementers-guide/src/runtime/router.md @@ -78,10 +78,12 @@ struct HrmpOpenChannelRequest { age: SessionIndex, /// The amount that the sender supplied at the time of creation of this request. sender_deposit: Balance, + /// The maximum message size that could be put into the channel. + max_message_size: u32, /// The maximum number of messages that can be pending in the channel at once. - limit_used_places: u32, + max_capacity: u32, /// The maximum total size of the messages that can be pending in the channel at once. - limit_used_bytes: u32, + max_total_size: u32, } /// A metadata of an HRMP channel. @@ -93,17 +95,17 @@ struct HrmpChannel { /// The maximum message size that could be put into the channel. limit_message_size: u32, /// The maximum number of messages that can be pending in the channel at once. - limit_used_places: u32, + max_capacity: u32, /// The maximum total size of the messages that can be pending in the channel at once. - limit_used_bytes: u32, + max_total_size: u32, /// The maximum message size that could be put into the channel. - limit_message_size: u32, + max_message_size: u32, /// The current number of messages pending in the channel. - /// Invariant: should be less or equal to `limit_used_places`. - used_places: u32, + /// Invariant: should be less or equal to `max_capacity`. + msg_count: u32, /// The total size in bytes of all message payloads in the channel. - /// Invariant: should be less or equal to `limit_used_bytes`. - used_bytes: u32, + /// Invariant: should be less or equal to `max_total_size`. + total_size: u32, /// A head of the Message Queue Chain for this channel. Each link in this chain has a form: /// `(prev_head, B, H(M))`, where /// - `prev_head`: is the previous value of `mqc_head` or zero if none. @@ -199,9 +201,9 @@ Candidate Acceptance Function: 1. Checks that horizontal messages are sorted by ascending recipient ParaId and there is no two horizontal messages have the same recipient. 1. For each horizontal message `M` with the channel `C` identified by `(sender, M.recipient)` check: 1. exists - 1. `M`'s payload size doesn't exceed a preconfigured limit `C.limit_message_size` - 1. `M`'s payload size summed with the `C.used_bytes` doesn't exceed a preconfigured limit `C.limit_used_bytes`. - 1. `C.used_places + 1` doesn't exceed a preconfigured limit `C.limit_used_places`. + 1. `M`'s payload size doesn't exceed a preconfigured limit `C.max_message_size` + 1. `M`'s payload size summed with the `C.total_size` doesn't exceed a preconfigured limit `C.max_total_size`. + 1. `C.msg_count + 1` doesn't exceed a preconfigured limit `C.max_capacity`. Candidate Enactment: @@ -209,16 +211,16 @@ Candidate Enactment: 1. For each horizontal message `HM` with the channel `C` identified by `(sender, HM.recipient)`: 1. Append `HM` into `HrmpChannelContents` that corresponds to `C` with `sent_at` equals to the current block number. 1. Locate or create an entry in `HrmpChannelDigests` for `HM.recipient` and append `sender` into the entry's list. - 1. Increment `C.used_places` - 1. Increment `C.used_bytes` by `HM`'s payload size + 1. Increment `C.msg_count` + 1. Increment `C.total_size` by `HM`'s payload size 1. Append a new link to the MQC and save the new head in `C.mqc_head`. Note that the current block number as of enactment is used for the link. * `prune_hrmp(recipient, new_hrmp_watermark)`: 1. From `HrmpChannelDigests` for `recipient` remove all entries up to an entry with block number equal to `new_hrmp_watermark`. 1. From the removed digests construct a set of paras that sent new messages within the interval between the old and new watermarks. 1. For each channel `C` identified by `(sender, recipient)` for each `sender` coming from the set, prune messages up to the `new_hrmp_watermark`. 1. For each pruned message `M` from channel `C`: - 1. Decrement `C.used_places` - 1. Decrement `C.used_bytes` by `M`'s payload size. + 1. Decrement `C.msg_count` + 1. Decrement `C.total_size` by `M`'s payload size. 1. Set `HrmpWatermarks` for `P` to be equal to `new_hrmp_watermark` > NOTE: That collecting digests can be inefficient and the time it takes grows very fast. Thanks to the aggresive > parametrization this shouldn't be a big of a deal. @@ -267,10 +269,10 @@ The following entry-points are meant to be used for HRMP channel management. Those entry-points are meant to be called from a parachain. `origin` is defined as the `ParaId` of the parachain executed the message. -* `hrmp_init_open_channel(recipient, max_places, max_message_size)`: +* `hrmp_init_open_channel(recipient, proposed_max_capacity, proposed_max_message_size)`: 1. Check that the `origin` is not `recipient`. - 1. Check that `max_places` is less or equal to `config.hrmp_channel_max_places` and greater than zero. - 1. Check that `max_message_size` is less or equal to `config.hrmp_channel_max_message_size` and greater than zero. + 1. Check that `proposed_max_capacity` is less or equal to `config.hrmp_channel_max_capacity` and greater than zero. + 1. Check that `proposed_max_message_size` is less or equal to `config.hrmp_channel_max_message_size` and greater than zero. 1. Check that `recipient` is a valid para. 1. Check that there is no existing channel for `(origin, recipient)` in `HrmpChannels`. 1. Check that there is no existing open channel request (`origin`, `recipient`) in `HrmpOpenChannelRequests`. @@ -284,15 +286,15 @@ the parachain executed the message. 1. Append `(origin, recipient)` to `HrmpOpenChannelRequestsList`. 1. Add a new entry to `HrmpOpenChannelRequests` for `(origin, recipient)` 1. Set `sender_deposit` to `config.hrmp_sender_deposit` - 1. Set `limit_used_places` to `max_places` - 1. Set `limit_message_size` to `max_message_size` - 1. Set `limit_used_bytes` to `config.hrmp_channel_max_size` + 1. Set `max_capacity` to `proposed_max_capacity` + 1. Set `max_message_size` to `proposed_max_message_size` + 1. Set `max_total_size` to `config.hrmp_channel_max_total_size` 1. Send a downward message to `recipient` notifying about an inbound HRMP channel request. - The DM is sent using `queue_downward_message`. - The DM is represented by the `HrmpNewChannelOpenRequest` XCM message. - `sender` is set to `origin`, - - `max_message_size` is set to `max_message_size`, - - `max_capacity` is set to `max_places`. + - `max_message_size` is set to `proposed_max_message_size`, + - `max_capacity` is set to `proposed_max_capacity`. * `hrmp_accept_open_channel(sender)`: 1. Check that there is an existing request between (`sender`, `origin`) in `HrmpOpenChannelRequests` 1. Check that it is not confirmed. diff --git a/roadmap/implementers-guide/src/types/runtime.md b/roadmap/implementers-guide/src/types/runtime.md index 10c342cd9670..fd9398b7beb4 100644 --- a/roadmap/implementers-guide/src/types/runtime.md +++ b/roadmap/implementers-guide/src/types/runtime.md @@ -77,9 +77,9 @@ struct HostConfiguration { /// The deposit that the recipient should provide for accepting opening an HRMP channel. pub hrmp_recipient_deposit: u32, /// The maximum number of messages allowed in an HRMP channel at once. - pub hrmp_channel_max_places: u32, + pub hrmp_channel_max_capacity: u32, /// The maximum total size of messages in bytes allowed in an HRMP channel at once. - pub hrmp_channel_max_size: u32, + pub hrmp_channel_max_total_size: u32, /// The maximum number of inbound HRMP channels a parachain is allowed to accept. pub hrmp_max_parachain_inbound_channels: u32, /// The maximum number of inbound HRMP channels a parathread is allowed to accept. From e1e4eb023b0349a64ca550f938b11c3d1ed85461 Mon Sep 17 00:00:00 2001 From: Sergey Shulepov Date: Wed, 28 Oct 2020 20:04:02 +0100 Subject: [PATCH 04/27] HRMP: Constrain the maximum number of HRMP messages per candidate This commit addresses the HRMP part of https://github.com/paritytech/polkadot/issues/1869 --- roadmap/implementers-guide/src/runtime/router.md | 1 + roadmap/implementers-guide/src/types/runtime.md | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/roadmap/implementers-guide/src/runtime/router.md b/roadmap/implementers-guide/src/runtime/router.md index d4dce49f3bf7..15cd37258e0c 100644 --- a/roadmap/implementers-guide/src/runtime/router.md +++ b/roadmap/implementers-guide/src/runtime/router.md @@ -198,6 +198,7 @@ Candidate Acceptance Function: 1. equal to the context's block number 1. or in `HrmpChannelDigests` for `P` an entry with the block number should exist * `check_outbound_hrmp(sender: ParaId, Vec)`: + 1. Checks that there are at most `config.hrmp_max_message_num_per_candidate` messages. 1. Checks that horizontal messages are sorted by ascending recipient ParaId and there is no two horizontal messages have the same recipient. 1. For each horizontal message `M` with the channel `C` identified by `(sender, M.recipient)` check: 1. exists diff --git a/roadmap/implementers-guide/src/types/runtime.md b/roadmap/implementers-guide/src/types/runtime.md index fd9398b7beb4..70c7237e1c6a 100644 --- a/roadmap/implementers-guide/src/types/runtime.md +++ b/roadmap/implementers-guide/src/types/runtime.md @@ -85,10 +85,16 @@ struct HostConfiguration { /// The maximum number of inbound HRMP channels a parathread is allowed to accept. pub hrmp_max_parathread_inbound_channels: u32, /// The maximum size of a message that could ever be put into an HRMP channel. + /// + /// This parameter affects the upper bound of size of `CandidateCommitments`. pub hrmp_channel_max_message_size: u32, /// The maximum number of outbound HRMP channels a parachain is allowed to open. pub hrmp_max_parachain_outbound_channels: u32, /// The maximum number of outbound HRMP channels a parathread is allowed to open. pub hrmp_max_parathread_outbound_channels: u32, + /// The maximum number of outbound HRMP messages can be sent by a candidate. + /// + /// This parameter affects the upper bound of size of `CandidateCommitments`. + pub hrmp_max_message_num_per_candidate: u32, } ``` From 8ef66ecb8f73db6a75cb241abac5d61f563a694f Mon Sep 17 00:00:00 2001 From: Sergei Shulepov Date: Wed, 30 Sep 2020 16:13:34 +0200 Subject: [PATCH 05/27] XCM: Introduce HRMP related message types --- xcm/src/v0/mod.rs | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/xcm/src/v0/mod.rs b/xcm/src/v0/mod.rs index d371f1e80976..67b6e4026c33 100644 --- a/xcm/src/v0/mod.rs +++ b/xcm/src/v0/mod.rs @@ -158,6 +158,51 @@ pub enum Xcm { /// /// Errors: RelayedFrom { superorigin: MultiLocation, inner: Box }, + + /// A message to notify about a new incoming HRMP channel. This message is meant to be sent by the + /// relay-chain to a para. + /// + /// - `sender`: The sender in the to-be opened channel. Also, the initiator of the channel opening. + /// - `max_message_size`: The maximum size of a message proposed by the sender. + /// - `max_capacity`: The maximum number of messages that can be queued in the channel. + /// + /// Safety: The message should originate directly from the relay-chain. + /// + /// Kind: *System Notification* + HrmpNewChannelOpenRequest { + #[codec(compact)] sender: u32, + #[codec(compact)] max_message_size: u32, + #[codec(compact)] max_capacity: u32, + }, + + /// A message to notify about that a previously sent open channel request has been accepted by + /// the recipient. That means that the channel will be opened during the next relay-chain session + /// change. This message is meant to be sent by the relay-chain to a para. + /// + /// Safety: The message should originate directly from the relay-chain. + /// + /// Kind: *System Notification* + /// + /// Errors: + HrmpChannelAccepted { + #[codec(compact)] recipient: u32, + }, + + /// A message to notify that the other party in an open channel decided to close it. In particular, + /// `inititator` is going to close the channel opened from `sender` to the `recipient`. The close + /// will be enacted at the next relay-chain session change. This message is meant to be sent by + /// the relay-chain to a para. + /// + /// Safety: The message should originate directly from the relay-chain. + /// + /// Kind: *System Notification* + /// + /// Errors: + HrmpChannelClosing { + #[codec(compact)] initiator: u32, + #[codec(compact)] sender: u32, + #[codec(compact)] recipient: u32, + }, } impl From for VersionedXcm { From 196c4a4bf247156a8e8b8d25e14b142696cddb34 Mon Sep 17 00:00:00 2001 From: Sergei Shulepov Date: Mon, 14 Sep 2020 20:01:43 +0200 Subject: [PATCH 06/27] HRMP: Data structures and plumbing --- core-primitives/src/lib.rs | 19 +++++++++++++++++++ node/collation-generation/src/lib.rs | 4 ++++ node/core/backing/src/lib.rs | 10 ++++++++++ node/core/candidate-validation/src/lib.rs | 6 ++++++ node/primitives/src/lib.rs | 8 ++++++-- parachain/src/primitives.rs | 18 ++++++++++++++++-- .../adder/src/wasm_validation.rs | 7 +++++-- primitives/src/v1.rs | 18 +++++++++++++----- 8 files changed, 79 insertions(+), 11 deletions(-) diff --git a/core-primitives/src/lib.rs b/core-primitives/src/lib.rs index 0e080fff6f7c..e29457f05b96 100644 --- a/core-primitives/src/lib.rs +++ b/core-primitives/src/lib.rs @@ -102,6 +102,25 @@ pub struct InboundDownwardMessage { pub msg: DownwardMessage, } +/// An HRMP message seen from the perspective of a recipient. +#[derive(codec::Encode, codec::Decode, Clone, sp_runtime::RuntimeDebug, PartialEq)] +pub struct InboundHrmpMessage { + /// The block number at which this message was sent. + /// Specifically, it is the block number at which the candidate that sends this message was + /// enacted. + pub sent_at: BlockNumber, + /// The message payload. + pub data: sp_std::vec::Vec, +} + +#[derive(codec::Encode, codec::Decode, Clone, sp_runtime::RuntimeDebug, PartialEq, Eq, Hash)] +pub struct OutboundHrmpMessage { + /// The para that will get this message in its downward message queue. + pub recipient: Id, + /// The message payload. + pub data: sp_std::vec::Vec, +} + /// V1 primitives. pub mod v1 { pub use super::*; diff --git a/node/collation-generation/src/lib.rs b/node/collation-generation/src/lib.rs index 1b4d978589e0..bb4b9eb8ee68 100644 --- a/node/collation-generation/src/lib.rs +++ b/node/collation-generation/src/lib.rs @@ -274,10 +274,12 @@ async fn handle_new_activations( let commitments = CandidateCommitments { upward_messages: collation.upward_messages, + horizontal_messages: collation.horizontal_messages, new_validation_code: collation.new_validation_code, head_data: collation.head_data, erasure_root, processed_downward_messages: collation.processed_downward_messages, + hrmp_watermark: collation.hrmp_watermark, }; let ccr = CandidateReceipt { @@ -382,12 +384,14 @@ mod tests { fn test_collation() -> Collation { Collation { upward_messages: Default::default(), + horizontal_messages: Default::default(), new_validation_code: Default::default(), head_data: Default::default(), proof_of_validity: PoV { block_data: BlockData(Vec::new()), }, processed_downward_messages: Default::default(), + hrmp_watermark: Default::default(), } } diff --git a/node/core/backing/src/lib.rs b/node/core/backing/src/lib.rs index 0cb1c40cdcb3..b8a9d942ab15 100644 --- a/node/core/backing/src/lib.rs +++ b/node/core/backing/src/lib.rs @@ -717,10 +717,12 @@ impl CandidateBackingJob { let commitments = CandidateCommitments { upward_messages: outputs.upward_messages, + horizontal_messages: outputs.horizontal_messages, erasure_root, new_validation_code: outputs.new_validation_code, head_data: outputs.head_data, processed_downward_messages: outputs.processed_downward_messages, + hrmp_watermark: outputs.hrmp_watermark, }; let res = match with_commitments(commitments) { @@ -1200,9 +1202,11 @@ mod tests { tx.send(Ok( ValidationResult::Valid(ValidationOutputs { head_data: expected_head_data.clone(), + horizontal_messages: Vec::new(), upward_messages: Vec::new(), new_validation_code: None, processed_downward_messages: 0, + hrmp_watermark: 0, }, test_state.validation_data.persisted), )).unwrap(); } @@ -1328,8 +1332,10 @@ mod tests { ValidationResult::Valid(ValidationOutputs { head_data: expected_head_data.clone(), upward_messages: Vec::new(), + horizontal_messages: Vec::new(), new_validation_code: None, processed_downward_messages: 0, + hrmp_watermark: 0, }, test_state.validation_data.persisted), )).unwrap(); } @@ -1477,8 +1483,10 @@ mod tests { ValidationResult::Valid(ValidationOutputs { head_data: expected_head_data.clone(), upward_messages: Vec::new(), + horizontal_messages: Vec::new(), new_validation_code: None, processed_downward_messages: 0, + hrmp_watermark: 0, }, test_state.validation_data.persisted), )).unwrap(); } @@ -1660,8 +1668,10 @@ mod tests { ValidationResult::Valid(ValidationOutputs { head_data: expected_head_data.clone(), upward_messages: Vec::new(), + horizontal_messages: Vec::new(), new_validation_code: None, processed_downward_messages: 0, + hrmp_watermark: 0, }, test_state.validation_data.persisted), )).unwrap(); } diff --git a/node/core/candidate-validation/src/lib.rs b/node/core/candidate-validation/src/lib.rs index f141319fd3da..29ceb0d33abd 100644 --- a/node/core/candidate-validation/src/lib.rs +++ b/node/core/candidate-validation/src/lib.rs @@ -489,8 +489,10 @@ fn validate_candidate_exhaustive( let outputs = ValidationOutputs { head_data: res.head_data, upward_messages: res.upward_messages, + horizontal_messages: res.horizontal_messages, new_validation_code: res.new_validation_code, processed_downward_messages: res.processed_downward_messages, + hrmp_watermark: res.hrmp_watermark, }; Ok(ValidationResult::Valid(outputs, persisted_validation_data)) } @@ -833,7 +835,9 @@ mod tests { head_data: HeadData(vec![1, 1, 1]), new_validation_code: Some(vec![2, 2, 2].into()), upward_messages: Vec::new(), + horizontal_messages: Vec::new(), processed_downward_messages: 0, + hrmp_watermark: 0, }; let v = validate_candidate_exhaustive::( @@ -848,7 +852,9 @@ mod tests { assert_matches!(v, ValidationResult::Valid(outputs, used_validation_data) => { assert_eq!(outputs.head_data, HeadData(vec![1, 1, 1])); assert_eq!(outputs.upward_messages, Vec::::new()); + assert_eq!(outputs.horizontal_messages, Vec::new()); assert_eq!(outputs.new_validation_code, Some(vec![2, 2, 2].into())); + assert_eq!(outputs.hrmp_watermark, 0); assert_eq!(used_validation_data, validation_data); }); } diff --git a/node/primitives/src/lib.rs b/node/primitives/src/lib.rs index 45affc5756a3..9368e5ad8e77 100644 --- a/node/primitives/src/lib.rs +++ b/node/primitives/src/lib.rs @@ -28,7 +28,7 @@ use polkadot_primitives::v1::{ Hash, CommittedCandidateReceipt, CandidateReceipt, CompactStatement, EncodeAs, Signed, SigningContext, ValidatorIndex, ValidatorId, UpwardMessage, ValidationCode, PersistedValidationData, ValidationData, - HeadData, PoV, CollatorPair, Id as ParaId, ValidationOutputs, CandidateHash, + HeadData, PoV, CollatorPair, Id as ParaId, OutboundHrmpMessage, ValidationOutputs, CandidateHash, }; use polkadot_statement_table::{ generic::{ @@ -252,9 +252,11 @@ impl std::convert::TryFrom for MisbehaviorReport { /// - does not contain the erasure root; that's computed at the Polkadot level, not at Cumulus /// - contains a proof of validity. #[derive(Clone, Encode, Decode)] -pub struct Collation { +pub struct Collation { /// Messages destined to be interpreted by the Relay chain itself. pub upward_messages: Vec, + /// The horizontal messages sent by the parachain. + pub horizontal_messages: Vec>, /// New validation code. pub new_validation_code: Option, /// The head-data produced as a result of execution. @@ -263,6 +265,8 @@ pub struct Collation { pub proof_of_validity: PoV, /// The number of messages processed from the DMQ. pub processed_downward_messages: u32, + /// The mark which specifies the block number up to which all inbound HRMP messages are processed. + pub hrmp_watermark: BlockNumber, } /// Configuration for the collation generator diff --git a/parachain/src/primitives.rs b/parachain/src/primitives.rs index 2e1549ba970c..c1ae971f10fe 100644 --- a/parachain/src/primitives.rs +++ b/parachain/src/primitives.rs @@ -28,7 +28,7 @@ use serde::{Serialize, Deserialize}; #[cfg(feature = "std")] use sp_core::bytes; -use polkadot_core_primitives::Hash; +use polkadot_core_primitives::{Hash, OutboundHrmpMessage}; /// Block number type used by the relay chain. pub use polkadot_core_primitives::BlockNumber as RelayChainBlockNumber; @@ -186,6 +186,16 @@ impl AccountIdConversion for Id { } } +/// A type that uniquely identifies an HRMP channel. An HRMP channel is unidirectional +#[derive(Clone, PartialEq, Eq, Encode, Decode, RuntimeDebug)] +#[cfg_attr(feature = "std", derive(Hash))] +pub struct HrmpChannelId { + /// The para that acts as the sender in this channel. + pub sender: Id, + /// The para that acts as the recipient in this channel. + pub recipient: Id, +} + /// A message from a parachain to its Relay Chain. pub type UpwardMessage = Vec; @@ -212,7 +222,7 @@ pub struct ValidationParams { } /// The result of parachain validation. -// TODO: egress and balance uploads (https://github.com/paritytech/polkadot/issues/220) +// TODO: balance uploads (https://github.com/paritytech/polkadot/issues/220) #[derive(PartialEq, Eq, Encode)] #[cfg_attr(feature = "std", derive(Debug, Decode))] pub struct ValidationResult { @@ -222,8 +232,12 @@ pub struct ValidationResult { pub new_validation_code: Option, /// Upward messages send by the Parachain. pub upward_messages: Vec, + /// Outbound horizontal messages sent by the parachain. + pub horizontal_messages: Vec>, /// Number of downward messages that were processed by the Parachain. /// /// It is expected that the Parachain processes them from first to last. pub processed_downward_messages: u32, + /// The mark which specifies the block number up to which all inbound HRMP messages are processed. + pub hrmp_watermark: RelayChainBlockNumber, } diff --git a/parachain/test-parachains/adder/src/wasm_validation.rs b/parachain/test-parachains/adder/src/wasm_validation.rs index f009b5530db6..3de7c2311e10 100644 --- a/parachain/test-parachains/adder/src/wasm_validation.rs +++ b/parachain/test-parachains/adder/src/wasm_validation.rs @@ -17,12 +17,13 @@ //! WASM validation for adder parachain. use crate::{HeadData, BlockData}; -use core::{intrinsics, panic}; +use core::panic; +use sp_std::vec::Vec; use parachain::primitives::{ValidationResult, HeadData as GenericHeadData}; use codec::{Encode, Decode}; #[no_mangle] -pub extern fn validate_block(params: *const u8, len: usize) -> u64 { +pub extern "C" fn validate_block(params: *const u8, len: usize) -> u64 { let params = unsafe { parachain::load_params(params, len) }; let parent_head = HeadData::decode(&mut ¶ms.parent_head.0[..]) .expect("invalid parent head format."); @@ -38,7 +39,9 @@ pub extern fn validate_block(params: *const u8, len: usize) -> u64 { head_data: GenericHeadData(new_head.encode()), new_validation_code: None, upward_messages: sp_std::vec::Vec::new(), + horizontal_messages: sp_std::vec::Vec::new(), processed_downward_messages: 0, + hrmp_watermark: params.relay_chain_height, } ) } diff --git a/primitives/src/v1.rs b/primitives/src/v1.rs index 6b3a2342f1d4..955f87111977 100644 --- a/primitives/src/v1.rs +++ b/primitives/src/v1.rs @@ -29,14 +29,14 @@ pub use runtime_primitives::traits::{BlakeTwo256, Hash as HashT}; // Export some core primitives. pub use polkadot_core_primitives::v1::{ - BlockNumber, Moment, Signature, AccountPublic, AccountId, AccountIndex, - ChainId, Hash, Nonce, Balance, Header, Block, BlockId, UncheckedExtrinsic, - Remark, DownwardMessage, InboundDownwardMessage, CandidateHash, + BlockNumber, Moment, Signature, AccountPublic, AccountId, AccountIndex, ChainId, Hash, Nonce, + Balance, Header, Block, BlockId, UncheckedExtrinsic, Remark, DownwardMessage, + InboundDownwardMessage, CandidateHash, InboundHrmpMessage, OutboundHrmpMessage, }; // Export some polkadot-parachain primitives pub use polkadot_parachain::primitives::{ - Id, LOWEST_USER_ID, UpwardMessage, HeadData, BlockData, ValidationCode, + Id, LOWEST_USER_ID, HrmpChannelId, UpwardMessage, HeadData, BlockData, ValidationCode, }; // Export some basic parachain primitives from v0. @@ -317,18 +317,24 @@ pub struct ValidationOutputs { pub head_data: HeadData, /// Upward messages to the relay chain. pub upward_messages: Vec, + /// The horizontal messages sent by the parachain. + pub horizontal_messages: Vec>, /// The new validation code submitted by the execution, if any. pub new_validation_code: Option, /// The number of messages processed from the DMQ. pub processed_downward_messages: u32, + /// The mark which specifies the block number up to which all inbound HRMP messages are processed. + pub hrmp_watermark: BlockNumber, } /// Commitments made in a `CandidateReceipt`. Many of these are outputs of validation. #[derive(PartialEq, Eq, Clone, Encode, Decode)] #[cfg_attr(feature = "std", derive(Debug, Default, Hash))] -pub struct CandidateCommitments { +pub struct CandidateCommitments { /// Messages destined to be interpreted by the Relay chain itself. pub upward_messages: Vec, + /// Horizontal messages sent by the parachain. + pub horizontal_messages: Vec>, /// The root of a block's erasure encoding Merkle tree. pub erasure_root: Hash, /// New validation code. @@ -337,6 +343,8 @@ pub struct CandidateCommitments { pub head_data: HeadData, /// The number of messages processed from the DMQ. pub processed_downward_messages: u32, + /// The mark which specifies the block number up to which all inbound HRMP messages are processed. + pub hrmp_watermark: N, } impl CandidateCommitments { From 71bb7a6aa432221f82229898ffa39ebac2494181 Mon Sep 17 00:00:00 2001 From: Sergei Shulepov Date: Sat, 19 Sep 2020 20:12:21 +0200 Subject: [PATCH 07/27] HRMP: Configuration --- runtime/parachains/src/configuration.rs | 182 +++++++++++++++++++++++- 1 file changed, 181 insertions(+), 1 deletion(-) diff --git a/runtime/parachains/src/configuration.rs b/runtime/parachains/src/configuration.rs index 39330407433e..49e54b2c3191 100644 --- a/runtime/parachains/src/configuration.rs +++ b/runtime/parachains/src/configuration.rs @@ -19,7 +19,7 @@ //! Configuration can change only at session boundaries and is buffered until then. use sp_std::prelude::*; -use primitives::v1::ValidatorId; +use primitives::v1::{Balance, ValidatorId}; use frame_support::{ decl_storage, decl_module, decl_error, dispatch::DispatchResult, @@ -84,6 +84,32 @@ pub struct HostConfiguration { /// /// This parameter affects the size upper bound of the `CandidateCommitments`. pub max_upward_message_num_per_candidate: u32, + /// Number of sessions after which an HRMP open channel request expires. + pub hrmp_open_request_ttl: u32, + /// The deposit that the sender should provide for opening an HRMP channel. + pub hrmp_sender_deposit: Balance, + /// The deposit that the recipient should provide for accepting opening an HRMP channel. + pub hrmp_recipient_deposit: Balance, + /// The maximum number of messages allowed in an HRMP channel at once. + pub hrmp_channel_max_capacity: u32, + /// The maximum total size of messages in bytes allowed in an HRMP channel at once. + pub hrmp_channel_max_total_size: u32, + /// The maximum number of inbound HRMP channels a parachain is allowed to accept. + pub hrmp_max_parachain_inbound_channels: u32, + /// The maximum number of inbound HRMP channels a parathread is allowed to accept. + pub hrmp_max_parathread_inbound_channels: u32, + /// The maximum size of a message that could ever be put into an HRMP channel. + /// + /// This parameter affects the upper bound of size of `CandidateCommitments`. + pub hrmp_channel_max_message_size: u32, + /// The maximum number of outbound HRMP channels a parachain is allowed to open. + pub hrmp_max_parachain_outbound_channels: u32, + /// The maximum number of outbound HRMP channels a parathread is allowed to open. + pub hrmp_max_parathread_outbound_channels: u32, + /// The maximum number of outbound HRMP messages can be sent by a candidate. + /// + /// This parameter affects the upper bound of size of `CandidateCommitments`. + pub hrmp_max_message_num_per_candidate: u32, } pub trait Trait: frame_system::Trait { } @@ -276,6 +302,105 @@ decl_module! { }); Ok(()) } + + #[weight = (1_000, DispatchClass::Operational)] + pub fn set_hrmp_open_request_ttl(origin, new: u32) -> DispatchResult { + ensure_root(origin)?; + Self::update_config_member(|config| { + sp_std::mem::replace(&mut config.hrmp_open_request_ttl, new) != new + }); + Ok(()) + } + + #[weight = (1_000, DispatchClass::Operational)] + pub fn set_hrmp_sender_deposit(origin, new: Balance) -> DispatchResult { + ensure_root(origin)?; + Self::update_config_member(|config| { + sp_std::mem::replace(&mut config.hrmp_sender_deposit, new) != new + }); + Ok(()) + } + + #[weight = (1_000, DispatchClass::Operational)] + pub fn set_hrmp_recipient_deposit(origin, new: Balance) -> DispatchResult { + ensure_root(origin)?; + Self::update_config_member(|config| { + sp_std::mem::replace(&mut config.hrmp_recipient_deposit, new) != new + }); + Ok(()) + } + + #[weight = (1_000, DispatchClass::Operational)] + pub fn set_hrmp_channel_max_capacity(origin, new: u32) -> DispatchResult { + ensure_root(origin)?; + Self::update_config_member(|config| { + sp_std::mem::replace(&mut config.hrmp_channel_max_capacity, new) != new + }); + Ok(()) + } + + #[weight = (1_000, DispatchClass::Operational)] + pub fn set_hrmp_channel_max_total_size(origin, new: u32) -> DispatchResult { + ensure_root(origin)?; + Self::update_config_member(|config| { + sp_std::mem::replace(&mut config.hrmp_channel_max_total_size, new) != new + }); + Ok(()) + } + + #[weight = (1_000, DispatchClass::Operational)] + pub fn set_hrmp_max_parachain_inbound_channels(origin, new: u32) -> DispatchResult { + ensure_root(origin)?; + Self::update_config_member(|config| { + sp_std::mem::replace(&mut config.hrmp_max_parachain_inbound_channels, new) != new + }); + Ok(()) + } + + #[weight = (1_000, DispatchClass::Operational)] + pub fn set_hrmp_max_parathread_inbound_channels(origin, new: u32) -> DispatchResult { + ensure_root(origin)?; + Self::update_config_member(|config| { + sp_std::mem::replace(&mut config.hrmp_max_parathread_inbound_channels, new) != new + }); + Ok(()) + } + + #[weight = (1_000, DispatchClass::Operational)] + pub fn set_hrmp_channel_max_message_size(origin, new: u32) -> DispatchResult { + ensure_root(origin)?; + Self::update_config_member(|config| { + sp_std::mem::replace(&mut config.hrmp_channel_max_message_size, new) != new + }); + Ok(()) + } + + #[weight = (1_000, DispatchClass::Operational)] + pub fn set_hrmp_max_parachain_outbound_channels(origin, new: u32) -> DispatchResult { + ensure_root(origin)?; + Self::update_config_member(|config| { + sp_std::mem::replace(&mut config.hrmp_max_parachain_outbound_channels, new) != new + }); + Ok(()) + } + + #[weight = (1_000, DispatchClass::Operational)] + pub fn set_hrmp_max_parathread_outbound_channels(origin, new: u32) -> DispatchResult { + ensure_root(origin)?; + Self::update_config_member(|config| { + sp_std::mem::replace(&mut config.hrmp_max_parathread_outbound_channels, new) != new + }); + Ok(()) + } + + #[weight = (1_000, DispatchClass::Operational)] + pub fn set_hrmp_max_message_num_per_candidate(origin, new: u32) -> DispatchResult { + ensure_root(origin)?; + Self::update_config_member(|config| { + sp_std::mem::replace(&mut config.hrmp_max_message_num_per_candidate, new) != new + }); + Ok(()) + } } } @@ -360,6 +485,17 @@ mod tests { preferred_dispatchable_upward_messages_step_weight: 20000, max_upward_message_size: 448, max_upward_message_num_per_candidate: 5, + hrmp_open_request_ttl: 1312, + hrmp_sender_deposit: 22, + hrmp_recipient_deposit: 4905, + hrmp_channel_max_capacity: 3921, + hrmp_channel_max_total_size: 7687, + hrmp_max_parachain_inbound_channels: 3722, + hrmp_max_parathread_inbound_channels: 1967, + hrmp_channel_max_message_size: 8192, + hrmp_max_parachain_outbound_channels: 100, + hrmp_max_parathread_outbound_channels: 200, + hrmp_max_message_num_per_candidate: 20, }; assert!(::PendingConfig::get().is_none()); @@ -415,6 +551,50 @@ mod tests { Configuration::set_max_upward_message_num_per_candidate( Origin::root(), new_config.max_upward_message_num_per_candidate, ).unwrap(); + Configuration::set_hrmp_open_request_ttl( + Origin::root(), + new_config.hrmp_open_request_ttl, + ).unwrap(); + Configuration::set_hrmp_sender_deposit( + Origin::root(), + new_config.hrmp_sender_deposit, + ).unwrap(); + Configuration::set_hrmp_recipient_deposit( + Origin::root(), + new_config.hrmp_recipient_deposit, + ).unwrap(); + Configuration::set_hrmp_channel_max_capacity( + Origin::root(), + new_config.hrmp_channel_max_capacity, + ).unwrap(); + Configuration::set_hrmp_channel_max_total_size( + Origin::root(), + new_config.hrmp_channel_max_total_size, + ).unwrap(); + Configuration::set_hrmp_max_parachain_inbound_channels( + Origin::root(), + new_config.hrmp_max_parachain_inbound_channels, + ).unwrap(); + Configuration::set_hrmp_max_parathread_inbound_channels( + Origin::root(), + new_config.hrmp_max_parathread_inbound_channels, + ).unwrap(); + Configuration::set_hrmp_channel_max_message_size( + Origin::root(), + new_config.hrmp_channel_max_message_size, + ).unwrap(); + Configuration::set_hrmp_max_parachain_outbound_channels( + Origin::root(), + new_config.hrmp_max_parachain_outbound_channels, + ).unwrap(); + Configuration::set_hrmp_max_parathread_outbound_channels( + Origin::root(), + new_config.hrmp_max_parathread_outbound_channels, + ).unwrap(); + Configuration::set_hrmp_max_message_num_per_candidate( + Origin::root(), + new_config.hrmp_max_message_num_per_candidate, + ).unwrap(); assert_eq!(::PendingConfig::get(), Some(new_config)); }) From 3dacd77e89e782557337c82fd47608c04fdc466e Mon Sep 17 00:00:00 2001 From: Sergei Shulepov Date: Tue, 6 Oct 2020 13:27:15 +0200 Subject: [PATCH 08/27] HRMP: Data layout --- runtime/parachains/src/router.rs | 73 +++++++++++++++++++++++++-- runtime/parachains/src/router/hrmp.rs | 64 +++++++++++++++++++++++ 2 files changed, 132 insertions(+), 5 deletions(-) create mode 100644 runtime/parachains/src/router/hrmp.rs diff --git a/runtime/parachains/src/router.rs b/runtime/parachains/src/router.rs index b773bd9c1270..d70d91af4bcc 100644 --- a/runtime/parachains/src/router.rs +++ b/runtime/parachains/src/router.rs @@ -20,18 +20,19 @@ //! routing the messages at their destinations and informing the parachains about the incoming //! messages. -use crate::{ - configuration, - initializer, -}; +use crate::{configuration, initializer}; use sp_std::prelude::*; use frame_support::{decl_error, decl_module, decl_storage, weights::Weight}; use sp_std::collections::vec_deque::VecDeque; -use primitives::v1::{Id as ParaId, InboundDownwardMessage, Hash, UpwardMessage}; +use primitives::v1::{ + Id as ParaId, InboundDownwardMessage, Hash, UpwardMessage, HrmpChannelId, InboundHrmpMessage, +}; +mod hrmp; mod dmp; mod ump; +use hrmp::{HrmpOpenChannelRequest, HrmpChannel}; pub use dmp::QueueDownwardMessageError; pub use ump::UmpSink; @@ -103,6 +104,68 @@ decl_storage! { /// Invariant: /// - If `Some(para)`, then `para` must be present in `NeedsDispatch`. NextDispatchRoundStartWith: Option; + + /* + * Horizontally Relay-routed Message Passing (HRMP) + * + * HRMP related storage layout + */ + + /// The set of pending HRMP open channel requests. + /// + /// The set is accompanied by a list for iteration. + /// + /// Invariant: + /// - There are no channels that exists in list but not in the set and vice versa. + HrmpOpenChannelRequests: map hasher(twox_64_concat) HrmpChannelId => Option; + HrmpOpenChannelRequestsList: Vec; + + /// This mapping tracks how many open channel requests are inititated by a given sender para. + /// Invariant: `HrmpOpenChannelRequests` should contain the same number of items that has `(X, _)` + /// as the number of `HrmpOpenChannelRequestCount` for `X`. + HrmpOpenChannelRequestCount: map hasher(twox_64_concat) ParaId => u32; + /// This mapping tracks how many open channel requests were accepted by a given recipient para. + /// Invariant: `HrmpOpenChannelRequests` should contain the same number of items `(_, X)` with + /// `confirmed` set to true, as the number of `HrmpAcceptedChannelRequestCount` for `X`. + HrmpAcceptedChannelRequestCount: map hasher(twox_64_concat) ParaId => u32; + + /// A set of pending HRMP close channel requests that are going to be closed during the session change. + /// Used for checking if a given channel is registered for closure. + /// + /// The set is accompanied by a list for iteration. + /// + /// Invariant: + /// - There are no channels that exists in list but not in the set and vice versa. + HrmpCloseChannelRequests: map hasher(twox_64_concat) HrmpChannelId => Option<()>; + HrmpCloseChannelRequestsList: Vec; + + /// The HRMP watermark associated with each para. + /// Invariant: every para should be onboarded. + HrmpWatermarks: map hasher(twox_64_concat) ParaId => Option; + /// HRMP channel data associated with each para. + HrmpChannels: map hasher(twox_64_concat) HrmpChannelId => Option; + /// Ingress/egress indexes allow to find all the senders and receivers given the opposite + /// side. I.e. + /// + /// (a) ingress index allows to find all the senders for a given recipient. + /// (b) egress index allows to find all the recipients for a given sender. + /// + /// Invariants: + /// - for each ingress index entry for `P` each item `I` in the index should present in `HrmpChannels` + /// as `(I, P)`. + /// - for each egress index entry for `P` each item `E` in the index should present in `HrmpChannels` + /// as `(P, E)`. + /// - there should be no other dangling channels in `HrmpChannels`. + /// - the vectors are sorted. + HrmpIngressChannelsIndex: map hasher(twox_64_concat) ParaId => Vec; + HrmpEgressChannelsIndex: map hasher(twox_64_concat) ParaId => Vec; + /// Storage for the messages for each channel. + /// Invariant: cannot be non-empty if the corresponding channel in `HrmpChannels` is `None`. + HrmpChannelContents: map hasher(twox_64_concat) HrmpChannelId => Vec>; + /// Maintains a mapping that can be used to answer the question: + /// What paras sent a message at the given block number for a given reciever. + /// Invariant: The para ids vector is never empty. + HrmpChannelDigests: map hasher(twox_64_concat) ParaId => Vec<(T::BlockNumber, Vec)>; } } diff --git a/runtime/parachains/src/router/hrmp.rs b/runtime/parachains/src/router/hrmp.rs new file mode 100644 index 000000000000..1e8d9d1a4d7c --- /dev/null +++ b/runtime/parachains/src/router/hrmp.rs @@ -0,0 +1,64 @@ +// Copyright 2020 Parity Technologies (UK) Ltd. +// This file is part of Polkadot. + +// Polkadot is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Polkadot is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Polkadot. If not, see . + +use primitives::v1::{Balance, Hash, SessionIndex}; +use codec::{Encode, Decode}; + +/// A description of a request to open an HRMP channel. +#[derive(Encode, Decode)] +pub struct HrmpOpenChannelRequest { + /// Indicates if this request was confirmed by the recipient. + pub confirmed: bool, + /// How many session boundaries ago this request was seen. + pub age: SessionIndex, + /// The amount that the sender supplied at the time of creation of this request. + pub sender_deposit: Balance, + /// The maximum message size that could be put into the channel. + pub max_message_size: u32, + /// The maximum number of messages that can be pending in the channel at once. + pub max_capacity: u32, + /// The maximum total size of the messages that can be pending in the channel at once. + pub max_total_size: u32, +} + +/// A metadata of an HRMP channel. +#[derive(Encode, Decode)] +pub struct HrmpChannel { + /// The amount that the sender supplied as a deposit when opening this channel. + pub sender_deposit: Balance, + /// The amount that the recipient supplied as a deposit when accepting opening this channel. + pub recipient_deposit: Balance, + /// The maximum number of messages that can be pending in the channel at once. + pub max_capacity: u32, + /// The maximum total size of the messages that can be pending in the channel at once. + pub max_total_size: u32, + /// The maximum message size that could be put into the channel. + pub max_message_size: u32, + /// The current number of messages pending in the channel. + /// Invariant: should be less or equal to `max_capacity`.s`. + pub msg_count: u32, + /// The total size in bytes of all message payloads in the channel. + /// Invariant: should be less or equal to `max_total_size`. + pub total_size: u32, + /// A head of the Message Queue Chain for this channel. Each link in this chain has a form: + /// `(prev_head, B, H(M))`, where + /// - `prev_head`: is the previous value of `mqc_head` or zero if none. + /// - `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 + /// that no messages were previously added. + pub mqc_head: Option, +} From 6e7979b461c3ac03a539081fae12af84293665e6 Mon Sep 17 00:00:00 2001 From: Sergei Shulepov Date: Wed, 23 Sep 2020 17:02:47 +0200 Subject: [PATCH 09/27] HRMP: Acceptance & Enactment --- Cargo.lock | 1 + runtime/common/src/paras_registrar.rs | 1 + runtime/parachains/Cargo.toml | 2 + runtime/parachains/src/inclusion.rs | 60 +++ runtime/parachains/src/mock.rs | 1 + runtime/parachains/src/paras.rs | 6 + runtime/parachains/src/router.rs | 87 +++- runtime/parachains/src/router/hrmp.rs | 607 +++++++++++++++++++++++++- runtime/parachains/src/util.rs | 3 +- runtime/rococo/src/lib.rs | 1 + runtime/test-runtime/src/lib.rs | 1 + 11 files changed, 760 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 21944abf4519..d488bcadaf18 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5478,6 +5478,7 @@ dependencies = [ "sp-std", "sp-trie", "sp-version", + "xcm", ] [[package]] diff --git a/runtime/common/src/paras_registrar.rs b/runtime/common/src/paras_registrar.rs index ab58878abddd..dab0bb02e250 100644 --- a/runtime/common/src/paras_registrar.rs +++ b/runtime/common/src/paras_registrar.rs @@ -427,6 +427,7 @@ mod tests { impl router::Trait for Test { type UmpSink = (); + type Origin = Origin; } impl pallet_session::historical::Trait for Test { diff --git a/runtime/parachains/Cargo.toml b/runtime/parachains/Cargo.toml index 30815fc8b556..fac83aa3817f 100644 --- a/runtime/parachains/Cargo.toml +++ b/runtime/parachains/Cargo.toml @@ -33,6 +33,7 @@ pallet-vesting = { git = "https://github.com/paritytech/substrate", branch = "ma pallet-offences = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } frame-benchmarking = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false, optional = true } +xcm = { package = "xcm", path = "../../xcm", default-features = false } primitives = { package = "polkadot-primitives", path = "../../primitives", default-features = false } libsecp256k1 = { version = "0.3.2", default-features = false, optional = true } @@ -81,6 +82,7 @@ std = [ "frame-system/std", "pallet-timestamp/std", "pallet-vesting/std", + "xcm/std", ] runtime-benchmarks = [ "libsecp256k1/hmac", diff --git a/runtime/parachains/src/inclusion.rs b/runtime/parachains/src/inclusion.rs index 61bdd58bf285..c9cb162181da 100644 --- a/runtime/parachains/src/inclusion.rs +++ b/runtime/parachains/src/inclusion.rs @@ -157,6 +157,10 @@ decl_error! { IncorrectDownwardMessageHandling, /// At least one upward message sent does not pass the acceptance criteria. InvalidUpwardMessages, + /// The candidate didn't follow the rules of HRMP watermark advancement. + HrmpWatermarkMishandling, + /// The HRMP messages sent by the candidate is not valid. + InvalidOutboundHrmp, } } @@ -415,6 +419,8 @@ impl Module { &candidate.candidate.commitments.new_validation_code, candidate.candidate.commitments.processed_downward_messages, &candidate.candidate.commitments.upward_messages, + candidate.candidate.commitments.hrmp_watermark, + &candidate.candidate.commitments.horizontal_messages, )?; for (i, assignment) in scheduled[skip..].iter().enumerate() { @@ -548,6 +554,8 @@ impl Module { &validation_outputs.new_validation_code, validation_outputs.processed_downward_messages, &validation_outputs.upward_messages, + validation_outputs.hrmp_watermark, + &validation_outputs.horizontal_messages, ) } @@ -578,6 +586,14 @@ impl Module { receipt.descriptor.para_id, commitments.upward_messages, ); + weight += >::prune_hrmp( + receipt.descriptor.para_id, + T::BlockNumber::from(commitments.hrmp_watermark), + ); + weight += >::queue_outbound_hrmp( + receipt.descriptor.para_id, + commitments.horizontal_messages, + ); Self::deposit_event( Event::::CandidateIncluded(plain, commitments.head_data.clone()) @@ -702,6 +718,8 @@ impl CandidateCheckContext { new_validation_code: &Option, processed_downward_messages: u32, upward_messages: &[primitives::v1::UpwardMessage], + hrmp_watermark: primitives::v1::BlockNumber, + horizontal_messages: &[primitives::v1::OutboundHrmpMessage], ) -> Result<(), DispatchError> { ensure!( head_data.0.len() <= self.config.max_head_data_size as _, @@ -739,6 +757,23 @@ impl CandidateCheckContext { ), Error::::InvalidUpwardMessages, ); + ensure!( + >::check_hrmp_watermark( + para_id, + self.relay_parent_number, + // TODO: Hmm, we should settle on a single represenation of T::BlockNumber + T::BlockNumber::from(hrmp_watermark), + ), + Error::::HrmpWatermarkMishandling, + ); + ensure!( + >::check_outbound_hrmp( + &self.config, + para_id, + horizontal_messages, + ), + Error::::InvalidOutboundHrmp, + ); Ok(()) } @@ -946,6 +981,7 @@ mod tests { relay_parent: Hash, persisted_validation_data_hash: Hash, new_validation_code: Option, + hrmp_watermark: BlockNumber, } impl TestCandidateBuilder { @@ -961,6 +997,7 @@ mod tests { commitments: CandidateCommitments { head_data: self.head_data, new_validation_code: self.new_validation_code, + hrmp_watermark: self.hrmp_watermark, ..Default::default() }, } @@ -1359,6 +1396,9 @@ mod tests { let chain_b = ParaId::from(2); let thread_a = ParaId::from(3); + // The block number of the relay-parent for testing. + const RELAY_PARENT_NUM: BlockNumber = 4; + let paras = vec![(chain_a, true), (chain_b, true), (thread_a, false)]; let validators = vec![ Sr25519Keyring::Alice, @@ -1421,6 +1461,7 @@ mod tests { relay_parent: System::parent_hash(), pov_hash: Hash::from([1; 32]), persisted_validation_data_hash: make_vdata_hash(chain_a).unwrap(), + hrmp_watermark: RELAY_PARENT_NUM, ..Default::default() }.build(); collator_sign_candidate( @@ -1454,6 +1495,7 @@ mod tests { relay_parent: System::parent_hash(), pov_hash: Hash::from([1; 32]), persisted_validation_data_hash: make_vdata_hash(chain_a).unwrap(), + hrmp_watermark: RELAY_PARENT_NUM, ..Default::default() }.build(); let mut candidate_b = TestCandidateBuilder { @@ -1461,6 +1503,7 @@ mod tests { relay_parent: System::parent_hash(), pov_hash: Hash::from([2; 32]), persisted_validation_data_hash: make_vdata_hash(chain_b).unwrap(), + hrmp_watermark: RELAY_PARENT_NUM, ..Default::default() }.build(); @@ -1510,6 +1553,7 @@ mod tests { relay_parent: System::parent_hash(), pov_hash: Hash::from([1; 32]), persisted_validation_data_hash: make_vdata_hash(chain_a).unwrap(), + hrmp_watermark: RELAY_PARENT_NUM, ..Default::default() }.build(); collator_sign_candidate( @@ -1579,6 +1623,7 @@ mod tests { relay_parent: System::parent_hash(), pov_hash: Hash::from([1; 32]), persisted_validation_data_hash: make_vdata_hash(thread_a).unwrap(), + hrmp_watermark: RELAY_PARENT_NUM, ..Default::default() }.build(); @@ -1618,6 +1663,7 @@ mod tests { relay_parent: System::parent_hash(), pov_hash: Hash::from([1; 32]), persisted_validation_data_hash: make_vdata_hash(thread_a).unwrap(), + hrmp_watermark: RELAY_PARENT_NUM, ..Default::default() }.build(); @@ -1656,6 +1702,7 @@ mod tests { relay_parent: System::parent_hash(), pov_hash: Hash::from([1; 32]), persisted_validation_data_hash: make_vdata_hash(chain_a).unwrap(), + hrmp_watermark: RELAY_PARENT_NUM, ..Default::default() }.build(); @@ -1703,6 +1750,7 @@ mod tests { relay_parent: System::parent_hash(), pov_hash: Hash::from([1; 32]), persisted_validation_data_hash: make_vdata_hash(chain_a).unwrap(), + hrmp_watermark: RELAY_PARENT_NUM, ..Default::default() }.build(); @@ -1743,6 +1791,7 @@ mod tests { pov_hash: Hash::from([1; 32]), new_validation_code: Some(vec![5, 6, 7, 8].into()), persisted_validation_data_hash: make_vdata_hash(chain_a).unwrap(), + hrmp_watermark: RELAY_PARENT_NUM, ..Default::default() }.build(); @@ -1785,6 +1834,7 @@ mod tests { relay_parent: System::parent_hash(), pov_hash: Hash::from([1; 32]), persisted_validation_data_hash: [42u8; 32].into(), + hrmp_watermark: RELAY_PARENT_NUM, ..Default::default() }.build(); @@ -1820,6 +1870,9 @@ mod tests { let chain_b = ParaId::from(2); let thread_a = ParaId::from(3); + // The block number of the relay-parent for testing. + const RELAY_PARENT_NUM: BlockNumber = 4; + let paras = vec![(chain_a, true), (chain_b, true), (thread_a, false)]; let validators = vec![ Sr25519Keyring::Alice, @@ -1880,6 +1933,7 @@ mod tests { relay_parent: System::parent_hash(), pov_hash: Hash::from([1; 32]), persisted_validation_data_hash: make_vdata_hash(chain_a).unwrap(), + hrmp_watermark: RELAY_PARENT_NUM, ..Default::default() }.build(); collator_sign_candidate( @@ -1892,6 +1946,7 @@ mod tests { relay_parent: System::parent_hash(), pov_hash: Hash::from([2; 32]), persisted_validation_data_hash: make_vdata_hash(chain_b).unwrap(), + hrmp_watermark: RELAY_PARENT_NUM, ..Default::default() }.build(); collator_sign_candidate( @@ -1904,6 +1959,7 @@ mod tests { relay_parent: System::parent_hash(), pov_hash: Hash::from([3; 32]), persisted_validation_data_hash: make_vdata_hash(thread_a).unwrap(), + hrmp_watermark: RELAY_PARENT_NUM, ..Default::default() }.build(); collator_sign_candidate( @@ -2001,6 +2057,9 @@ mod tests { fn can_include_candidate_with_ok_code_upgrade() { let chain_a = ParaId::from(1); + // The block number of the relay-parent for testing. + const RELAY_PARENT_NUM: BlockNumber = 4; + let paras = vec![(chain_a, true)]; let validators = vec![ Sr25519Keyring::Alice, @@ -2044,6 +2103,7 @@ mod tests { pov_hash: Hash::from([1; 32]), persisted_validation_data_hash: make_vdata_hash(chain_a).unwrap(), new_validation_code: Some(vec![1, 2, 3].into()), + hrmp_watermark: RELAY_PARENT_NUM, ..Default::default() }.build(); collator_sign_candidate( diff --git a/runtime/parachains/src/mock.rs b/runtime/parachains/src/mock.rs index 4fdd6e0e6155..3da3a6448128 100644 --- a/runtime/parachains/src/mock.rs +++ b/runtime/parachains/src/mock.rs @@ -109,6 +109,7 @@ impl crate::paras::Trait for Test { } impl crate::router::Trait for Test { + type Origin = Origin; type UmpSink = crate::router::MockUmpSink; } diff --git a/runtime/parachains/src/paras.rs b/runtime/parachains/src/paras.rs index 4d7f1e2a07c5..84bdf6cf73ad 100644 --- a/runtime/parachains/src/paras.rs +++ b/runtime/parachains/src/paras.rs @@ -541,6 +541,12 @@ impl Module { } } + /// Returns whether the given ID refers to a valid para. + pub(crate) fn is_valid_para(id: ParaId) -> bool { + Self::parachains().binary_search(&id).is_ok() + || Self::is_parathread(id) + } + /// Whether a para ID corresponds to any live parathread. pub(crate) fn is_parathread(id: ParaId) -> bool { Parathreads::get(&id).is_some() diff --git a/runtime/parachains/src/router.rs b/runtime/parachains/src/router.rs index d70d91af4bcc..5955e910cd7d 100644 --- a/runtime/parachains/src/router.rs +++ b/runtime/parachains/src/router.rs @@ -20,16 +20,16 @@ //! routing the messages at their destinations and informing the parachains about the incoming //! messages. -use crate::{configuration, initializer}; +use crate::{configuration, paras, initializer, ensure_parachain}; use sp_std::prelude::*; -use frame_support::{decl_error, decl_module, decl_storage, weights::Weight}; +use frame_support::{decl_error, decl_module, decl_storage, dispatch::DispatchResult, weights::Weight}; use sp_std::collections::vec_deque::VecDeque; use primitives::v1::{ Id as ParaId, InboundDownwardMessage, Hash, UpwardMessage, HrmpChannelId, InboundHrmpMessage, }; -mod hrmp; mod dmp; +mod hrmp; mod ump; use hrmp::{HrmpOpenChannelRequest, HrmpChannel}; @@ -39,7 +39,11 @@ pub use ump::UmpSink; #[cfg(test)] pub use ump::mock_sink::MockUmpSink; -pub trait Trait: frame_system::Trait + configuration::Trait { +pub trait Trait: frame_system::Trait + configuration::Trait + paras::Trait { + type Origin: From + + From<::Origin> + + Into::Origin>>; + /// A place where all received upward messages are funneled. type UmpSink: UmpSink; } @@ -170,13 +174,75 @@ decl_storage! { } decl_error! { - pub enum Error for Module { } + pub enum Error for Module { + /// The sender tried to open a channel to themselves. + OpenHrmpChannelToSelf, + /// The recipient is not a valid para. + OpenHrmpChannelInvalidRecipient, + /// The requested capacity is zero. + OpenHrmpChannelZeroCapacity, + /// The requested capacity exceeds the global limit. + OpenHrmpChannelCapacityExceedsLimit, + /// The requested maximum message size is 0. + OpenHrmpChannelZeroMessageSize, + /// The open request requested the message size that exceeds the global limit. + OpenHrmpChannelMessageSizeExceedsLimit, + /// The channel already exists + OpenHrmpChannelAlreadyExists, + /// There is already a request to open the same channel. + OpenHrmpChannelAlreadyRequested, + /// The sender already has the maximum number of allowed outbound channels. + OpenHrmpChannelLimitExceeded, + /// The channel from the sender to the origin doesn't exist. + AcceptHrmpChannelDoesntExist, + /// The channel is already confirmed. + AcceptHrmpChannelAlreadyConfirmed, + /// The recipient already has the maximum number of allowed inbound channels. + AcceptHrmpChannelLimitExceeded, + /// The origin tries to close a channel where it is neither the sender nor the recipient. + CloseHrmpChannelUnauthorized, + /// The channel to be closed doesn't exist. + CloseHrmpChannelDoesntExist, + /// The channel close request is already requested. + CloseHrmpChannelAlreadyUnderway, + } } decl_module! { /// The router module. pub struct Module for enum Call where origin: ::Origin { type Error = Error; + + #[weight = 0] + fn hrmp_init_open_channel( + origin, + recipient: ParaId, + proposed_max_capacity: u32, + proposed_max_message_size: u32, + ) -> DispatchResult { + let origin = ensure_parachain(::Origin::from(origin))?; + Self::init_open_channel( + origin, + recipient, + proposed_max_capacity, + proposed_max_message_size + )?; + Ok(()) + } + + #[weight = 0] + fn hrmp_accept_open_channel(origin, sender: ParaId) -> DispatchResult { + let origin = ensure_parachain(::Origin::from(origin))?; + Self::accept_open_channel(origin, sender)?; + Ok(()) + } + + #[weight = 0] + fn hrmp_close_channel(origin, channel_id: HrmpChannelId) -> DispatchResult { + let origin = ensure_parachain(::Origin::from(origin))?; + Self::close_channel(origin, channel_id)?; + Ok(()) + } } } @@ -191,12 +257,21 @@ impl Module { /// Called by the initializer to note that a new session has started. pub(crate) fn initializer_on_new_session( - _notification: &initializer::SessionChangeNotification, + notification: &initializer::SessionChangeNotification, ) { + Self::perform_outgoing_para_cleanup(); + Self::process_hrmp_open_channel_requests(¬ification.prev_config); + Self::process_hrmp_close_channel_requests(); + } + + /// Iterate over all paras that were registered for offboarding and remove all the data + /// associated with them. + fn perform_outgoing_para_cleanup() { let outgoing = OutgoingParas::take(); for outgoing_para in outgoing { Self::clean_dmp_after_outgoing(outgoing_para); Self::clean_ump_after_outgoing(outgoing_para); + Self::clean_hrmp_after_outgoing(outgoing_para); } } diff --git a/runtime/parachains/src/router/hrmp.rs b/runtime/parachains/src/router/hrmp.rs index 1e8d9d1a4d7c..d1c3fc9ca71e 100644 --- a/runtime/parachains/src/router/hrmp.rs +++ b/runtime/parachains/src/router/hrmp.rs @@ -14,8 +14,21 @@ // You should have received a copy of the GNU General Public License // along with Polkadot. If not, see . -use primitives::v1::{Balance, Hash, SessionIndex}; -use codec::{Encode, Decode}; +use super::{Module, Store, Trait, Error as DispatchError, dmp}; +use crate::{ + configuration::{self, HostConfiguration}, + paras, +}; +use codec::{Decode, Encode}; +use frame_support::{traits::Get, weights::Weight, StorageMap, StorageValue, ensure}; +use primitives::v1::{ + Balance, Hash, HrmpChannelId, Id as ParaId, InboundHrmpMessage, OutboundHrmpMessage, + SessionIndex, +}; +use sp_runtime::traits::{BlakeTwo256, Hash as HashT}; +use sp_std::collections::btree_set::BTreeSet; +use sp_std::mem; +use sp_std::prelude::*; /// A description of a request to open an HRMP channel. #[derive(Encode, Decode)] @@ -36,6 +49,7 @@ pub struct HrmpOpenChannelRequest { /// A metadata of an HRMP channel. #[derive(Encode, Decode)] +#[cfg_attr(test, derive(Debug))] pub struct HrmpChannel { /// The amount that the sender supplied as a deposit when opening this channel. pub sender_deposit: Balance, @@ -62,3 +76,592 @@ pub struct HrmpChannel { /// that no messages were previously added. pub mqc_head: Option, } + +/// Routines and getters related to HRMP. +impl Module { + /// Remove all storage entries associated with the given para. + pub(super) fn clean_hrmp_after_outgoing(outgoing_para: ParaId) { + ::HrmpOpenChannelRequestCount::remove(&outgoing_para); + ::HrmpAcceptedChannelRequestCount::remove(&outgoing_para); + + // close all channels where the outgoing para acts as the recipient. + for sender in ::HrmpIngressChannelsIndex::take(&outgoing_para) { + Self::close_hrmp_channel(&HrmpChannelId { + sender, + recipient: outgoing_para.clone(), + }); + } + // close all channels where the outgoing para acts as the sender. + for recipient in ::HrmpEgressChannelsIndex::take(&outgoing_para) { + Self::close_hrmp_channel(&HrmpChannelId { + sender: outgoing_para.clone(), + recipient, + }); + } + } + + /// Iterate over all open channel requests and: + /// + /// - prune the stale requests + /// - enact the confirmed requests + pub(super) fn process_hrmp_open_channel_requests(config: &HostConfiguration) { + let mut open_req_channels = ::HrmpOpenChannelRequestsList::get(); + if open_req_channels.is_empty() { + return; + } + + // iterate the vector starting from the end making our way to the beginning. This way we + // can leverage `swap_remove` to efficiently remove an item during iteration. + let mut idx = open_req_channels.len(); + loop { + // bail if we've iterated over all items. + if idx == 0 { + break; + } + + idx -= 1; + let channel_id = open_req_channels[idx].clone(); + let mut request = ::HrmpOpenChannelRequests::get(&channel_id) + .expect( + "can't be `None` due to the invariant that the list contains the same items as the set; qed" + ); + + if request.confirmed { + if >::is_valid_para(channel_id.sender) + && >::is_valid_para(channel_id.recipient) + { + ::HrmpChannels::insert( + &channel_id, + HrmpChannel { + sender_deposit: request.sender_deposit, + recipient_deposit: config.hrmp_recipient_deposit, + max_capacity: request.max_capacity, + max_total_size: request.max_total_size, + max_message_size: request.max_message_size, + msg_count: 0, + total_size: 0, + mqc_head: None, + }, + ); + + ::HrmpIngressChannelsIndex::mutate(&channel_id.recipient, |v| { + if let Err(i) = v.binary_search(&channel_id.sender) { + v.insert(i, channel_id.sender); + } + }); + ::HrmpEgressChannelsIndex::mutate(&channel_id.sender, |v| { + if let Err(i) = v.binary_search(&channel_id.recipient) { + v.insert(i, channel_id.recipient); + } + }); + } + + let new_open_channel_req_cnt = + ::HrmpOpenChannelRequestCount::get(&channel_id.sender) + .saturating_sub(1); + if new_open_channel_req_cnt != 0 { + ::HrmpOpenChannelRequestCount::insert( + &channel_id.sender, + new_open_channel_req_cnt, + ); + } else { + ::HrmpOpenChannelRequestCount::remove(&channel_id.sender); + } + + let new_accepted_channel_req_cnt = + ::HrmpAcceptedChannelRequestCount::get(&channel_id.recipient) + .saturating_sub(1); + if new_accepted_channel_req_cnt != 0 { + ::HrmpAcceptedChannelRequestCount::insert( + &channel_id.recipient, + new_accepted_channel_req_cnt, + ); + } else { + ::HrmpAcceptedChannelRequestCount::remove(&channel_id.recipient); + } + + let _ = open_req_channels.swap_remove(idx); + ::HrmpOpenChannelRequests::remove(&channel_id); + } else { + request.age += 1; + if request.age == config.hrmp_open_request_ttl { + // got stale + + ::HrmpOpenChannelRequestCount::mutate(&channel_id.sender, |v| { + *v -= 1; + }); + + // TODO: return deposit https://github.com/paritytech/polkadot/issues/1907 + + let _ = open_req_channels.swap_remove(idx); + ::HrmpOpenChannelRequests::remove(&channel_id); + } + } + } + + ::HrmpOpenChannelRequestsList::put(open_req_channels); + } + + /// Iterate over all close channel requests unconditionally closing the channels. + pub(super) fn process_hrmp_close_channel_requests() { + let close_reqs = ::HrmpCloseChannelRequestsList::take(); + for condemned_ch_id in close_reqs { + ::HrmpCloseChannelRequests::remove(&condemned_ch_id); + Self::close_hrmp_channel(&condemned_ch_id); + + // clean up the indexes. + ::HrmpEgressChannelsIndex::mutate(&condemned_ch_id.sender, |v| { + if let Ok(i) = v.binary_search(&condemned_ch_id.recipient) { + v.remove(i); + } + }); + ::HrmpIngressChannelsIndex::mutate(&condemned_ch_id.recipient, |v| { + if let Ok(i) = v.binary_search(&condemned_ch_id.sender) { + v.remove(i); + } + }); + } + } + + /// Close and remove the designated HRMP channel. + /// + /// This includes returning the deposits. However, it doesn't include updating the ingress/egress + /// indicies. + pub(super) fn close_hrmp_channel(channel_id: &HrmpChannelId) { + // TODO: return deposit https://github.com/paritytech/polkadot/issues/1907 + + ::HrmpChannels::remove(channel_id); + ::HrmpChannelContents::remove(channel_id); + } + + /// Check that the candidate of the given recipient controls the HRMP watermark properly. + pub(crate) fn check_hrmp_watermark( + recipient: ParaId, + relay_chain_parent_number: T::BlockNumber, + new_hrmp_watermark: T::BlockNumber, + ) -> bool { + // First, check where the watermark CANNOT legally land. + // + // (a) For ensuring that messages are eventually, a rule requires each parablock new + // watermark should be greater than the last one. + // + // (b) However, a parachain cannot read into "the future", therefore the watermark should + // not be greater than the relay-chain context block which the parablock refers to. + if let Some(last_watermark) = ::HrmpWatermarks::get(&recipient) { + if new_hrmp_watermark <= last_watermark { + return false; + } + } + if new_hrmp_watermark > relay_chain_parent_number { + return false; + } + + // Second, check where the watermark CAN land. It's one of the following: + // + // (a) The relay parent block number. + // (b) A relay-chain block in which this para received at least one message. + if new_hrmp_watermark == relay_chain_parent_number { + true + } else { + let digest = ::HrmpChannelDigests::get(&recipient); + digest + .binary_search_by_key(&new_hrmp_watermark, |(block_no, _)| *block_no) + .is_ok() + } + } + + pub(crate) fn check_outbound_hrmp( + config: &HostConfiguration, + sender: ParaId, + out_hrmp_msgs: &[OutboundHrmpMessage], + ) -> bool { + if out_hrmp_msgs.len() as u32 > config.hrmp_max_message_num_per_candidate { + return false; + } + + let mut last_recipient = None::; + + for out_msg in out_hrmp_msgs { + match last_recipient { + // the messages must be sorted in ascending order and there must be no two messages sent + // to the same recipient. Thus we can check that every recipient is strictly greater than + // the previous one. + Some(last_recipient) if out_msg.recipient <= last_recipient => { + return false; + } + _ => last_recipient = Some(out_msg.recipient), + } + + let channel_id = HrmpChannelId { + sender, + recipient: out_msg.recipient, + }; + + let channel = match ::HrmpChannels::get(&channel_id) { + Some(channel) => channel, + None => return false, + }; + + if out_msg.data.len() as u32 > channel.max_message_size { + return false; + } + + if channel.total_size + out_msg.data.len() as u32 > channel.max_total_size { + return false; + } + + if channel.msg_count + 1 > channel.max_capacity { + return false; + } + } + + true + } + + pub(crate) fn prune_hrmp(recipient: ParaId, new_hrmp_watermark: T::BlockNumber) -> Weight { + let mut weight = 0; + + // sift through the incoming messages digest to collect the paras that sent at least one + // message to this parachain between the old and new watermarks. + let senders = ::HrmpChannelDigests::mutate(&recipient, |digest| { + let mut senders = BTreeSet::new(); + let mut residue = Vec::with_capacity(digest.len()); + for (block_no, paras_sent_msg) in mem::replace(digest, Vec::new()) { + if block_no <= new_hrmp_watermark { + senders.extend(paras_sent_msg); + } else { + residue.push((block_no, paras_sent_msg)); + } + } + *digest = residue; + senders + }); + weight += T::DbWeight::get().reads_writes(1, 1); + + // having all senders we can trivially find out the channels which we need to prune. + let channels_to_prune = senders + .into_iter() + .map(|sender| HrmpChannelId { sender, recipient }); + for channel_id in channels_to_prune { + // prune each channel up to the new watermark keeping track how many messages we removed + // and what is the total byte size of them. + let (cnt, size) = + ::HrmpChannelContents::mutate(&channel_id, |outbound_messages| { + let (mut cnt, mut size) = (0, 0); + + let mut residue = Vec::with_capacity(outbound_messages.len()); + for msg in mem::replace(outbound_messages, Vec::new()).into_iter() { + if msg.sent_at <= new_hrmp_watermark { + cnt += 1; + size += msg.data.len(); + } else { + residue.push(msg); + } + } + *outbound_messages = residue; + + (cnt, size) + }); + + // update the channel metadata. + ::HrmpChannels::mutate(&channel_id, |channel| { + if let Some(ref mut channel) = channel { + channel.msg_count -= cnt as u32; + channel.total_size -= size as u32; + } + }); + + weight += T::DbWeight::get().reads_writes(2, 2); + } + + ::HrmpWatermarks::insert(&recipient, new_hrmp_watermark); + weight += T::DbWeight::get().reads_writes(0, 1); + + weight + } + + /// Process the outbound HRMP messages by putting them into the appropriate recipient queues. + /// + /// Returns the amount of weight consumed. + pub(crate) fn queue_outbound_hrmp( + sender: ParaId, + out_hrmp_msgs: Vec>, + ) -> Weight { + let mut weight = 0; + let now = >::block_number(); + + for out_msg in out_hrmp_msgs { + let channel_id = HrmpChannelId { + sender, + recipient: out_msg.recipient, + }; + + let mut channel = match ::HrmpChannels::get(&channel_id) { + Some(channel) => channel, + None => { + // apparently, that since acceptance of this candidate the recipient was + // offboarded and the channel no longer exists. + continue; + } + }; + + let inbound = InboundHrmpMessage { + sent_at: now, + data: out_msg.data, + }; + + // book keeping + channel.msg_count += 1; + channel.total_size += inbound.data.len() as u32; + + // compute the new MQC head of the channel + let prev_head = channel.mqc_head.clone().unwrap_or(Default::default()); + let new_head = BlakeTwo256::hash_of(&( + prev_head, + inbound.sent_at, + T::Hashing::hash_of(&inbound.data), + )); + channel.mqc_head = Some(new_head); + + ::HrmpChannels::insert(&channel_id, channel); + ::HrmpChannelContents::append(&channel_id, inbound); + + weight += T::DbWeight::get().reads_writes(2, 2); + } + + weight + } + + pub(super) fn init_open_channel( + origin: ParaId, + recipient: ParaId, + proposed_max_capacity: u32, + proposed_max_message_size: u32, + ) -> Result<(), DispatchError> { + ensure!( + origin != recipient, + DispatchError::::OpenHrmpChannelToSelf + ); + ensure!( + >::is_valid_para(recipient), + DispatchError::::OpenHrmpChannelInvalidRecipient, + ); + + let config = >::config(); + ensure!( + proposed_max_capacity > 0, + DispatchError::::OpenHrmpChannelZeroCapacity, + ); + ensure!( + proposed_max_capacity <= config.hrmp_channel_max_capacity, + DispatchError::::OpenHrmpChannelCapacityExceedsLimit, + ); + ensure!( + proposed_max_message_size > 0, + DispatchError::::OpenHrmpChannelZeroMessageSize, + ); + ensure!( + proposed_max_message_size <= config.hrmp_channel_max_message_size, + DispatchError::::OpenHrmpChannelMessageSizeExceedsLimit, + ); + + let channel_id = HrmpChannelId { + sender: origin, + recipient, + }; + ensure!( + ::HrmpOpenChannelRequests::get(&channel_id).is_none(), + DispatchError::::OpenHrmpChannelAlreadyExists, + ); + ensure!( + ::HrmpChannels::get(&channel_id).is_none(), + DispatchError::::OpenHrmpChannelAlreadyRequested, + ); + + let egress_cnt = + ::HrmpEgressChannelsIndex::decode_len(&origin).unwrap_or(0) as u32; + let open_req_cnt = ::HrmpOpenChannelRequestCount::get(&origin); + let channel_num_limit = if >::is_parathread(origin) { + config.hrmp_max_parathread_outbound_channels + } else { + config.hrmp_max_parachain_outbound_channels + }; + ensure!( + egress_cnt + open_req_cnt < channel_num_limit, + DispatchError::::OpenHrmpChannelLimitExceeded, + ); + + // TODO: Deposit https://github.com/paritytech/polkadot/issues/1907 + + ::HrmpOpenChannelRequestCount::insert(&origin, open_req_cnt + 1); + ::HrmpOpenChannelRequests::insert( + &channel_id, + HrmpOpenChannelRequest { + confirmed: false, + age: 0, + sender_deposit: config.hrmp_sender_deposit, + max_capacity: proposed_max_capacity, + max_message_size: proposed_max_message_size, + max_total_size: config.hrmp_channel_max_total_size, + }, + ); + ::HrmpOpenChannelRequestsList::append(channel_id); + + let notification_bytes = { + use xcm::v0::Xcm; + use codec::Encode as _; + + Xcm::HrmpNewChannelOpenRequest { + sender: u32::from(origin), + max_capacity: proposed_max_capacity, + max_message_size: proposed_max_message_size, + } + .encode() + }; + if let Err(dmp::QueueDownwardMessageError::ExceedsMaxMessageSize) = + Self::queue_downward_message(&config, recipient, notification_bytes) + { + // this should never happen unless the max downward message size is configured to an + // jokingly small number. + debug_assert!(false); + } + + Ok(()) + } + + pub(super) fn accept_open_channel( + origin: ParaId, + sender: ParaId, + ) -> Result<(), DispatchError> { + let channel_id = HrmpChannelId { + sender, + recipient: origin, + }; + let mut channel_req = ::HrmpOpenChannelRequests::get(&channel_id) + .ok_or(DispatchError::::AcceptHrmpChannelDoesntExist)?; + ensure!( + !channel_req.confirmed, + DispatchError::::AcceptHrmpChannelAlreadyConfirmed, + ); + + // check if by accepting this open channel request, this parachain would exceed the + // number of inbound channels. + let config = >::config(); + let channel_num_limit = if >::is_parathread(origin) { + config.hrmp_max_parathread_inbound_channels + } else { + config.hrmp_max_parachain_inbound_channels + }; + let ingress_cnt = + ::HrmpIngressChannelsIndex::decode_len(&origin).unwrap_or(0) as u32; + let accepted_cnt = ::HrmpAcceptedChannelRequestCount::get(&origin); + ensure!( + ingress_cnt + accepted_cnt < channel_num_limit, + DispatchError::::AcceptHrmpChannelLimitExceeded, + ); + + // TODO: Deposit https://github.com/paritytech/polkadot/issues/1907 + + // persist the updated open channel request and then increment the number of accepted + // channels. + channel_req.confirmed = true; + ::HrmpOpenChannelRequests::insert(&channel_id, channel_req); + ::HrmpAcceptedChannelRequestCount::insert(&origin, accepted_cnt + 1); + + let notification_bytes = { + use xcm::v0::Xcm; + use codec::Encode as _; + + Xcm::HrmpChannelAccepted { + recipient: u32::from(origin), + } + .encode() + }; + if let Err(dmp::QueueDownwardMessageError::ExceedsMaxMessageSize) = + Self::queue_downward_message(&config, sender, notification_bytes) + { + // this should never happen unless the max downward message size is configured to an + // jokingly small number. + debug_assert!(false); + } + + Ok(()) + } + + pub(super) fn close_channel( + origin: ParaId, + channel_id: HrmpChannelId, + ) -> Result<(), DispatchError> { + // check if the origin is allowed to close the channel. + ensure!( + origin == channel_id.sender || origin == channel_id.recipient, + DispatchError::::CloseHrmpChannelUnauthorized, + ); + + // check if the channel requested to close does exist. + ensure!( + ::HrmpChannels::get(&channel_id).is_some(), + DispatchError::::CloseHrmpChannelDoesntExist, + ); + + // check that there is no outstanding close request for this channel + ensure!( + ::HrmpCloseChannelRequests::get(&channel_id).is_none(), + DispatchError::::CloseHrmpChannelAlreadyUnderway, + ); + + ::HrmpCloseChannelRequests::insert(&channel_id, ()); + ::HrmpCloseChannelRequestsList::append(channel_id.clone()); + + let config = >::config(); + let notification_bytes = { + use xcm::v0::Xcm; + use codec::Encode as _; + + Xcm::HrmpChannelClosing { + initiator: u32::from(origin), + sender: u32::from(channel_id.sender), + recipient: u32::from(channel_id.recipient), + } + .encode() + }; + let opposite_party = if origin == channel_id.sender { + channel_id.recipient + } else { + channel_id.sender + }; + if let Err(dmp::QueueDownwardMessageError::ExceedsMaxMessageSize) = + Self::queue_downward_message(&config, opposite_party, notification_bytes) + { + // this should never happen unless the max downward message size is configured to an + // jokingly small number. + debug_assert!(false); + } + + Ok(()) + } + + /// Returns the list of MQC heads for the inbound channels of the given recipient para paired + /// with the sender para ids. This vector is sorted ascending by the para id and doesn't contain + /// multiple entries with the same sender. + pub(crate) fn hrmp_mqc_heads(recipient: ParaId) -> Vec<(ParaId, Hash)> { + let sender_set = ::HrmpIngressChannelsIndex::get(&recipient); + + // The ingress channels vector is sorted, thus `mqc_heads` is sorted as well. + let mut mqc_heads = Vec::with_capacity(sender_set.len()); + for sender in sender_set { + let channel_metadata = + ::HrmpChannels::get(&HrmpChannelId { sender, recipient }); + let mqc_head = channel_metadata + .and_then(|metadata| metadata.mqc_head) + .unwrap_or(Hash::default()); + mqc_heads.push((sender, mqc_head)); + } + + mqc_heads + } +} + +#[cfg(test)] +mod tests { +} diff --git a/runtime/parachains/src/util.rs b/runtime/parachains/src/util.rs index 0f6becfd6005..34946de3e3d2 100644 --- a/runtime/parachains/src/util.rs +++ b/runtime/parachains/src/util.rs @@ -19,7 +19,6 @@ use sp_runtime::traits::{One, Saturating}; use primitives::v1::{Id as ParaId, PersistedValidationData, TransientValidationData}; -use sp_std::prelude::*; use crate::{configuration, paras, router}; @@ -34,7 +33,7 @@ pub fn make_persisted_validation_data( Some(PersistedValidationData { parent_head: >::para_head(¶_id)?, block_number: relay_parent_number, - hrmp_mqc_heads: Vec::new(), + hrmp_mqc_heads: >::hrmp_mqc_heads(para_id), dmq_mqc_head: >::dmq_mqc_head(para_id), }) } diff --git a/runtime/rococo/src/lib.rs b/runtime/rococo/src/lib.rs index 4c5f594fc31d..947f30971743 100644 --- a/runtime/rococo/src/lib.rs +++ b/runtime/rococo/src/lib.rs @@ -532,6 +532,7 @@ impl parachains_paras::Trait for Runtime { } impl parachains_router::Trait for Runtime { + type Origin = Origin; type UmpSink = (); // TODO: #1873 To be handled by the XCM receiver. } diff --git a/runtime/test-runtime/src/lib.rs b/runtime/test-runtime/src/lib.rs index 0ed03f99a6d2..a481ff9ba1b9 100644 --- a/runtime/test-runtime/src/lib.rs +++ b/runtime/test-runtime/src/lib.rs @@ -453,6 +453,7 @@ impl paras::Trait for Runtime { } impl router::Trait for Runtime { + type Origin = Origin; type UmpSink = (); } From 836387400a591df59b5d2d66b4aabe40dc830d1f Mon Sep 17 00:00:00 2001 From: Sergey Shulepov Date: Wed, 4 Nov 2020 11:46:28 +0100 Subject: [PATCH 10/27] HRMP: Test base logic --- runtime/parachains/src/router/hrmp.rs | 332 ++++++++++++++++++++++++++ 1 file changed, 332 insertions(+) diff --git a/runtime/parachains/src/router/hrmp.rs b/runtime/parachains/src/router/hrmp.rs index d1c3fc9ca71e..2819cea52347 100644 --- a/runtime/parachains/src/router/hrmp.rs +++ b/runtime/parachains/src/router/hrmp.rs @@ -664,4 +664,336 @@ impl Module { #[cfg(test)] mod tests { + use super::*; + use crate::router::tests::default_genesis_config; + use crate::mock::{Configuration, System, Paras, Router, new_test_ext}; + use primitives::v1::BlockNumber; + use std::collections::HashSet; + + pub(crate) fn run_to_block(to: BlockNumber, new_session: Option>) { + use frame_support::traits::{OnFinalize as _, OnInitialize as _}; + + while System::block_number() < to { + let b = System::block_number(); + + // NOTE: this is in reverse initialization order. + Router::initializer_finalize(); + Paras::initializer_finalize(); + + System::on_finalize(b); + + System::on_initialize(b + 1); + System::set_block_number(b + 1); + + if new_session.as_ref().map_or(false, |v| v.contains(&(b + 1))) { + Router::initializer_on_new_session(&Default::default()); + Paras::initializer_on_new_session(&Default::default()); + } + + // NOTE: this is in initialization order. + Paras::initializer_initialize(b + 1); + Router::initializer_initialize(b + 1); + } + } + + struct GenesisConfigBuilder { + hrmp_channel_max_capacity: u32, + hrmp_channel_max_message_size: u32, + hrmp_max_parathread_outbound_channels: u32, + hrmp_max_parachain_outbound_channels: u32, + hrmp_max_parathread_inbound_channels: u32, + hrmp_max_parachain_inbound_channels: u32, + hrmp_max_message_num_per_candidate: u32, + hrmp_channel_max_total_size: u32, + } + + impl Default for GenesisConfigBuilder { + fn default() -> Self { + Self { + hrmp_channel_max_capacity: 2, + hrmp_channel_max_message_size: 8, + hrmp_max_parathread_outbound_channels: 1, + hrmp_max_parachain_outbound_channels: 2, + hrmp_max_parathread_inbound_channels: 1, + hrmp_max_parachain_inbound_channels: 2, + hrmp_max_message_num_per_candidate: 2, + hrmp_channel_max_total_size: 16, + } + } + } + + impl GenesisConfigBuilder { + fn build(self) -> crate::mock::GenesisConfig { + let mut genesis = default_genesis_config(); + let config = &mut genesis.configuration.config; + config.hrmp_channel_max_capacity = self.hrmp_channel_max_capacity; + config.hrmp_channel_max_message_size = self.hrmp_channel_max_message_size; + config.hrmp_max_parathread_outbound_channels = + self.hrmp_max_parathread_outbound_channels; + config.hrmp_max_parachain_outbound_channels = self.hrmp_max_parachain_outbound_channels; + config.hrmp_max_parathread_inbound_channels = self.hrmp_max_parathread_inbound_channels; + config.hrmp_max_parachain_inbound_channels = self.hrmp_max_parachain_inbound_channels; + config.hrmp_max_message_num_per_candidate = self.hrmp_max_message_num_per_candidate; + config.hrmp_channel_max_total_size = self.hrmp_channel_max_total_size; + genesis + } + } + + fn register_parachain(id: ParaId) { + Paras::schedule_para_initialize( + id, + crate::paras::ParaGenesisArgs { + parachain: true, + genesis_head: vec![1].into(), + validation_code: vec![1].into(), + }, + ); + } + + fn channel_exists(sender: ParaId, recipient: ParaId) -> bool { + ::HrmpChannels::get(&HrmpChannelId { sender, recipient }).is_some() + } + + fn assert_storage_consistency_exhaustive() { + use frame_support::IterableStorageMap; + + assert_eq!( + ::HrmpOpenChannelRequests::iter() + .map(|(k, _)| k) + .collect::>(), + ::HrmpOpenChannelRequestsList::get() + .into_iter() + .collect::>(), + ); + + // verify that the set of keys in `HrmpOpenChannelRequestCount` corresponds to the set + // of _senders_ in `HrmpOpenChannelRequests`. + // + // having ensured that, we can go ahead and go over all counts and verify that they match. + assert_eq!( + ::HrmpOpenChannelRequestCount::iter() + .map(|(k, _)| k) + .collect::>(), + ::HrmpOpenChannelRequests::iter() + .map(|(k, _)| k.sender) + .collect::>(), + ); + for (open_channel_initiator, expected_num) in + ::HrmpOpenChannelRequestCount::iter() + { + let actual_num = ::HrmpOpenChannelRequests::iter() + .filter(|(ch, _)| ch.sender == open_channel_initiator) + .count() as u32; + assert_eq!(expected_num, actual_num); + } + + // The same as above, but for accepted channel request count. Note that we are interested + // only in confirmed open requests. + assert_eq!( + ::HrmpAcceptedChannelRequestCount::iter() + .map(|(k, _)| k) + .collect::>(), + ::HrmpOpenChannelRequests::iter() + .filter(|(_, v)| v.confirmed) + .map(|(k, _)| k.recipient) + .collect::>(), + ); + for (channel_recipient, expected_num) in + ::HrmpAcceptedChannelRequestCount::iter() + { + let actual_num = ::HrmpOpenChannelRequests::iter() + .filter(|(ch, v)| ch.recipient == channel_recipient && v.confirmed) + .count() as u32; + assert_eq!(expected_num, actual_num); + } + + assert_eq!( + ::HrmpCloseChannelRequests::iter() + .map(|(k, _)| k) + .collect::>(), + ::HrmpCloseChannelRequestsList::get() + .into_iter() + .collect::>(), + ); + + // A HRMP watermark can be None for an onboarded parachain. However, an offboarded parachain + // cannot have an HRMP watermark: it should've been cleanup. + assert_contains_only_onboarded(::HrmpWatermarks::iter().map(|(k, _)| k)); + + // An entry in `HrmpChannels` indicates that the channel is open. Only open channels can + // have contents. + for (non_empty_channel, contents) in ::HrmpChannelContents::iter() { + assert!(::HrmpChannels::contains_key( + &non_empty_channel + )); + + // pedantic check: there should be no empty vectors in storage, those should be modeled + // by a removed kv pair. + assert!(!contents.is_empty()); + } + + // Senders and recipients must be onboarded. Otherwise, all channels associated with them + // are removed. + assert_contains_only_onboarded( + ::HrmpChannels::iter().flat_map(|(k, _)| vec![k.sender, k.recipient]), + ); + + // Check the docs for `HrmpIngressChannelsIndex` and `HrmpEgressChannelsIndex` in decl + // storage to get an index what are the channel mappings indexes. + // + // Here, from indexes. + // + // ingress egress + // + // a -> [x, y] x -> [a, b] + // b -> [x, z] y -> [a] + // z -> [b] + // + // we derive a list of channels they represent. + // + // (a, x) (a, x) + // (a, y) (a, y) + // (b, x) (b, x) + // (b, y) (b, y) + // + // and then that we compare that to the channel list in the `HrmpChannels`. + let channel_set_derived_from_ingress = ::HrmpIngressChannelsIndex::iter() + .flat_map(|(p, v)| v.into_iter().map(|i| (i, p)).collect::>()) + .collect::>(); + let channel_set_derived_from_egress = ::HrmpEgressChannelsIndex::iter() + .flat_map(|(p, v)| v.into_iter().map(|e| (p, e)).collect::>()) + .collect::>(); + let channel_set_ground_truth = ::HrmpChannels::iter() + .map(|(k, _)| (k.sender, k.recipient)) + .collect::>(); + assert_eq!( + channel_set_derived_from_ingress, + channel_set_derived_from_egress + ); + assert_eq!(channel_set_derived_from_egress, channel_set_ground_truth); + + fn assert_contains_only_onboarded(iter: impl Iterator) { + for para in iter { + assert!(Paras::is_valid_para(para)) + } + } + } + + #[test] + fn empty_state_consistent_state() { + new_test_ext(GenesisConfigBuilder::default().build()).execute_with(|| { + assert_storage_consistency_exhaustive(); + }); + } + + #[test] + fn open_channel_works() { + let para_a = 1.into(); + let para_b = 3.into(); + + new_test_ext(GenesisConfigBuilder::default().build()).execute_with(|| { + // We need both A & B to be registered and alive parachains. + register_parachain(para_a); + register_parachain(para_b); + + run_to_block(5, Some(vec![5])); + Router::init_open_channel(para_a, para_b, 2, 8).unwrap(); + assert_storage_consistency_exhaustive(); + + Router::accept_open_channel(para_b, para_a).unwrap(); + assert_storage_consistency_exhaustive(); + + // Advance to a block 6, but without session change. That means that the channel has + // not been created yet. + run_to_block(6, None); + assert!(!channel_exists(para_a, para_b)); + assert_storage_consistency_exhaustive(); + + // Now let the session change happen and thus open the channel. + run_to_block(8, Some(vec![8])); + assert!(channel_exists(para_a, para_b)); + }); + } + + #[test] + fn close_channel_works() { + let para_a = 5.into(); + let para_b = 2.into(); + + new_test_ext(GenesisConfigBuilder::default().build()).execute_with(|| { + register_parachain(para_a); + register_parachain(para_b); + + run_to_block(5, Some(vec![5])); + Router::init_open_channel(para_a, para_b, 2, 8).unwrap(); + Router::accept_open_channel(para_b, para_a).unwrap(); + + run_to_block(6, Some(vec![6])); + assert!(channel_exists(para_a, para_b)); + + // Close the channel. The effect is not immediate, but rather deferred to the next + // session change. + Router::close_channel( + para_b, + HrmpChannelId { + sender: para_a, + recipient: para_b, + }, + ) + .unwrap(); + assert!(channel_exists(para_a, para_b)); + assert_storage_consistency_exhaustive(); + + // After the session change the channel should be closed. + run_to_block(8, Some(vec![8])); + assert!(!channel_exists(para_a, para_b)); + assert_storage_consistency_exhaustive(); + }); + } + + #[test] + fn send_recv_messages() { + let para_a = 32.into(); + let para_b = 64.into(); + + let mut genesis = GenesisConfigBuilder::default(); + genesis.hrmp_channel_max_message_size = 20; + genesis.hrmp_channel_max_total_size = 20; + new_test_ext(genesis.build()).execute_with(|| { + register_parachain(para_a); + register_parachain(para_b); + + run_to_block(5, Some(vec![5])); + Router::init_open_channel(para_a, para_b, 2, 20).unwrap(); + Router::accept_open_channel(para_b, para_a).unwrap(); + + // On Block 6: + // A sends a message to B + run_to_block(6, Some(vec![6])); + assert!(channel_exists(para_a, para_b)); + let msgs = vec![OutboundHrmpMessage { + recipient: para_b, + data: b"this is an emergency".to_vec(), + }]; + let config = Configuration::config(); + assert!(Router::check_outbound_hrmp( + &config, + para_a, + &msgs, + )); + let _ = Router::queue_outbound_hrmp(para_a, msgs); + assert_storage_consistency_exhaustive(); + + // On Block 7: + // B receives the message sent by A. B sets the watermark to 6. + run_to_block(7, None); + assert!(Router::check_hrmp_watermark( + para_b, + 7, + 6, + )); + let _ = Router::prune_hrmp(para_b, 6); + assert_storage_consistency_exhaustive(); + }); + } } From e6a84dc2f5ba795c0ae8e24028bb6606cdf1256c Mon Sep 17 00:00:00 2001 From: Sergey Shulepov Date: Mon, 2 Nov 2020 18:23:36 +0100 Subject: [PATCH 11/27] Update adder collator --- parachain/test-parachains/adder/collator/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/parachain/test-parachains/adder/collator/src/lib.rs b/parachain/test-parachains/adder/collator/src/lib.rs index 7246fb9f63ec..ba2923a752b0 100644 --- a/parachain/test-parachains/adder/collator/src/lib.rs +++ b/parachain/test-parachains/adder/collator/src/lib.rs @@ -114,10 +114,12 @@ impl Collator { let collation = Collation { upward_messages: Vec::new(), + horizontal_messages: Vec::new(), new_validation_code: None, head_data: head_data.encode().into(), proof_of_validity: PoV { block_data: block_data.encode().into() }, processed_downward_messages: 0, + hrmp_watermark: validation_data.persisted.block_number, }; async move { Some(collation) }.boxed() From 19f01ac81cc4ffbb9eb54d382f164ac269171626 Mon Sep 17 00:00:00 2001 From: Sergey Shulepov Date: Tue, 3 Nov 2020 14:25:37 +0100 Subject: [PATCH 12/27] HRMP: Runtime API for accessing inbound messages Also, removing some redundant fully-qualified names. --- node/core/runtime-api/src/lib.rs | 82 ++++++++++++++++++- node/subsystem/src/messages.rs | 9 +- primitives/src/v1.rs | 5 ++ runtime/kusama/src/lib.rs | 10 ++- runtime/parachains/src/router/hrmp.rs | 23 +++++- runtime/parachains/src/runtime_api_impl/v1.rs | 10 ++- runtime/polkadot/src/lib.rs | 11 ++- runtime/rococo/src/lib.rs | 11 ++- runtime/test-runtime/src/lib.rs | 10 ++- runtime/westend/src/lib.rs | 10 ++- 10 files changed, 166 insertions(+), 15 deletions(-) diff --git a/node/core/runtime-api/src/lib.rs b/node/core/runtime-api/src/lib.rs index cd0719ccb79a..cf266e578d14 100644 --- a/node/core/runtime-api/src/lib.rs +++ b/node/core/runtime-api/src/lib.rs @@ -130,6 +130,7 @@ fn make_runtime_api_request( Request::CandidateEvents(sender) => query!(candidate_events(), sender), Request::ValidatorDiscovery(ids, sender) => query!(validator_discovery(ids), sender), Request::DmqContents(id, sender) => query!(dmq_contents(id), sender), + Request::InboundHrmpChannelsContents(id, sender) => query!(inbound_hrmp_channels_contents(id), sender), } } @@ -180,12 +181,11 @@ mod tests { ValidatorId, ValidatorIndex, GroupRotationInfo, CoreState, PersistedValidationData, Id as ParaId, OccupiedCoreAssumption, ValidationData, SessionIndex, ValidationCode, CommittedCandidateReceipt, CandidateEvent, AuthorityDiscoveryId, InboundDownwardMessage, - BlockNumber, + BlockNumber, InboundHrmpMessage, }; use polkadot_node_subsystem_test_helpers as test_helpers; use sp_core::testing::TaskExecutor; - - use std::collections::HashMap; + use std::collections::{HashMap, BTreeMap}; use futures::channel::oneshot; #[derive(Default, Clone)] @@ -201,6 +201,7 @@ mod tests { candidate_pending_availability: HashMap, candidate_events: Vec, dmq_contents: HashMap>, + hrmp_channels: HashMap>>, } impl ProvideRuntimeApi for MockRuntimeApi { @@ -306,9 +307,16 @@ mod tests { fn dmq_contents( &self, recipient: ParaId, - ) -> Vec { + ) -> Vec { self.dmq_contents.get(&recipient).map(|q| q.clone()).unwrap_or_default() } + + fn inbound_hrmp_channels_contents( + &self, + recipient: ParaId + ) -> BTreeMap> { + self.hrmp_channels.get(&recipient).map(|q| q.clone()).unwrap_or_default() + } } } @@ -701,6 +709,72 @@ mod tests { futures::executor::block_on(future::join(subsystem_task, test_task)); } + #[test] + fn requests_inbound_hrmp_channels_contents() { + let (ctx, mut ctx_handle) = test_helpers::make_subsystem_context(TaskExecutor::new()); + + let relay_parent = [1; 32].into(); + let para_a = 99.into(); + let para_b = 66.into(); + let para_c = 33.into(); + + let para_b_inbound_channels = [ + (para_a, vec![]), + ( + para_c, + vec![InboundHrmpMessage { + sent_at: 1, + data: "๐™€=๐™ˆ๐˜พยฒ".as_bytes().to_owned(), + }], + ), + ] + .iter() + .cloned() + .collect::>(); + + let runtime_api = Arc::new({ + let mut runtime_api = MockRuntimeApi::default(); + + runtime_api.hrmp_channels.insert(para_a, BTreeMap::new()); + runtime_api + .hrmp_channels + .insert(para_b, para_b_inbound_channels.clone()); + + runtime_api + }); + + let subsystem = RuntimeApiSubsystem::new(runtime_api.clone(), Metrics(None)); + let subsystem_task = run(ctx, subsystem).map(|x| x.unwrap()); + let test_task = async move { + let (tx, rx) = oneshot::channel(); + ctx_handle + .send(FromOverseer::Communication { + msg: RuntimeApiMessage::Request( + relay_parent, + Request::InboundHrmpChannelsContents(para_a, tx), + ), + }) + .await; + assert_eq!(rx.await.unwrap().unwrap(), BTreeMap::new()); + + let (tx, rx) = oneshot::channel(); + ctx_handle + .send(FromOverseer::Communication { + msg: RuntimeApiMessage::Request( + relay_parent, + Request::InboundHrmpChannelsContents(para_b, tx), + ), + }) + .await; + assert_eq!(rx.await.unwrap().unwrap(), para_b_inbound_channels,); + + ctx_handle + .send(FromOverseer::Signal(OverseerSignal::Conclude)) + .await; + }; + futures::executor::block_on(future::join(subsystem_task, test_task)); + } + #[test] fn requests_historical_code() { let (ctx, mut ctx_handle) = test_helpers::make_subsystem_context(TaskExecutor::new()); diff --git a/node/subsystem/src/messages.rs b/node/subsystem/src/messages.rs index 9198d46c5eb3..0d395f216741 100644 --- a/node/subsystem/src/messages.rs +++ b/node/subsystem/src/messages.rs @@ -37,9 +37,10 @@ use polkadot_primitives::v1::{ GroupRotationInfo, Hash, Id as ParaId, OccupiedCoreAssumption, PersistedValidationData, PoV, SessionIndex, SignedAvailabilityBitfield, ValidationCode, ValidatorId, ValidationData, CandidateHash, - ValidatorIndex, ValidatorSignature, InboundDownwardMessage, + ValidatorIndex, ValidatorSignature, InboundDownwardMessage, InboundHrmpMessage, }; use std::sync::Arc; +use std::collections::btree_map::BTreeMap; /// A notification of a new backed candidate. #[derive(Debug)] @@ -452,6 +453,12 @@ pub enum RuntimeApiRequest { ParaId, RuntimeApiSender>>, ), + /// Get the contents of all channels addressed to the given recipient. Channels that have no + /// messages in them are also included. + InboundHrmpChannelsContents( + ParaId, + RuntimeApiSender>>>, + ), } /// A message to the Runtime API subsystem. diff --git a/primitives/src/v1.rs b/primitives/src/v1.rs index 955f87111977..72af26b546bc 100644 --- a/primitives/src/v1.rs +++ b/primitives/src/v1.rs @@ -17,6 +17,7 @@ //! V1 Primitives. use sp_std::prelude::*; +use sp_std::collections::btree_map::BTreeMap; use parity_scale_codec::{Encode, Decode}; use bitvec::vec::BitVec; @@ -743,6 +744,10 @@ sp_api::decl_runtime_apis! { fn dmq_contents( recipient: Id, ) -> Vec>; + + /// Get the contents of all channels addressed to the given recipient. Channels that have no + /// messages in them are also included. + fn inbound_hrmp_channels_contents(recipient: Id) -> BTreeMap>>; } } diff --git a/runtime/kusama/src/lib.rs b/runtime/kusama/src/lib.rs index 0add25c1da1d..39193d0dc75b 100644 --- a/runtime/kusama/src/lib.rs +++ b/runtime/kusama/src/lib.rs @@ -22,12 +22,14 @@ use pallet_transaction_payment::CurrencyAdapter; use sp_std::prelude::*; +use sp_std::collections::btree_map::BTreeMap; use sp_core::u32_trait::{_1, _2, _3, _5}; use codec::{Encode, Decode}; use primitives::v1::{ AccountId, AccountIndex, Balance, BlockNumber, CandidateEvent, CommittedCandidateReceipt, CoreState, GroupRotationInfo, Hash, Id, Moment, Nonce, OccupiedCoreAssumption, PersistedValidationData, Signature, ValidationCode, ValidationData, ValidatorId, ValidatorIndex, + InboundDownwardMessage, InboundHrmpMessage, }; use runtime_common::{ claims, SlowAdjustingFeeUpdate, CurrencyToVote, @@ -1112,9 +1114,15 @@ sp_api::impl_runtime_apis! { fn dmq_contents( _recipient: Id, - ) -> Vec> { + ) -> Vec> { Vec::new() } + + fn inbound_hrmp_channels_contents( + _recipient: Id + ) -> BTreeMap>> { + BTreeMap::new() + } } impl fg_primitives::GrandpaApi for Runtime { diff --git a/runtime/parachains/src/router/hrmp.rs b/runtime/parachains/src/router/hrmp.rs index 2819cea52347..8549e001479f 100644 --- a/runtime/parachains/src/router/hrmp.rs +++ b/runtime/parachains/src/router/hrmp.rs @@ -26,7 +26,7 @@ use primitives::v1::{ SessionIndex, }; use sp_runtime::traits::{BlakeTwo256, Hash as HashT}; -use sp_std::collections::btree_set::BTreeSet; +use sp_std::collections::{btree_set::BTreeSet, btree_map::BTreeMap}; use sp_std::mem; use sp_std::prelude::*; @@ -62,10 +62,10 @@ pub struct HrmpChannel { /// The maximum message size that could be put into the channel. pub max_message_size: u32, /// The current number of messages pending in the channel. - /// Invariant: should be less or equal to `max_capacity`.s`. + /// Invariant: should be less or equal to `max_capacity`.s`. pub msg_count: u32, /// The total size in bytes of all message payloads in the channel. - /// Invariant: should be less or equal to `max_total_size`. + /// Invariant: should be less or equal to `max_total_size`. pub total_size: u32, /// A head of the Message Queue Chain for this channel. Each link in this chain has a form: /// `(prev_head, B, H(M))`, where @@ -660,6 +660,23 @@ impl Module { mqc_heads } + + /// Returns contents of all channels addressed to the given recipient. Channels that have no + /// messages in them are also included. + pub(crate) fn inbound_hrmp_channels_contents( + recipient: ParaId, + ) -> BTreeMap>> { + let sender_set = ::HrmpIngressChannelsIndex::get(&recipient); + + let mut inbound_hrmp_channels_contents = BTreeMap::new(); + for sender in sender_set { + let channel_contents = + ::HrmpChannelContents::get(&HrmpChannelId { sender, recipient }); + inbound_hrmp_channels_contents.insert(sender, channel_contents); + } + + inbound_hrmp_channels_contents + } } #[cfg(test)] diff --git a/runtime/parachains/src/runtime_api_impl/v1.rs b/runtime/parachains/src/runtime_api_impl/v1.rs index 896205626934..1f7050661fc8 100644 --- a/runtime/parachains/src/runtime_api_impl/v1.rs +++ b/runtime/parachains/src/runtime_api_impl/v1.rs @@ -18,12 +18,13 @@ //! functions. use sp_std::prelude::*; +use sp_std::collections::btree_map::BTreeMap; use primitives::v1::{ ValidatorId, ValidatorIndex, GroupRotationInfo, CoreState, ValidationData, Id as ParaId, OccupiedCoreAssumption, SessionIndex, ValidationCode, CommittedCandidateReceipt, ScheduledCore, OccupiedCore, CoreOccupied, CoreIndex, GroupIndex, CandidateEvent, PersistedValidationData, AuthorityDiscoveryId, - InboundDownwardMessage, + InboundDownwardMessage, InboundHrmpMessage, }; use sp_runtime::traits::Zero; use frame_support::debug; @@ -328,3 +329,10 @@ pub fn dmq_contents( ) -> Vec> { >::dmq_contents(recipient) } + +/// Implementation for the `inbound_hrmp_channels_contents` function of the runtime API. +pub fn inbound_hrmp_channels_contents( + recipient: ParaId, +) -> BTreeMap>> { + >::inbound_hrmp_channels_contents(recipient) +} diff --git a/runtime/polkadot/src/lib.rs b/runtime/polkadot/src/lib.rs index fe4baf3b9f63..a18a63b14758 100644 --- a/runtime/polkadot/src/lib.rs +++ b/runtime/polkadot/src/lib.rs @@ -30,12 +30,14 @@ use runtime_common::{ }; use sp_std::prelude::*; +use sp_std::collections::btree_map::BTreeMap; use sp_core::u32_trait::{_1, _2, _3, _4, _5}; use codec::{Encode, Decode}; use primitives::v1::{ AccountId, AccountIndex, Balance, BlockNumber, CandidateEvent, CommittedCandidateReceipt, CoreState, GroupRotationInfo, Hash, Id, Moment, Nonce, OccupiedCoreAssumption, PersistedValidationData, Signature, ValidationCode, ValidationData, ValidatorId, ValidatorIndex, + InboundDownwardMessage, InboundHrmpMessage, }; use sp_runtime::{ create_runtime_str, generic, impl_opaque_keys, ModuleId, ApplyExtrinsicResult, @@ -1106,9 +1108,16 @@ sp_api::impl_runtime_apis! { fn dmq_contents( _recipient: Id, - ) -> Vec> { + ) -> Vec> { Vec::new() } + + fn inbound_hrmp_channels_contents( + _recipient: Id + ) -> BTreeMap>> { + BTreeMap::new() + } + } impl fg_primitives::GrandpaApi for Runtime { diff --git a/runtime/rococo/src/lib.rs b/runtime/rococo/src/lib.rs index 947f30971743..d9c2a839c4f1 100644 --- a/runtime/rococo/src/lib.rs +++ b/runtime/rococo/src/lib.rs @@ -22,12 +22,13 @@ use pallet_transaction_payment::CurrencyAdapter; use sp_std::prelude::*; +use sp_std::collections::btree_map::BTreeMap; use codec::Encode; use primitives::v1::{ AccountId, AccountIndex, Balance, BlockNumber, Hash, Nonce, Signature, Moment, GroupRotationInfo, CoreState, Id, ValidationData, ValidationCode, CandidateEvent, ValidatorId, ValidatorIndex, CommittedCandidateReceipt, OccupiedCoreAssumption, - PersistedValidationData, + PersistedValidationData, InboundDownwardMessage, InboundHrmpMessage, }; use runtime_common::{ SlowAdjustingFeeUpdate, @@ -682,9 +683,15 @@ sp_api::impl_runtime_apis! { runtime_api_impl::validator_discovery::(validators) } - fn dmq_contents(recipient: Id) -> Vec> { + fn dmq_contents(recipient: Id) -> Vec> { runtime_api_impl::dmq_contents::(recipient) } + + fn inbound_hrmp_channels_contents( + recipient: Id + ) -> BTreeMap>> { + runtime_api_impl::inbound_hrmp_channels_contents::(recipient) + } } impl fg_primitives::GrandpaApi for Runtime { diff --git a/runtime/test-runtime/src/lib.rs b/runtime/test-runtime/src/lib.rs index a481ff9ba1b9..c597710a73c7 100644 --- a/runtime/test-runtime/src/lib.rs +++ b/runtime/test-runtime/src/lib.rs @@ -22,6 +22,7 @@ use pallet_transaction_payment::CurrencyAdapter; use sp_std::prelude::*; +use sp_std::collections::btree_map::BTreeMap; use codec::Encode; use polkadot_runtime_parachains::{ configuration, @@ -36,6 +37,7 @@ use primitives::v1::{ AccountId, AccountIndex, Balance, BlockNumber, CandidateEvent, CommittedCandidateReceipt, CoreState, GroupRotationInfo, Hash as HashT, Id as ParaId, Moment, Nonce, OccupiedCoreAssumption, PersistedValidationData, Signature, ValidationCode, ValidationData, ValidatorId, ValidatorIndex, + InboundDownwardMessage, InboundHrmpMessage, }; use runtime_common::{ claims, SlowAdjustingFeeUpdate, paras_sudo_wrapper, @@ -669,9 +671,15 @@ sp_api::impl_runtime_apis! { fn dmq_contents( recipient: ParaId, - ) -> Vec> { + ) -> Vec> { runtime_impl::dmq_contents::(recipient) } + + fn inbound_hrmp_channels_contents( + recipient: ParaId, + ) -> BTreeMap>> { + runtime_impl::inbound_hrmp_channels_contents::(recipient) + } } impl fg_primitives::GrandpaApi for Runtime { diff --git a/runtime/westend/src/lib.rs b/runtime/westend/src/lib.rs index 754780ce2030..42df9f7ba338 100644 --- a/runtime/westend/src/lib.rs +++ b/runtime/westend/src/lib.rs @@ -22,11 +22,13 @@ use pallet_transaction_payment::CurrencyAdapter; use sp_std::prelude::*; +use sp_std::collections::btree_map::BTreeMap; use codec::{Encode, Decode}; use primitives::v1::{ AccountId, AccountIndex, Balance, BlockNumber, CandidateEvent, CommittedCandidateReceipt, CoreState, GroupRotationInfo, Hash, Id, Moment, Nonce, OccupiedCoreAssumption, PersistedValidationData, Signature, ValidationCode, ValidationData, ValidatorId, ValidatorIndex, + InboundDownwardMessage, InboundHrmpMessage, }; use runtime_common::{ SlowAdjustingFeeUpdate, CurrencyToVote, @@ -856,9 +858,15 @@ sp_api::impl_runtime_apis! { fn dmq_contents( _recipient: Id, - ) -> Vec> { + ) -> Vec> { Vec::new() } + + fn inbound_hrmp_channels_contents( + _recipient: Id + ) -> BTreeMap>> { + BTreeMap::new() + } } impl fg_primitives::GrandpaApi for Runtime { From d61440ba6936d1fc5bb21c086285b42f171dcaab Mon Sep 17 00:00:00 2001 From: Sergey Shulepov Date: Tue, 3 Nov 2020 15:15:48 +0100 Subject: [PATCH 13/27] HRMP: Add diagnostic logging in acceptance criteria --- runtime/parachains/src/router/hrmp.rs | 91 ++++++++++++++++++++++----- 1 file changed, 75 insertions(+), 16 deletions(-) diff --git a/runtime/parachains/src/router/hrmp.rs b/runtime/parachains/src/router/hrmp.rs index 8549e001479f..94a5d846131a 100644 --- a/runtime/parachains/src/router/hrmp.rs +++ b/runtime/parachains/src/router/hrmp.rs @@ -20,7 +20,9 @@ use crate::{ paras, }; use codec::{Decode, Encode}; -use frame_support::{traits::Get, weights::Weight, StorageMap, StorageValue, ensure}; +use frame_support::{ + traits::Get, weights::Weight, StorageMap, StorageValue, ensure, debug::native as log, +}; use primitives::v1::{ Balance, Hash, HrmpChannelId, Id as ParaId, InboundHrmpMessage, OutboundHrmpMessage, SessionIndex, @@ -77,6 +79,8 @@ pub struct HrmpChannel { pub mqc_head: Option, } +const LOG_TARGET: &str = "runtime-parachains::hrmp"; + /// Routines and getters related to HRMP. impl Module { /// Remove all storage entries associated with the given para. @@ -249,10 +253,22 @@ impl Module { // not be greater than the relay-chain context block which the parablock refers to. if let Some(last_watermark) = ::HrmpWatermarks::get(&recipient) { if new_hrmp_watermark <= last_watermark { + log::warn!( + target: LOG_TARGET, + "the HRMP watermark is not advanced relative to the last watermark ({} > {})", + new_hrmp_watermark, + last_watermark, + ); return false; } } if new_hrmp_watermark > relay_chain_parent_number { + log::warn!( + target: LOG_TARGET, + "the HRMP watermark is ahead the relay-parent ({} > {})", + new_hrmp_watermark, + relay_chain_parent_number, + ); return false; } @@ -264,9 +280,17 @@ impl Module { true } else { let digest = ::HrmpChannelDigests::get(&recipient); - digest + if !digest .binary_search_by_key(&new_hrmp_watermark, |(block_no, _)| *block_no) .is_ok() + { + log::warn!( + target: LOG_TARGET, + "the HRMP watermark ({}) doesn't land on a block with messages received", + new_hrmp_watermark, + ); + } + true } } @@ -276,17 +300,28 @@ impl Module { out_hrmp_msgs: &[OutboundHrmpMessage], ) -> bool { if out_hrmp_msgs.len() as u32 > config.hrmp_max_message_num_per_candidate { + log::warn!( + target: LOG_TARGET, + "more HRMP messages than permitted by config ({} > {})", + out_hrmp_msgs.len(), + config.hrmp_max_message_num_per_candidate, + ); return false; } let mut last_recipient = None::; - for out_msg in out_hrmp_msgs { + for (idx, out_msg) in out_hrmp_msgs.iter().enumerate() { match last_recipient { // the messages must be sorted in ascending order and there must be no two messages sent // to the same recipient. Thus we can check that every recipient is strictly greater than // the previous one. Some(last_recipient) if out_msg.recipient <= last_recipient => { + log::warn!( + target: LOG_TARGET, + "the HRMP messages are not sorted (at index {})", + idx, + ); return false; } _ => last_recipient = Some(out_msg.recipient), @@ -299,18 +334,50 @@ impl Module { let channel = match ::HrmpChannels::get(&channel_id) { Some(channel) => channel, - None => return false, + None => { + log::warn!( + target: LOG_TARGET, + "the HRMP message at index {} is sent to a non existent channel {}->{}", + idx, + channel_id.sender, + channel_id.recipient, + ); + return false; + } }; if out_msg.data.len() as u32 > channel.max_message_size { + log::warn!( + target: LOG_TARGET, + "the HRMP message at index {} exceeds the negotiated channel maximum message size ({} > {})", + idx, + out_msg.data.len(), + channel.max_message_size, + ); return false; } - if channel.total_size + out_msg.data.len() as u32 > channel.max_total_size { + let new_total_size = channel.total_size + out_msg.data.len() as u32; + if new_total_size > channel.max_total_size { + log::warn!( + target: LOG_TARGET, + "sending the HRMP message at index {} would exceed the neogitiated channel total size ({} > {})", + idx, + new_total_size, + channel.max_total_size, + ); return false; } - if channel.msg_count + 1 > channel.max_capacity { + let new_msg_count = channel.msg_count + 1; + if new_msg_count > channel.max_capacity { + log::warn!( + target: LOG_TARGET, + "sending the HRMP message at index {} would exceed the neogitiated channel capacity ({} > {})", + idx, + new_msg_count, + channel.max_capacity, + ); return false; } } @@ -993,22 +1060,14 @@ mod tests { data: b"this is an emergency".to_vec(), }]; let config = Configuration::config(); - assert!(Router::check_outbound_hrmp( - &config, - para_a, - &msgs, - )); + assert!(Router::check_outbound_hrmp(&config, para_a, &msgs,)); let _ = Router::queue_outbound_hrmp(para_a, msgs); assert_storage_consistency_exhaustive(); // On Block 7: // B receives the message sent by A. B sets the watermark to 6. run_to_block(7, None); - assert!(Router::check_hrmp_watermark( - para_b, - 7, - 6, - )); + assert!(Router::check_hrmp_watermark(para_b, 7, 6,)); let _ = Router::prune_hrmp(para_b, 6); assert_storage_consistency_exhaustive(); }); From 71aa0bb20e6b9bd9e3bdd6277b1d337555e09456 Mon Sep 17 00:00:00 2001 From: Sergey Shulepov Date: Wed, 4 Nov 2020 17:43:14 +0100 Subject: [PATCH 14/27] HRMP: Additional tests --- runtime/parachains/src/router/hrmp.rs | 134 ++++++++++++++++++++++++-- 1 file changed, 128 insertions(+), 6 deletions(-) diff --git a/runtime/parachains/src/router/hrmp.rs b/runtime/parachains/src/router/hrmp.rs index 94a5d846131a..23379eefc9ca 100644 --- a/runtime/parachains/src/router/hrmp.rs +++ b/runtime/parachains/src/router/hrmp.rs @@ -752,7 +752,7 @@ mod tests { use crate::router::tests::default_genesis_config; use crate::mock::{Configuration, System, Paras, Router, new_test_ext}; use primitives::v1::BlockNumber; - use std::collections::HashSet; + use std::collections::{BTreeMap, HashSet}; pub(crate) fn run_to_block(to: BlockNumber, new_session: Option>) { use frame_support::traits::{OnFinalize as _, OnInitialize as _}; @@ -770,8 +770,9 @@ mod tests { System::set_block_number(b + 1); if new_session.as_ref().map_or(false, |v| v.contains(&(b + 1))) { - Router::initializer_on_new_session(&Default::default()); + // NOTE: this is in initialization order. Paras::initializer_on_new_session(&Default::default()); + Router::initializer_on_new_session(&Default::default()); } // NOTE: this is in initialization order. @@ -823,6 +824,9 @@ mod tests { } } + // TODO: + // - rename: schedule_initialize + // - impl Iterator fn register_parachain(id: ParaId) { Paras::schedule_para_initialize( id, @@ -834,6 +838,10 @@ mod tests { ); } + fn deregister_parachain(id: ParaId) { + Paras::schedule_para_cleanup(id); + } + fn channel_exists(sender: ParaId, recipient: ParaId) -> bool { ::HrmpChannels::get(&HrmpChannelId { sender, recipient }).is_some() } @@ -902,7 +910,10 @@ mod tests { // A HRMP watermark can be None for an onboarded parachain. However, an offboarded parachain // cannot have an HRMP watermark: it should've been cleanup. - assert_contains_only_onboarded(::HrmpWatermarks::iter().map(|(k, _)| k)); + assert_contains_only_onboarded( + ::HrmpWatermarks::iter().map(|(k, _)| k), + "HRMP watermarks should contain only onboarded paras", + ); // An entry in `HrmpChannels` indicates that the channel is open. Only open channels can // have contents. @@ -920,6 +931,7 @@ mod tests { // are removed. assert_contains_only_onboarded( ::HrmpChannels::iter().flat_map(|(k, _)| vec![k.sender, k.recipient]), + "senders and recipients in all channels should be onboarded", ); // Check the docs for `HrmpIngressChannelsIndex` and `HrmpEgressChannelsIndex` in decl @@ -956,13 +968,33 @@ mod tests { ); assert_eq!(channel_set_derived_from_egress, channel_set_ground_truth); - fn assert_contains_only_onboarded(iter: impl Iterator) { + ::HrmpIngressChannelsIndex::iter() + .map(|(_, v)| v) + .for_each(|v| assert_is_sorted(&v, "HrmpIngressChannelsIndex")); + ::HrmpEgressChannelsIndex::iter() + .map(|(_, v)| v) + .for_each(|v| assert_is_sorted(&v, "HrmpIngressChannelsIndex")); + + fn assert_contains_only_onboarded(iter: impl Iterator, cause: &str) { for para in iter { - assert!(Paras::is_valid_para(para)) + assert!( + Paras::is_valid_para(para), + "{}: {} para is offboarded", + cause, + para + ); } } } + fn assert_is_sorted(slice: &[T], id: &str) { + assert!( + slice.windows(2).all(|xs| xs[0] <= xs[1]), + "{} supposed to be sorted", + id + ); + } + #[test] fn empty_state_consistent_state() { new_test_ext(GenesisConfigBuilder::default().build()).execute_with(|| { @@ -1060,7 +1092,7 @@ mod tests { data: b"this is an emergency".to_vec(), }]; let config = Configuration::config(); - assert!(Router::check_outbound_hrmp(&config, para_a, &msgs,)); + assert!(Router::check_outbound_hrmp(&config, para_a, &msgs)); let _ = Router::queue_outbound_hrmp(para_a, msgs); assert_storage_consistency_exhaustive(); @@ -1072,4 +1104,94 @@ mod tests { assert_storage_consistency_exhaustive(); }); } + + #[test] + fn accept_incoming_request_and_offboard() { + let para_a = 32.into(); + let para_b = 64.into(); + + new_test_ext(GenesisConfigBuilder::default().build()).execute_with(|| { + register_parachain(para_a); + register_parachain(para_b); + + run_to_block(5, Some(vec![5])); + Router::init_open_channel(para_a, para_b, 2, 8).unwrap(); + Router::accept_open_channel(para_b, para_a).unwrap(); + deregister_parachain(para_a); + + // On Block 6: session change. The channel should not be created. + run_to_block(6, Some(vec![6])); + assert!(!Paras::is_valid_para(para_a)); + assert!(!channel_exists(para_a, para_b)); + assert_storage_consistency_exhaustive(); + }); + } + + #[test] + fn check_sent_messages() { + let para_a = 32.into(); + let para_b = 64.into(); + let para_c = 97.into(); + + new_test_ext(GenesisConfigBuilder::default().build()).execute_with(|| { + register_parachain(para_a); + register_parachain(para_b); + register_parachain(para_c); + + run_to_block(5, Some(vec![5])); + + // Open two channels to the same receiver, b: + // a -> b, c -> b + Router::init_open_channel(para_a, para_b, 2, 8).unwrap(); + Router::accept_open_channel(para_b, para_a).unwrap(); + Router::init_open_channel(para_c, para_b, 2, 8).unwrap(); + Router::accept_open_channel(para_b, para_c).unwrap(); + + // On Block 6: session change. + run_to_block(6, Some(vec![6])); + assert!(Paras::is_valid_para(para_a)); + + let msgs = vec![OutboundHrmpMessage { + recipient: para_b, + data: b"knock".to_vec(), + }]; + let config = Configuration::config(); + assert!(Router::check_outbound_hrmp(&config, para_a, &msgs)); + let _ = Router::queue_outbound_hrmp(para_a, msgs.clone()); + + // Verify that the sent messages are there and that also the empty channels are present. + let mqc_heads = Router::hrmp_mqc_heads(para_b); + let contents = Router::inbound_hrmp_channels_contents(para_b); + assert_eq!( + contents, + vec![ + ( + para_a, + vec![InboundHrmpMessage { + sent_at: 6, + data: b"knock".to_vec(), + }] + ), + (para_c, vec![]) + ] + .into_iter() + .collect::>(), + ); + assert_eq!( + mqc_heads, + vec![ + ( + para_a, + hex_literal::hex!( + "3bba6404e59c91f51deb2ae78f1273ebe75896850713e13f8c0eba4b0996c483" + ) + .into() + ), + (para_c, Default::default()) + ], + ); + + assert_storage_consistency_exhaustive(); + }); + } } From f4b03ceaad3065fc598ba242534096080f7a1f20 Mon Sep 17 00:00:00 2001 From: Sergey Shulepov Date: Thu, 5 Nov 2020 11:45:42 +0100 Subject: [PATCH 15/27] Self-review fixes --- core-primitives/src/lib.rs | 1 + parachain/src/primitives.rs | 9 ++++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/core-primitives/src/lib.rs b/core-primitives/src/lib.rs index e29457f05b96..fc325985e0f6 100644 --- a/core-primitives/src/lib.rs +++ b/core-primitives/src/lib.rs @@ -113,6 +113,7 @@ pub struct InboundHrmpMessage { pub data: sp_std::vec::Vec, } +/// An HRMP message seen from the perspective of a sender. #[derive(codec::Encode, codec::Decode, Clone, sp_runtime::RuntimeDebug, PartialEq, Eq, Hash)] pub struct OutboundHrmpMessage { /// The para that will get this message in its downward message queue. diff --git a/parachain/src/primitives.rs b/parachain/src/primitives.rs index c1ae971f10fe..421d1d38315a 100644 --- a/parachain/src/primitives.rs +++ b/parachain/src/primitives.rs @@ -186,7 +186,14 @@ impl AccountIdConversion for Id { } } -/// A type that uniquely identifies an HRMP channel. An HRMP channel is unidirectional +/// A type that uniquely identifies an HRMP channel. An HRMP channel is established between two paras. +/// In text, we use the notation `(A, B)` to specify a channel between A and B. The channels are +/// unidirectional, meaning that `(A, B)` and `(B, A)` refer to different channels. The convention is +/// that we use the first item tuple for the sender and the second for the recipient. Only one channel +/// is allowed between two participants in one direction, i.e. there cannot be 2 different channels +/// identified by `(A, B)`. +/// +/// `HrmpChannelId` has a defined ordering: first `sender` and tie is resolved by `recipient`. #[derive(Clone, PartialEq, Eq, Encode, Decode, RuntimeDebug)] #[cfg_attr(feature = "std", derive(Hash))] pub struct HrmpChannelId { From 0fe6c9a7652999ef3ac230253fc2cbdab3c3cc23 Mon Sep 17 00:00:00 2001 From: Sergey Shulepov Date: Thu, 5 Nov 2020 12:09:45 +0100 Subject: [PATCH 16/27] save test refactorings for the next time --- runtime/parachains/src/router/hrmp.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/runtime/parachains/src/router/hrmp.rs b/runtime/parachains/src/router/hrmp.rs index 23379eefc9ca..493ce55c5897 100644 --- a/runtime/parachains/src/router/hrmp.rs +++ b/runtime/parachains/src/router/hrmp.rs @@ -824,9 +824,6 @@ mod tests { } } - // TODO: - // - rename: schedule_initialize - // - impl Iterator fn register_parachain(id: ParaId) { Paras::schedule_para_initialize( id, From 0dce9f881175a5d8816daf6f1a130e94e44f467d Mon Sep 17 00:00:00 2001 From: Sergey Shulepov Date: Thu, 5 Nov 2020 12:23:44 +0100 Subject: [PATCH 17/27] Missed a return statement. --- runtime/parachains/src/router/hrmp.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/runtime/parachains/src/router/hrmp.rs b/runtime/parachains/src/router/hrmp.rs index 493ce55c5897..834abf94ca34 100644 --- a/runtime/parachains/src/router/hrmp.rs +++ b/runtime/parachains/src/router/hrmp.rs @@ -289,6 +289,7 @@ impl Module { "the HRMP watermark ({}) doesn't land on a block with messages received", new_hrmp_watermark, ); + return false; } true } From 35553271f6e6f95a700eb50b8b9a6fafa71c0d3d Mon Sep 17 00:00:00 2001 From: Sergey Shulepov Date: Thu, 5 Nov 2020 12:26:00 +0100 Subject: [PATCH 18/27] a formatting blip --- runtime/parachains/src/router/hrmp.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/parachains/src/router/hrmp.rs b/runtime/parachains/src/router/hrmp.rs index 834abf94ca34..c625ccda530e 100644 --- a/runtime/parachains/src/router/hrmp.rs +++ b/runtime/parachains/src/router/hrmp.rs @@ -1097,7 +1097,7 @@ mod tests { // On Block 7: // B receives the message sent by A. B sets the watermark to 6. run_to_block(7, None); - assert!(Router::check_hrmp_watermark(para_b, 7, 6,)); + assert!(Router::check_hrmp_watermark(para_b, 7, 6)); let _ = Router::prune_hrmp(para_b, 6); assert_storage_consistency_exhaustive(); }); From af7707519104d600118ea9c89328a1fdc0b8e4c1 Mon Sep 17 00:00:00 2001 From: Sergey Shulepov Date: Thu, 5 Nov 2020 15:53:38 +0100 Subject: [PATCH 19/27] Add missing logic for appending HRMP digests --- runtime/parachains/src/router/hrmp.rs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/runtime/parachains/src/router/hrmp.rs b/runtime/parachains/src/router/hrmp.rs index c625ccda530e..f29c34c03361 100644 --- a/runtime/parachains/src/router/hrmp.rs +++ b/runtime/parachains/src/router/hrmp.rs @@ -494,6 +494,31 @@ impl Module { ::HrmpChannels::insert(&channel_id, channel); ::HrmpChannelContents::append(&channel_id, inbound); + // The digests are sorted in ascending by block number order. Assuming absence of + // contextual execution, there are only two possible scenarios here: + // + // (a) It's the first time anybody sends a message to this recipient within this block. + // In this case, the digest vector would be empty or the block number of the latest + // entry is smaller than the current. + // + // (b) Somebody has already sent a message within the current block. That means that + // the block number of the latest entry is equal to the current. + // + // Note that having the latest entry greater than the current block number is a logical + // error. + let mut recipient_digest = + ::HrmpChannelDigests::get(&channel_id.recipient); + if let Some(cur_block_digest) = recipient_digest + .last_mut() + .filter(|(block_no, _)| *block_no == now) + .map(|(_, ref mut d)| d) + { + cur_block_digest.push(sender); + } else { + recipient_digest.push((now, vec![sender])); + } + ::HrmpChannelDigests::insert(&channel_id.recipient, recipient_digest); + weight += T::DbWeight::get().reads_writes(2, 2); } From 1b2368bdd99c3d8e4f9065a12d4b9ef854008bb1 Mon Sep 17 00:00:00 2001 From: Sergey Shulepov Date: Thu, 5 Nov 2020 15:55:50 +0100 Subject: [PATCH 20/27] Remove the channel contents vectors which became empty --- runtime/parachains/src/router/hrmp.rs | 44 +++++++++++++-------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/runtime/parachains/src/router/hrmp.rs b/runtime/parachains/src/router/hrmp.rs index f29c34c03361..19fd0397ed99 100644 --- a/runtime/parachains/src/router/hrmp.rs +++ b/runtime/parachains/src/router/hrmp.rs @@ -393,15 +393,15 @@ impl Module { // message to this parachain between the old and new watermarks. let senders = ::HrmpChannelDigests::mutate(&recipient, |digest| { let mut senders = BTreeSet::new(); - let mut residue = Vec::with_capacity(digest.len()); + let mut leftover = Vec::with_capacity(digest.len()); for (block_no, paras_sent_msg) in mem::replace(digest, Vec::new()) { if block_no <= new_hrmp_watermark { senders.extend(paras_sent_msg); } else { - residue.push((block_no, paras_sent_msg)); + leftover.push((block_no, paras_sent_msg)); } } - *digest = residue; + *digest = leftover; senders }); weight += T::DbWeight::get().reads_writes(1, 1); @@ -413,29 +413,29 @@ impl Module { for channel_id in channels_to_prune { // prune each channel up to the new watermark keeping track how many messages we removed // and what is the total byte size of them. - let (cnt, size) = - ::HrmpChannelContents::mutate(&channel_id, |outbound_messages| { - let (mut cnt, mut size) = (0, 0); - - let mut residue = Vec::with_capacity(outbound_messages.len()); - for msg in mem::replace(outbound_messages, Vec::new()).into_iter() { - if msg.sent_at <= new_hrmp_watermark { - cnt += 1; - size += msg.data.len(); - } else { - residue.push(msg); - } - } - *outbound_messages = residue; - - (cnt, size) - }); + let (mut pruned_cnt, mut pruned_size) = (0, 0); + + let contents = ::HrmpChannelContents::get(&channel_id); + let mut leftover = Vec::with_capacity(contents.len()); + for msg in contents { + if msg.sent_at <= new_hrmp_watermark { + pruned_cnt += 1; + pruned_size += msg.data.len(); + } else { + leftover.push(msg); + } + } + if !leftover.is_empty() { + ::HrmpChannelContents::insert(&channel_id, leftover); + } else { + ::HrmpChannelContents::remove(&channel_id); + } // update the channel metadata. ::HrmpChannels::mutate(&channel_id, |channel| { if let Some(ref mut channel) = channel { - channel.msg_count -= cnt as u32; - channel.total_size -= size as u32; + channel.msg_count -= pruned_cnt as u32; + channel.total_size -= pruned_size as u32; } }); From 02945d9e783d123a09e8e08f5ee40c70e7efbecc Mon Sep 17 00:00:00 2001 From: Sergey Shulepov Date: Thu, 5 Nov 2020 16:29:10 +0100 Subject: [PATCH 21/27] Tighten HRMP channel digests invariants. --- .../implementers-guide/src/runtime/router.md | 5 +++- runtime/parachains/src/router.rs | 5 +++- runtime/parachains/src/router/hrmp.rs | 24 +++++++++++++++++++ 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/roadmap/implementers-guide/src/runtime/router.md b/roadmap/implementers-guide/src/runtime/router.md index 15cd37258e0c..6ed4df75fd5b 100644 --- a/roadmap/implementers-guide/src/runtime/router.md +++ b/roadmap/implementers-guide/src/runtime/router.md @@ -172,7 +172,10 @@ HrmpEgressChannelsIndex: map ParaId => Vec; HrmpChannelContents: map HrmpChannelId => Vec; /// Maintains a mapping that can be used to answer the question: /// What paras sent a message at the given block number for a given reciever. -/// Invariant: The para ids vector is never empty. +/// Invariants: +/// - The para ids vector is never empty. +/// - The para ids vector doesn't contain duplicates. +/// - The outer vector is sorted ascending by block number and there are no duplicates. HrmpChannelDigests: map ParaId => Vec<(BlockNumber, Vec)>; ``` diff --git a/runtime/parachains/src/router.rs b/runtime/parachains/src/router.rs index 5955e910cd7d..5f2d4a3c07b0 100644 --- a/runtime/parachains/src/router.rs +++ b/runtime/parachains/src/router.rs @@ -168,7 +168,10 @@ decl_storage! { HrmpChannelContents: map hasher(twox_64_concat) HrmpChannelId => Vec>; /// Maintains a mapping that can be used to answer the question: /// What paras sent a message at the given block number for a given reciever. - /// Invariant: The para ids vector is never empty. + /// Invariants: + /// - The para ids vector is never empty. + /// - The para ids vector doesn't contain duplicates. + /// - The outer vector is sorted ascending by block number and there are no duplicates. HrmpChannelDigests: map hasher(twox_64_concat) ParaId => Vec<(T::BlockNumber, Vec)>; } } diff --git a/runtime/parachains/src/router/hrmp.rs b/runtime/parachains/src/router/hrmp.rs index 19fd0397ed99..8666071faa6f 100644 --- a/runtime/parachains/src/router/hrmp.rs +++ b/runtime/parachains/src/router/hrmp.rs @@ -998,6 +998,30 @@ mod tests { .map(|(_, v)| v) .for_each(|v| assert_is_sorted(&v, "HrmpIngressChannelsIndex")); + assert_contains_only_onboarded( + ::HrmpChannelDigests::iter().map(|(k, _)| k), + "HRMP channel digests should contain only onboarded paras", + ); + for (_digest_for_para, digest) in ::HrmpChannelDigests::iter() { + // Assert that items are in **strictly** ascending order. The strictness also implies + // there are no duplicates. + assert!(digest.windows(2).all(|xs| xs[0].0 < xs[1].0)); + + for (_, mut senders) in digest { + assert!(!senders.is_empty()); + + // check for duplicates. For that we sort the vector, then perform deduplication. + // if the vector stayed the same, there are no duplicates. + senders.sort(); + let orig_senders = senders.clone(); + senders.dedup(); + assert_eq!( + orig_senders, senders, + "duplicates removed implies existence of duplicates" + ); + } + } + fn assert_contains_only_onboarded(iter: impl Iterator, cause: &str) { for para in iter { assert!( From 8448078f732ab1ef7afbffb71064f67ba3c02c73 Mon Sep 17 00:00:00 2001 From: Sergei Shulepov Date: Fri, 6 Nov 2020 12:56:50 +0100 Subject: [PATCH 22/27] Apply suggestions from code review Co-authored-by: Peter Goodspeed-Niklaus --- roadmap/implementers-guide/src/runtime/inclusion.md | 2 +- roadmap/implementers-guide/src/runtime/router.md | 4 +--- runtime/parachains/src/router/hrmp.rs | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/roadmap/implementers-guide/src/runtime/inclusion.md b/roadmap/implementers-guide/src/runtime/inclusion.md index aca0534f7c09..46f3e5211674 100644 --- a/roadmap/implementers-guide/src/runtime/inclusion.md +++ b/roadmap/implementers-guide/src/runtime/inclusion.md @@ -70,7 +70,7 @@ All failed checks should lead to an unrecoverable error making the block invalid 1. call `Router::check_upward_messages(para, commitments.upward_messages)` to check that the upward messages are valid. 1. call `Router::check_processed_downward_messages(para, commitments.processed_downward_messages)` to check that the DMQ is properly drained. 1. call `Router::check_hrmp_watermark(para, commitments.hrmp_watermark)` for each candidate to check rules of processing the HRMP watermark. - 1. using `Router::check_outbound_hrmp(sender, commitments.horizontal_messages)` ensure that the each candidate send a valid set of horizontal messages + 1. using `Router::check_outbound_hrmp(sender, commitments.horizontal_messages)` ensure that the each candidate sent a valid set of horizontal messages 1. create an entry in the `PendingAvailability` map for each backed candidate with a blank `availability_votes` bitfield. 1. create a corresponding entry in the `PendingAvailabilityCommitments` with the commitments. 1. Return a `Vec` of all scheduled cores of the list of passed assignments that a candidate was successfully backed for, sorted ascending by CoreIndex. diff --git a/roadmap/implementers-guide/src/runtime/router.md b/roadmap/implementers-guide/src/runtime/router.md index 6ed4df75fd5b..de410f469548 100644 --- a/roadmap/implementers-guide/src/runtime/router.md +++ b/roadmap/implementers-guide/src/runtime/router.md @@ -92,8 +92,6 @@ struct HrmpChannel { sender_deposit: Balance, /// The amount that the recipient supplied as a deposit when accepting opening this channel. recipient_deposit: Balance, - /// The maximum message size that could be put into the channel. - limit_message_size: u32, /// The maximum number of messages that can be pending in the channel at once. max_capacity: u32, /// The maximum total size of the messages that can be pending in the channel at once. @@ -173,7 +171,7 @@ HrmpChannelContents: map HrmpChannelId => Vec; /// Maintains a mapping that can be used to answer the question: /// What paras sent a message at the given block number for a given reciever. /// Invariants: -/// - The para ids vector is never empty. +/// - The inner `Vec` is never empty. /// - The para ids vector doesn't contain duplicates. /// - The outer vector is sorted ascending by block number and there are no duplicates. HrmpChannelDigests: map ParaId => Vec<(BlockNumber, Vec)>; diff --git a/runtime/parachains/src/router/hrmp.rs b/runtime/parachains/src/router/hrmp.rs index 8666071faa6f..4b56af297f17 100644 --- a/runtime/parachains/src/router/hrmp.rs +++ b/runtime/parachains/src/router/hrmp.rs @@ -973,7 +973,7 @@ mod tests { // (a, x) (a, x) // (a, y) (a, y) // (b, x) (b, x) - // (b, y) (b, y) + // (b, z) (b, z) // // and then that we compare that to the channel list in the `HrmpChannels`. let channel_set_derived_from_ingress = ::HrmpIngressChannelsIndex::iter() From ee5078fc65f295b4e961fe3298b42fe07fcd97d4 Mon Sep 17 00:00:00 2001 From: Sergey Shulepov Date: Fri, 6 Nov 2020 12:49:15 +0100 Subject: [PATCH 23/27] Remove a note about sorting for channel id --- parachain/src/primitives.rs | 2 -- roadmap/implementers-guide/src/types/messages.md | 2 -- 2 files changed, 4 deletions(-) diff --git a/parachain/src/primitives.rs b/parachain/src/primitives.rs index 421d1d38315a..da49f26d302e 100644 --- a/parachain/src/primitives.rs +++ b/parachain/src/primitives.rs @@ -192,8 +192,6 @@ impl AccountIdConversion for Id { /// that we use the first item tuple for the sender and the second for the recipient. Only one channel /// is allowed between two participants in one direction, i.e. there cannot be 2 different channels /// identified by `(A, B)`. -/// -/// `HrmpChannelId` has a defined ordering: first `sender` and tie is resolved by `recipient`. #[derive(Clone, PartialEq, Eq, Encode, Decode, RuntimeDebug)] #[cfg_attr(feature = "std", derive(Hash))] pub struct HrmpChannelId { diff --git a/roadmap/implementers-guide/src/types/messages.md b/roadmap/implementers-guide/src/types/messages.md index 2462b969165e..8ea58d14e85d 100644 --- a/roadmap/implementers-guide/src/types/messages.md +++ b/roadmap/implementers-guide/src/types/messages.md @@ -42,8 +42,6 @@ that we use the first item tuple for the sender and the second for the recipient is allowed between two participants in one direction, i.e. there cannot be 2 different channels identified by `(A, B)`. -`HrmpChannelId` has a defined ordering: first `sender` and tie is resolved by `recipient`. - ```rust,ignore struct HrmpChannelId { sender: ParaId, From 37c1fdd817023a74b6285d95bf092e6041dd145a Mon Sep 17 00:00:00 2001 From: Sergey Shulepov Date: Fri, 6 Nov 2020 13:05:06 +0100 Subject: [PATCH 24/27] Add missing rustdocs to the configuration --- runtime/parachains/src/configuration.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/runtime/parachains/src/configuration.rs b/runtime/parachains/src/configuration.rs index 49e54b2c3191..716005cdada0 100644 --- a/runtime/parachains/src/configuration.rs +++ b/runtime/parachains/src/configuration.rs @@ -303,6 +303,7 @@ decl_module! { Ok(()) } + /// Sets the number of sessions after which an HRMP open channel request expires. #[weight = (1_000, DispatchClass::Operational)] pub fn set_hrmp_open_request_ttl(origin, new: u32) -> DispatchResult { ensure_root(origin)?; @@ -312,6 +313,7 @@ decl_module! { Ok(()) } + /// Sets the amount of funds that the sender should provide for opening an HRMP channel. #[weight = (1_000, DispatchClass::Operational)] pub fn set_hrmp_sender_deposit(origin, new: Balance) -> DispatchResult { ensure_root(origin)?; @@ -321,6 +323,8 @@ decl_module! { Ok(()) } + /// Sets the amount of funds that the recipient should provide for accepting opening an HRMP + /// channel. #[weight = (1_000, DispatchClass::Operational)] pub fn set_hrmp_recipient_deposit(origin, new: Balance) -> DispatchResult { ensure_root(origin)?; @@ -330,6 +334,7 @@ decl_module! { Ok(()) } + /// Sets the maximum number of messages allowed in an HRMP channel at once. #[weight = (1_000, DispatchClass::Operational)] pub fn set_hrmp_channel_max_capacity(origin, new: u32) -> DispatchResult { ensure_root(origin)?; @@ -339,6 +344,7 @@ decl_module! { Ok(()) } + /// Sets the maximum total size of messages in bytes allowed in an HRMP channel at once. #[weight = (1_000, DispatchClass::Operational)] pub fn set_hrmp_channel_max_total_size(origin, new: u32) -> DispatchResult { ensure_root(origin)?; @@ -348,6 +354,7 @@ decl_module! { Ok(()) } + /// Sets the maximum number of inbound HRMP channels a parachain is allowed to accept. #[weight = (1_000, DispatchClass::Operational)] pub fn set_hrmp_max_parachain_inbound_channels(origin, new: u32) -> DispatchResult { ensure_root(origin)?; @@ -357,6 +364,7 @@ decl_module! { Ok(()) } + /// Sets the maximum number of inbound HRMP channels a parathread is allowed to accept. #[weight = (1_000, DispatchClass::Operational)] pub fn set_hrmp_max_parathread_inbound_channels(origin, new: u32) -> DispatchResult { ensure_root(origin)?; @@ -366,6 +374,7 @@ decl_module! { Ok(()) } + /// Sets the maximum size of a message that could ever be put into an HRMP channel. #[weight = (1_000, DispatchClass::Operational)] pub fn set_hrmp_channel_max_message_size(origin, new: u32) -> DispatchResult { ensure_root(origin)?; @@ -375,6 +384,7 @@ decl_module! { Ok(()) } + /// Sets the maximum number of outbound HRMP channels a parachain is allowed to open. #[weight = (1_000, DispatchClass::Operational)] pub fn set_hrmp_max_parachain_outbound_channels(origin, new: u32) -> DispatchResult { ensure_root(origin)?; @@ -384,6 +394,7 @@ decl_module! { Ok(()) } + /// Sets the maximum number of outbound HRMP channels a parathread is allowed to open. #[weight = (1_000, DispatchClass::Operational)] pub fn set_hrmp_max_parathread_outbound_channels(origin, new: u32) -> DispatchResult { ensure_root(origin)?; @@ -393,6 +404,7 @@ decl_module! { Ok(()) } + /// Sets the maximum number of outbound HRMP messages can be sent by a candidate. #[weight = (1_000, DispatchClass::Operational)] pub fn set_hrmp_max_message_num_per_candidate(origin, new: u32) -> DispatchResult { ensure_root(origin)?; From be1013ce03076d125aa41d1516ac0c7ba4ba9b4f Mon Sep 17 00:00:00 2001 From: Sergey Shulepov Date: Fri, 6 Nov 2020 13:10:18 +0100 Subject: [PATCH 25/27] Clarify and update the invariant for HrmpChannelDigests --- roadmap/implementers-guide/src/runtime/router.md | 5 +++-- runtime/parachains/src/router.rs | 7 ++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/roadmap/implementers-guide/src/runtime/router.md b/roadmap/implementers-guide/src/runtime/router.md index de410f469548..9aadf9997c4e 100644 --- a/roadmap/implementers-guide/src/runtime/router.md +++ b/roadmap/implementers-guide/src/runtime/router.md @@ -172,8 +172,9 @@ HrmpChannelContents: map HrmpChannelId => Vec; /// What paras sent a message at the given block number for a given reciever. /// Invariants: /// - The inner `Vec` is never empty. -/// - The para ids vector doesn't contain duplicates. -/// - The outer vector is sorted ascending by block number and there are no duplicates. +/// - The inner `Vec` cannot store two same `ParaId`. +/// - The outer vector is sorted ascending by block number and cannot store two items with the same +/// block number. HrmpChannelDigests: map ParaId => Vec<(BlockNumber, Vec)>; ``` diff --git a/runtime/parachains/src/router.rs b/runtime/parachains/src/router.rs index 5f2d4a3c07b0..eeb989f00919 100644 --- a/runtime/parachains/src/router.rs +++ b/runtime/parachains/src/router.rs @@ -169,9 +169,10 @@ decl_storage! { /// Maintains a mapping that can be used to answer the question: /// What paras sent a message at the given block number for a given reciever. /// Invariants: - /// - The para ids vector is never empty. - /// - The para ids vector doesn't contain duplicates. - /// - The outer vector is sorted ascending by block number and there are no duplicates. + /// - The inner `Vec` is never empty. + /// - The inner `Vec` cannot store two same `ParaId`. + /// - The outer vector is sorted ascending by block number and cannot store two items with the same + /// block number. HrmpChannelDigests: map hasher(twox_64_concat) ParaId => Vec<(T::BlockNumber, Vec)>; } } From e546c3c906a46672c72bab98dcdb3fa5f6d78811 Mon Sep 17 00:00:00 2001 From: Sergey Shulepov Date: Fri, 6 Nov 2020 13:19:04 +0100 Subject: [PATCH 26/27] Make the onboarding invariant less sloppy Namely, introduce `Paras::is_valid_para` (in fact, it already is present in the implementation) and hook up the invariant to that. Note that this says "within a session" because I don't want to make it super strict on the session boundary. The logic on the session boundary should be extremely careful. --- roadmap/implementers-guide/src/runtime/paras.md | 1 + roadmap/implementers-guide/src/runtime/router.md | 5 ++++- runtime/parachains/src/router.rs | 5 ++++- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/roadmap/implementers-guide/src/runtime/paras.md b/roadmap/implementers-guide/src/runtime/paras.md index dbb169af1751..0958c88d510f 100644 --- a/roadmap/implementers-guide/src/runtime/paras.md +++ b/roadmap/implementers-guide/src/runtime/paras.md @@ -111,6 +111,7 @@ OutgoingParas: Vec; * `note_new_head(ParaId, HeadData, BlockNumber)`: note that a para has progressed to a new head, where the new head was executed in the context of a relay-chain block with given number. This will apply pending code upgrades based on the block number provided. * `validation_code_at(ParaId, at: BlockNumber, assume_intermediate: Option)`: Fetches the validation code to be used when validating a block in the context of the given relay-chain height. A second block number parameter may be used to tell the lookup to proceed as if an intermediate parablock has been included at the given relay-chain height. This may return past, current, or (with certain choices of `assume_intermediate`) future code. `assume_intermediate`, if provided, must be before `at`. If the validation code has been pruned, this will return `None`. * `is_parathread(ParaId) -> bool`: Returns true if the para ID references any live parathread. +* `is_valid_para(ParaId) -> bool`: Returns true if the para ID references either a live parathread or live parachain. * `last_code_upgrade(id: ParaId, include_future: bool) -> Option`: The block number of the last scheduled upgrade of the requested para. Includes future upgrades if the flag is set. This is the `expected_at` number, not the `activated_at` number. * `persisted_validation_data(id: ParaId) -> Option`: Get the PersistedValidationData of the given para, assuming the context is the parent block. Returns `None` if the para is not known. diff --git a/roadmap/implementers-guide/src/runtime/router.md b/roadmap/implementers-guide/src/runtime/router.md index 9aadf9997c4e..ef7ce8ceb7bd 100644 --- a/roadmap/implementers-guide/src/runtime/router.md +++ b/roadmap/implementers-guide/src/runtime/router.md @@ -146,9 +146,12 @@ HrmpCloseChannelRequests: map HrmpChannelId => Option<()>; HrmpCloseChannelRequestsList: Vec; /// The HRMP watermark associated with each para. -/// Invariant: every para should be onboarded. +/// Invariant: +/// - each para `P` used here as a key should satisfy `Paras::is_valid_para(P)` within a session. HrmpWatermarks: map ParaId => Option; /// HRMP channel data associated with each para. +/// Invariant: +/// - each participant in the channel should satisfy `Paras::is_valid_para(P)` within a session. HrmpChannels: map HrmpChannelId => Option; /// Ingress/egress indexes allow to find all the senders and receivers given the opposite /// side. I.e. diff --git a/runtime/parachains/src/router.rs b/runtime/parachains/src/router.rs index eeb989f00919..508e4da59051 100644 --- a/runtime/parachains/src/router.rs +++ b/runtime/parachains/src/router.rs @@ -144,9 +144,12 @@ decl_storage! { HrmpCloseChannelRequestsList: Vec; /// The HRMP watermark associated with each para. - /// Invariant: every para should be onboarded. + /// Invariant: + /// - each para `P` used here as a key should satisfy `Paras::is_valid_para(P)` within a session. HrmpWatermarks: map hasher(twox_64_concat) ParaId => Option; /// HRMP channel data associated with each para. + /// Invariant: + /// - each participant in the channel should satisfy `Paras::is_valid_para(P)` within a session. HrmpChannels: map hasher(twox_64_concat) HrmpChannelId => Option; /// Ingress/egress indexes allow to find all the senders and receivers given the opposite /// side. I.e. From 9e6e3c269ff3b33569a001dcc68c4c8ac5b58cf9 Mon Sep 17 00:00:00 2001 From: Sergey Shulepov Date: Fri, 6 Nov 2020 13:23:17 +0100 Subject: [PATCH 27/27] Make `CandidateCheckContext` use T::BlockNumber for hrmp_watermark --- runtime/parachains/src/inclusion.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/runtime/parachains/src/inclusion.rs b/runtime/parachains/src/inclusion.rs index c9cb162181da..b685be1bf3e8 100644 --- a/runtime/parachains/src/inclusion.rs +++ b/runtime/parachains/src/inclusion.rs @@ -419,7 +419,7 @@ impl Module { &candidate.candidate.commitments.new_validation_code, candidate.candidate.commitments.processed_downward_messages, &candidate.candidate.commitments.upward_messages, - candidate.candidate.commitments.hrmp_watermark, + T::BlockNumber::from(candidate.candidate.commitments.hrmp_watermark), &candidate.candidate.commitments.horizontal_messages, )?; @@ -554,7 +554,7 @@ impl Module { &validation_outputs.new_validation_code, validation_outputs.processed_downward_messages, &validation_outputs.upward_messages, - validation_outputs.hrmp_watermark, + T::BlockNumber::from(validation_outputs.hrmp_watermark), &validation_outputs.horizontal_messages, ) } @@ -718,7 +718,7 @@ impl CandidateCheckContext { new_validation_code: &Option, processed_downward_messages: u32, upward_messages: &[primitives::v1::UpwardMessage], - hrmp_watermark: primitives::v1::BlockNumber, + hrmp_watermark: T::BlockNumber, horizontal_messages: &[primitives::v1::OutboundHrmpMessage], ) -> Result<(), DispatchError> { ensure!( @@ -761,8 +761,7 @@ impl CandidateCheckContext { >::check_hrmp_watermark( para_id, self.relay_parent_number, - // TODO: Hmm, we should settle on a single represenation of T::BlockNumber - T::BlockNumber::from(hrmp_watermark), + hrmp_watermark, ), Error::::HrmpWatermarkMishandling, );