Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/hash_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,8 @@ static void secp256k1_sha256_finalize(secp256k1_sha256 *hash, unsigned char *out
int i;
/* The maximum message size of SHA256 is 2^64-1 bits. */
VERIFY_CHECK(hash->bytes < ((uint64_t)1 << 61));
secp256k1_write_be32(&sizedesc[0], hash->bytes >> 29);
secp256k1_write_be32(&sizedesc[4], hash->bytes << 3);
secp256k1_write_be32(&sizedesc[0], (uint32_t)(hash->bytes >> 29));
secp256k1_write_be32(&sizedesc[4], (uint32_t)(hash->bytes << 3));
secp256k1_sha256_write(hash, pad, 1 + ((119 - (hash->bytes % 64)) % 64));
secp256k1_sha256_write(hash, sizedesc, 8);
for (i = 0; i < 8; i++) {
Expand Down
4 changes: 2 additions & 2 deletions src/modinv64_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ static int64_t secp256k1_modinv64_divsteps_62_var(int64_t eta, uint64_t f0, uint
/* Transformation matrix; see comments in secp256k1_modinv64_divsteps_62. */
uint64_t u = 1, v = 0, q = 0, r = 1;
uint64_t f = f0, g = g0, m;
uint32_t w;
uint64_t w;
int i = 62, limit, zeros;

for (;;) {
Expand Down Expand Up @@ -326,7 +326,7 @@ static int64_t secp256k1_modinv64_posdivsteps_62_var(int64_t eta, uint64_t f0, u
/* Transformation matrix; see comments in secp256k1_modinv64_divsteps_62. */
uint64_t u = 1, v = 0, q = 0, r = 1;
uint64_t f = f0, g = g0, m;
uint32_t w;
uint64_t w;
int i = 62, limit, zeros;
int jac = *jacp;

Expand Down
8 changes: 4 additions & 4 deletions src/scalar_4x64_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ static int secp256k1_scalar_add(secp256k1_scalar *r, const secp256k1_scalar *a,
secp256k1_u128_accum_u64(&t, a->d[3]);
secp256k1_u128_accum_u64(&t, b->d[3]);
r->d[3] = secp256k1_u128_to_u64(&t); secp256k1_u128_rshift(&t, 64);
overflow = secp256k1_u128_to_u64(&t) + secp256k1_scalar_check_overflow(r);
overflow = (int)secp256k1_u128_to_u64(&t) + secp256k1_scalar_check_overflow(r);
Comment thread
csjones marked this conversation as resolved.
Outdated
VERIFY_CHECK(overflow == 0 || overflow == 1);
secp256k1_scalar_reduce(r, overflow);

Expand Down Expand Up @@ -634,7 +634,7 @@ static void secp256k1_scalar_reduce_512(secp256k1_scalar *r, const uint64_t *l)
sumadd_fast(n3);
extract_fast(m5);
VERIFY_CHECK(c0 <= 1);
m6 = c0;
m6 = (uint32_t)c0;

/* Reduce 385 bits into 258. */
/* p[0..4] = m[0..3] + m[4..6] * SECP256K1_N_C. */
Expand All @@ -654,7 +654,7 @@ static void secp256k1_scalar_reduce_512(secp256k1_scalar *r, const uint64_t *l)
muladd_fast(m6, SECP256K1_N_C_1);
sumadd_fast(m5);
extract_fast(p3);
p4 = c0 + m6;
p4 = (uint32_t)c0 + m6;
VERIFY_CHECK(p4 <= 2);
Comment on lines +658 to 659
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
p4 = (uint32_t)c0 + m6;
VERIFY_CHECK(p4 <= 2);
VERIFY_CHECK(c0 <= 1 && m6 <= 1);
p4 = (uint32_t)c0 + m6;

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change made

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ended up removing m6 <= 1 because it's already verified at line 637. The CI failed with c0 <= 1 and after running the ci script locally a few times, I changed it to c0 <= 2, which is consistent with the original p4 <= 2 check (since p4 = c0 + m6 and m6 <= 1). I also restored the p4 <= 2 check.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I see. I have no idea how the algorithm works. Still, this is a bit surprising to me because that means m6 == 0 always (but then it probably wouldn't be there in the first place) or m6 == 0 iff c0 == 2, and I don't see why the latter should hold.

Perhaps @sipa can provide some insights..


/* Reduce 258 bits into 256. */
Expand All @@ -674,7 +674,7 @@ static void secp256k1_scalar_reduce_512(secp256k1_scalar *r, const uint64_t *l)
#endif

/* Final reduction of r. */
secp256k1_scalar_reduce(r, c + secp256k1_scalar_check_overflow(r));
secp256k1_scalar_reduce(r, (unsigned int)c + secp256k1_scalar_check_overflow(r));
Comment thread
csjones marked this conversation as resolved.
}

static void secp256k1_scalar_mul_512(uint64_t *l8, const secp256k1_scalar *a, const secp256k1_scalar *b) {
Expand Down