-
Notifications
You must be signed in to change notification settings - Fork 16
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
Add Public Key Encryption #34
base: main
Are you sure you want to change the base?
Add Public Key Encryption #34
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.
On top of the in-line comments, can you please add further indication in the README on how to generate the parameters for the public key encryption case? I would also ask you to add a gtihub action for testing the new circuit.
As I run
python3 scripts/circuit_sk.py -n 4096 -qis '[
27424203952895201,
27424203952895203
]' -t 65537
I first run into this error (you can also see it from the GitHub action)
File "/Users/enricobottazzi/Developer/GitHub/greco/scripts/circuit_sk.py", line 478, in <module>
main(args)
File "/Users/enricobottazzi/Developer/GitHub/greco/scripts/circuit_sk.py", line 30, in main
bfv_crt = BFVCrt(crt_moduli, n, t, discrete_gaussian)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/enricobottazzi/Developer/GitHub/greco/scripts/venv/lib/python3.11/site-packages/bfv/bfv.py", line 342, in __init__
self.bfv_q = BFV(RLWE(n, crt_moduli.q, t, discrete_gauss))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/enricobottazzi/Developer/GitHub/greco/scripts/venv/lib/python3.11/site-packages/bfv/bfv.py", line 32, in __init__
assert np.gcd(q, t) == 1, "modulus q and t must be coprime"
^^^^^^^^^^^^
OverflowError: Python int too large to convert to C long
I think this might be caused by an updated version of np
which is a dependency on my bfv-py
repo. If you confirm that you are facing the same error, I’ll update the requirements.txt
in bfv-py
to pin to the correct (old) version of np
.
In general it seems that there are some small errors in the naming of the variables or in the related comments. Some errors might be carried from the original PR.
The tests seem to be passing correctly based on your generated data. But I'm quite outside the loop with Greco, so I will leave it to @0xjei to provide a more thorough review on the PR and the adherence of the protocol to what has been described in the paper
Yes, I can confirm I see the same numpy error. For the time being, I updated the I also had to make these changes in In def __len__(self) -> int:
return len(self.coefficients) In ct_1 = pk1_u.coefficients + e1 |
…nore in gitignore
hey @auryn-macmillan and @fhedude, Thanks for the update. I'm still getting the same error as Enrico mentioned above.
I'm just checking in to see if you need any help or hints on this and how I can be of service. Thanks! |
@0xjei, do you still get that error after doing That error is caused by an incompatible version of Numpy. There is also a subsequent error that requires the above changes to BFV.py. I think we should perhaps have Greco depend on an instance of BFV.py in the PSE GitHub org, rather than the one in @enricobottazzi profile. Just so we're not dependent on Enrico to merge fixes. |
yes, I'm still getting the same error after running the installation on a fresh
sure, we should move whatever Greco is dependent on under the same organization. I will get back to you on this asap. |
@0xjei I've just updated the requirements.txt to point to an updated version of bfv.py in the Gnosis Guild github org. Give it a try again now, the errors should be resolved. |
thank you @auryn-macmillan - I've got it working locally now, using |
Hi @auryn-macmillan and @0xjei, I've implemented the changes suggested by Enrico in my PR. You can check it out here. I hope this is helpful, and please let me know if there's anything else I can assist with |
Hey @Vishalkulkarni45 thx - can you just push your new changes back on your PR about public key encryption to ease the review process? tysm |
@Vishalkulkarni45 and @0xjei, we've shifted our efforts to supporting fhe.rs rather than bfv.py over in our fork of Greco.
Will try to address this in our fork as well. cc @fhedude |
This PR resolves the comments from unmerged PR #31.
I'll also make a PR back to the OPs fork so that these changes could be incorporated into the original PR.