Skip to content

Commit 9d25987

Browse files
committed
Improve verify check and comments
- Add explanation for 59 divsteps vs 2^62 scaling
1 parent 1bce1d1 commit 9d25987

File tree

2 files changed

+13
-11
lines changed

2 files changed

+13
-11
lines changed

src/modinv32_impl.h

+5-4
Original file line numberDiff line numberDiff line change
@@ -187,11 +187,11 @@ static int32_t secp256k1_modinv32_divsteps_30(int32_t theta, uint32_t f0, uint32
187187
for (i = 0; i < 30; ++i) {
188188
/* f must always be odd */
189189
VERIFY_CHECK((f & 1) == 1);
190+
/* Minimum trailing zeros count for matrix elements decreases in each iteration */
191+
VERIFY_CHECK(!((u | v | q | r) & (0xFFFFFFFFULL >> (i + 2))));
190192
/* Applying the matrix so far to the initial f,g gives current f,g. */
191193
VERIFY_CHECK((u >> (30 - i)) * f0 + (v >> (30 - i)) * g0 == f << i);
192194
VERIFY_CHECK((q >> (30 - i)) * f0 + (r >> (30 - i)) * g0 == g << i);
193-
/* At the beginning of every loop, the matrix variables are even. */
194-
VERIFY_CHECK(!((u | v | q | r) & 1));
195195
/* Compute conditional masks for (theta < 0) and for (g & 1). */
196196
c1 = theta >> 31;
197197
c2 = -(g & 1);
@@ -228,8 +228,9 @@ static int32_t secp256k1_modinv32_divsteps_30(int32_t theta, uint32_t f0, uint32
228228
VERIFY_CHECK(q * f0 + r * g0 == g << 30);
229229
/* The determinant of t must be a power of two. This guarantees that multiplication with t
230230
* does not change the gcd of f and g, apart from adding a power-of-2 factor to it (which
231-
* will be divided out again). As each divstep's individual matrix has determinant 2, the
232-
* aggregate of 30 of them will have determinant 2^30. */
231+
* will be divided out again). As each divstep's individual matrix has determinant 2^-1,
232+
* the aggregate of 30 of them will have determinant 2^-30. Multiplying with the initial
233+
* 2^30*identity (which has determinant 2^60) means the result has determinant 2^30. */
233234
VERIFY_CHECK((int64_t)t->u * t->r - (int64_t)t->v * t->q == ((int64_t)1) << 30);
234235
return theta;
235236
}

src/modinv64_impl.h

+8-7
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,9 @@ typedef struct {
146146
} secp256k1_modinv64_trans2x2;
147147

148148
/* Compute the transition matrix and theta for 59 divsteps (where theta=delta-1/2)
149-
* Note that the transformation matrix is scaled by 2^62 and not 2^59.
149+
* Although only 59 divsteps are performed, the resulting transformation matrix
150+
* is scaled by 2^62 to allow reuse of _update_de_62 and _update_fg_62 between
151+
* _modinv64 and _modinv64_var.
150152
*
151153
* Input: theta: initial theta
152154
* f0: bottom limb of initial f
@@ -165,11 +167,11 @@ static int64_t secp256k1_modinv64_divsteps_59(int64_t theta, uint64_t f0, uint64
165167
for (i = 3; i < 62; ++i) {
166168
/* f must always be odd */
167169
VERIFY_CHECK((f & 1) == 1);
170+
/* Minimum trailing zeros count for matrix elements decreases in each iteration */
171+
VERIFY_CHECK(!((u | v | q | r) & (0xFFFFFFFFFFFFFFFFULL >> (i - 1))));
168172
/* Applying the matrix so far to the initial f,g gives current f,g. */
169173
VERIFY_CHECK((u >> (62 - i)) * f0 + (v >> (62 - i)) * g0 == f << i);
170174
VERIFY_CHECK((q >> (62 - i)) * f0 + (r >> (62 - i)) * g0 == g << i);
171-
/* At the beginning of every loop, the matrix variables are even. */
172-
VERIFY_CHECK(!((u | v | q | r) & 1));
173175
/* Compute conditional masks for (theta < 0) and for (g & 1). */
174176
c1 = theta >> 63;
175177
c2 = -(g & 1);
@@ -206,10 +208,9 @@ static int64_t secp256k1_modinv64_divsteps_59(int64_t theta, uint64_t f0, uint64
206208
VERIFY_CHECK(q * f0 + r * g0 == g << 62);
207209
/* The determinant of t must be a power of two. This guarantees that multiplication with t
208210
* does not change the gcd of f and g, apart from adding a power-of-2 factor to it (which
209-
* will be divided out again). As each divstep's individual matrix has determinant 2, the
210-
* aggregate of 59 of them will have determinant 2^59. Multiplying with the initial
211-
* 8*identity (which has determinant 2^6) means the overall outputs has determinant
212-
* 2^65. */
211+
* will be divided out again). As each divstep's individual matrix has determinant 2^-1,
212+
* the aggregate of 59 of them will have determinant 2^-59. Multiplying with the initial
213+
* 2^62*identity (which has determinant 2^124) means the result has determinant 2^65. */
213214
VERIFY_CHECK((int128_t)t->u * t->r - (int128_t)t->v * t->q == ((int128_t)1) << 65);
214215
return theta;
215216
}

0 commit comments

Comments
 (0)