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

Expose Basic Gate (for AND gadget) #190

Merged
merged 8 commits into from
Nov 1, 2023
Merged

Conversation

jackryanservia
Copy link
Contributor

@jackryanservia jackryanservia commented Oct 23, 2023

Summary

This PR exposes the generic constraint to our JS bindings via a function called Basic and is part of the work necessary for the AND gadget.

Context

The AND gadget behaves similarly to the & operator in JavaScript and does not require a new custom gate. Instead, it uses the XOR gate and one generic gate. See OCaml implementation for context.

Tested

This work is tested in the upper layers of o1js with unit and e2e tests built on top of the exposed gate constraint.

AND gadget PR in o1js repo: o1-labs/o1js#1193

closes: o1-labs/o1js#1140

Base automatically changed from feature/XOR-gadget to main October 24, 2023 19:03
Copy link
Contributor Author

@jackryanservia jackryanservia left a comment

Choose a reason for hiding this comment

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

I think everything is good, but I left a few questions :).

@jackryanservia jackryanservia marked this pull request as ready for review October 24, 2023 21:14
Comment on lines 182 to 189
let basic sl l sr r so o sm sc =
Impl.with_label "generic_gate" (fun () ->
Impl.assert_
{ annotation = Some __LOC__
; basic =
Kimchi_backend_common.Plonk_constraint_system.Plonk_constraint.T
(Basic { l = (sl, l); r = (sr, r); o = (so, o); m = sm; c = sc })
} )
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I exposed Basic and configured the gate in TS insetead of implementing and_equation as in the OCaml code.

  1. Is this the right approach?
  2. Are the argument names appropriate?
  3. Is there anything else I should expose while I'm here?

Copy link
Collaborator

@mitschabaude mitschabaude Oct 25, 2023

Choose a reason for hiding this comment

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

1a) Yes it's the right approach to expose the generic gate in it's most general form and not specialized to the "and" logic. We definitely want this in o1js!
1b) I don't think we need it for the "and" gadget though. o1js creates the right generic gates when you do .add() and mul() and so on, which we could use for "and" (will comment on the other PR)
2) I think so. I guess sl means "the selector for the left input" etc?
3) No I think this is good, although I think we should out of principle make an effort to expose all gates to o1js, but that can be a separate PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

1b) Ahhhh! My assumption was that that would be less efficient than doing what I did, but I think I understand now. I gather from Anaïs that when I call .add(), it goes on to a stack, and then once there is enough on the stack for us to generate an efficient constraint we do so. Is that correct? (Don't worry about tying it all out here if not; maybe we can just do a short call sometime and I will make a list of questions).
2) Correct!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah what Anais wrote is true but she probably didn't know that a simple Field.mul() also flushes that stack into a constraint, so we don't need that complicated way of specifying the gate

Copy link
Collaborator

@mitschabaude mitschabaude left a comment

Choose a reason for hiding this comment

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

Approving with a small renaming ask

crypto/finite_field.ts Outdated Show resolved Hide resolved
Comment on lines 182 to 189
let basic sl l sr r so o sm sc =
Impl.with_label "generic_gate" (fun () ->
Impl.assert_
{ annotation = Some __LOC__
; basic =
Kimchi_backend_common.Plonk_constraint_system.Plonk_constraint.T
(Basic { l = (sl, l); r = (sr, r); o = (so, o); m = sm; c = sc })
} )
Copy link
Collaborator

@mitschabaude mitschabaude Oct 25, 2023

Choose a reason for hiding this comment

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

1a) Yes it's the right approach to expose the generic gate in it's most general form and not specialized to the "and" logic. We definitely want this in o1js!
1b) I don't think we need it for the "and" gadget though. o1js creates the right generic gates when you do .add() and mul() and so on, which we could use for "and" (will comment on the other PR)
2) I think so. I guess sl means "the selector for the left input" etc?
3) No I think this is good, although I think we should out of principle make an effort to expose all gates to o1js, but that can be a separate PR

ocaml/lib/snarky_bindings.ml Outdated Show resolved Hide resolved
@jackryanservia jackryanservia requested a review from a team as a code owner November 1, 2023 05:48
@jackryanservia jackryanservia merged commit d72652c into main Nov 1, 2023
1 check passed
@jackryanservia jackryanservia deleted the feature/AND-gadget branch November 1, 2023 10:07
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.

Port AND gadget to TypeScript
2 participants