Skip to content

chore: ecc group audit 1- remove dead code, documentation and test #20886

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.

Sign up for GitHub

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

Merged
kashbrti merged 17 commits into from
Mar 3, 2026
Merged

chore: ecc group audit 1- remove dead code, documentation and test #20886

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
3b03665
chore: opt sol verifier scope + renaming (#20069)
iakovenkos Feb 2, 2026
04b0805
chore: graph tooling audit 0 (#19691)
ledwards2225 Feb 2, 2026
c4ffbdc
Merge branch 'next' into merge-train/barretenberg
Feb 3, 2026
f051a11
Merge branch 'next' into merge-train/barretenberg
Feb 3, 2026
2dcc347
Merge branch 'next' into merge-train/barretenberg
Feb 3, 2026
52c5916
Merge branch 'next' into merge-train/barretenberg
Feb 3, 2026
ec6866f
Merge branch 'merge-train/barretenberg' of github.com:AztecProtocol/a…
kashbrti Feb 4, 2026
33ff1ab
Merge branch 'merge-train/barretenberg' of github.com:AztecProtocol/a…
kashbrti Feb 6, 2026
23d3574
removing dead code
kashbrti Feb 26, 2026
3c8bb2f
Merge branch 'merge-train/barretenberg' of github.com:AztecProtocol/a…
kashbrti Feb 26, 2026
432fbbf
Merge branch 'merge-train/barretenberg' into kb/group_audit
kashbrti Feb 26, 2026
168e7ca
cleanup
kashbrti Mar 2, 2026
6b70cdb
docs
kashbrti Mar 2, 2026
b738a31
edge case testing
kashbrti Mar 2, 2026
dde80e4
Merge branch 'merge-train/barretenberg' into kb/group_audit
kashbrti Mar 2, 2026
a5bd087
nits
kashbrti Mar 3, 2026
a228c4b
Merge branch 'merge-train/barretenberg' into kb/group_audit
kashbrti Mar 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions barretenberg/cpp/src/barretenberg/ecc/groups/affine_element.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,6 @@ template <typename Fq_, typename Fr_, typename Params_> class alignas(64) affine

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

template <typename BaseField = Fq,
typename CompileTimeEnabled = std::enable_if_t<(BaseField::modulus >> 255) == uint256_t(0), void>>
[[nodiscard]] constexpr uint256_t compress() const noexcept;

static constexpr affine_element infinity();
constexpr affine_element set_infinity() const noexcept;
constexpr void self_set_infinity() noexcept;
Expand Down
7 changes: 4 additions & 3 deletions barretenberg/cpp/src/barretenberg/ecc/groups/affine_element.test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,10 @@ template <typename G1> class TestAffineElement : public testing::Test {
{
for (size_t i = 0; i < 10; i++) {
affine_element P = affine_element(element::random_element());
uint256_t compressed = P.compress();
uint256_t compressed = uint256_t(P.x);
if (uint256_t(P.y).get_bit(0)) {
compressed.data[3] |= 0x8000000000000000ULL;

@suyash67 suyash67 Mar 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe declare 0x8000000000000000ULL as a static constant somewhere, especially if its being used in multiple places.

}
affine_element Q = affine_element::from_compressed(compressed);
EXPECT_EQ(P, Q);
}
Expand Down Expand Up @@ -168,8 +171,6 @@ template <typename G1> class TestAffineElement : public testing::Test {
affine_element R(0, P.y);
ASSERT_FALSE(P == R);
}
// Regression test to ensure that the point at infinity is not equal to its coordinate-wise reduction, which may lie
// on the curve, depending on the y-coordinate.
static void test_infinity_ordering_regression()
{
affine_element P(0, 1);
Expand Down
18 changes: 0 additions & 18 deletions barretenberg/cpp/src/barretenberg/ecc/groups/affine_element_impl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,18 +80,6 @@ constexpr affine_element<Fq, Fr, T> affine_element<Fq, Fr, T>::operator*(const F
return bb::group_elements::element(*this) * exponent;
}

template <class Fq, class Fr, class T>
template <typename BaseField, typename CompileTimeEnabled>

constexpr uint256_t affine_element<Fq, Fr, T>::compress() const noexcept
{
uint256_t out(x);
if (uint256_t(y).get_bit(0)) {
out.data[3] = out.data[3] | 0x8000000000000000ULL;
}
return out;
}

template <class Fq, class Fr, class T> constexpr affine_element<Fq, Fr, T> affine_element<Fq, Fr, T>::infinity()
{
affine_element e{};
Expand Down Expand Up @@ -157,15 +145,9 @@ constexpr bool affine_element<Fq, Fr, T>::operator==(const affine_element& other
return !only_one_is_infinity && (both_infinity || ((x == other.x) && (y == other.y)));
}

/**
* Comparison operators (for std::sort)
*
* @details CAUTION!! Don't use this operator. It has no meaning other than for use by std::sort.
**/
template <class Fq, class Fr, class T>
constexpr bool affine_element<Fq, Fr, T>::operator>(const affine_element& other) const noexcept
{
// We are setting point at infinity to always be the lowest element
if (is_point_at_infinity()) {
return false;
}
Expand Down
7 changes: 3 additions & 4 deletions barretenberg/cpp/src/barretenberg/ecc/groups/element.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ template <class Fq, class Fr, class Params> class alignas(32) element {

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

constexpr element operator+(const element& other) const noexcept;
constexpr element operator+(const affine_element<Fq, Fr, Params>& other) const noexcept;
Expand Down Expand Up @@ -146,9 +145,9 @@ template <class Fq, class Fr, class Params> class alignas(32) element {
// }
// return { x, y, Fq::one() };
// }
static void conditional_negate_affine(const affine_element<Fq, Fr, Params>& in,
affine_element<Fq, Fr, Params>& out,
uint64_t predicate) noexcept;
// TODO(https://github.com/AztecProtocol/barretenberg/issues/908) point at inifinty isn't handled
// To reenable this do NOT do use MSGPACK_FIELDS macro below, instead follow the logic in affine_element
// MSGPACK_FIELDS(x, y, z);

friend std::ostream& operator<<(std::ostream& os, const element& a)
{
Expand Down
101 changes: 0 additions & 101 deletions barretenberg/cpp/src/barretenberg/ecc/groups/element_impl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,99 +155,6 @@ template <class Fq, class Fr, class T> constexpr element<Fq, Fr, T> element<Fq,
return result;
}

template <class Fq, class Fr, class T>
constexpr void element<Fq, Fr, T>::self_mixed_add_or_sub(const affine_element<Fq, Fr, T>& other,
const uint64_t predicate) noexcept
{
if constexpr (Fq::modulus.data[3] >= MODULUS_TOP_LIMB_LARGE_THRESHOLD) {
if (is_point_at_infinity()) {
conditional_negate_affine(other, *(affine_element<Fq, Fr, T>*)this, predicate); // NOLINT
z = Fq::one();
return;
}
} else {
const bool edge_case_trigger = x.is_msb_set() || other.x.is_msb_set();
if (edge_case_trigger) {
if (x.is_msb_set()) {
conditional_negate_affine(other, *(affine_element<Fq, Fr, T>*)this, predicate); // NOLINT
z = Fq::one();
}
return;
}
}

// T0 = z1.z1
Fq T0 = z.sqr();

// T1 = x2.t0 - x1 = x2.z1.z1 - x1
Fq T1 = other.x * T0;
T1 -= x;

// T2 = T0.z1 = z1.z1.z1
// T2 = T2.y2 - y1 = y2.z1.z1.z1 - y1
Fq T2 = z * T0;
T2 *= other.y;
T2.self_conditional_negate(predicate);
T2 -= y;

if (__builtin_expect(T1.is_zero(), 0)) {
if (T2.is_zero()) {
// y2 equals y1, x2 equals x1, double x1
self_dbl();
return;
}
self_set_infinity();
return;
}

// T2 = 2T2 = 2(y2.z1.z1.z1 - y1) = R
// z3 = z1 + H
T2 += T2;
z += T1;

// T3 = T1*T1 = HH
Fq T3 = T1.sqr();

// z3 = z3 - z1z1 - HH
T0 += T3;

// z3 = (z1 + H)*(z1 + H)
z.self_sqr();
z -= T0;

// T3 = 4HH
T3 += T3;
T3 += T3;

// T1 = T1*T3 = 4HHH
T1 *= T3;

// T3 = T3 * x1 = 4HH*x1
T3 *= x;

// T0 = 2T3
T0 = T3 + T3;

// T0 = T0 + T1 = 2(4HH*x1) + 4HHH
T0 += T1;
x = T2.sqr();

// x3 = x3 - T0 = R*R - 8HH*x1 -4HHH
x -= T0;

// T3 = T3 - x3 = 4HH*x1 - x3
T3 -= x;

T1 *= y;
T1 += T1;

// T3 = T2 * T3 = R*(4HH*x1 - x3)
T3 *= T2;

// y3 = T3 - T1
y = T3 - T1;
}

template <class Fq, class Fr, class T>
constexpr element<Fq, Fr, T> element<Fq, Fr, T>::operator+=(const affine_element<Fq, Fr, T>& other) noexcept
{
Expand Down Expand Up @@ -1057,14 +964,6 @@ std::vector<affine_element<Fq, Fr, T>> element<Fq, Fr, T>::batch_mul_with_endomo
return work_elements;
}

template <typename Fq, typename Fr, typename T>
void element<Fq, Fr, T>::conditional_negate_affine(const affine_element<Fq, Fr, T>& in,
affine_element<Fq, Fr, T>& out,
const uint64_t predicate) noexcept
{
out = { in.x, predicate ? -in.y : in.y };
}

template <typename Fq, typename Fr, typename T>
void element<Fq, Fr, T>::batch_normalize(element* elements, const size_t num_elements) noexcept
{
Expand Down
4 changes: 0 additions & 4 deletions barretenberg/cpp/src/barretenberg/ecc/groups/group.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,6 @@ template <typename Fq_, typename Fr_, typename Params> class group {
}
return derive_generators(domain_bytes, num_generators, starting_index);
}

BB_INLINE static void conditional_negate_affine(const affine_element* src,
affine_element* dest,
uint64_t predicate);
};

} // namespace bb
Expand Down
157 changes: 1 addition & 156 deletions barretenberg/cpp/src/barretenberg/ecc/groups/group_impl_asm.tcc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,161 +2,6 @@

#ifndef DISABLE_ASM

#include "barretenberg/ecc/groups/group.hpp"

@kashbrti kashbrti Mar 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this was only used in self_mixed_add_or_sub which was never used. our scalar multiplication function already does the conditional negation

@suyash67 suyash67 Mar 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Delete the file altogether?

#include <cstdint>

namespace bb {
// copies src into dest. n.b. both src and dest must be aligned on 32 byte boundaries
// template <typename Fq, typename Fr, typename Params>
// inline void group<Fq, Fr, Params>::copy(const affine_element* src, affine_element*
// dest)
// {
// if constexpr (Params::small_elements) {
// #if defined __AVX__ && defined USE_AVX
// ASSERT((((uintptr_t)src & 0x1f) == 0));
// ASSERT((((uintptr_t)dest & 0x1f) == 0));
// __asm__ __volatile__("vmovdqa 0(%0), %%ymm0 \n\t"
// "vmovdqa 32(%0), %%ymm1 \n\t"
// "vmovdqa %%ymm0, 0(%1) \n\t"
// "vmovdqa %%ymm1, 32(%1) \n\t"
// :
// : "r"(src), "r"(dest)
// : "%ymm0", "%ymm1", "memory");
// #else
// *dest = *src;
// #endif
// } else {
// *dest = *src;
// }
// }

// // copies src into dest. n.b. both src and dest must be aligned on 32 byte boundaries
// template <typename Fq, typename Fr, typename Params>
// inline void group<Fq, Fr, Params>::copy(const element* src, element* dest)
// {
// if constexpr (Params::small_elements) {
// #if defined __AVX__ && defined USE_AVX
// ASSERT((((uintptr_t)src & 0x1f) == 0));
// ASSERT((((uintptr_t)dest & 0x1f) == 0));
// __asm__ __volatile__("vmovdqa 0(%0), %%ymm0 \n\t"
// "vmovdqa 32(%0), %%ymm1 \n\t"
// "vmovdqa 64(%0), %%ymm2 \n\t"
// "vmovdqa %%ymm0, 0(%1) \n\t"
// "vmovdqa %%ymm1, 32(%1) \n\t"
// "vmovdqa %%ymm2, 64(%1) \n\t"
// :
// : "r"(src), "r"(dest)
// : "%ymm0", "%ymm1", "%ymm2", "memory");
// #else
// *dest = *src;
// #endif
// } else {
// *dest = src;
// }
// }

// copies src into dest, inverting y-coordinate if 'predicate' is true
// n.b. requires src and dest to be aligned on 32 byte boundary
template <typename Fq, typename Fr, typename Params>
inline void group<Fq, Fr, Params>::conditional_negate_affine(const affine_element* src,
affine_element* dest,
uint64_t predicate)
{
constexpr uint256_t twice_modulus = Fq::modulus + Fq::modulus;

constexpr uint64_t twice_modulus_0 = twice_modulus.data[0];
constexpr uint64_t twice_modulus_1 = twice_modulus.data[1];
constexpr uint64_t twice_modulus_2 = twice_modulus.data[2];
constexpr uint64_t twice_modulus_3 = twice_modulus.data[3];

if constexpr (Params::small_elements) {
#if defined __AVX__ && defined USE_AVX
BB_ASSERT_EQ(((uintptr_t)src & 0x1f, 0));
BB_ASSERT_EQ(((uintptr_t)dest & 0x1f, 0));
__asm__ __volatile__("xorq %%r8, %%r8 \n\t"
"movq 32(%0), %%r8 \n\t"
"movq 40(%0), %%r9 \n\t"
"movq 48(%0), %%r10 \n\t"
"movq 56(%0), %%r11 \n\t"
"movq %[modulus_0], %%r12 \n\t"
"movq %[modulus_1], %%r13 \n\t"
"movq %[modulus_2], %%r14 \n\t"
"movq %[modulus_3], %%r15 \n\t"
"subq %%r8, %%r12 \n\t"
"sbbq %%r9, %%r13 \n\t"
"sbbq %%r10, %%r14 \n\t"
"sbbq %%r11, %%r15 \n\t"
"testq %2, %2 \n\t"
"cmovnzq %%r12, %%r8 \n\t"
"cmovnzq %%r13, %%r9 \n\t"
"cmovnzq %%r14, %%r10 \n\t"
"cmovnzq %%r15, %%r11 \n\t"
"vmovdqa 0(%0), %%ymm0 \n\t"
"vmovdqa %%ymm0, 0(%1) \n\t"
"movq %%r8, 32(%1) \n\t"
"movq %%r9, 40(%1) \n\t"
"movq %%r10, 48(%1) \n\t"
"movq %%r11, 56(%1) \n\t"
:
: "r"(src),
"r"(dest),
"r"(predicate),
[modulus_0] "i"(twice_modulus_0),
[modulus_1] "i"(twice_modulus_1),
[modulus_2] "i"(twice_modulus_2),
[modulus_3] "i"(twice_modulus_3)
: "%r8", "%r9", "%r10", "%r11", "%r12", "%r13", "%r14", "%r15", "%ymm0", "memory", "cc");
#else
__asm__ __volatile__("xorq %%r8, %%r8 \n\t"
"movq 32(%0), %%r8 \n\t"
"movq 40(%0), %%r9 \n\t"
"movq 48(%0), %%r10 \n\t"
"movq 56(%0), %%r11 \n\t"
"movq %[modulus_0], %%r12 \n\t"
"movq %[modulus_1], %%r13 \n\t"
"movq %[modulus_2], %%r14 \n\t"
"movq %[modulus_3], %%r15 \n\t"
"subq %%r8, %%r12 \n\t"
"sbbq %%r9, %%r13 \n\t"
"sbbq %%r10, %%r14 \n\t"
"sbbq %%r11, %%r15 \n\t"
"testq %2, %2 \n\t"
"cmovnzq %%r12, %%r8 \n\t"
"cmovnzq %%r13, %%r9 \n\t"
"cmovnzq %%r14, %%r10 \n\t"
"cmovnzq %%r15, %%r11 \n\t"
"movq 0(%0), %%r12 \n\t"
"movq 8(%0), %%r13 \n\t"
"movq 16(%0), %%r14 \n\t"
"movq 24(%0), %%r15 \n\t"
"movq %%r8, 32(%1) \n\t"
"movq %%r9, 40(%1) \n\t"
"movq %%r10, 48(%1) \n\t"
"movq %%r11, 56(%1) \n\t"
"movq %%r12, 0(%1) \n\t"
"movq %%r13, 8(%1) \n\t"
"movq %%r14, 16(%1) \n\t"
"movq %%r15, 24(%1) \n\t"
:
: "r"(src),
"r"(dest),
"r"(predicate),
[modulus_0] "i"(twice_modulus_0),
[modulus_1] "i"(twice_modulus_1),
[modulus_2] "i"(twice_modulus_2),
[modulus_3] "i"(twice_modulus_3)
: "%r8", "%r9", "%r10", "%r11", "%r12", "%r13", "%r14", "%r15", "memory", "cc");
#endif
} else {
if (predicate) { // NOLINT
Fq::__copy(src->x, dest->x);
dest->y = -src->y;
} else {
copy_affine(*src, *dest);
}
}
}

} // namespace bb
namespace bb {} // namespace bb

#endif
Loading
Loading