Skip to content
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

Merged
merged 1 commit into from
Mar 24, 2025
Merged

feat: add trivial enc/dec for strings #2146

merged 1 commit into from
Mar 24, 2025

Conversation

tmontaigu
Copy link
Contributor

@tmontaigu tmontaigu commented Mar 3, 2025

This change is Reviewable

@cla-bot cla-bot bot added the cla-signed label Mar 3, 2025
@tmontaigu tmontaigu requested a review from mayeul-zama March 3, 2025 16:18
@tmontaigu tmontaigu force-pushed the tm/string-trivial branch 2 times, most recently from ed6b853 to 05b231b Compare March 4, 2025 16:20
Copy link
Contributor

@mayeul-zama mayeul-zama left a 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

Copy link
Contributor Author

@tmontaigu tmontaigu left a 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

@tmontaigu tmontaigu force-pushed the tm/string-trivial branch from 05b231b to 8ce72a3 Compare March 5, 2025 14:20
@tmontaigu tmontaigu requested a review from mayeul-zama March 10, 2025 10:04
Copy link
Contributor

@mayeul-zama mayeul-zama left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Thanks!

Reviewable status: 2 of 4 files reviewed, 1 unresolved discussion

@tmontaigu tmontaigu merged commit bbcad43 into main Mar 24, 2025
96 of 97 checks passed
@tmontaigu tmontaigu deleted the tm/string-trivial branch March 24, 2025 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants