-
Notifications
You must be signed in to change notification settings - Fork 15
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
Added Public Key Encryption circuit #31
base: main
Are you sure you want to change the base?
Added Public Key Encryption circuit #31
Conversation
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.
Thanks for the PR. There are some things to update that I added to the comments. The logic seems to work. Anyway I noticed that there are a lot of repeated structues between the secret key and the public key encryption. We should aim to reduce duplication as much as possible. In general, the difference between the new lines of code and the deleted ones as result of the PR should be minimized as much as possible.
The frist matrix operations at page 13 of the paper is exactly the same as the one performed in secret key encryption. Therefore I propose to generalize the operations inside a RLWE struct that is consumed both by the secret key encryption and pub key encryption circuits. The pub key circuit will also have the operations included in the second matrix at page 13.
My suggestion is to proceed as follow:
- Work on a fresh new branch and start with the python scripts and create this rlwe functionality that are consumed across both the scripts. Once you are done, let's do the first round of review.
- Together with that, please provide the matematical proof on the ranges of p2i and p1i
- Then let's move to the circuit implementation in rust and do the same. Once you are done, let's do the second round of review.
- Last step is to update the benchmarks and the read me and the github action script
- While doing so keep in mind the comments that I provided on this PR
from bfv.discrete_gauss import DiscreteGaussian | ||
from bfv.polynomial import Polynomial, poly_div | ||
from random import randint | ||
import copy | ||
from utils import assign_to_circuit, count_advice_cells_needed_for_poly_range_check, print_advice_cells_info | ||
import argparse | ||
import json | ||
import numpy as np |
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.
not required
@@ -1,13 +1,18 @@ | |||
import os | |||
from bfv.crt import CRTModuli | |||
from bfv.bfv import BFVCrt | |||
from bfv.bfv import BFV |
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.
not required
@@ -287,10 +292,11 @@ def main(args): | |||
|
|||
# sanity check. The coefficients of ai * s + e should be in the range $- (N \cdot \frac{q_i - 1}{2} + B), N \cdot \frac{q_i - 1}{2} + B]$ | |||
bound = int((qis[i] - 1) / 2) * n + b | |||
res = ais[i] * s + e | |||
print(f" sk r2 bound = {bound}") |
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.
rmv
@@ -0,0 +1,99 @@ | |||
import os |
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 file shouldn't be here. If you want to test it, do it externally or fork the python bfv dependency and add a test there
qis = json.loads(qis) | ||
t = args.t | ||
|
||
t = 65537 |
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.
These parameters shouldn't be encoded
@@ -54,6 +54,14 @@ impl<F: ScalarField> PolyAssigned<F> { | |||
range.check_less_than_safe(ctx_gate, shifted_coeff, (2 * upper_bound) + 1); | |||
} | |||
} | |||
// pub fn range_check_128(&self,ctx_gate: &mut Context<F>,range: &RangeChip<F>,upper_bound: u128){ |
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.
Remove if not needed
use super::{test_params, BfvPkEncryptionCircuit}; | ||
|
||
#[test] | ||
fn test_pk_enc_valid() { |
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 test should use the mock prover. Otherwise it's same as the next test pk_enc_full_prover
let rlc_circuit = RlcExecutor::new(mock_builder, pk_enc_circuit); | ||
|
||
// 4. Run the mock prover | ||
let invalid_mock_prover = MockProver::run( |
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 test is not actually failing for the reason that it is supposed to fail. Instead it is failing because NOT ENOUGH ADVICE COLUMNS
, you need to increase the k
factor to 22 and you will see that the error disappear. Also please check the invalid range also on p2i and p1i
Okay , I will remove the unwanted code and make the required changes as mentioned in the comment |
src/[pk_encryption_circuit.rs
as described in feat: Support Pub Key Encryption #16pk_encryption_circuit