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

Provide From<ed25519::PublicKey> for x25519::PublicKey instead of TryFrom #133

Open
arnauorriols opened this issue Apr 25, 2022 · 2 comments

Comments

@arnauorriols
Copy link

arnauorriols commented Apr 25, 2022

Description

Is there a particular case where the conversion would fail? Particularly, the current implementation implies there might be a case where decompressing the ed25519 public key as a CompressedEdwardsY might fail. However, a comment also states that in the happy path of the conversion "pk is a valid ed25519::PublicKey". This is however already an invariant of the conversion (as we are converting from a ed25519::PublicKey), making one think the conversion is infallible in practice:

match curve25519_dalek::edwards::CompressedEdwardsY(y_bytes).decompress() {
    Some(decompressed_edwards) => {
        // `pk` is a valid `ed25519::PublicKey` hence contains valid `EdwardsPoint`.
        // x25519 uses Montgomery form, and `x25519::PublicKey` is just a `MontgomeryPoint`.
        // `MontgomeryPoint` can be constructed from `EdwardsPoint` with `to_montgomery()` method.
        // Can't construct `x25519::PublicKey` directly from `MontgomeryPoint`,
        // do it via intermediate bytes.
        Ok(PublicKey::from_bytes(decompressed_edwards.to_montgomery().to_bytes()))
    }
    None => Err(crate::error::Error::ConvertError {
        from: "ed25519 public key",
        to: "x25519 public key",
    }),
}

Motivation

If the conversion is in fact infallible, the trait should be From instead of TryFrom. Currently https://github.com/iotaledger/streams relies on this conversion being infallible. If it in fact isn't, I'd appreciate some clarification so that we can adjust our error handling.

Are you planning to do it yourself in a pull request?

Yes (upon confirmation)

@semenov-vladyslav
Copy link
Collaborator

That makes sense, ed25519 public key must always be valid thus compressed y coordinate must always decompress.

@Wollac
Copy link
Contributor

Wollac commented May 18, 2022

@arnauorriols what is your status on this one, is it still relevant?
If that is the case, could you open the PR adding an infallible From?

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

No branches or pull requests

3 participants