Conversation
`key insert --scheme` previously rejected the hybrid post-quantum
schemes used by Quip, even though the patched LocalKeystore in this
fork already knows how to sign/verify with them once they're stored.
The blocker was only the URI→public-bytes conversion at insert time.
Add a local `InsertKeyScheme` enum in insert_key.rs that extends the
global `CryptoScheme` with `hybrid-babe-h344` and `hybrid-grandpa-h144`.
The dispatch is inlined rather than going through `with_crypto_scheme!`
because the global macro is also used by `generate`/`sign`/`inspect`/
`verify`, whose helpers require `Pair::Public: Into<MultiSigner>` —
a bound the hybrid public types don't satisfy. Keeping the global
macro/enum unchanged means those commands are untouched.
Adds quip-crypto-primitives as an sc-cli dep — mirrors what
sc-keystore already does.
Usage:
quip-network-node key insert --base-path <p> \
--key-type babe --suri "<mnemonic>" \
--scheme hybrid-babe-h344
quip-network-node key insert --base-path <p> \
--key-type gran --suri "<mnemonic>" \
--scheme hybrid-grandpa-h144
LocalKeystore::key_file_path used the raw hex of the public key as the
on-disk filename. For 32-byte classical pubkeys the result (72 chars)
fits comfortably under NAME_MAX (255 bytes on Linux/macOS), but for the
1344-byte hybrid post-quantum pubkeys (sr25519+ML-DSA-44, ed25519+
ML-DSA-44) the filename would be ~2700 chars and File::create fails.
For pubkeys whose raw hex would exceed a conservative budget the file
now uses a blake2_256 hash for the filename and a JSON envelope for the
content:
{"public": "0x<full pubkey hex>", "suri": "<mnemonic>"}
The full pubkey lives inside the envelope so raw_public_keys can still
recover it. Classical pubkeys keep the legacy filename-encodes-pubkey
format, so existing keystores on disk remain readable.
read_suri_from_keystore_file does the format detection on the read
side. Both `key_phrase_by_type` and `raw_public_keys` were updated to
walk either format.
All 10 sc-keystore unit tests still pass; the dispatch is gated on the
pubkey length so classical schemes are unaffected.
🤖 Augment PR SummarySummary: Introduces Quip hybrid cryptography primitives (classical + ML‑DSA‑44) and wires them into Substrate consensus/keystore paths. Changes:
Technical notes: This PR changes consensus-facing key/signature sizes and VRF/story derivation behavior; downstream integrators should review keystore compatibility and any assumptions about signature/VRF encodings. 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| /// | ||
| /// Returns the SCALE encoded VRF signature if key is found and supported, | ||
| /// `None` if the key doesn't exist, or an error when something failed. | ||
| fn vrf_sign_with( |
There was a problem hiding this comment.
substrate/primitives/keystore/src/lib.rs:586: vrf_sign_with’s default implementation always returns KeyNotSupported, but the doc comment claims sr25519 is supported and BABE now calls vrf_sign_with on the keystore. This can break non-LocalKeystore implementations that previously worked via sr25519_vrf_sign (e.g. MemoryKeystore).
Severity: high
⏳ Generating Fix in Augment link...
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| /// Panics if `ctx.len() > 255`, because the current wire format encodes the | ||
| /// context length in a single byte. | ||
| pub fn prepare_message(version: u8, label: &[u8], msg: &[u8], ctx: &[u8]) -> Vec<u8> { | ||
| assert!(ctx.len() <= 255, "ctx must be at most 255 bytes"); |
There was a problem hiding this comment.
quip/primitives/crypto/src/domain.rs:23: prepare_message enforces ctx.len() <= 255 via assert!, which will panic in no_std callers and could crash consensus/runtime if an oversized context is ever passed in. Consider making this constraint fallible (or otherwise ensuring inputs are bounded at the API boundary) so invalid input can’t trigger a panic.
Severity: medium
⏳ Generating Fix in Augment link...
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| @@ -169,27 +168,13 @@ pub mod v1 { | |||
| /// Compute the randomness associated with this VRF output. | |||
| pub fn compute_randomness( | |||
There was a problem hiding this comment.
polkadot/node/primitives/src/approval/mod.rs:169: compute_randomness now ignores authorities, randomness, and epoch_index and no longer checks authority_index bounds, so it can’t return AuthorityOutOfBounds/SchnorrkelSignature anymore. Please double-check that this still matches the intended RelayVRFStory semantics and that invalid authority_index values can’t silently pass through this path.
Severity: medium
⏳ Generating Fix in Augment link...
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| &babe_vrf_data, | ||
| ); | ||
| if let Ok(Some(vrf_signature)) = result { | ||
| let Ok(vrf_signature) = Decode::decode(&mut &vrf_signature[..]) else { |
There was a problem hiding this comment.
substrate/client/consensus/babe/src/authorship.rs:270: If the keystore returns malformed VRF bytes here, the code silently continues and just skips the key, which can mask keystore corruption/misconfiguration and lead to missed slot claims. The secondary-slot path maps decode failures into a ValidationError, so handling primary-slot decode failures similarly would improve consistency/observability.
Severity: medium
⏳ Generating Fix in Augment link...
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| zstd = { version = "0.12.4", default-features = false } | ||
|
|
||
| ## quip | ||
| quip-crypto-primitives = { path = "quip/primitives/crypto", default-features = false } |
There was a problem hiding this comment.
Cargo.toml:1501 (general PR metadata): The PR description still contains the unfilled template sections (Description/Integration/Review Notes checklist), which makes it hard to review and integrate this large consensus/crypto change safely. Please replace the template text with concrete motivation, integration steps, and reviewer guidance.
Severity: low
⏳ Generating Fix in Augment link...
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
ruseinov
changed the title
…ning feat: wasm-compatible signing
…ytech#11992) Recent rustc requires `-Z json-target-spec` to opt into the JSON target spec format whenever `--target=*.json` is used. Without this, builds that go through `polkavm-linker::target_json_path` fail with: error: `.json` target specs require -Zjson-target-spec Fix the two places in the workspace that invoke cargo with a JSON target spec for the Riscv runtime: - substrate-wasm-builder (`wasm_project.rs`): pass the flag for `RuntimeTarget::Riscv`. `RUSTC_BOOTSTRAP=1` is already set by the preceding `-Z build-std` block (Riscv always opts into build-std). - pallet-revive-fixtures (`builder.rs`): refactor the inline rustc version detection to expose major/minor and derive both `new_immediate_abort` (1.92+) and `needs_json_target_spec` (1.95+) from them. The flag is gated on rustc 1.95+ where it was introduced. Older rustc doesn't recognize it; later rustc requires it. --------- Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Pass -Zjson-target-spec when building with a .json target spec (paritytech#11992)
3 participants
✄ -----------------------------------------------------------------------------
Thank you for your Pull Request! 🙏 Please make sure it follows the contribution guidelines outlined in this
document and fill out the
sections below. Once you're ready to submit your PR for review, please delete this section and leave only the text under
the "Description" heading.
Description
A concise description of what your PR is doing, and what potential issue it is solving. Use Github semantic
linking
to link the PR to an issue that must be closed once this is merged.
Integration
In depth notes about how this PR should be integrated by downstream projects. This part is
mandatory, and should be reviewed by reviewers, if the PR does NOT have the
R0-no-crate-publish-requiredlabel. In case of aR0-no-crate-publish-required, it can beignored.
Review Notes
In depth notes about the implementation details of your PR. This should be the main guide for reviewers to
understand your approach and effectively review it. If too long, use
<details>.Imagine that someone who is depending on the old code wants to integrate your new code and the only information that
they get is this section. It helps to include example usage and default value here, with a
diffcode-block to showpossibly integration.
Include your leftover TODOs, if any, here.
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 commandsYou can remove the "Checklist" section once all have been checked. Thank you for your contribution!
✄ -----------------------------------------------------------------------------