Skip to content

Commit

Permalink
Fix NonZero violation with Point::default
Browse files Browse the repository at this point in the history
Default for NonZero Point was zero 🤦
  • Loading branch information
LLFourn committed May 3, 2023
1 parent d102384 commit 5fcc773
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 14 deletions.
5 changes: 1 addition & 4 deletions schnorr_fun/src/frost.rs
Original file line number Diff line number Diff line change
Expand Up @@ -832,10 +832,7 @@ impl<H: Digest<OutputSize = U32> + Clone, NG> Frost<H, NG> {
g!({ agg_nonce[0] } + binding_coeff * { agg_nonce[1] })
.normalize()
.non_zero()
.unwrap_or_else(|| {
// Use the same trick as the MuSig spec
G.normalize()
})
.unwrap_or(Point::generator())
.into_point_with_even_y();

let challenge = self
Expand Down
6 changes: 1 addition & 5 deletions schnorr_fun/src/musig.rs
Original file line number Diff line number Diff line change
Expand Up @@ -540,11 +540,7 @@ impl<H: Digest<OutputSize = U32> + Clone, NG> MuSig<H, NG> {
let (R, r_needs_negation) = g!({ agg_Rs.0[0] } + b * { agg_Rs.0[1] })
.normalize()
.non_zero()
.unwrap_or_else(|| {
// if final nonce is zero we set it to generator as in MuSig spec
debug_assert!(G.is_y_even());
G.normalize()
})
.unwrap_or(Point::generator())
.into_point_with_even_y();

for R_i in &mut Rs {
Expand Down
47 changes: 42 additions & 5 deletions secp256kfun/src/point.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use rand_core::RngCore;
/// - `S`: A [`Secrecy`] to determine whether operations on this point should be done in constant-time or not. By default points are [`Public`] so operations run in variable time.
/// - `Z`: A [`ZeroChoice`] to keep track of whether the point might be zero (the point at infinity) or is guaranteed to be non-zero.
///
/// # Serialization
/// ## Serialization
///
/// Only points that are normalized (i.e. `T` ≠ `NonNormal`) can be serialized. A Point that is
/// `EvenY` points serialize to and from their 32-byte x-only representation.
Expand All @@ -46,13 +46,26 @@ use rand_core::RngCore;
/// [`Public`]: crate::marker::Public
/// [`is_zero`]: crate::Point::is_zero
/// [_identity element_]: https://en.wikipedia.org/wiki/Identity_element
#[derive(Default)]
pub struct Point<T = Normal, S = Public, Z = NonZero>(
pub(crate) backend::Point,
pub(crate) T,
PhantomData<(Z, S)>,
);

/// The default for `Point`<_,_,Zero>` is [`Point::zero`].
impl<T: Default, S> Default for Point<T, S, Zero> {
fn default() -> Self {
Point::zero()
}
}

/// The default for `Point`<_,_,Zero>` is [`Point::generator`].
impl<T: Default + PointType, S> Default for Point<T, S, NonZero> {
fn default() -> Self {
Point::generator()
}
}

impl<Z, S, T: Clone> Clone for Point<T, S, Z> {
fn clone(&self) -> Self {
Point::from_inner(self.0, self.1.clone())
Expand Down Expand Up @@ -147,7 +160,7 @@ impl<Z: ZeroChoice> Point<Normal, Public, Z> {
}
}

impl<T: PointType, S> Point<T, S, NonZero> {
impl<T, S> Point<T, S, NonZero> {
/// Converts this point into the point with the same x-coordinate but with
/// an even y-coordinate. Returns a Point marked `EvenY` with a `bool`
/// indicating whether the point had to be negated to make its y-coordinate
Expand All @@ -159,12 +172,36 @@ impl<T: PointType, S> Point<T, S, NonZero> {
/// let point = Point::random(&mut rand::thread_rng());
/// let (point_with_even_y, was_odd) = point.clone().into_point_with_even_y();
/// ```
pub fn into_point_with_even_y(self) -> (Point<EvenY, S, NonZero>, bool) {
pub fn into_point_with_even_y(self) -> (Point<EvenY, S, NonZero>, bool)
where
T: PointType,
{
let normalized = self.normalize();
let needs_negation = !normalized.is_y_even();
let negated = normalized.conditional_negate(needs_negation);
(Point::from_inner(negated.0, EvenY), needs_negation)
}

/// Returns the generator point [`G`] defined in [_Standards for Efficient Cryptography_].
///
/// This is sometimes more useful than just using `secp256kfun::G` since it allows the compiler
/// to infer types.
///
/// ## Examples
///
/// ```
/// use secp256kfun::{marker::*, Point, G};
/// assert_eq!(Point::<Normal, Public, _>::generator(), *G);
/// ```
///
/// [_Standards for Efficient Cryptography_]: https://www.secg.org/sec1-v2.pdf
/// [`G`]: crate::G
pub fn generator() -> Self
where
T: Default,
{
Self::from_inner(backend::G_POINT, T::default())
}
}

impl Point<EvenY, Public, NonZero> {
Expand Down Expand Up @@ -401,7 +438,7 @@ impl<T, S> Point<T, S, Zero> {
/// [`identity_element`]: https://en.wikipedia.org/wiki/Identity_element
pub fn zero() -> Self
where
T: PointType + Default,
T: Default,
{
Self::from_inner(backend::Point::zero(), T::default())
}
Expand Down

0 comments on commit 5fcc773

Please sign in to comment.