Kanas/omni node aura authority id type based on metadata checks#11107
Kanas/omni node aura authority id type based on metadata checks#11107iulianbarbu merged 28 commits intoparitytech:masterfrom
Conversation
This PR addresses issue paritytech#11026 by adding warnings when the library makes assumptions about Aura authority ID types and implements optional metadata-based detection. The `polkadot-omni-node-lib` currently assumes that the Aura authority ID type is `ed25519` for `asset-hub-polkadot` and `sr25519` for all other chains. This PR adds warnings to make these assumptions explicit and implements optional metadata-based detection to automatically determine the correct Aura authority ID type from runtime metadata when available. Changes: 1. Emit warning when assuming `ed25519` for `asset-hub-polkadot`/`statemint` 2. Emit warning when defaulting to `sr25519` for other chains 3. Add metadata-based detection for Aura authority ID type (optional step 3) 4. Proper fallback logic: defaults to `ed25519` for asset-hub-polkadot/statemint, `sr25519` for others when metadata is unavailable Closes paritytech#11026 No breaking changes. This is a non-breaking enhancement that adds warnings and improves detection logic. Downstream projects will see warnings when assumptions are made, but behavior remains the same. **Implementation details:** 1. **Warnings in `polkadot-parachain` RuntimeResolver:** - Warns when assuming `ed25519` for `asset-hub-polkadot`/`statemint` - Warns when defaulting to `sr25519` for other chains 2. **Metadata detection in `polkadot-omni-node-lib`:** - Added `aura_consensus_id()` method to `MetadataInspector` that scans runtime metadata types for `sp_consensus_aura::sr25519::AuthorityId` or `sp_consensus_aura::ed25519::AuthorityId` - If detection succeeds, uses detected type (no warning) - If detection fails, falls back to chain spec ID check: - `asset-hub-polkadot`/`statemint` → `ed25519` with warning - Others → `sr25519` with warning - Handles both cases: when metadata check fails entirely, and when metadata exists but detection returns None 3. **Test coverage:** - Added test for `aura_consensus_id()` to verify metadata detection works correctly with test runtime The warnings follow the existing codebase style: - `polkadot-parachain`: Simple warnings without emoji (matching existing style) - `polkadot-omni-node-lib`: Warnings with emoji (matching existing style)
…ks correctly with test runtime
|
/cmd prdoc |
|
User @cursoragent, please sign the CLA here. |
d5ed816 to
70cd5d2
Compare
|
@iulianbarbu Kindly review |
|
@iulianbarbu I've made all changes kindly review now |
|
Please fix clippy as well. CI is not green. |
|
Review required! Latest push from author must always be reviewed |
Co-authored-by: Iulian Barbu <14218860+iulianbarbu@users.noreply.github.com>
|
@iulianbarbu I pushed a commit to resolve Ci errors kindly check |
iulianbarbu
left a comment
There was a problem hiding this comment.
Super sorry for the late review here! Everything looks good, will approve but I would add one more test first against assethub-polkadot chain spec. We can add a tests/chain-specs dir here to be used to infer runtime metadata that tests the other cases of detecting the authority id type.
No worries! Thanks for the review! That’s a good suggestion tho. I will add the test and and introduce a tests/chain-specs directory |
serban300
left a comment
There was a problem hiding this comment.
Thanks for the contribution ! Looks good to me, modulo Iulian's comment
I only removed that test because I was following @serban300 suggestion to delete the entire chain-specs folder! I definitely want it tested too. verify it locally against the fellowship AHP like you mentioned and lets see |
franciscoaguirre
left a comment
There was a problem hiding this comment.
Looks good to me. @serban300 @iulianbarbu you think we can merge?
It's almost good to merge. It's pending on this |
iulianbarbu
left a comment
There was a problem hiding this comment.
Built the compact.compressed ahp and resulted in a 3.6 MB chain spec (with on-chain-release-build feature on). This is smaller than the one pushed before, and running your prev test as before locally takes ~4s. Definitely shorter than before (@serban300 had it running for 1min42s), maybe worth re-adding it, but I don't have a strong opinion of adding it now - can be in a follow-up.
7da81d6
|
Glad to see this merged🤗 |
|
/bot tip small |
Didn't go trough?🙁 |
|
/tip small |
|
@Kanasjnr Hey 👋, thanks for your contribution. We offer to propose a tip for you to OpenGov 🤩 |
Polkadot Address 12pCUGSwoW4Xek48TLUHCFhrvAdjmciMMLJoRJD8HWP5saXH |
iirc you have to add either to profile bio or in the PR description |
It's on my profile bio already 🙁 |
@mordamax Done🙁 |
|
/tip small |
|
@Kanasjnr Invalid network: "`Polkadot". Please select one of: polkadot, kusama, rococo, westend. |
|
/tip small |
|
Tip for 12pCUGSwoW4Xek48TLUHCFhrvAdjmciMMLJoRJD8HWP5saXH referendum status is 👎: InvalidTxError: { |
|
Ok that's differnt story :) i will followup here soon |
was supposed to be small p i added capital but i changed it already tho🙈 |
|
@Kanasjnr as i mentioned i will followup The issue was not the case of first latter unfortunately |
ohh my bad |
|
/tip small |
|
The referendum has appeared on Polkassembly. |

Description
This PR implements optional metadata-based detection to automatically determine the correct Aura authority ID type from runtime metadata.
The library currently assumes that the Aura authority ID type is
ed25519forasset-hub-polkadot/statemintandsr25519for all other chains. This PR adds the ability to detect the correct type from runtime metadata when available.Further implementation of #11026
polkadot address: 12pCUGSwoW4Xek48TLUHCFhrvAdjmciMMLJoRJD8HWP5saXH
Integration
No integration changes required. This is a non-breaking enhancement that improves detection logic. Behavior remains backward compatible with existing fallback mechanisms.
Review Notes
Implementation Overview
This PR implements optional metadata detection for Aura authority IDs:
Changes Made
File:
cumulus/polkadot-omni-node/lib/src/common/runtime.rsAdded
aura_consensus_id()method toMetadataInspector:sp_consensus_aura::sr25519::AuthorityIdorsp_consensus_aura::ed25519::AuthorityIdSome(AuraConsensusId)if found,NoneotherwiseUpdated
DefaultRuntimeResolver::runtime():metadata_inspector.aura_consensus_id()for metadata-based detectionNoneAdded test coverage:
aura_consensus_id()correctly detectssr25519from test runtime metadataExample Behavior
Metadata detection workflow:
sr25519ored25519)ed25519for asset-hub-polkadot/statemint,sr25519for others)Code Example
Testing
test_aura_consensus_id()that verifies metadata detection works correctly with the test runtime (which usessr25519)Notes
Checklist
Trequired)/cmd label <label-name>to add labelsBot Commands
You can use the following bot commands in comments to help manage your PR:
Labeling (Self-service for contributors):
/cmd label T1-FRAME- Add a single label/cmd label T1-FRAME R0-no-crate-publish-required- Add multiple labels/cmd label T6-XCM D2-substantial I5-enhancement- Add multiple labels at onceOther useful commands:
/cmd fmt- Format code (cargo +nightly fmt and taplo)/cmd prdoc- Generate PR documentation/cmd bench- Run benchmarks/cmd update-ui- Update UI tests/cmd --help- Show help for all available commands