Skip to content

Commit 2799c35

Browse files
kashbrtiiakovenkosledwards2225claudeAztecBot
authored
chore: ecc group audit 1- remove dead code, documentation and test (#20886)
there's lots of code that is never used in ecc/group module, especially in WNAF implementation. This PR removes most of the unused code. We also: - add documentation for WNAF - add edge case testing for WNAF --------- Co-authored-by: sergei iakovenko <105737703+iakovenkos@users.noreply.github.com> Co-authored-by: ledwards2225 <98505400+ledwards2225@users.noreply.github.com> Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com> Co-authored-by: AztecBot <tech@aztecprotocol.com>
1 parent 694e53f commit 2799c35

10 files changed

Lines changed: 171 additions & 731 deletions

File tree

‎barretenberg/cpp/src/barretenberg/ecc/groups/affine_element.hpp‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,11 @@
1717
#include <vector>
1818

1919
namespace bb::group_elements {
20+
21+
// MSB of the top 64-bit limb in a uint256_t (bit 255). Used in point compression to encode the
22+
// y-coordinate parity bit, and cleared when recovering the x-coordinate.
23+
static constexpr uint64_t UINT256_TOP_LIMB_MSB = 0x8000000000000000ULL;
24+
2025
template <typename T>
2126
concept SupportsHashToCurve = T::can_hash_to_curve;
2227
template <typename Fq_, typename Fr_, typename Params_> class alignas(64) affine_element {
@@ -80,10 +85,6 @@ template <typename Fq_, typename Fr_, typename Params_> class alignas(64) affine
8085

8186
constexpr affine_element operator*(const Fr& exponent) const noexcept;
8287

83-
template <typename BaseField = Fq,
84-
typename CompileTimeEnabled = std::enable_if_t<(BaseField::modulus >> 255) == uint256_t(0), void>>
85-
[[nodiscard]] constexpr uint256_t compress() const noexcept;
86-
8788
static constexpr affine_element infinity();
8889
constexpr affine_element set_infinity() const noexcept;
8990
constexpr void self_set_infinity() noexcept;

‎barretenberg/cpp/src/barretenberg/ecc/groups/affine_element.test.cpp‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,10 @@ template <typename G1> class TestAffineElement : public testing::Test {
139139
{
140140
for (size_t i = 0; i < 10; i++) {
141141
affine_element P = affine_element(element::random_element());
142-
uint256_t compressed = P.compress();
142+
uint256_t compressed = uint256_t(P.x);
143+
if (uint256_t(P.y).get_bit(0)) {
144+
compressed.data[3] |= group_elements::UINT256_TOP_LIMB_MSB;
145+
}
143146
affine_element Q = affine_element::from_compressed(compressed);
144147
EXPECT_EQ(P, Q);
145148
}
@@ -168,8 +171,6 @@ template <typename G1> class TestAffineElement : public testing::Test {
168171
affine_element R(0, P.y);
169172
ASSERT_FALSE(P == R);
170173
}
171-
// Regression test to ensure that the point at infinity is not equal to its coordinate-wise reduction, which may lie
172-
// on the curve, depending on the y-coordinate.
173174
static void test_infinity_ordering_regression()
174175
{
175176
affine_element P(0, 1);

‎barretenberg/cpp/src/barretenberg/ecc/groups/affine_element_impl.hpp‎

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ template <typename BaseField, typename CompileTimeEnabled>
2121
constexpr affine_element<Fq, Fr, T> affine_element<Fq, Fr, T>::from_compressed(const uint256_t& compressed) noexcept
2222
{
2323
uint256_t x_coordinate = compressed;
24-
x_coordinate.data[3] = x_coordinate.data[3] & (~0x8000000000000000ULL);
24+
x_coordinate.data[3] = x_coordinate.data[3] & (~UINT256_TOP_LIMB_MSB);
2525
bool y_bit = compressed.get_bit(255);
2626

2727
Fq x = Fq(x_coordinate);
@@ -80,18 +80,6 @@ constexpr affine_element<Fq, Fr, T> affine_element<Fq, Fr, T>::operator*(const F
8080
return bb::group_elements::element(*this) * exponent;
8181
}
8282

83-
template <class Fq, class Fr, class T>
84-
template <typename BaseField, typename CompileTimeEnabled>
85-
86-
constexpr uint256_t affine_element<Fq, Fr, T>::compress() const noexcept
87-
{
88-
uint256_t out(x);
89-
if (uint256_t(y).get_bit(0)) {
90-
out.data[3] = out.data[3] | 0x8000000000000000ULL;
91-
}
92-
return out;
93-
}
94-
9583
template <class Fq, class Fr, class T> constexpr affine_element<Fq, Fr, T> affine_element<Fq, Fr, T>::infinity()
9684
{
9785
affine_element e{};
@@ -157,15 +145,9 @@ constexpr bool affine_element<Fq, Fr, T>::operator==(const affine_element& other
157145
return !only_one_is_infinity && (both_infinity || ((x == other.x) && (y == other.y)));
158146
}
159147

160-
/**
161-
* Comparison operators (for std::sort)
162-
*
163-
* @details CAUTION!! Don't use this operator. It has no meaning other than for use by std::sort.
164-
**/
165148
template <class Fq, class Fr, class T>
166149
constexpr bool affine_element<Fq, Fr, T>::operator>(const affine_element& other) const noexcept
167150
{
168-
// We are setting point at infinity to always be the lowest element
169151
if (is_point_at_infinity()) {
170152
return false;
171153
}

‎barretenberg/cpp/src/barretenberg/ecc/groups/element.hpp‎

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@ template <class Fq, class Fr, class Params> class alignas(32) element {
5959

6060
constexpr element dbl() const noexcept;
6161
constexpr void self_dbl() noexcept;
62-
constexpr void self_mixed_add_or_sub(const affine_element<Fq, Fr, Params>& other, uint64_t predicate) noexcept;
6362

6463
constexpr element operator+(const element& other) const noexcept;
6564
constexpr element operator+(const affine_element<Fq, Fr, Params>& other) const noexcept;
@@ -128,27 +127,6 @@ template <class Fq, class Fr, class Params> class alignas(32) element {
128127

129128
template <typename = typename std::enable_if<Params::can_hash_to_curve>>
130129
static element random_coordinates_on_curve(numeric::RNG* engine = nullptr) noexcept;
131-
// {
132-
// bool found_one = false;
133-
// Fq yy;
134-
// Fq x;
135-
// Fq y;
136-
// Fq t0;
137-
// while (!found_one) {
138-
// x = Fq::random_element(engine);
139-
// yy = x.sqr() * x + Params::b;
140-
// if constexpr (Params::has_a) {
141-
// yy += (x * Params::a);
142-
// }
143-
// y = yy.sqrt();
144-
// t0 = y.sqr();
145-
// found_one = (yy == t0);
146-
// }
147-
// return { x, y, Fq::one() };
148-
// }
149-
static void conditional_negate_affine(const affine_element<Fq, Fr, Params>& in,
150-
affine_element<Fq, Fr, Params>& out,
151-
uint64_t predicate) noexcept;
152130

153131
friend std::ostream& operator<<(std::ostream& os, const element& a)
154132
{
@@ -162,10 +140,6 @@ template <class Fq, class Fr, class Params> std::ostream& operator<<(std::ostrea
162140
return os << "x:" << e.x << " y:" << e.y << " z:" << e.z;
163141
}
164142

165-
// constexpr element<Fq, Fr, Params>::one = element<Fq, Fr, Params>{ Params::one_x, Params::one_y, Fq::one() };
166-
// constexpr element<Fq, Fr, Params>::point_at_infinity = one.set_infinity();
167-
// constexpr element<Fq, Fr, Params>::curve_b = Params::b;
168-
169143
} // namespace bb::group_elements
170144

171145
#include "./element_impl.hpp"

‎barretenberg/cpp/src/barretenberg/ecc/groups/element_impl.hpp‎

Lines changed: 0 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -155,99 +155,6 @@ template <class Fq, class Fr, class T> constexpr element<Fq, Fr, T> element<Fq,
155155
return result;
156156
}
157157

158-
template <class Fq, class Fr, class T>
159-
constexpr void element<Fq, Fr, T>::self_mixed_add_or_sub(const affine_element<Fq, Fr, T>& other,
160-
const uint64_t predicate) noexcept
161-
{
162-
if constexpr (Fq::modulus.data[3] >= MODULUS_TOP_LIMB_LARGE_THRESHOLD) {
163-
if (is_point_at_infinity()) {
164-
conditional_negate_affine(other, *(affine_element<Fq, Fr, T>*)this, predicate); // NOLINT
165-
z = Fq::one();
166-
return;
167-
}
168-
} else {
169-
const bool edge_case_trigger = x.is_msb_set() || other.x.is_msb_set();
170-
if (edge_case_trigger) {
171-
if (x.is_msb_set()) {
172-
conditional_negate_affine(other, *(affine_element<Fq, Fr, T>*)this, predicate); // NOLINT
173-
z = Fq::one();
174-
}
175-
return;
176-
}
177-
}
178-
179-
// T0 = z1.z1
180-
Fq T0 = z.sqr();
181-
182-
// T1 = x2.t0 - x1 = x2.z1.z1 - x1
183-
Fq T1 = other.x * T0;
184-
T1 -= x;
185-
186-
// T2 = T0.z1 = z1.z1.z1
187-
// T2 = T2.y2 - y1 = y2.z1.z1.z1 - y1
188-
Fq T2 = z * T0;
189-
T2 *= other.y;
190-
T2.self_conditional_negate(predicate);
191-
T2 -= y;
192-
193-
if (__builtin_expect(T1.is_zero(), 0)) {
194-
if (T2.is_zero()) {
195-
// y2 equals y1, x2 equals x1, double x1
196-
self_dbl();
197-
return;
198-
}
199-
self_set_infinity();
200-
return;
201-
}
202-
203-
// T2 = 2T2 = 2(y2.z1.z1.z1 - y1) = R
204-
// z3 = z1 + H
205-
T2 += T2;
206-
z += T1;
207-
208-
// T3 = T1*T1 = HH
209-
Fq T3 = T1.sqr();
210-
211-
// z3 = z3 - z1z1 - HH
212-
T0 += T3;
213-
214-
// z3 = (z1 + H)*(z1 + H)
215-
z.self_sqr();
216-
z -= T0;
217-
218-
// T3 = 4HH
219-
T3 += T3;
220-
T3 += T3;
221-
222-
// T1 = T1*T3 = 4HHH
223-
T1 *= T3;
224-
225-
// T3 = T3 * x1 = 4HH*x1
226-
T3 *= x;
227-
228-
// T0 = 2T3
229-
T0 = T3 + T3;
230-
231-
// T0 = T0 + T1 = 2(4HH*x1) + 4HHH
232-
T0 += T1;
233-
x = T2.sqr();
234-
235-
// x3 = x3 - T0 = R*R - 8HH*x1 -4HHH
236-
x -= T0;
237-
238-
// T3 = T3 - x3 = 4HH*x1 - x3
239-
T3 -= x;
240-
241-
T1 *= y;
242-
T1 += T1;
243-
244-
// T3 = T2 * T3 = R*(4HH*x1 - x3)
245-
T3 *= T2;
246-
247-
// y3 = T3 - T1
248-
y = T3 - T1;
249-
}
250-
251158
template <class Fq, class Fr, class T>
252159
constexpr element<Fq, Fr, T> element<Fq, Fr, T>::operator+=(const affine_element<Fq, Fr, T>& other) noexcept
253160
{
@@ -1057,14 +964,6 @@ std::vector<affine_element<Fq, Fr, T>> element<Fq, Fr, T>::batch_mul_with_endomo
1057964
return work_elements;
1058965
}
1059966

1060-
template <typename Fq, typename Fr, typename T>
1061-
void element<Fq, Fr, T>::conditional_negate_affine(const affine_element<Fq, Fr, T>& in,
1062-
affine_element<Fq, Fr, T>& out,
1063-
const uint64_t predicate) noexcept
1064-
{
1065-
out = { in.x, predicate ? -in.y : in.y };
1066-
}
1067-
1068967
template <typename Fq, typename Fr, typename T>
1069968
void element<Fq, Fr, T>::batch_normalize(element* elements, const size_t num_elements) noexcept
1070969
{

‎barretenberg/cpp/src/barretenberg/ecc/groups/group.hpp‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -130,16 +130,6 @@ template <typename Fq_, typename Fr_, typename Params> class group {
130130
}
131131
return derive_generators(domain_bytes, num_generators, starting_index);
132132
}
133-
134-
BB_INLINE static void conditional_negate_affine(const affine_element* src,
135-
affine_element* dest,
136-
uint64_t predicate);
137133
};
138134

139135
} // namespace bb
140-
141-
#ifdef DISABLE_ASM
142-
#include "group_impl_int128.tcc"
143-
#else
144-
#include "group_impl_asm.tcc"
145-
#endif

0 commit comments

Comments
 (0)