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

Making serialization functions specific instead of generic and removing helper code. #39

Merged
merged 12 commits into from
Aug 15, 2023

Conversation

xvzcf
Copy link
Contributor

@xvzcf xvzcf commented Aug 11, 2023

  • I've removed the generic serialize and deserialize functions that take in the number of bits to use as a parameter, and replaced them by specific functions that work on 1, 4, 10, and 12 bits only
  • This meant the BitVector code was no longer needed, so I've removed that too

Copy link
Member

@franziskuskiefer franziskuskiefer left a comment

Choose a reason for hiding this comment

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

Thanks!
I left a few comments for some more improvements. If you want to push some of them into followups, just mark them as such.

Please run rustfmt on all the code.

src/kem/kyber768/sampling.rs Show resolved Hide resolved
src/kem/kyber768/serialize.rs Outdated Show resolved Hide resolved
src/kem/kyber768/ind_cpa.rs Outdated Show resolved Hide resolved
src/kem/kyber768/ind_cpa.rs Outdated Show resolved Hide resolved
src/kem/kyber768/ind_cpa.rs Outdated Show resolved Hide resolved
Copy link
Member

@franziskuskiefer franziskuskiefer left a comment

Choose a reason for hiding this comment

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

Thanks.
I'm approving but please add some comments. When the comments are resolved you can merge it.

src/kem/kyber768/serialize.rs Outdated Show resolved Hide resolved
src/kem/kyber768/sampling.rs Show resolved Hide resolved
@xvzcf xvzcf merged commit 055c700 into dev Aug 15, 2023
6 checks passed
@xvzcf xvzcf deleted the kyber-serialize branch August 15, 2023 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants