-
Notifications
You must be signed in to change notification settings - Fork 166
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: add trivial enc/dec for strings #2146
Conversation
ed6b853
to
05b231b
Compare
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.
Reviewed 3 of 4 files at r1, all commit messages.
Reviewable status: 3 of 4 files reviewed, 3 unresolved discussions
tfhe/src/strings/server_key/mod.rs
line 62 at r1 (raw file):
pub fn trivial_encrypt_ascii(&self, str: &str, padding: Option<u32>) -> FheString { let ck = self.inner.borrow();
Should be called sk
tfhe/src/high_level_api/strings/ascii/mod.rs
line 306 at r1 (raw file):
let str = str.as_ref(); let (sliced, padding) = if str.len() >= size { (&str[..size], 0)
It would be safer to return an error in that case IMO
And let the user do the shortening themselves if they need it
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.
Reviewable status: 3 of 4 files reviewed, 3 unresolved discussions (waiting on @mayeul-zama)
tfhe/src/high_level_api/strings/ascii/mod.rs
line 306 at r1 (raw file):
Previously, mayeul-zama wrote…
It would be safer to return an error in that case IMO
And let the user do the shortening themselves if they need it
Not sure I agree with that, the real encrypt variant does the same thing
Also to me, it would be weird that the function adds padding but refuses to truncate
tfhe/src/strings/server_key/mod.rs
line 62 at r1 (raw file):
Previously, mayeul-zama wrote…
Should be called
sk
true
05b231b
to
8ce72a3
Compare
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.
Reviewable status: 2 of 4 files reviewed, 1 unresolved discussion
This change is