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

add native field chip using native arithmetic operations & grumpkin curve chip #136

Closed
wants to merge 8 commits into from
Closed

add native field chip using native arithmetic operations & grumpkin curve chip #136

wants to merge 8 commits into from

Conversation

odyssey2077
Copy link
Contributor

No description provided.

jonathanpwang
jonathanpwang requested changes Sep 7, 2023
View reviewed changes
Copy link
Contributor

@jonathanpwang jonathanpwang left a comment

Choose a reason for hiding this comment

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

Initial lookthrough

halo2-ecc/src/fields/native_fp.rs Outdated Show resolved Hide resolved
halo2-base/src/lib.rs Show resolved Hide resolved
halo2-ecc/src/fields/native_fp.rs Outdated Show resolved Hide resolved
halo2-ecc/src/fields/native_fp.rs Outdated Show resolved Hide resolved
halo2-ecc/src/fields/native_fp.rs Outdated Show resolved Hide resolved
halo2-ecc/src/fields/native_fp.rs Outdated Show resolved Hide resolved
halo2-ecc/src/fields/native_fp.rs Outdated Show resolved Hide resolved
halo2-ecc/src/grumpkin/mod.rs Outdated Show resolved Hide resolved
halo2-ecc/src/grumpkin/mod.rs Outdated Show resolved Hide resolved
jonathanpwang
jonathanpwang reviewed Sep 11, 2023
View reviewed changes
Copy link
Contributor

@jonathanpwang jonathanpwang left a comment

Choose a reason for hiding this comment

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

Another minor comment, otherwise LGTM.

Can't merge this into develop (that's our prod branch). I'm about to open a PR to merge develop into community-edition, will change base to that branch after.

halo2-ecc/src/fields/native_fp.rs Outdated Show resolved Hide resolved
@jonathanpwang jonathanpwang changed the base branch from develop to develop-ce September 14, 2023 15:13
@jonathanpwang
Copy link
Contributor

Changed base to develop-ce

jonathanpwang
jonathanpwang approved these changes Sep 21, 2023
View reviewed changes
Copy link
Contributor

@jonathanpwang jonathanpwang left a comment

Choose a reason for hiding this comment

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

LGTM!

One last nit: can you run cargo clippy to fix formatting? The CI isn't on for develop-ce so it's not catching it.

@jonathanpwang jonathanpwang deleted the branch axiom-crypto:develop September 21, 2023 19:02
@jonathanpwang
Copy link
Contributor

jonathanpwang commented Sep 21, 2023 •
edited
Loading

Edit: changed base to community-edition now that develop-ce was merged

@jonathanpwang jonathanpwang reopened this Sep 21, 2023
@jonathanpwang jonathanpwang changed the base branch from develop-ce to community-edition September 21, 2023 19:07
@jonathanpwang jonathanpwang self-requested a review September 21, 2023 19:07
jonathanpwang
jonathanpwang requested changes Sep 21, 2023
View reviewed changes
Copy link
Contributor

@jonathanpwang jonathanpwang left a comment

Choose a reason for hiding this comment

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

Merge community-edition and fix clippy

@odyssey2077 odyssey2077 changed the base branch from community-edition to develop September 21, 2023 20:48
@jonathanpwang
Copy link
Contributor

Replaced by #164

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonathanpwang jonathanpwang jonathanpwang requested changes

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

Successfully merging this pull request may close these issues.
2 participants
@odyssey2077 @jonathanpwang