Cleanup UMP signal handling#12240
Conversation
Make intent clear and correct.
|
All GitHub workflows were cancelled due to failure one of the required jobs. |
mchristou
left a comment
mchristou
left a comment
There was a problem hiding this comment.
Nice refactor of UMP signal handling!
| /// `peer_id` is a plain (non-`Option`) type, so the contract is "always override". Was not my | ||
| /// original idea (had optional override in mind), but it is fine either way. |
There was a problem hiding this comment.
This looks like a leftover
There was a problem hiding this comment.
Not really, but if I am the only one understanding it, we can remove the comment 🙈
There was a problem hiding this comment.
Feel free to merge, if that's the only nit and just remove the comment.
| /// | ||
| /// `peer_id` is a plain (non-`Option`) type, so the contract is "always override". Was not my | ||
| /// original idea (had optional override in mind), but it is fine either way. | ||
| pub fn from_resubmission(signed_info: &SignedSchedulingInfo) -> Self { |
There was a problem hiding this comment.
nit: from_scheduling_info is better IMO.
| /// Signed `claim_queue_offset` exceeds the runtime cap. The resubmission override takes the | ||
| /// offset from the signed payload, bypassing the in-block check `pallet_parachain_system` | ||
| /// applies to the block's own `CoreInfo` digest — so we re-apply the bound here, else a | ||
| /// resubmitter could sign an out-of-range offset. |
There was a problem hiding this comment.
nit: this is too verbose and talks about applying the bound, but this is just error variant definition.. I'd just say that this error is emitted when the scheduling proof offset override is larger than what runtime enforces as maximum.
| if signals.select_core.replace((selector, offset)).is_some() { | ||
| panic!( | ||
| "Parachain emitted more than one `SelectCore` UMP signal; \ | ||
| only the last block of a PoV may emit one" |
There was a problem hiding this comment.
We should remove "only the last block of a PoV may emit one" which make sense for block bundling, but this error is generic.
There was a problem hiding this comment.
Not sure what you mean here.
3 participants
No description provided.