Skip to content

Make copy_key optional in fastcrypto-pq - #1008

Open
jonas-lj wants to merge 2 commits into
mainfrom
jonas/mldsa65-gate-copy-key
Open

Make copy_key optional in fastcrypto-pq#1008
jonas-lj wants to merge 2 commits into
mainfrom
jonas/mldsa65-gate-copy-key

Conversation

@jonas-lj

Copy link
Copy Markdown
Contributor

No description provided.

@jonas-lj
jonas-lj marked this pull request as ready for review August 24, 2026 11:07
@jonas-lj
jonas-lj requested a review from mahdi-mysten August 24, 2026 11:07
@jonas-lj
jonas-lj force-pushed the jonas/mldsa65-gate-copy-key branch from 25f6dc5 to b3e7c45 Compare August 24, 2026 11:08
@jonas-lj
jonas-lj marked this pull request as draft August 24, 2026 11:08
@jonas-lj
jonas-lj marked this pull request as ready for review August 24, 2026 11:16
fastcrypto-pq depended on fastcrypto with features = ["copy_key"] and implemented
KeyPair::copy without the cfg gate the other schemes use. Because Cargo unifies
features per build, that turned copy_key on for the whole dependency graph, making
Ed25519KeyPair::copy and friends available in crates that deliberately built without
it. Forward the feature instead and gate MLDSA65KeyPair::copy like ed25519 does.

The two tests that only used copy() to reach the private key now build it from the
keypair bytes, so they keep running without the feature.
@jonas-lj
jonas-lj force-pushed the jonas/mldsa65-gate-copy-key branch from b3e7c45 to f1f0e43 Compare August 24, 2026 12:35
@jonas-lj
jonas-lj marked this pull request as draft August 24, 2026 12:36
@jonas-lj
jonas-lj marked this pull request as ready for review August 24, 2026 12:37
this is bc  failed with E0046 before this push
@mahdi-mysten

Copy link
Copy Markdown
Collaborator

@jonas-lj Pushed a small fix on top: fastcrypto-cli was hardcoding copy_key, so a plain cargo check --workspace still failed with E0046 on the pq impl. Not critical, but better to clean it up and not leave some dust under the rug.

Comment thread fastcrypto-pq/Cargo.toml
[dependencies]
# copy_key is not optional here
fastcrypto = { path = "../fastcrypto", features = ["copy_key"] }
fastcrypto = { path = "../fastcrypto" }

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

cargo check --workspace fails on

impl KeyPair for MLDSA65KeyPair {

with error:

error[E0046]: not all trait items implemented, missing: `copy`
   --> fastcrypto-pq/src/mldsa65/mod.rs:179:1

the core issue I think is:

fastcrypto = { path = "../fastcrypto", features = ["copy_key"] }

hardcodes features = ["copy_key"], so feature unification turns it on for the shared fastcrypto build.

}

serialize_deserialize_with_to_from_bytes!(MLDSA65KeyPair, MLDSA65_KEYPAIR_LENGTH);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

add #[cfg(feature = "copy_key")] here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants