Skip to content

Commit a022220

Browse files
zac-williamsoncodygunton
zac-williamson
and
codygunton
authored
fix: ECCVM correctly handles points at infinity and group operation edge cases (#6388)
Please read [contributing guidelines](CONTRIBUTING.md) and remove this line. --------- Co-authored-by: codygunton <codygunton@gmail.com>
1 parent 17f6e1d commit a022220

19 files changed

Lines changed: 1106 additions & 305 deletions

‎barretenberg/cpp/src/barretenberg/benchmark/relations_bench/relations.bench.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ BENCHMARK(execute_relation_for_values<ECCVMFlavor, ECCVMPointTableRelation<Fq>>)
117117
BENCHMARK(execute_relation_for_values<ECCVMFlavor, ECCVMSetRelation<Fq>>);
118118
BENCHMARK(execute_relation_for_values<ECCVMFlavor, ECCVMTranscriptRelation<Fq>>);
119119
BENCHMARK(execute_relation_for_values<ECCVMFlavor, ECCVMWnafRelation<Fq>>);
120+
BENCHMARK(execute_relation_for_values<ECCVMFlavor, ECCVMBoolsRelation<Fq>>);
120121

121122
} // namespace bb::benchmark::relations
122123

‎barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.test.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ TEST_F(ClientIVCTests, BasicFailure)
119119
* @brief Prove and verify accumulation of an arbitrary set of circuits
120120
*
121121
*/
122-
TEST_F(ClientIVCTests, DISABLED_BasicLarge)
122+
TEST_F(ClientIVCTests, BasicLarge)
123123
{
124124
ClientIVC ivc;
125125

@@ -142,7 +142,7 @@ TEST_F(ClientIVCTests, DISABLED_BasicLarge)
142142
* @brief Using a structured trace allows for the accumulation of circuits of varying size
143143
*
144144
*/
145-
TEST_F(ClientIVCTests, DISABLED_BasicStructured)
145+
TEST_F(ClientIVCTests, BasicStructured)
146146
{
147147
ClientIVC ivc;
148148
ivc.structured_flag = true;

‎barretenberg/cpp/src/barretenberg/eccvm/eccvm_circuit_builder.hpp‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -120,15 +120,10 @@ class ECCVMCircuitBuilder {
120120
// populate opqueue and mul indices
121121
for (const auto& op : raw_ops) {
122122
if (op.mul) {
123-
if (op.z1 != 0 || op.z2 != 0) {
123+
if ((op.z1 != 0 || op.z2 != 0) && !op.base_point.is_point_at_infinity()) {
124124
msm_opqueue_index.push_back(op_idx);
125125
msm_mul_index.emplace_back(msm_count, active_mul_count);
126-
}
127-
if (op.z1 != 0) {
128-
active_mul_count++;
129-
}
130-
if (op.z2 != 0) {
131-
active_mul_count++;
126+
active_mul_count += static_cast<size_t>(op.z1 != 0) + static_cast<size_t>(op.z2 != 0);
132127
}
133128
} else if (active_mul_count > 0) {
134129
msm_sizes.push_back(active_mul_count);
@@ -138,7 +133,7 @@ class ECCVMCircuitBuilder {
138133
op_idx++;
139134
}
140135
// if last op is a mul we have not correctly computed the total number of msms
141-
if (raw_ops.back().mul) {
136+
if (raw_ops.back().mul && active_mul_count > 0) {
142137
msm_sizes.push_back(active_mul_count);
143138
msm_count++;
144139
}
@@ -152,7 +147,7 @@ class ECCVMCircuitBuilder {
152147
for (size_t i = start; i < end; i++) {
153148
const auto& op = raw_ops[msm_opqueue_index[i]];
154149
auto [msm_index, mul_index] = msm_mul_index[i];
155-
if (op.z1 != 0) {
150+
if (op.z1 != 0 && !op.base_point.is_point_at_infinity()) {
156151
ASSERT(result.size() > msm_index);
157152
ASSERT(result[msm_index].size() > mul_index);
158153
result[msm_index][mul_index] = (ScalarMul{
@@ -165,7 +160,7 @@ class ECCVMCircuitBuilder {
165160
});
166161
mul_index++;
167162
}
168-
if (op.z2 != 0) {
163+
if (op.z2 != 0 && !op.base_point.is_point_at_infinity()) {
169164
ASSERT(result.size() > msm_index);
170165
ASSERT(result[msm_index].size() > mul_index);
171166
auto endo_point = AffineElement{ op.base_point.x * FF::cube_root_of_unity(), -op.base_point.y };

‎barretenberg/cpp/src/barretenberg/eccvm/eccvm_circuit_builder.test.cpp‎

Lines changed: 204 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,10 @@ TEST(ECCVMCircuitBuilderTests, BaseCase)
1818
typename G1::element a = generators[0];
1919
typename G1::element b = generators[1];
2020
typename G1::element c = generators[2];
21+
typename G1::element point_at_infinity = G1::point_at_infinity;
2122
Fr x = Fr::random_element(&engine);
2223
Fr y = Fr::random_element(&engine);
24+
Fr zero_scalar = 0;
2325

2426
std::shared_ptr<ECCOpQueue> op_queue = std::make_shared<ECCOpQueue>();
2527

@@ -29,14 +31,49 @@ TEST(ECCVMCircuitBuilderTests, BaseCase)
2931
op_queue->mul_accumulate(b, y);
3032
op_queue->add_accumulate(a);
3133
op_queue->mul_accumulate(b, x);
34+
op_queue->no_op();
35+
op_queue->add_accumulate(b);
3236
op_queue->eq_and_reset();
3337
op_queue->add_accumulate(c);
3438
op_queue->mul_accumulate(a, x);
39+
op_queue->mul_accumulate(point_at_infinity, x);
3540
op_queue->mul_accumulate(b, x);
3641
op_queue->eq_and_reset();
3742
op_queue->mul_accumulate(a, x);
3843
op_queue->mul_accumulate(b, x);
44+
op_queue->mul_accumulate(point_at_infinity, zero_scalar);
3945
op_queue->mul_accumulate(c, x);
46+
op_queue->eq_and_reset();
47+
op_queue->mul_accumulate(point_at_infinity, zero_scalar);
48+
op_queue->mul_accumulate(point_at_infinity, x);
49+
op_queue->mul_accumulate(point_at_infinity, zero_scalar);
50+
op_queue->add_accumulate(a);
51+
op_queue->eq_and_reset();
52+
op_queue->add_accumulate(a);
53+
op_queue->add_accumulate(point_at_infinity);
54+
op_queue->eq_and_reset();
55+
op_queue->add_accumulate(point_at_infinity);
56+
op_queue->eq_and_reset();
57+
op_queue->mul_accumulate(point_at_infinity, x);
58+
op_queue->mul_accumulate(point_at_infinity, -x);
59+
op_queue->eq_and_reset();
60+
op_queue->add_accumulate(a);
61+
op_queue->mul_accumulate(point_at_infinity, x);
62+
op_queue->mul_accumulate(point_at_infinity, -x);
63+
op_queue->add_accumulate(a);
64+
op_queue->add_accumulate(a);
65+
op_queue->eq_and_reset();
66+
67+
ECCVMCircuitBuilder circuit{ op_queue };
68+
bool result = ECCVMTraceChecker::check(circuit);
69+
EXPECT_EQ(result, true);
70+
}
71+
72+
TEST(ECCVMCircuitBuilderTests, NoOp)
73+
{
74+
std::shared_ptr<ECCOpQueue> op_queue = std::make_shared<ECCOpQueue>();
75+
76+
op_queue->no_op();
4077

4178
ECCVMCircuitBuilder circuit{ op_queue };
4279
bool result = ECCVMTraceChecker::check(circuit, &engine);
@@ -72,6 +109,109 @@ TEST(ECCVMCircuitBuilderTests, Mul)
72109
EXPECT_EQ(result, true);
73110
}
74111

112+
TEST(ECCVMCircuitBuilderTests, MulInfinity)
113+
{
114+
std::shared_ptr<ECCOpQueue> op_queue = std::make_shared<ECCOpQueue>();
115+
116+
auto generators = G1::derive_generators("test generators", 3);
117+
typename G1::element a = generators[0];
118+
Fr x = Fr::random_element(&engine);
119+
G1::element b = -a * x;
120+
// G1::affine_element c = G1::affine_point_at_infinity;
121+
op_queue->add_accumulate(b);
122+
op_queue->mul_accumulate(a, x);
123+
op_queue->eq_and_reset();
124+
ECCVMCircuitBuilder circuit{ op_queue };
125+
bool result = ECCVMTraceChecker::check(circuit);
126+
EXPECT_EQ(result, true);
127+
}
128+
129+
// Validate we do not trigger edge cases of addition formulae when we have identical mul inputs
130+
TEST(ECCVMCircuitBuilderTests, MulOverIdenticalInputs)
131+
{
132+
std::shared_ptr<ECCOpQueue> op_queue = std::make_shared<ECCOpQueue>();
133+
134+
auto generators = G1::derive_generators("test generators", 3);
135+
typename G1::element a = generators[0];
136+
Fr x = Fr::random_element(&engine);
137+
op_queue->mul_accumulate(a, x);
138+
op_queue->mul_accumulate(a, x);
139+
op_queue->eq_and_reset();
140+
ECCVMCircuitBuilder circuit{ op_queue };
141+
bool result = ECCVMTraceChecker::check(circuit);
142+
EXPECT_EQ(result, true);
143+
}
144+
145+
TEST(ECCVMCircuitBuilderTests, MSMProducesInfinity)
146+
{
147+
std::shared_ptr<ECCOpQueue> op_queue = std::make_shared<ECCOpQueue>();
148+
149+
auto generators = G1::derive_generators("test generators", 3);
150+
typename G1::element a = generators[0];
151+
Fr x = Fr::random_element(&engine);
152+
op_queue->add_accumulate(a);
153+
op_queue->mul_accumulate(a, x);
154+
op_queue->mul_accumulate(a, -x);
155+
op_queue->eq_and_reset();
156+
ECCVMCircuitBuilder circuit{ op_queue };
157+
bool result = ECCVMTraceChecker::check(circuit);
158+
EXPECT_EQ(result, true);
159+
}
160+
161+
TEST(ECCVMCircuitBuilderTests, MSMOverPointAtInfinity)
162+
{
163+
std::shared_ptr<ECCOpQueue> op_queue = std::make_shared<ECCOpQueue>();
164+
165+
auto generators = G1::derive_generators("test generators", 3);
166+
typename G1::element point_at_infinity = G1::point_at_infinity;
167+
typename G1::element b = generators[0];
168+
Fr x = Fr::random_element(&engine);
169+
Fr zero_scalar = 0;
170+
171+
// validate including points at infinity in a multiscalar multiplication does not effect result
172+
{
173+
op_queue->mul_accumulate(b, x);
174+
op_queue->mul_accumulate(point_at_infinity, x);
175+
op_queue->eq_and_reset();
176+
ECCVMCircuitBuilder circuit{ op_queue };
177+
bool result = ECCVMTraceChecker::check(circuit);
178+
EXPECT_EQ(result, true);
179+
}
180+
// validate multiplying a point at infinity by nonzero scalar produces point at infinity
181+
{
182+
op_queue->mul_accumulate(point_at_infinity, x);
183+
op_queue->eq_and_reset();
184+
ECCVMCircuitBuilder circuit{ op_queue };
185+
bool result = ECCVMTraceChecker::check(circuit);
186+
EXPECT_EQ(result, true);
187+
}
188+
// validate multiplying a point by zero produces point at infinity
189+
{
190+
op_queue->mul_accumulate(b, zero_scalar);
191+
op_queue->eq_and_reset();
192+
ECCVMCircuitBuilder circuit{ op_queue };
193+
bool result = ECCVMTraceChecker::check(circuit);
194+
EXPECT_EQ(result, true);
195+
}
196+
// validate multiplying a point at infinity by zero produces a point at infinity
197+
{
198+
op_queue->mul_accumulate(point_at_infinity, zero_scalar);
199+
op_queue->eq_and_reset();
200+
ECCVMCircuitBuilder circuit{ op_queue };
201+
bool result = ECCVMTraceChecker::check(circuit);
202+
EXPECT_EQ(result, true);
203+
}
204+
// validate an MSM made entirely of points at infinity / zero scalars produces a point at infinity
205+
{
206+
op_queue->mul_accumulate(point_at_infinity, x);
207+
op_queue->mul_accumulate(b, zero_scalar);
208+
op_queue->eq_and_reset();
209+
ECCVMCircuitBuilder circuit{ op_queue };
210+
bool result = ECCVMTraceChecker::check(circuit);
211+
EXPECT_EQ(result, true);
212+
}
213+
}
214+
75215
TEST(ECCVMCircuitBuilderTests, ShortMul)
76216
{
77217
std::shared_ptr<ECCOpQueue> op_queue = std::make_shared<ECCOpQueue>();
@@ -243,3 +383,67 @@ TEST(ECCVMCircuitBuilderTests, MSM)
243383
bool result = ECCVMTraceChecker::check(circuit, &engine);
244384
EXPECT_EQ(result, true);
245385
}
386+
387+
TEST(ECCVMCircuitBuilderTests, EqAgainstPointAtInfinity)
388+
{
389+
std::shared_ptr<ECCOpQueue> op_queue = std::make_shared<ECCOpQueue>();
390+
391+
auto generators = G1::derive_generators("test generators", 3);
392+
typename G1::element a = generators[0];
393+
a.self_set_infinity();
394+
395+
op_queue->add_accumulate(a);
396+
op_queue->eq_and_reset();
397+
398+
ECCVMCircuitBuilder circuit{ op_queue };
399+
bool result = ECCVMTraceChecker::check(circuit);
400+
EXPECT_EQ(result, true);
401+
}
402+
403+
TEST(ECCVMCircuitBuilderTests, AddPointAtInfinity)
404+
{
405+
std::shared_ptr<ECCOpQueue> op_queue = std::make_shared<ECCOpQueue>();
406+
407+
auto generators = G1::derive_generators("test generators", 3);
408+
typename G1::element a = generators[0];
409+
typename G1::element b = generators[0];
410+
b.self_set_infinity();
411+
412+
op_queue->add_accumulate(a);
413+
op_queue->add_accumulate(b);
414+
op_queue->eq_and_reset();
415+
416+
ECCVMCircuitBuilder circuit{ op_queue };
417+
bool result = ECCVMTraceChecker::check(circuit);
418+
EXPECT_EQ(result, true);
419+
}
420+
421+
TEST(ECCVMCircuitBuilderTests, AddProducesPointAtInfinity)
422+
{
423+
std::shared_ptr<ECCOpQueue> op_queue = std::make_shared<ECCOpQueue>();
424+
425+
auto generators = G1::derive_generators("test generators", 3);
426+
typename G1::element a = generators[0];
427+
428+
op_queue->add_accumulate(a);
429+
op_queue->add_accumulate(-a);
430+
op_queue->eq_and_reset();
431+
ECCVMCircuitBuilder circuit{ op_queue };
432+
bool result = ECCVMTraceChecker::check(circuit);
433+
EXPECT_EQ(result, true);
434+
}
435+
436+
TEST(ECCVMCircuitBuilderTests, AddProducesDouble)
437+
{
438+
std::shared_ptr<ECCOpQueue> op_queue = std::make_shared<ECCOpQueue>();
439+
440+
auto generators = G1::derive_generators("test generators", 3);
441+
typename G1::element a = generators[0];
442+
443+
op_queue->add_accumulate(a);
444+
op_queue->add_accumulate(a);
445+
op_queue->eq_and_reset();
446+
ECCVMCircuitBuilder circuit{ op_queue };
447+
bool result = ECCVMTraceChecker::check(circuit);
448+
EXPECT_EQ(result, true);
449+
}

0 commit comments

Comments
 (0)