iotaledger / crypto.rs

The canonical source of cryptographic ground-truth for IOTA projects that use Rust.
https://iota.org
64 stars 22 forks source link

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

Open arnauorriols opened 2 years ago

arnauorriols commented 2 years ago

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 commented 2 years ago

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

Wollac commented 2 years ago

@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?