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

Unify Import verifier usage across parachain template and omninode #7195

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

MrishoLukamba
Copy link

@MrishoLukamba MrishoLukamba commented Jan 16, 2025

@skunert
Copy link
Contributor

skunert commented Jan 16, 2025

Thanks for looking into this! The changes as proposed here cannot be merged. Let me expand on the context:

Context

The import queue used in the omni-node is shared across various chains, including our system chains (some of which are quite old). Historically, these chains followed a pattern where they would:

  1. Start with relay-chain consensus (blocks 0 to X)
  2. Later migrate to AURA consensus (blocks X+1 onwards)

The import queue needs to maintain backwards compatibility with these dual-consensus chains. When starting a fresh node, it begins downloading initial blocks that were authored with relay-chain consensus. The changes in this PR would cause these initial block imports to fail because we're exclusively using the AURA verifier. The verifier that was present before detects whether we have switched to AURA and then uses the required verifier.

Specific Changes Needed

The following verifier needs to be replaced:

let aura_verifier = cumulus_client_consensus_aura::build_verifier::<

With this verifier:

let verifier = Verifier::<P, _, _, _> {

You may need to adjust the crate to make the second verifier publicly available.

The overall wrapping verifier should remain in place to handle path selection based on the consensus in use:

Apologies for not providing more info in the original issue!

@MrishoLukamba
Copy link
Author

Thanks for looking into this! The changes as proposed here cannot be merged. Let me expand on the context:

Context

The import queue used in the omni-node is shared across various chains, including our system chains (some of which are quite old). Historically, these chains followed a pattern where they would:

  1. Start with relay-chain consensus (blocks 0 to X)
  2. Later migrate to AURA consensus (blocks X+1 onwards)

The import queue needs to maintain backwards compatibility with these dual-consensus chains. When starting a fresh node, it begins downloading initial blocks that were authored with relay-chain consensus. The changes in this PR would cause these initial block imports to fail because we're exclusively using the AURA verifier. The verifier that was present before detects whether we have switched to AURA and then uses the required verifier.

Specific Changes Needed

The following verifier needs to be replaced:

let aura_verifier = cumulus_client_consensus_aura::build_verifier::<

With this verifier:

let verifier = Verifier::<P, _, _, _> {

You may need to adjust the crate to make the second verifier publicly available.

The overall wrapping verifier should remain in place to handle path selection based on the consensus in use:

Apologies for not providing more info in the original issue!

Okey on it

@MrishoLukamba
Copy link
Author

@skunert done

@bkchr
Copy link
Member

bkchr commented Jan 20, 2025

/cmd fmt

@bkchr bkchr added the T0-node This PR/Issue is related to the topic “node”. label Jan 20, 2025
Copy link

Command "fmt" has started 🚀 See logs here

Copy link

Command "fmt" has finished ✅ See logs here

@MrishoLukamba
Copy link
Author

MrishoLukamba commented Jan 21, 2025

/cmd fmt

done @bkchr @skunert

Copy link
Contributor

@skunert skunert left a comment

Choose a reason for hiding this comment

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

Looks Good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T0-node This PR/Issue is related to the topic “node”.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unify Import verifier usage across parachain template and omninode.
3 participants