Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

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

Merged
merged 4 commits into from
Jan 6, 2025
Merged

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

merged 4 commits into from
Jan 6, 2025

Conversation

skunert
Copy link
Contributor

@skunert skunert commented Jan 6, 2025 •
edited
Loading

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:

polkadot-sdk/substrate/client/service/src/client/client.rs

Line 488 in 3d9eddb

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 mentioned this pull request Jan 6, 2025
Closed
@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
@skunert @github-actions
Avoid incomplete block import pipeline with full verifying import que…
1c581ae 
…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
Copy link

Successfully created backport PR for stable2407:

@paritytech-cmd-bot-polkadot-sdk paritytech-cmd-bot-polkadot-sdk bot mentioned this pull request Jan 6, 2025
Merged
@paritytech-cmd-bot-polkadot-sdk
Copy link

Successfully created backport PR for stable2409:

github-actions bot pushed a commit that referenced this pull request Jan 6, 2025
@skunert @github-actions
Avoid incomplete block import pipeline with full verifying import que…
51f7a0e 
…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 paritytech-cmd-bot-polkadot-sdk bot mentioned this pull request Jan 6, 2025
Merged
@paritytech-cmd-bot-polkadot-sdk
Copy link

Successfully created backport PR for stable2412:

github-actions bot pushed a commit that referenced this pull request Jan 6, 2025
@skunert @github-actions
Avoid incomplete block import pipeline with full verifying import que…
10a9d6a 
…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 paritytech-cmd-bot-polkadot-sdk bot mentioned this pull request Jan 6, 2025
Merged
ordian added a commit that referenced this pull request Jan 7, 2025
@ordian
Merge branch 'master' into ao-fix-issue-64
f2f031c 
* 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
@paritytech-cmd-bot-polkadot-sdk @skunert
[stable2407] Backport #7050 (#7058)
826bb49 
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>
EgorPopelyaev pushed a commit that referenced this pull request Jan 13, 2025
@paritytech-cmd-bot-polkadot-sdk @skunert
[stable2409] Backport #7050 (#7059)
5d7a7c0 
Backport #7050 into `stable2409` 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>
EgorPopelyaev pushed a commit that referenced this pull request Jan 14, 2025
@paritytech-cmd-bot-polkadot-sdk @skunert
[stable2412] Backport #7050 (#7060)
d5f981d 
Backport #7050 into `stable2412` 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>
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@michalkucharczyk michalkucharczyk michalkucharczyk approved these changes

@bkchr bkchr bkchr approved these changes

@dmitry-markin dmitry-markin dmitry-markin approved these changes

@lexnv lexnv lexnv approved these changes

Assignees
No one assigned
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
SDK Node
Status: done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Muse chain not syncing when using --sync warp as the syncing method
5 participants
@skunert @michalkucharczyk @bkchr @dmitry-markin @lexnv