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

Add PrimeFieldBits support to Scalar #579

Merged
merged 11 commits into from
Sep 20, 2023
18 changes: 17 additions & 1 deletion curve25519-dalek/src/scalar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ use cfg_if::cfg_if;

#[cfg(feature = "group")]
use {
group::ff::{Field, FromUniformBytes, PrimeField},
group::ff::{Field, FromUniformBytes, PrimeField, PrimeFieldBits},
rand_core::RngCore,
};

Expand Down Expand Up @@ -1321,6 +1321,22 @@ impl PrimeField for Scalar {
};
}

#[cfg(feature = "group")]
impl PrimeFieldBits for Scalar {
type ReprBits = [u8; 32];

fn to_le_bits(&self) -> FieldBits<Self::ReprBits> {
self.to_repr().into()
}

fn char_le_bits() -> FieldBits<Self::ReprBits> {
let mut bytes = (Scalar::ZERO - Scalar::ONE).to_repr();
bytes[0] += 1;
debug_assert_eq!(DScalar::from_bytes_mod_order(bytes), DScalar::ZERO);
bytes.into()
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should work but haven't tested it

Suggested change
let mut bytes = (Scalar::ZERO - Scalar::ONE).to_repr();
bytes[0] += 1;
debug_assert_eq!(DScalar::from_bytes_mod_order(bytes), DScalar::ZERO);
bytes.into()
crate::constants::BASEPOINT_ORDER.into()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll test it now, thanks.

Copy link
Contributor Author

@kayabaNerve kayabaNerve Sep 12, 2023

Choose a reason for hiding this comment

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

That's a Scalar, not a byte array. Doesn't that break safety to have an unreduced Scalar in system? Especially now that Scalar addition assumes only unreduced Scalars exist? I'd guess it doesn't in practice, as it won't have any shouldn't never be set bits set, yet still sounds like something which should be a [u8; 32].

Copy link
Contributor Author

Choose a reason for hiding this comment

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

to_bytes().into() works, will push that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't that break safety to have an unreduced Scalar in system?

Yeah, seems a bit odd to have it in the public API if it's unreduced /cc @rozbb

Copy link
Contributor

@pinkforest pinkforest Sep 12, 2023

Choose a reason for hiding this comment

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

BASEPOINT_ORDER will be marked deprecated from 4.1.1 - this will be BASEPOINT_ORDER_PRIVATE as pub(crate)

}
}

#[cfg(feature = "group")]
impl FromUniformBytes<64> for Scalar {
fn from_uniform_bytes(bytes: &[u8; 64]) -> Self {
Expand Down