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

Fix #2088 again #2182

Merged
merged 2 commits into from
Mar 29, 2022
Merged

Fix #2088 again #2182

merged 2 commits into from
Mar 29, 2022

Conversation

tomaka
Copy link
Contributor

@tomaka tomaka commented Mar 28, 2022

Fix #2088 again
Fix #2147 as well

This is a fix of #2106, which was the previous attempt at fixing #2088

What #2106 didn't do is check whether the block to report is equal to the finalized block.

What happens is:

  • When the parachain sync service initializes, it starts to fetch the parahead corresponding the latest relay chain finalized block and to fetch the parahead corresponding to each relay chain non-finalized block
  • The fetch of the relay chain finalized block succeeds, and we report this block. We not only reports it as new block but also as finalized.
  • The fetch of one of the relay chain non-finalized blocks succeeds, and the code that this PR modifies runs. But because the code before this PR only looks at non-finalized blocks, it doesn't detect that we have already reports this new block, and thus we report it again.

As a result, we have two different blocks with the same hash, which isn't supposed to happen, and triggers a panic in the state machines on top.

Copy link
Contributor

@mergify mergify bot left a comment

Choose a reason for hiding this comment

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

Automatically approving tomaka's pull requests. This auto-approval will be removed once more maintainers are active.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 28, 2022

twiggy diff report

Difference in .wasm size before and after this pull request.


 Delta Bytes │ Item
─────────────┼──────────────────────────────────────────────────────────────────────────────────────────────────────
         +15 ┊ <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll::h42eb11ae074aced3
         +15 ┊ Σ [1 Total Rows]

Copy link
Contributor

@melekes melekes left a comment

Choose a reason for hiding this comment

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

👍

@tomaka tomaka added the automerge Automatically merge pull request as soon as possible label Mar 29, 2022
@mergify mergify bot merged commit 76d375b into paritytech:main Mar 29, 2022
@tomaka tomaka deleted the fix-2088-again branch March 29, 2022 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Automatically merge pull request as soon as possible
Projects
None yet
2 participants