Skip to content

Commit 20f414c

Browse files
author
notnotraju
committed
responded to sergei's comments.
1 parent fefe1df commit 20f414c

File tree

1 file changed

+9
-9
lines changed

1 file changed

+9
-9
lines changed

barretenberg/cpp/src/barretenberg/relations/ecc_vm/ecc_msm_relation_impl.hpp

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -133,16 +133,16 @@ void ECCVMMSMRelationImpl<FF>::accumulate(ContainerOverSubrelations& accumulator
133133
* The boolean column q_add describes whether a round is an ADDITION round.
134134
* The values of q_add are Prover-defined. We need to ensure they set q_add correctly. We will do this via a
135135
* multiset-equality check (formerly called a "strict lookup"), which allows the various tables to "communicate".
136-
* On a high level, we ensure that this table "reads" (pc, round, wnaf_slice), another table (Precomputed) "writes"
136+
* On a high level, this table "reads" (pc, round, wnaf_slice), another table (Precomputed) "writes"
137137
* a potentially different set of (pc, round, wnaf_slice), and we demand that the reads match the writes.
138138
* Alternatively said, the MSM columns spawn a multiset of tuples of the form (pc, round, wnaf_slice), the
139139
* Precomputed Table columns spawn a potentially different multiset of tuples of the form (pc, round, wnaf_slice),
140140
* and we _check_ that these two multisets match.
141141
*
142142
* The above description does not reference how we will _prove_ that the two multisets are equal. As usual, we use a
143-
* grand product argument. A happy byproduct of this is that we can use the grand product technique is powerful
144-
* enough to allow our multiset equality testing to support _conditional adds_; this means that we only add a tuple
145-
* if some particular condition occurs.
143+
* grand product argument. A happy byproduct of this is that we can use the grand product technique, which is
144+
* powerful enough to allow our multiset equality testing to support _conditional adds_; this means that we only add
145+
* a tuple if some particular condition occurs.
146146
*
147147
* This (pc, round, wnaf_slice) multiset equality testing is made more difficult by the fact that the values of
148148
* `precomputed_pc` are _not the same_ as the values of `msm_pc`. The former indexes over every (non-trivial, 128
@@ -189,12 +189,12 @@ void ECCVMMSMRelationImpl<FF>::accumulate(ContainerOverSubrelations& accumulator
189189
* 6. The lookup table is implemented correctly.
190190
*
191191
* First of all, note the asymmetry: we do not explicitly add tuples corresponding to skew on the MSM side of the
192-
* table. Indeeed, this in implicit with `msm_round == 32`. Now, the point is that the pair (pc, round) uniquely
192+
* table. Indeeed, this is implicit with `msm_round == 32`. Now, the point is that the pair (pc, round) uniquely
193193
* specifies the point + wNAF digit that we are processing (and adding to the accumulator) and both `pc` and `round`
194194
* are directly constrained to be monotonic.
195195
*
196196
* Suppose the Prover sets `q_addK = 0` when an honest Prover would set `q_addK == 1`. Then there would be some (pc,
197-
* round, wnaf_slice) that the Precomputed table add to its multiset that the prover did not add. The Prover can
197+
* round, wnaf_slice) that the Precomputed table added to its multiset that the prover did not add. The Prover can
198198
* _never_ "compensate" for this, as `pc` is locally constrained to be monotonic and `round` is constrained to be
199199
* periodic; this means that the Prover has "lost their chance" to add this element to the multiset and hence the
200200
* multiset equality check will fail.
@@ -453,7 +453,7 @@ void ECCVMMSMRelationImpl<FF>::accumulate(ContainerOverSubrelations& accumulator
453453
std::get<17>(accumulator) += (q_add * q_double + q_add * q_skew + q_double * q_skew) * scaling_factor;
454454

455455
// Validate that if q_add = 1 or q_skew = 1, add1 also is 1
456-
// Optimize(@zac-williamson): could just get rid of add1 as a column, as it is a linear combination, see issue #2222
456+
// NOTE(#2222): could just get rid of add1 as a column, as it is a linear combination.
457457
std::get<32>(accumulator) += (add1 - q_add - q_skew) * scaling_factor;
458458

459459
// ROUND TRANSITION LOGIC
@@ -498,8 +498,8 @@ void ECCVMMSMRelationImpl<FF>::accumulate(ContainerOverSubrelations& accumulator
498498
// if double, next add = 1. As q_double, q_add, and q_skew are mutually exclusive, this suffices to force
499499
// q_double_shift == q_skew_shift == 0.
500500
std::get<22>(accumulator) += q_double * (-q_add_shift + 1) * scaling_factor;
501-
// if the current row is has q_skew == 1 and the next row is _not_ an MSM transition, then q_skew_shift = 1.
502-
// this forces q_skew to precisely correspond to the rows where `round == 32`. Indeed, not that the first q_skew
501+
// if the current row has q_skew == 1 and the next row is _not_ an MSM transition, then q_skew_shift = 1.
502+
// this forces q_skew to precisely correspond to the rows where `round == 32`. Indeed, note that the first q_skew
503503
// bit is set correctly:
504504
// round == 31, round_transition == 1 ==> q_skew_shift == 1. (if, to the contrary, q_double_shift == 1, then
505505
// the q_add_shift_shift == 1, but we assume that we have correctly constrained the q_adds via the multiset

0 commit comments

Comments
 (0)