Conversation
kubkon
left a comment
kubkon
left a comment
There was a problem hiding this comment.
Looks good! Just a couple of things to address before we can merge it.
src/enc.rs
Outdated
| } | ||
|
|
||
| #[inline] | ||
| fn enc_i32(key_pair: &KeyPair, value: i32) -> Self { |
There was a problem hiding this comment.
Hmm, it looks like an ideal candidate for a generic. Could we perhaps make enc generic, thus accepting either i32 or u32? In the future, we could think of some constraints, but for now, a simple generic type would work OK IMHO.
There was a problem hiding this comment.
Another thing, are enc_i32 and dec_i32 used anywhere? They don't seem though 🤔
| impl SubAssign for Enc { | ||
| #[inline] | ||
| fn sub_assign(&mut self, rhs: Self) { | ||
| *self = *self - rhs |
There was a problem hiding this comment.
Does this also work if we're encrypting/decrypting u32? Or is there some trick to it?
| (y as i32) - MODULUSI32 | ||
| } else { | ||
| y as i32 |
There was a problem hiding this comment.
Hmm, casting a u32 to i32 like this could potentially lead to a difficult to track overflow, am I right? In such cases, we could use a TryInto trait
let y: u32 = self.0;
// ...
y.try_into()? - MODULUSI32and wrap everything inside a TryInto rather than Into for Mod231. What do you think? Or the overflows are not possible here perhaps?
src/algebra/m231.rs
Outdated
| let z: Result<Mod231, _> = Mod231::try_from(y); | ||
| z == Ok(x) |
There was a problem hiding this comment.
FWIW, in unit tests, it is possible to return a Result which saves the unwrapping step. I'll redo this test to demonstrate what I mean.
2 participants
Interpret numbers in the range
0..2^30-1as nonnegative and numbers in the range2^30..2^31-2as negative.For example
-1corresponds to2^31-2