Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Runtime: unify and harden UMP signal checks in check_core_index #6179

Closed
sandreim opened this issue Oct 22, 2024 · 3 comments · Fixed by #6217
Closed

Runtime: unify and harden UMP signal checks in check_core_index #6179

sandreim opened this issue Oct 22, 2024 · 3 comments · Fixed by #6217
Assignees

Comments

@sandreim
Copy link
Contributor

sandreim commented Oct 22, 2024

Currently in para_inherent we do some filtering of candidates which includes checking if the core index is correct, but we do not check the count of UMP signals or format. If these are invalid we assume the parachain cannot use elastic scaling and we check the core index in the descriptor.

We need to move the UMP checks from verify_backed_candidate to the filtering stage in para_inherent so old nodes that don't check the number of UMP messages offchain filter candidates and don't panic during block production if a parachain sends a malformed signal.

I am not an expert on that code but it appears to me that we can move the verify_backed_candidate as last step in filtering stage just to ensure that filtering bugs on the node don't make the author panic.

@sandreim sandreim converted this from a draft issue Oct 22, 2024
@alindima
Copy link
Contributor

We're already doing verify_backed_candidate during candidate filtering:

match check_ctx.verify_backed_candidate(
so we won't panic during block production.

@alindima
Copy link
Contributor

alindima commented Oct 23, 2024

I opened another issue for refactoring the runtime code so that we only do candidate validation during filtering and remove this code from inclusion: #6186

It makes sense however to harden the core index check as you suggest here: #6178 (comment)

@sandreim
Copy link
Contributor Author

I opened another issue for refactoring the runtime code so that we only do candidate validation during filtering and remove this code from inclusion: #6186

It makes sense however to harden the core index check as you suggest here: #6178 (comment)

yup, sounds good to me.

As you say, we already call verify_backed_candidate in filtering, so we just need to keep filtering UMPSignal in check_upward_messages, but without the signal count checks.

@alindima alindima changed the title Runtime: move ump checks at filtering stage. Runtime: unify and harden UMP signal checks in check_core_index Oct 24, 2024
github-merge-queue bot pushed a commit that referenced this issue Nov 5, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Completed in parachains team board Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Completed
Development

Successfully merging a pull request may close this issue.

2 participants