-
Notifications
You must be signed in to change notification settings - Fork 277
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(sdk-lib-mpc): add implementation of paillier-blum #3940
Conversation
No top level dependency changes detected. Learn more about Socket for GitHub ↗︎ |
f919ce8
to
95284d5
Compare
|
||
describe('prove and verify', function () { | ||
// This test takes quite some time ~ 18 seconds | ||
// Test takes > 30s in CI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will have to address this in a follow up on why this takes too long. Might have to make the paillier key a mocked key.
@@ -2,7 +2,7 @@ | |||
|
|||
module.exports = { | |||
require: 'ts-node/register', | |||
timeout: '20000', | |||
timeout: '40000', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bitgo/sdk-lib-mpc: creating paillier blum proof should not throw an error, test case 0:
@bitgo/sdk-lib-mpc: Error: Timeout of 20000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/home/runner/work/BitGoJS/BitGoJS/modules/sdk-lib-mpc/test/unit/tss/ecdsa/paillierBlumProof.ts)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will need to be optimized in a follow up.
b7f5404
to
64ee68a
Compare
); | ||
h = (h * h) % N; | ||
if (gcd(h, N) === BigInt(1)) { | ||
return h; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sampling of y here deviates from https://eprint.iacr.org/2020/492.pdf section 4.3 and the non-interactive version at https://www.zkdocs.com/docs/zkdocs/zero-knowledge-protocols/product-primes/paillier_blum_modulus/#non-interactive-protocol
Here it seems to be sampling y so that it will be the same as y' by implicitly making a = 0, b = 0 (and thus not including a, b in the proof). This will make only certain co-primes of N be picked as the challenge y, violating generateY() being a random oracle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, for y' (and therefore y here) to be suitable so that its 4th root is well defined, it needs to be a quadratic residue modulo N. Here by making y = h^2 and y (mod N) = 1, y is indeed a quadratic residue N but only for the cases where remainder is 1. Other quadratic residues are missed where x^2 === y (mod N) is other values (e.g. 4, 9, etc.). This further limits the sampling domain of the challenge value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could refer to Fireblocks' implementation https://github.com/fireblocks/mpc-lib/blob/main/src/common/crypto/paillier/paillier_zkp.c#L648
Basically, due to p, q === 3 mod 4 and jacobi(w, N) === -1, it's guaranteed one of (0, 0), (0, 1), (1, 0), (1,1) for (a, b) will make y' a quadratic residue modulo N.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
const beta = randBetween(sqrtN0 << (ELL + EPSILON), -sqrtN0 << (ELL + EPSILON)); | ||
const rho = randBetween((nHat * n0) << ELL, -(nHat * n0) << ELL); | ||
// Commit to p. | ||
const mu = randBetween(BigInt(1) << ELL, BigInt(-1) << ELL); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think mu should be
randBetween(BigInt(nHat) << ELL, BigInt(-nHat) << ELL)
according to https://eprint.iacr.org/2020/492.pdf B.4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
const mu = randBetween(BigInt(1) << ELL, BigInt(-1) << ELL); | ||
const P = (modPow(s, p, nHat) * modPow(t, mu, nHat)) % nHat; | ||
// Commit to q. | ||
const nu = randBetween(BigInt(1) << ELL, BigInt(-1) << ELL); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as mu
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
const nu = randBetween(BigInt(1) << ELL, BigInt(-1) << ELL); | ||
const Q = (modPow(s, q, nHat) * modPow(t, nu, nHat)) % nHat; | ||
// Commit to alpha. | ||
const x = randBetween(BigInt(1) << (ELL + EPSILON), BigInt(-1) << (ELL + EPSILON)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 and -1 should be nHat and -nHat respectively.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
const x = randBetween(BigInt(1) << (ELL + EPSILON), BigInt(-1) << (ELL + EPSILON)); | ||
const A = (modPow(s, alpha, nHat) * modPow(t, x, nHat)) % nHat; | ||
// Commit to beta. | ||
const y = randBetween(BigInt(1) << (ELL + EPSILON), BigInt(-1) << (ELL + EPSILON)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 and -1 should be nHat and -nHat respectively.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
138a699
to
2f8c8cc
Compare
.update('$') | ||
.digest() | ||
); | ||
if (y > BigInt(0) && y < N) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
y should also be coprime of N
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
} | ||
if ( | ||
modPow(t, (p - BigInt(1)) / BigInt(2), p) === BigInt(1) && | ||
modPow(t, (q - BigInt(1)) / BigInt(2), q) === BigInt(1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can also be done by checking if y_i and w are quadratic residue of p or q first by calculating
modPow(y_i, (p - 1) / 2, p), modPow(t, (q - 1) / 2, q), modPow(w, (p -1)/2, p) and modPow(w, (q -1)/2, q) as above, the result can be used to determine the value of a_i and b_i. This does need 2 more calculations, so on average it evens out with the try-and-error approach.
Since p, q === 3 mod 4 and are primes, "If p ≡ 3 (mod 4) the negative of a residue modulo p is a nonresidue and the negative of a nonresidue is a residue." according to https://en.wikipedia.org/wiki/Quadratic_residue. Jacobi(w, N) = -1 means w is quadratic residue to one of p, q and nonresidue to the other.
For example, if (y_i/p) = 1 (residue), (y_i/q) = -1 (nonresidue), (w/p) = 1, (w/q) = -1, then a_i = 0, b_i = 1.
let t; | ||
while (true) { | ||
t = y_i; | ||
ab[i] = (await randBits(2))[0]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since there're only 4 cases, I wonder if it's sufficient to just enumerate them, just in case random generation gives duplicate values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Flushing from the review queue, let me know when its ready to review
github's still having issue syncing this PR to the latest commits, but they've addressed comments on the paillier blum proof. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@zahin-mohammad I don't know how to resolve the issue this "prettier" issue with mockNoSmallFactorsProof.json and mockPaillierBlumProof.json. Can you please help resolve? |
15c7b2b
15c7b2b
to
c53c2f3
Compare
66774d5
to
4df6424
Compare
4df6424
to
519e200
Compare
Rebased |
TICKET: WP-132