-
Notifications
You must be signed in to change notification settings - Fork 270
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.
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
fix: cycle_group validate_is_on_curve bug #4494
Changes from 3 commits
1ab819b
c10521d
5ad9af0
696edbe
16693a5
244eec7
92796be
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,6 +48,43 @@ template <class Builder> class CycleGroupTest : public ::testing::Test { | |
using CircuitTypes = ::testing::Types<bb::StandardCircuitBuilder, bb::UltraCircuitBuilder>; | ||
TYPED_TEST_SUITE(CycleGroupTest, CircuitTypes); | ||
|
||
TYPED_TEST(CycleGroupTest, TestValidateOnCurveSucceed) | ||
lucasxia01 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
STDLIB_TYPE_ALIASES; | ||
auto builder = Builder(); | ||
lucasxia01 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
auto lhs = TestFixture::generators[0]; | ||
cycle_group_ct a = cycle_group_ct::from_witness(&builder, lhs); | ||
a.validate_is_on_curve(); | ||
EXPECT_FALSE(builder.failed()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd recommend doing the check_circuit, too. failed() doesn't check if constraints hold |
||
} | ||
|
||
TYPED_TEST(CycleGroupTest, TestValidateOnCurveInfinitySucceed) | ||
{ | ||
STDLIB_TYPE_ALIASES; | ||
auto builder = Builder(); | ||
|
||
auto x = stdlib::field_t<Builder>::from_witness(&builder, 1); | ||
lucasxia01 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
auto y = stdlib::field_t<Builder>::from_witness(&builder, 1); | ||
|
||
cycle_group_ct a(x, y, true); | ||
lucasxia01 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
a.validate_is_on_curve(); | ||
EXPECT_FALSE(builder.failed()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd recommend doing the check_circuit, too. failed() doesn't check if constraints hold |
||
} | ||
|
||
TYPED_TEST(CycleGroupTest, TestValidateOnCurveFail) | ||
{ | ||
STDLIB_TYPE_ALIASES; | ||
auto builder = Builder(); | ||
|
||
auto x = stdlib::field_t<Builder>::from_witness(&builder, 1); | ||
auto y = stdlib::field_t<Builder>::from_witness(&builder, 1); | ||
|
||
cycle_group_ct a(x, y, false); | ||
a.validate_is_on_curve(); | ||
EXPECT_TRUE(builder.failed()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd recommend doing the check_circuit, too. failed() doesn't check if constraints hold |
||
} | ||
|
||
TYPED_TEST(CycleGroupTest, TestDbl) | ||
{ | ||
STDLIB_TYPE_ALIASES; | ||
|
@@ -436,8 +473,8 @@ TYPED_TEST(CycleGroupTest, TestBatchMul) | |
EXPECT_TRUE(result.is_point_at_infinity().get_value()); | ||
} | ||
|
||
// case 5, fixed-base MSM with inputs that are combinations of constant and witnesses (group elements are in lookup | ||
// table) | ||
// case 5, fixed-base MSM with inputs that are combinations of constant and witnesses (group elements are in | ||
// lookup table) | ||
{ | ||
std::vector<cycle_group_ct> points; | ||
std::vector<typename cycle_group_ct::cycle_scalar> scalars; | ||
|
@@ -465,8 +502,8 @@ TYPED_TEST(CycleGroupTest, TestBatchMul) | |
EXPECT_EQ(result.get_value(), crypto::pedersen_commitment::commit_native(scalars_native)); | ||
} | ||
|
||
// case 6, fixed-base MSM with inputs that are combinations of constant and witnesses (some group elements are in | ||
// lookup table) | ||
// case 6, fixed-base MSM with inputs that are combinations of constant and witnesses (some group elements are | ||
// in lookup table) | ||
{ | ||
std::vector<cycle_group_ct> points; | ||
std::vector<typename cycle_group_ct::cycle_scalar> scalars; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changing a hardcoded hash should be fine here, but someone should double check me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI should automatically detect if you computed incorrectly