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.

Sign up for GitHub

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

Jump to bottom

refactor: fix a few code base warts + create uint64 package #42

Merged
merged 6 commits into from
Feb 25, 2024
Merged

refactor: fix a few code base warts + create uint64 package #42

merged 6 commits into from
Feb 25, 2024

Conversation

huitseeker
Copy link
Member

TL;DR:

  • splits off the uint64 gadgets used in Keccak/Sha512, into its own crate,
  • removes superfluous dependencies on PrimeField,
  • cleans up minor Rust warts.

@huitseeker
fix: PrimeFieldBits implies PrimeField
c3a8340
@huitseeker
fix: use alloc_infallible, make Clone derivation automatic
dff9e1e
@huitseeker
fix: use a few Result/Option patterns
d086e96
@huitseeker
chore: remove obsolete dependency
d9084e1
@huitseeker
chore: Refactor uint64 module with external bellpepper-uint64 package
bf00694 
- Replaced the local `uint64` modules with the `bellpepper_uint64` package in the `keccak` and `sha512` crates.
- Removed `UInt64` struct and associated functions/tests from the `keccak` & `sha512` crates.
- Created a `uint64` crate with details including license (both MIT and Apache 2.0), authors, description, and dependencies.
- Updated `Cargo.toml` files for the `sha512` and `keccak` crates, adding `bellpepper-uint64` as a new dependency, and for the project, including `uint64` in the project members.
- Created comprehensive README and license files for the new `uint64` crate.
@avras
Copy link
Collaborator

avras commented Feb 24, 2024

The LICENSE file change in #36 resulted in my name in the root MIT and APACHE license files. We could replace my name with "Bellpepper Gadget Developers" and include the change in this PR. The authors of each of the crates are credited in the "authors" fields of the respective Cargo.toml files.

adr1anh
adr1anh approved these changes Feb 25, 2024
View reviewed changes
Copy link

@adr1anh adr1anh left a comment

Choose a reason for hiding this comment

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

LGTM

@huitseeker huitseeker merged commit 68172e9 into argumentcomputer:main Feb 25, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adr1anh adr1anh adr1anh approved these changes

@wwared wwared Awaiting requested review from wwared

@avras avras Awaiting requested review from avras

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@huitseeker @avras @adr1anh