From 4e5d15e5eed0481df192e333b96343c0deb0d9d0 Mon Sep 17 00:00:00 2001 From: Martin Minkov Date: Tue, 23 Jul 2024 08:58:10 -0700 Subject: [PATCH 1/7] fix(finite-field): ensure isSquare and sqrt methods take input modulo p fix(finite-field): optimize equal method by avoiding modulo operation when possible refactor(finite-field): use more descriptive variable names in equal method The isSquare and sqrt methods were not taking the input modulo p, which could lead to incorrect results for inputs outside the field. The equal method was performing a modulo operation unnecessarily in some cases, which could be avoided by first checking if the inputs are already in the correct range. The variable names in the equal method were also improved for clarity. --- crypto/finite-field.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/crypto/finite-field.ts b/crypto/finite-field.ts index d7300e96..db1cb459 100644 --- a/crypto/finite-field.ts +++ b/crypto/finite-field.ts @@ -300,10 +300,10 @@ function createField( return mod(x * x, p); }, isSquare(x: bigint) { - return isSquare(x, p); + return isSquare(mod(x, p), p); }, sqrt(x: bigint) { - return sqrt(x, p, oddFactor, twoadicRoot, twoadicity); + return sqrt(mod(x, p), p, oddFactor, twoadicRoot, twoadicity); }, power(x: bigint, n: bigint) { return power(x, n, p); @@ -317,7 +317,9 @@ function createField( return mod(z, p); }, equal(x: bigint, y: bigint) { - return mod(x - y, p) === 0n; + let x_ = x >= 0n && x < p ? x : mod(x, p); + let y_ = y >= 0n && y < p ? y : mod(y, p); + return x_ === y_; }, isEven(x: bigint) { return !(x & 1n); From 901bddef9692e04f2004515788b873c50eadea30 Mon Sep 17 00:00:00 2001 From: Martin Minkov Date: Tue, 23 Jul 2024 09:16:29 -0700 Subject: [PATCH 2/7] test(finite-field): add tests for non-canonical zero handling in sqrt and isSquare methods The commit adds two new test cases to the finite field unit tests: 1. It asserts that calling `sqrt` with a non-canonical zero (i.e., `p`) should return the same result as calling it with a canonical zero (`0n`). This ensures that the `sqrt` method handles non-canonical zero inputs correctly. 2. It asserts that calling `isSquare` with a non-canonical zero (`p`) should return the same result as calling it with a canonical zero (`0n`). This ensures that the `isSquare` method treats non-canonical zero inputs the same way as canonical zero. These tests were added to improve the robustness of the finite field implementation by verifying that it correctly handles non-canonical zero values, which are equivalent to the canonical zero value in the field. --- crypto/finite-field.unit-test.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crypto/finite-field.unit-test.ts b/crypto/finite-field.unit-test.ts index 8080d861..15f08a3c 100644 --- a/crypto/finite-field.unit-test.ts +++ b/crypto/finite-field.unit-test.ts @@ -70,9 +70,15 @@ for (let F of fields) { let squareX = F.square(x); assert(F.isSquare(squareX), 'square + isSquare'); assert([x, F.negate(x)].includes(F.sqrt(squareX)!), 'square + sqrt'); + assert.equal(F.sqrt(0n), F.sqrt(p), 'sqrt handles non-canonical 0'); if (F.M >= 2n) { assert(F.isSquare(p - 1n), 'isSquare -1'); + assert.equal( + F.isSquare(0n), + F.isSquare(p), + 'isSquare handles non-canonical 0' + ); let i = F.power(F.twoadicRoot, 1n << (F.M - 2n)); assert([i, F.negate(i)].includes(F.sqrt(p - 1n)!), 'sqrt -1'); } From 261522bf3fd080c6972e0e50d3516666ed9ff1af Mon Sep 17 00:00:00 2001 From: Martin Minkov Date: Tue, 30 Jul 2024 08:34:27 -0700 Subject: [PATCH 3/7] refactor(finite-field): simplify negate function to use mod(-x, p) instead of p - x for improved readability and consistency with other functions fix(finite-field): use mod in isEven function to ensure the result is always within the field range and avoid potential issues with large numbers --- crypto/finite-field.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crypto/finite-field.ts b/crypto/finite-field.ts index db1cb459..d78e1209 100644 --- a/crypto/finite-field.ts +++ b/crypto/finite-field.ts @@ -282,7 +282,7 @@ function createField( return mod(2n ** BigInt(bits) - (x + 1n), p); }, negate(x: bigint) { - return x === 0n ? 0n : p - x; + return x === 0n ? 0n : mod(-x, p); }, sub(x: bigint, y: bigint) { return mod(x - y, p); @@ -322,7 +322,7 @@ function createField( return x_ === y_; }, isEven(x: bigint) { - return !(x & 1n); + return !mod(x & 1n, p); }, random() { return randomField(p, sizeInBytes, hiBitMask); From cc3f63228f6caa025a39b761b1e38faa8c50e69e Mon Sep 17 00:00:00 2001 From: Martin Minkov Date: Tue, 30 Jul 2024 08:37:39 -0700 Subject: [PATCH 4/7] refactor(finite-field): improve sqrt and isSquare functions by modding input to ensure it's in the field The changes in the `sqrt` and `isSquare` functions were made to ensure that the input value is always within the finite field before performing any operations. This is done by modding the input value by the field's prime modulus `p` at the beginning of each function. The reason for this change is to handle cases where the input value might be outside the field, which could lead to incorrect results or even errors. By modding the input, we guarantee that all operations are performed on values within the field, ensuring the correctness of the results. This refactoring improves the robustness and reliability of the `sqrt` and `isSquare` functions, making them more consistent with the expected behavior of finite field arithmetic. --- crypto/finite-field.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/crypto/finite-field.ts b/crypto/finite-field.ts index d78e1209..600caefe 100644 --- a/crypto/finite-field.ts +++ b/crypto/finite-field.ts @@ -184,12 +184,13 @@ function fastInverse( return s; } -function sqrt(n: bigint, p: bigint, Q: bigint, c: bigint, M: bigint) { +function sqrt(n_: bigint, p: bigint, Q: bigint, c: bigint, M: bigint) { // https://en.wikipedia.org/wiki/Tonelli-Shanks_algorithm#The_algorithm // variable naming is the same as in that link ^ // Q is what we call `t` elsewhere - the odd factor in p - 1 // c is a known primitive root of unity // M is the twoadicity = exponent of 2 in factorization of p - 1 + let n = mod(n_, p); if (n === 0n) return 0n; let t = power(n, (Q - 1n) >> 1n, p); // n^(Q - 1)/2 let R = mod(t * n, p); // n^((Q - 1)/2 + 1) = n^((Q + 1)/2) @@ -212,7 +213,8 @@ function sqrt(n: bigint, p: bigint, Q: bigint, c: bigint, M: bigint) { } } -function isSquare(x: bigint, p: bigint) { +function isSquare(x_: bigint, p: bigint) { + let x = mod(x_, p); if (x === 0n) return true; let sqrt1 = power(x, (p - 1n) / 2n, p); return sqrt1 === 1n; @@ -300,10 +302,10 @@ function createField( return mod(x * x, p); }, isSquare(x: bigint) { - return isSquare(mod(x, p), p); + return isSquare(x, p); }, sqrt(x: bigint) { - return sqrt(mod(x, p), p, oddFactor, twoadicRoot, twoadicity); + return sqrt(x, p, oddFactor, twoadicRoot, twoadicity); }, power(x: bigint, n: bigint) { return power(x, n, p); From 3674c62f0f112c7bb70535e14f85e6823af12f3d Mon Sep 17 00:00:00 2001 From: Martin Minkov Date: Tue, 30 Jul 2024 10:01:21 -0700 Subject: [PATCH 5/7] test(finite-field): add test cases for non-canonical zero - Assert that negating non-canonical zero (p) returns canonical zero (0n) - Assert that non-canonical zero (p) is considered even These additional test cases ensure that the finite field implementation handles non-canonical zero correctly by treating it as equivalent to the canonical zero representation. --- crypto/finite-field.unit-test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crypto/finite-field.unit-test.ts b/crypto/finite-field.unit-test.ts index 15f08a3c..358971dc 100644 --- a/crypto/finite-field.unit-test.ts +++ b/crypto/finite-field.unit-test.ts @@ -28,9 +28,11 @@ for (let F of fields) { assert.equal(F.sub(3n, 3n), 0n, 'sub'); assert.equal(F.sub(3n, 8n), p - 5n, 'sub'); assert.equal(F.negate(5n), p - 5n, 'negate'); + assert.equal(F.negate(p), 0n, 'non-canonical 0 is negated'); assert.equal(F.add(x, F.negate(x)), 0n, 'add & negate'); assert.equal(F.sub(F.add(x, y), x), y, 'add & sub'); assert.equal(F.isEven(17n), false, 'isEven'); + assert.equal(F.isEven(p), true, 'non-canonical 0 is even'); assert.equal(F.isEven(p - 1n), true, 'isEven'); assert.equal(F.mul(p - 1n, 2n), p - 2n, 'mul'); From ba836e37e74920ba85124b7606640589da4b9d54 Mon Sep 17 00:00:00 2001 From: Martin Minkov Date: Tue, 30 Jul 2024 10:04:37 -0700 Subject: [PATCH 6/7] fix(finite-field): check for even number correctly by first reducing the number modulo p before checking the least significant bit This fixes a bug where the isEven function would incorrectly classify numbers greater than the field size as even. By first reducing the number modulo p, we ensure that the least significant bit correctly determines the parity of the number within the field. --- crypto/finite-field.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crypto/finite-field.ts b/crypto/finite-field.ts index 600caefe..1fa5bb1b 100644 --- a/crypto/finite-field.ts +++ b/crypto/finite-field.ts @@ -324,7 +324,7 @@ function createField( return x_ === y_; }, isEven(x: bigint) { - return !mod(x & 1n, p); + return !(mod(x, p) & 1n); }, random() { return randomField(p, sizeInBytes, hiBitMask); From 71f2e698dadcdfc62c76a72248c0df71cfd39d4c Mon Sep 17 00:00:00 2001 From: Martin Minkov Date: Tue, 30 Jul 2024 10:05:09 -0700 Subject: [PATCH 7/7] docs(finite-field): add comment explaining the logic behind the equal method implementation --- crypto/finite-field.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/crypto/finite-field.ts b/crypto/finite-field.ts index 1fa5bb1b..ac3d4cda 100644 --- a/crypto/finite-field.ts +++ b/crypto/finite-field.ts @@ -319,6 +319,7 @@ function createField( return mod(z, p); }, equal(x: bigint, y: bigint) { + // We check if x and y are both in the range [0, p). If they are, can do a simple comparison. Otherwise, we need to reduce them to the proper canonical field range. let x_ = x >= 0n && x < p ? x : mod(x, p); let y_ = y >= 0n && y < p ? y : mod(y, p); return x_ === y_;