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

UMP, HRMP: CandidateCommitments size limits #1869

Closed
pepyakin opened this issue Oct 28, 2020 · 0 comments · Fixed by #1900
Closed

UMP, HRMP: CandidateCommitments size limits #1869

pepyakin opened this issue Oct 28, 2020 · 0 comments · Fixed by #1900

Comments

@pepyakin
Copy link
Contributor

pepyakin commented Oct 28, 2020

basically, currently, at the moment CandidateCommitments is mostly of constant size with exception of messages. HRMP and UMP outbound messages inflate the candidate commitments. HRMP has some upper bound of size: a parachain can open a limited number of channels and the sizes of messages are limited as well. UMP has only a limit on number of messages and these messages can be of any size.

Therefore, if I am not mistaken, this means that CandidateCommitments can be inflated as well. Not sure, which would be the next limit (perhaps networking? or do we have a special limit for CandidateCommitments?), but I think we are better off introducing a dedicated limit or limits, that would effectively limit the overall size taken in candidate commitments.

Now, apart from the size concern there is also the weight. But now after dekindification (#1702, also tackled in this PR), all the messages are byte blobs anyway, so UMP delegates this to a higher level. UMP dispatching aside, we need to weigh all this machinery that propels it.

I had this in mind and I was planning to discuss this on the second call with you (Rob) and Al, but that never happened and then I forgot until I was writing these docs.

Originally posted by @pepyakin in #1679 (comment)

@pepyakin pepyakin mentioned this issue Oct 28, 2020
2 tasks
@pepyakin pepyakin added this to Design in Batch: Message-passing Oct 28, 2020
pepyakin added a commit that referenced this issue Oct 28, 2020
This commit addresses the UMP part of #1869
pepyakin added a commit that referenced this issue Oct 28, 2020
This commit addresses the HRMP part of and closes #1869
pepyakin added a commit that referenced this issue Oct 28, 2020
This commit addresses the HRMP part of and closes #1869
pepyakin added a commit that referenced this issue Oct 28, 2020
pepyakin added a commit that referenced this issue Oct 29, 2020
This commit addresses the UMP part of #1869
pepyakin added a commit that referenced this issue Oct 29, 2020
This commit addresses the UMP part of #1869
pepyakin added a commit that referenced this issue Oct 29, 2020
ghost pushed a commit that referenced this issue Nov 2, 2020
* UMP: Update the impl guide

* UMP: Incorporate XCM related changes into the guide

* UMP: Data structures and configuration

* UMP: Initial plumbing

* UMP: Data layout

* UMP: Acceptance criteria & enactment

* UMP: Fix dispatcher bug and add the test for it

* UMP: Constrain the maximum size of an UMP message

This commit addresses the UMP part of #1869

* Fix failing test due to misconfiguration

* Make the type of RelayDispatchQueueSize be more apparent in the guide

* Revert renaming `max_upward_queue_capacity` to `max_upward_queue_count`

* convert spaces to tabs

Co-authored-by: Bernhard Schuster <bernhard@ahoi.io>

* Update runtime/parachains/src/router/ump.rs

Co-authored-by: Bernhard Schuster <bernhard@ahoi.io>

Co-authored-by: Bernhard Schuster <bernhard@ahoi.io>
pepyakin added a commit that referenced this issue Nov 2, 2020
@pepyakin pepyakin mentioned this issue Nov 2, 2020
3 tasks
pepyakin added a commit that referenced this issue Nov 4, 2020
pepyakin added a commit that referenced this issue Nov 5, 2020
@ghost ghost closed this as completed in #1900 Nov 6, 2020
ghost pushed a commit that referenced this issue Nov 6, 2020
* HRMP: Update the impl guide

* HRMP: Incorporate the channel notifications into the guide

* HRMP: Renaming in the impl guide

* HRMP: Constrain the maximum number of HRMP messages per candidate

This commit addresses the HRMP part of #1869

* XCM: Introduce HRMP related message types

* HRMP: Data structures and plumbing

* HRMP: Configuration

* HRMP: Data layout

* HRMP: Acceptance & Enactment

* HRMP: Test base logic

* Update adder collator

* HRMP: Runtime API for accessing inbound messages

Also, removing some redundant fully-qualified names.

* HRMP: Add diagnostic logging in acceptance criteria

* HRMP: Additional tests

* Self-review fixes

* save test refactorings for the next time

* Missed a return statement.

* a formatting blip

* Add missing logic for appending HRMP digests

* Remove the channel contents vectors which became empty

* Tighten HRMP channel digests invariants.

* Apply suggestions from code review

Co-authored-by: Peter Goodspeed-Niklaus <coriolinus@users.noreply.github.com>

* Remove a note about sorting for channel id

* Add missing rustdocs to the configuration

* Clarify and update the invariant for HrmpChannelDigests

* 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.

* Make `CandidateCheckContext` use T::BlockNumber for hrmp_watermark

Co-authored-by: Peter Goodspeed-Niklaus <coriolinus@users.noreply.github.com>
@pepyakin pepyakin moved this from Design to Done in Batch: Message-passing Nov 6, 2020
ggwpez pushed a commit to ggwpez/runtimes that referenced this issue Mar 10, 2023
* UMP: Update the impl guide

* UMP: Incorporate XCM related changes into the guide

* UMP: Data structures and configuration

* UMP: Initial plumbing

* UMP: Data layout

* UMP: Acceptance criteria & enactment

* UMP: Fix dispatcher bug and add the test for it

* UMP: Constrain the maximum size of an UMP message

This commit addresses the UMP part of paritytech/polkadot#1869

* Fix failing test due to misconfiguration

* Make the type of RelayDispatchQueueSize be more apparent in the guide

* Revert renaming `max_upward_queue_capacity` to `max_upward_queue_count`

* convert spaces to tabs

Co-authored-by: Bernhard Schuster <bernhard@ahoi.io>

* Update runtime/parachains/src/router/ump.rs

Co-authored-by: Bernhard Schuster <bernhard@ahoi.io>

Co-authored-by: Bernhard Schuster <bernhard@ahoi.io>
ggwpez pushed a commit to ggwpez/runtimes that referenced this issue Mar 10, 2023
* HRMP: Update the impl guide

* HRMP: Incorporate the channel notifications into the guide

* HRMP: Renaming in the impl guide

* HRMP: Constrain the maximum number of HRMP messages per candidate

This commit addresses the HRMP part of paritytech/polkadot#1869

* XCM: Introduce HRMP related message types

* HRMP: Data structures and plumbing

* HRMP: Configuration

* HRMP: Data layout

* HRMP: Acceptance & Enactment

* HRMP: Test base logic

* Update adder collator

* HRMP: Runtime API for accessing inbound messages

Also, removing some redundant fully-qualified names.

* HRMP: Add diagnostic logging in acceptance criteria

* HRMP: Additional tests

* Self-review fixes

* save test refactorings for the next time

* Missed a return statement.

* a formatting blip

* Add missing logic for appending HRMP digests

* Remove the channel contents vectors which became empty

* Tighten HRMP channel digests invariants.

* Apply suggestions from code review

Co-authored-by: Peter Goodspeed-Niklaus <coriolinus@users.noreply.github.com>

* Remove a note about sorting for channel id

* Add missing rustdocs to the configuration

* Clarify and update the invariant for HrmpChannelDigests

* 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.

* Make `CandidateCheckContext` use T::BlockNumber for hrmp_watermark

Co-authored-by: Peter Goodspeed-Niklaus <coriolinus@users.noreply.github.com>
ggwpez pushed a commit to ggwpez/runtimes that referenced this issue Jul 13, 2023
* HRMP: Update the impl guide

* HRMP: Incorporate the channel notifications into the guide

* HRMP: Renaming in the impl guide

* HRMP: Constrain the maximum number of HRMP messages per candidate

This commit addresses the HRMP part of paritytech/polkadot#1869

* XCM: Introduce HRMP related message types

* HRMP: Data structures and plumbing

* HRMP: Configuration

* HRMP: Data layout

* HRMP: Acceptance & Enactment

* HRMP: Test base logic

* Update adder collator

* HRMP: Runtime API for accessing inbound messages

Also, removing some redundant fully-qualified names.

* HRMP: Add diagnostic logging in acceptance criteria

* HRMP: Additional tests

* Self-review fixes

* save test refactorings for the next time

* Missed a return statement.

* a formatting blip

* Add missing logic for appending HRMP digests

* Remove the channel contents vectors which became empty

* Tighten HRMP channel digests invariants.

* Apply suggestions from code review

Co-authored-by: Peter Goodspeed-Niklaus <coriolinus@users.noreply.github.com>

* Remove a note about sorting for channel id

* Add missing rustdocs to the configuration

* Clarify and update the invariant for HrmpChannelDigests

* 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.

* Make `CandidateCheckContext` use T::BlockNumber for hrmp_watermark

Co-authored-by: Peter Goodspeed-Niklaus <coriolinus@users.noreply.github.com>
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

1 participant