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

Fast zkapp proofs with max_proofs_verified=0 #11053

Merged
merged 39 commits into from
Jun 7, 2022

Conversation

mitschabaude
Copy link
Member

@mitschabaude mitschabaude commented May 23, 2022

This adds the snarkyjs changes required to take advantage of #11097 / #11185
To get an idea of the impact, here are snark performance timings taken from the snarkyjs unit test in CI:

  • develop: compile 280-350 sec, prove 76-87 sec
  • this PR: compile 35-38 sec, prove 21-22 sec

This also includes some smaller snarkyjs bindings changes, whose JS counterparts are mostly merged with o1-labs/o1js#171

  • Closes Fix Field(number) in snarkyjs #10960
  • enables Field(10n), Field('-10') (bigints, strings with a minus sign)
  • adds Field.fromX constructors, Field.toBigInt, Field.ORDER (a bigint), Field.minusOne
  • adds Bool.true, Bool.false, Bool.assertTrue, Bool.assertFalse

TODO:

  • fix replayer test

@mitschabaude mitschabaude added the ci-build-me Add this label to trigger a circle+buildkite build for this branch label May 23, 2022
@mitschabaude mitschabaude changed the base branch from simplify-per-branch-data to pickles-wrap-hack May 26, 2022 12:35
@mitschabaude mitschabaude marked this pull request as ready for review June 2, 2022 09:22
@mitschabaude mitschabaude requested a review from a team as a code owner June 2, 2022 09:22
@mitschabaude mitschabaude requested review from mrmr1993 and a team as code owners June 2, 2022 09:22
Base automatically changed from pickles-wrap-hack to develop June 3, 2022 04:27
@mitschabaude mitschabaude requested a review from a team as a code owner June 4, 2022 09:53
@mitschabaude mitschabaude changed the base branch from develop to dynamic-lagrange-commitment-selection June 6, 2022 07:52
@mitschabaude mitschabaude requested review from bkase, psteckler and a team as code owners June 6, 2022 07:52
Base automatically changed from dynamic-lagrange-commitment-selection to develop June 7, 2022 02:44
@@ -51,30 +51,56 @@ module As_field = struct

let of_field_obj (x : field_class Js.t) : t = Obj.magic x

let of_number_exn (value : t) : Impl.Field.t =
Copy link
Member

Choose a reason for hiding this comment

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

These changes are unrelated. Please open a separate PR for them in future.

@@ -228,6 +254,11 @@ let optdef_arg_method (type a) class_ (name : string)
in
Js.Unsafe.set prototype (Js.string name) meth

let to_js_bigint =
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated

@@ -278,6 +309,7 @@ let () =
method_ "sizeInFields" (fun _this : int -> 1) ;
method_ "toFields" (fun this : field_class Js.t Js.js_array Js.t ->
singleton_array this ) ;
method_ "toBigInt" (fun this -> to_string this##.value |> to_js_bigint) ;
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated

@@ -358,6 +390,10 @@ let () =
in
field_class##.one := mk Field.one ;
field_class##.zero := mk Field.zero ;
field_class##.minusOne := mk @@ Field.negate Field.one ;
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated

@@ -494,7 +530,11 @@ let () =
else Field.Constant.of_string s ) )
with Failure _ -> Js.Opt.empty )
| _ ->
Js.Opt.empty )
Js.Opt.empty ) ;
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated

@@ -1731,15 +1764,19 @@ let pickles_compile (choices : pickles_rule_js Js.js_array Js.t) =
(* TODO: get rid of Obj.magic, this should be an empty "H3.T" *)
let prevs = Obj.magic [] in
let statement = Zkapp_statement.(statement_js |> of_js |> to_constant) in
prover ?handler:None prevs statement |> Promise_js_helpers.to_js
let proof_promise = prover ?handler:None prevs statement in
Copy link
Member

Choose a reason for hiding this comment

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

Nit: unnecessary let ... = ... in

method_ "toField" (fun this : field_class Js.t ->
new%js field_constr (As_field.of_field (this##.value :> Field.t)) ) ;
add_op1 "not" Boolean.not ;
add_op2 "and" Boolean.( &&& ) ;
add_op2 "or" Boolean.( ||| ) ;
method_ "assertEquals" (fun this (y : As_bool.t) : unit ->
Boolean.Assert.( = ) this##.value (As_bool.value y) ) ;
method_ "assertTrue" (fun this : unit -> Boolean.Assert.is_true this##.value) ;
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated

@@ -5,8 +5,8 @@ let () =
let js_layout =
`Assoc
[ ("Parties", layout Parties.deriver)
; ("BalanceChange", layout Fields_derivers_zkapps.Derivers.balance_change)
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated

let ok = await Ledger.verifyPartyProof(statement, proof, verificationKey.data);
toc();
console.log("did proof verify?", ok);
if (!ok) console.log("proof didn't verify");
Copy link
Member

Choose a reason for hiding this comment

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

Nit: is this not redundant with the log immediately above?

@@ -90,6 +91,16 @@ partiesJsonInitialize = signFeePayer(partiesJsonInitialize, senderKey, {
});
toc();

// verify the proof
tic("verify transaction proof");
Copy link
Member

Choose a reason for hiding this comment

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

Nit: tic and tock would be better served by self-documenting names. Consider e.g. startTiming and endTiming.
(Not to change in this PR, it's already a grab-bag, lets not make it worse.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-build-me Add this label to trigger a circle+buildkite build for this branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix Field(number) in snarkyjs
2 participants