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

Cleanup/roots of unities setup functions #37

Merged
merged 47 commits into from
Jan 14, 2025

Conversation

anupsv
Copy link
Collaborator

@anupsv anupsv commented Jan 12, 2025

Removing 2 functions
Removing unwanted struct fields
Changing tests and bench functions
Addresses: #28

@anupsv anupsv requested review from bxue-l2 and samlaf January 12, 2025 03:36
tests/kzg_test.rs Show resolved Hide resolved
src/helpers.rs Outdated Show resolved Hide resolved
src/kzg.rs Outdated Show resolved Hide resolved
benches/bench_kzg_commit.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@bxue-l2 bxue-l2 left a comment

Choose a reason for hiding this comment

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

thanks for cleaning up

benches/bench_kzg_commit.rs Outdated Show resolved Hide resolved
src/kzg.rs Outdated Show resolved Hide resolved
/// let max_size = 64;
/// let bytes = to_byte_array(&elements, max_size);
/// assert_eq!(bytes.len(), 64);
/// // bytes will contain up to max_size bytes from the encoded elements
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed this cause its the wrong example. Also as it's not a public function doctests can't work with it, so I just kept the documentation part since it's useful.

Copy link
Contributor

@samlaf samlaf left a comment

Choose a reason for hiding this comment

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

LGTM after you remove the is_zero function and fix the ci (think you're prob still using the private calculate_roots_of_unity in some doccomment somewhere?)

@anupsv
Copy link
Collaborator Author

anupsv commented Jan 14, 2025

calculate_roots_of_unity

Yep, fixed both. Apparently vscode's refactor symbol needs more work on detection.

@anupsv anupsv merged commit 8afe7c0 into master Jan 14, 2025
1 check passed
@anupsv anupsv deleted the cleanup/roots-of-unities-setup-functions branch January 14, 2025 19:23
anupsv added a commit that referenced this pull request Jan 15, 2025
* adding bare changes for batch verification

* adding some comments

* adding more comments

* moving back to sha2

* removing a test which is no longer needed. Removing methods no longer needed

* updates to method visibility, updating tests

* fmt fixes

* clean up

* cleanup, optimization, inline docs

* removing unwanted const

* more docs and cleanup

* formatting

* removing unwanted comments

* cargo fmt and clippy

* adding test for point at infinity

* cleaner errors, cleanup

* adding another test case

* removing unwanted errors

* adding fixes per comments

* adding 4844 spec references

* comment fixes

* formatting, adding index out of bound check, removing print statement

* removing unwanted test, adding test for evaluate_polynomial_in_evaluation_form

* moving test to bottom section

* Update src/polynomial.rs

Co-authored-by: Samuel Laferriere <[email protected]>

* Update src/kzg.rs

Co-authored-by: Samuel Laferriere <[email protected]>

* Update src/kzg.rs

Co-authored-by: Samuel Laferriere <[email protected]>

* Update src/kzg.rs

Co-authored-by: Samuel Laferriere <[email protected]>

* Update src/helpers.rs

Co-authored-by: Samuel Laferriere <[email protected]>

* updating deps, and toolchain to 1.84

* removing errors test, no longer useful

* adding to_byte_array arg explanation

* fmt fixes

* fmt and clippy fixes

* fixing function names and fmt

* clippy fixes

* removing unwanted setup functions

* removing vars from struct

* fixing function name and comments

* fixing naming for tests

* fixing naming in benches

* formatting

* removing is_zero

---------

Co-authored-by: anupsv <[email protected]>
Co-authored-by: Samuel Laferriere
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.

3 participants