fix: biggroup and cycle group point at infinity handling #19925
Conversation
suyash67
force-pushed
the
sb/biggroup-fixes-spearbit
branch
3 times, most recently
from
e36d1ac to
7088c52
Compare
| bigfield<Builder, T>::bigfield(Builder* parent_context) | ||
| : context(parent_context) | ||
| , binary_basis_limbs{ Limb(bb::fr(0)), Limb(bb::fr(0)), Limb(bb::fr(0)), Limb(bb::fr(0)) } | ||
| , binary_basis_limbs{ Limb(field_t<Builder>(parent_context, bb::fr(0))), |
There was a problem hiding this comment.
This was initialising the constant limbs with nullptr context. Fixed it to use parent_context.
| Limb(bb::fr(value.slice(NUM_LIMB_BITS, NUM_LIMB_BITS * 2))), | ||
| Limb(bb::fr(value.slice(NUM_LIMB_BITS * 2, NUM_LIMB_BITS * 3))), | ||
| Limb(bb::fr(value.slice(NUM_LIMB_BITS * 3, NUM_LIMB_BITS * 4))) } | ||
| , binary_basis_limbs{ Limb(field_t<Builder>(parent_context, bb::fr(value.slice(0, NUM_LIMB_BITS)))), |
There was a problem hiding this comment.
Same as above, we must initialise constants with parent_context. Otherwise the code crashes at some point due to context = nullptr.
| } | ||
|
|
||
| template <typename Builder, typename T> | ||
| void bigfield<Builder, T>::assert_zero_if(const bool_t<Builder>& predicate, std::string const& msg) const |
There was a problem hiding this comment.
New function: asserts that a bigfield is zero (all limbs are 0) if predicate is true. We need this in biggroup to ensure that the coordinates are 0 if the infinity flag is true.
| template <typename Builder, typename T> | ||
| void bigfield<Builder, T>::assert_less_than(const uint256_t& upper_limit, std::string const& msg) const | ||
| { | ||
| // For constant bigfields, just verify the value is in range (no circuit constraints needed) |
There was a problem hiding this comment.
This didn't have a branch handling constants
| } | ||
|
|
||
| // Test assert_zero_if with a zero bigfield - should always pass regardless of predicate | ||
| static void test_assert_zero_if_zero_value(bool predicate_value, InputType predicate_type) |
There was a problem hiding this comment.
Tests for assert_zero_if
| static constexpr size_t PUBLIC_INPUTS_SIZE = BIGGROUP_PUBLIC_INPUTS_SIZE; | ||
|
|
||
| element(); | ||
| element(const typename NativeGroup::affine_element& input); |
There was a problem hiding this comment.
Removed this constructor
| // The infinity flag is automatically set based on whether both coordinates are zero. | ||
| // By default, we validate that the point is on the curve | ||
| element(const Fq& x, const Fq& y, const bool assert_on_curve = true); | ||
| element(const Fq& x, const Fq& y, const bool_ct& is_infinity, bool assert_on_curve = true); |
There was a problem hiding this comment.
Made this constructor private because it directly sets the infinity flag.
|
|
||
| // Create _is_infinity as a witness for consistency with points from arithmetic operations. | ||
| // Security: Since assert_on_curve is true, validate_on_curve() enforces that if infinity=true, then x = y = 0. | ||
| bool_ct is_infinity = bool_ct(witness_ct(ctx, false)); |
There was a problem hiding this comment.
Creating infinity flag as a witness but ensure the coordinates are (0, 0) if the flag is true (in validate_on_curve())
|
|
||
| // Convert point into point + (2ⁱ)⋅(δG_offset) | ||
| points.push_back(_points[i] + running_point); | ||
| points.push_back(_points[i].add_internal(running_point)); |
There was a problem hiding this comment.
Use internal operators for efficiency
suyash67
force-pushed
the
sb/biggroup-fixes-spearbit
branch
2 times, most recently
from
ada0089 to
1d6fdf9
Compare
…yway create and access witness pai with operators. we must maintain the canonical form: that's what matters for security.
suyash67
force-pushed
the
sb/biggroup-fixes-spearbit
branch
from
250ef2e to
160516a
Compare
wip