secretflow/yacl

[Question]: Pack before comparison

maths644311798 opened this issue · 1 comments

In ecc/lib25519_group.cc,

bool Lib25519Group::PointEqual(const EcPoint& p1, const EcPoint& p2) const {
  if (IsInfinity(p1) && IsInfinity(p2)) {
    return true;
  }

  const auto* p1p = CastP3(p1);
  const auto* p2p = CastP3(p2);

  // p1 = (X1/Z1, Y1/Z1) = ((X1*Z2)/(Z1*Z2), (Y1*Z2)/(Z1*Z2));
  // P2 = (X2/Z2, Y2/Z2) = ((Z1*X2)/(Z1*Z2), (Z1*Y2)/(Z1*Z2));
  fe25519 a;
  fe25519 b;
  fe25519_mul(&a, &p1p->x, &p2p->z);
  fe25519_mul(&b, &p1p->z, &p2p->x);
  for (size_t i = 0; i < sizeof(fe25519) / sizeof(a.v[0]); ++i) {
    if (a.v[i] != b.v[i]) {
      return false;
    }
  }

  fe25519_mul(&a, &p1p->y, &p2p->z);
  fe25519_mul(&b, &p1p->z, &p2p->y);
  uint128_t buf_a[2];
  uint128_t buf_b[2];
  fe25519_pack(reinterpret_cast<unsigned char*>(buf_a), &a);
  fe25519_pack(reinterpret_cast<unsigned char*>(buf_b), &b);
  return buf_a[0] == buf_b[0] && buf_a[1] == buf_b[1];
}

Why pack before comparison? Why not just compare a.v and b.v in the comparison of Y1Z2 and Z1Y2?

Thanks for your question! Indeed fe25519_pack is not a good choice. fe25519_iseq_vartime might be better here.