Skip to content

lookahead collator: only build on top of scheduled relay parents#1429

Merged
slumber merged 4 commits into
masterfrom
slumber-lookahead-on-scheduled-rp
Sep 12, 2023
Merged

lookahead collator: only build on top of scheduled relay parents#1429
slumber merged 4 commits into
masterfrom
slumber-lookahead-on-scheduled-rp

Conversation

@slumber

@slumber slumber commented Sep 6, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Partially addresses #1400

@slumber
lookahead collator: filter out relay parents
23698f7 
if the parachain is not scheduled
@slumber slumber added the T9-cumulus This PR/Issue is related to cumulus. label Sep 6, 2023
@slumber slumber requested review from rphmeier and sandreim September 6, 2023 14:47
@sandreim

sandreim commented Sep 7, 2023

Copy link
Copy Markdown
Contributor

With async backing enabled, no candidates are backed with this PR and latest Westend(master) runtime.

@sandreim

sandreim commented Sep 7, 2023

Copy link
Copy Markdown
Contributor

With async backing enabled, no candidates are backed with this PR and latest Westend(master) runtime.

Actually I haven't built it with network-protocol-staging and that explains the issue.

sandreim
sandreim approved these changes Sep 7, 2023
View reviewed changes
Comment thread cumulus/client/consensus/aura/src/collators/lookahead.rs
@paritytech-ci paritytech-ci requested review from a team September 7, 2023 12:08
@sandreim

sandreim commented Sep 7, 2023

Copy link
Copy Markdown
Contributor

Looks good on Versi!

@rphmeier

rphmeier commented Sep 8, 2023

Copy link
Copy Markdown
Contributor

The lookahead collator should only build on chains where all in-scope and off-chain blocks are built on allowed relay-parents.

This PR only does half of the issue.
It changes collators not to build on disallowed relay-parents, but collators will still build on top of blocks which had disallowed relay parents. The parent search would need to be altered in order to fix this.

I'm OK with merging as-is, since an entirely-honest collator set would have no issues. But we should leave #1400 open

@paritytech-cicd-pr

paritytech-cicd-pr commented Sep 12, 2023

Copy link
Copy Markdown

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-clippy
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3659191

@slumber slumber force-pushed the slumber-lookahead-on-scheduled-rp branch from 98121a5 to 345d613 Compare September 12, 2023 13:33
@slumber slumber enabled auto-merge (squash) September 12, 2023 13:37
@slumber slumber merged commit 09630fc into master Sep 12, 2023
@slumber slumber deleted the slumber-lookahead-on-scheduled-rp branch September 12, 2023 14:15
@kiltbot kiltbot mentioned this pull request Oct 19, 2023
Closed
@ahmadkaouk ahmadkaouk mentioned this pull request Nov 16, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sandreim sandreim sandreim approved these changes

+1 more reviewer

@rphmeier rphmeier rphmeier approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

T9-cumulus This PR/Issue is related to cumulus.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@slumber @sandreim @rphmeier @paritytech-cicd-pr @patriciobcs