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

Avoid incomplete block import pipeline with full verifying import queue #7050

Merged
merged 4 commits into from
Jan 6, 2025

Conversation

skunert
Copy link
Contributor

@skunert skunert commented Jan 6, 2025

Problem

In the parachain template we use the fully verifying import queue
which does extra equivocation checks.

However, when we import a warp synced block with state, we don't set a fork choice, leading to an incomplete block import pipeline and error here:

let fork_choice = fork_choice.ok_or(Error::IncompletePipeline)?;

This renders warp sync useless for chains using this import queue.

Fix

The fix is to always import a block with state as best block, as we already do in the normal Aura Verifier.
In a follow up we should also take another look into unifying the usage of the different import queues.

fixes paritytech/project-mythical#256

@skunert skunert added the T0-node This PR/Issue is related to the topic “node”. label Jan 6, 2025
@skunert skunert requested review from bkchr and a team January 6, 2025 10:28
@skunert
Copy link
Contributor Author

skunert commented Jan 6, 2025

/cmd prdoc --bump patch --audience node_dev

@skunert skunert added the A4-needs-backport Pull request must be backported to all maintained releases. label Jan 6, 2025
@skunert skunert enabled auto-merge January 6, 2025 12:10
@skunert skunert added this pull request to the merge queue Jan 6, 2025
Merged via the queue into master with commit 1dcff3d Jan 6, 2025
199 of 201 checks passed
@skunert skunert deleted the skunert/fix-fork-choice-verifying branch January 6, 2025 13:41
github-actions bot pushed a commit that referenced this pull request Jan 6, 2025
…ue (#7050)

## Problem
In the parachain template we use the [fully verifying import queue

](https://github.com/paritytech/polkadot-sdk/blob/3d9eddbeb262277c79f2b93b9efb5af95a3a35a8/cumulus/client/consensus/aura/src/equivocation_import_queue.rs#L224-L224)
which does extra equivocation checks.

However, when we import a warp synced block with state, we don't set a
fork choice, leading to an incomplete block import pipeline and error
here:
https://github.com/paritytech/polkadot-sdk/blob/3d9eddbeb262277c79f2b93b9efb5af95a3a35a8/substrate/client/service/src/client/client.rs#L488-L488

This renders warp sync useless for chains using this import queue.

## Fix
The fix is to always import a block with state as best block, as we
already do in the normal Aura Verifier.
In a follow up we should also take another look into unifying the usage
of the different import queues.

fixes paritytech/project-mythical#256

---------

Co-authored-by: command-bot <>
(cherry picked from commit 1dcff3d)
@paritytech-cmd-bot-polkadot-sdk

Successfully created backport PR for stable2407:

@paritytech-cmd-bot-polkadot-sdk

Successfully created backport PR for stable2409:

github-actions bot pushed a commit that referenced this pull request Jan 6, 2025
…ue (#7050)

## Problem
In the parachain template we use the [fully verifying import queue

](https://github.com/paritytech/polkadot-sdk/blob/3d9eddbeb262277c79f2b93b9efb5af95a3a35a8/cumulus/client/consensus/aura/src/equivocation_import_queue.rs#L224-L224)
which does extra equivocation checks.

However, when we import a warp synced block with state, we don't set a
fork choice, leading to an incomplete block import pipeline and error
here:
https://github.com/paritytech/polkadot-sdk/blob/3d9eddbeb262277c79f2b93b9efb5af95a3a35a8/substrate/client/service/src/client/client.rs#L488-L488

This renders warp sync useless for chains using this import queue.

## Fix
The fix is to always import a block with state as best block, as we
already do in the normal Aura Verifier.
In a follow up we should also take another look into unifying the usage
of the different import queues.

fixes paritytech/project-mythical#256

---------

Co-authored-by: command-bot <>
(cherry picked from commit 1dcff3d)
@paritytech-cmd-bot-polkadot-sdk

Successfully created backport PR for stable2412:

github-actions bot pushed a commit that referenced this pull request Jan 6, 2025
…ue (#7050)

## Problem
In the parachain template we use the [fully verifying import queue

](https://github.com/paritytech/polkadot-sdk/blob/3d9eddbeb262277c79f2b93b9efb5af95a3a35a8/cumulus/client/consensus/aura/src/equivocation_import_queue.rs#L224-L224)
which does extra equivocation checks.

However, when we import a warp synced block with state, we don't set a
fork choice, leading to an incomplete block import pipeline and error
here:
https://github.com/paritytech/polkadot-sdk/blob/3d9eddbeb262277c79f2b93b9efb5af95a3a35a8/substrate/client/service/src/client/client.rs#L488-L488

This renders warp sync useless for chains using this import queue.

## Fix
The fix is to always import a block with state as best block, as we
already do in the normal Aura Verifier.
In a follow up we should also take another look into unifying the usage
of the different import queues.

fixes paritytech/project-mythical#256

---------

Co-authored-by: command-bot <>
(cherry picked from commit 1dcff3d)
ordian added a commit that referenced this pull request Jan 7, 2025
* master:
  workflows: add debug input for sync templates act (#7057)
  Remove usage of `sp-std` from Substrate (#7043)
  Fix typos (#7027)
  [core-fellowship] Add permissionless import_member (#7030)
  Avoid incomplete block import pipeline with full verifying import queue (#7050)
EgorPopelyaev pushed a commit that referenced this pull request Jan 8, 2025
Backport #7050 into `stable2407` from skunert.

See the
[documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md)
on how to use this bot.

<!--
  # To be used by other automation, do not modify:
  original-pr-number: #${pull_number}
-->

Co-authored-by: Sebastian Kunert <skunert49@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A4-needs-backport Pull request must be backported to all maintained releases. T0-node This PR/Issue is related to the topic “node”.
Projects
Status: done
Development

Successfully merging this pull request may close these issues.

Muse chain not syncing when using --sync warp as the syncing method
5 participants