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

Conversation

pepyakin
Copy link
Contributor

@pepyakin pepyakin commented Nov 2, 2020

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.

@github-actions github-actions bot added the A3-in_progress Pull request is in progress. No review needed at this stage. label Nov 2, 2020
@pepyakin pepyakin mentioned this pull request Nov 2, 2020
2 tasks
@pepyakin pepyakin added B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Nov 3, 2020
@pepyakin pepyakin marked this pull request as ready for review November 4, 2020 17:10
@pepyakin pepyakin removed the A3-in_progress Pull request is in progress. No review needed at this stage. label Nov 4, 2020
@github-actions github-actions bot added the A0-please_review Pull request needs code review. label Nov 4, 2020
Copy link
Contributor

@rphmeier rphmeier left a 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

pepyakin and others added 6 commits November 6, 2020 12:56
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.
@pepyakin
Copy link
Contributor Author

pepyakin commented Nov 6, 2020

bot merge

@ghost
Copy link

ghost commented Nov 6, 2020

Trying merge.

@ghost ghost merged commit 7355366 into master Nov 6, 2020
@ghost ghost deleted the ser-hrmp branch November 6, 2020 15:35
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 {
Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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 🐻

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
4 participants