-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Implement HRMP #1900
Conversation
This commit addresses the HRMP part of #1869
Also, removing some redundant fully-qualified names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some comments, but looks OK
Co-authored-by: Peter Goodspeed-Niklaus <coriolinus@users.noreply.github.com>
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.
bot merge |
Trying merge. |
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 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be preferred to have platform independent integer width length for len
, since this is exported.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess, this won't harm, but note that this code is for wasm only and we assume that it's 32 bits only.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's just a beartrap in the woods, soon to be forgotten of its existence, and somebody will step into it eventually - it might not be a 🐻
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR subsumes the Router PR: #1679
Closes #1400
Closes #1642
Closes #1663
Closes #1704
Closes #1806
Closes #1869
Regarding #1663 I decided to go with a simple implementation: just to send a DM to notify paras. For now, it makes sense to start with a simpler approach to see how it works and then extend it as needed. My concern about this solution was that these notifications should be processed in a timely manner but a downward message queue can be clogged. Well, that's a bit too forward looking since most likely the queues won't be filled in the foreseeable future (assuming we can upgrade later). Apart from that, if that becomes an issue, it is possible for the PVF to enact messages out-of-order therefore getting some sort of QoS. But again I doubt that this will be required for now.