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.

Sign up for GitHub

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

Lookahead collator: do not try to build upon an unknown block #4036

Closed
wants to merge 5 commits into from
Closed

Lookahead collator: do not try to build upon an unknown block #4036

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b4a6e13
Do not try to build upon an unknown block
s0me0ne-unkn0wn Apr 9, 2024
73469a8
Move check into outer loop
s0me0ne-unkn0wn Apr 9, 2024
49bbf49
Merge branch 'master' into s0me0ne/no-build-upon-unknown-block
s0me0ne-unkn0wn Apr 9, 2024
19c1113
Merge branch 'master' into s0me0ne/no-build-upon-unknown-block
s0me0ne-unkn0wn Apr 10, 2024
36b9ea4
Merge branch 'master' into s0me0ne/no-build-upon-unknown-block
s0me0ne-unkn0wn Apr 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions cumulus/client/consensus/aura/src/collators/lookahead.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,9 +315,10 @@ where
let mut parent_header = initial_parent.header;
let overseer_handle = &mut params.overseer_handle;

// We mainly call this to inform users at genesis if there is a mismatch with the
// on-chain data.
collator.collator_service().check_block_status(parent_hash, &parent_header);
Copy link
Contributor

@skunert skunert Apr 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Could we not directly call continue here? Why move it to the loop below?
Parent hash and parent header below will only change if we built a new block ortselves, so that should always return true in subsequent calls anyway (or I am missing something).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

parent_hash and parent_header get updated at the bottom of the inner loop. On the other hand, they are updated to values we get after a successful collation, which are unlikely to be unknown or bad 🤔 Need to think a bit more, thanks for pointing out!

Copy link
Contributor

Choose a reason for hiding this comment

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

parent_hash and parent_header get updated at the bottom of the inner loop. On the other hand, they are updated to values we get after a successful collation, which are unlikely to be unknown or bad 🤔 Need to think a bit more, thanks for pointing out!

Exactly, that is what I meant. We build a block successfully and set parent to it. But since we built it ourselves it will for sure be in-chain. If the first parent itself if messed up we will abort before entering the 0..2 loop.

// Do not try to build upon an unknown, pruned or bad block
if !collator.collator_service().check_block_status(parent_hash, &parent_header) {
continue
}

// This needs to change to support elastic scaling, but for continuously
// scheduled chains this ensures that the backlog will grow steadily.
Expand Down
Loading