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

Kyber spec. #15

Merged
merged 22 commits into from
Aug 2, 2023
Merged

Kyber spec. #15

merged 22 commits into from
Aug 2, 2023

Conversation

xvzcf
Copy link
Contributor

@xvzcf xvzcf commented Jul 10, 2023

No description provided.

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!
Looks pretty good already. I added some thoughts.

specs/kyber/rust-toolchain.toml Outdated Show resolved Hide resolved
specs/kyber/Cargo.toml Outdated Show resolved Hide resolved
specs/kyber/src/bit_vector.rs Outdated Show resolved Hide resolved
specs/kyber/src/bit_vector.rs Outdated Show resolved Hide resolved
specs/kyber/src/encoding.rs Outdated Show resolved Hide resolved
specs/kyber/src/encoding.rs Outdated Show resolved Hide resolved
@franziskuskiefer franziskuskiefer self-requested a review July 14, 2023 08:28
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, looks pretty good already.
I left some comments inline and there are some general things

  • Run cargo fmt on the code
  • add .gitignore for Cargo.lock and target
  • The hax issue after formatting are because the modules are resorted. This shouldn't happen but is a known bug. Add empty lines between the module declarations in lib.rs. Then you should be fine.
  • Error handling should be done consistently. Introduce an error enum or use u8 with error codes and only unwrap when you are certain that the call can't error. Each of these cases will need a proof.

specs/kyber/src/serialize.rs Outdated Show resolved Hide resolved
specs/kyber/src/lib.rs Outdated Show resolved Hide resolved
specs/kyber/src/lib.rs Outdated Show resolved Hide resolved
specs/kyber/src/lib.rs Outdated Show resolved Hide resolved
specs/kyber/src/field.rs Outdated Show resolved Hide resolved
specs/kyber/src/ind_cpa.rs Outdated Show resolved Hide resolved
specs/kyber/src/ring.rs Outdated Show resolved Hide resolved
specs/kyber/src/field.rs Outdated Show resolved Hide resolved
specs/kyber/src/sampling.rs Outdated Show resolved Hide resolved
specs/kyber/src/serialize.rs Outdated Show resolved Hide resolved
@franziskuskiefer franziskuskiefer changed the title First steps toward a Kyber spec. Kyber spec. Aug 1, 2023
@franziskuskiefer franziskuskiefer marked this pull request as ready for review August 2, 2023 09:27
@franziskuskiefer franziskuskiefer merged commit 622481c into dev Aug 2, 2023
6 checks passed
@franziskuskiefer franziskuskiefer deleted the kyber-spec branch August 2, 2023 09:53
@franziskuskiefer franziskuskiefer mentioned this pull request Aug 2, 2023
3 tasks
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