Breakdown the Router module on Dmp, Ump, Hrmp modules #1939
Conversation
pepyakin
added
A0-please_review
Now we have: DMP, UMP and Router module.
pepyakin
force-pushed
the
ser-breakdown-router
branch
from
3d738b5
to
536a914
Compare
|
borked rebase, converting into draft. UPD. fixed |
Co-authored-by: Bernhard Schuster <bernhard@ahoi.io>
Co-authored-by: Bernhard Schuster <bernhard@ahoi.io>
There was a problem hiding this comment.
Overall seems fine with just minor nits. The only real question I have: from the commit messages, it seems like you extracted the DMP, UMP, HRMP modules from existing implementations. However, I don't see where the previous implementation has been removed. If there is dead code remaining, it would be good to get rid of it.
|
|
||
| Candidate Acceptance Function: | ||
|
|
||
| * `check_upward_messages(P: ParaId, Vec<UpwardMessage>`): |
There was a problem hiding this comment.
messages should be named.
There was a problem hiding this comment.
should it though? Isn't it already clear that when we say "the messages" it refers to those?
I am not entirely against that, but in that case we should probably address it throughout the whole doc
|
|
||
| The following routine is intended to be called in the same time when `Paras::schedule_para_cleanup` is called. | ||
|
|
||
| `schedule_para_cleanup(ParaId)`: |
There was a problem hiding this comment.
again, the same logic as here
|
I've addressed some of the uncontroversial comments. The router module implementation was removed in this commit Additionally, I've noticed that hrmp.rs has quite a lot of repetition, all these HRMP prefixes/postfixes. I will get rid of em' but in a separate PR. |
|
Yeah, @coriolinus you were right! That commit did not include removal of the (sub-)modules themselves. |
|
bot merge |
|
Trying merge. |
ghost
merged commit 
Even though this PR looks big it actually isn't. What we do here is we split the Router module on three separate independent modules: Dmp, Ump and Hrmp, and update tests and plumbing. I tried to collate a proper commit history so I'd recommend going over all commits.
As I said in our parachains team call, there is a not so nice duplication of
OutgoingParas. On the bonus side, the external modules (such as registrar) now can call into a single entrypoint rather than calling both Router and Parasschedule_para_cleanup. In future, we would need to tackle this.