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

Conversation

@montekki
Copy link
Contributor

@montekki montekki commented Jun 8, 2020

Biggest question is how the upward message queues work when parachains' code is changed on session boundaries?

Fixes #1138

@montekki montekki requested a review from rphmeier June 8, 2020 12:21
@rphmeier
Copy link
Contributor

rphmeier commented Jun 8, 2020

Biggest question is how the upward message queues work when parachains' code is changed on session boundaries?

What case in specific are you worried about?

What do you think about introducing a Routing module for upwards/downwards messages and then later XCMP?

@montekki
Copy link
Contributor Author

montekki commented Jun 8, 2020

What case in specific are you worried about?

Suppose we have messages [msg1 .. msgN] enqueued and the runtime code changes and it no longer accept some (if not all) of those messages.

What do you think about introducing a Routing module for upwards/downwards messages and then later XCMP?

That would probably be reasonable, but I need to better understand the difference between XCMP and upwards/downwards.

@rphmeier
Copy link
Contributor

rphmeier commented Jun 8, 2020

Suppose we have messages [msg1 .. msgN] enqueued and the runtime code changes and it no longer accept some (if not all) of those messages.

Valid concern, but it is easier to handle this at the parachain level. Parachain runtimes need to discard messages they cannot interpret anyway, and in practice they will take care to maintain backwards compatibility.

@gavofyork gavofyork added the A3-in_progress Pull request is in progress. No review needed at this stage. label Jun 9, 2020
@rphmeier
Copy link
Contributor

rphmeier commented Jun 9, 2020

Some discussion on queue limit sizes (I believe described as Watermarks` in existing code) would be helpful

@montekki montekki added the B0-silent Changes should not be mentioned in any release notes label Jun 15, 2020
@montekki montekki marked this pull request as ready for review June 15, 2020 11:24
@montekki
Copy link
Contributor Author

@rphmeier The note_new_head now has HeadData as a parameter type and the messages are not passed in any way if I am not mistaken, I think that this should change.

* `MAX_QUEUE_COUNT` - Total number of messages allowed in the parachain -> relay-chain message queue
* `WATERMARK_QUEUE_SIZE` - Total size of messages allowed in the parachain -> relay-chain message queue before which no further messages may be added to it.

In other words, if the queue contains more messages or the total message size in the queue is greater than these contants, `note_new_head` whill fail on the stage of checking the new head.
Copy link
Contributor

Choose a reason for hiding this comment

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

note_new_head happens after availability, whereas we need this check to happen before backing. It's not clear what should happen if note_new_head is fallible - so a candidate could be backed, become available, but then get dropped from the chain because of a queue size check that should have been done earlier?

So it's clear that this size check should be done as part of the process_candidates function of the Inclusion module.

But also, this logic doesn't belong in the paras module. What does it have to do with the head of the chain and validation code upgrades? This logic should go in a new Routing module which will handle upwards/downwards messages and later XCMP logic

@montekki montekki closed this Jun 17, 2020
@montekki montekki mentioned this pull request Jun 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

A3-in_progress Pull request is in progress. No review needed at this stage. B0-silent Changes should not be mentioned in any release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Guide: Upwards Messages

4 participants