o1-labs / snarky

OCaml DSL for verifiable computation
MIT License
494 stars 74 forks source link

move assert_equals to checked #776

Closed mimoo closed 1 year ago

mimoo commented 1 year ago

to fix a bug I mention in the comment here https://github.com/o1-labs/snarky/pull/752#issuecomment-1376600672

I'll copy/paste the issue:

The issue

running into some error, that can be reproduce from mina by doing:

make snarkyjs
cd src/lib/snarky_js_bindings/test_module 
npm i
node simple-zkapp-mock-apply.js

the issue is an assert_equals function that does not handle constant variables correctly.

The reason is that it's using what's in checked.ml (which is "impure" as it adds a constraint):

  let assert_ ?label c = add_constraint (Constraint.override_label c label)

  let assert_equal ?label x y = assert_ (Constraint.equal ?label x y)

whereas we want to use the wrapper that's introduced in utils.ml (which handles the constant correctly, and keep the logic "pure"):

  let assert_equal ?label x y =
    match (x, y) with
    | Cvar0.Constant x, Cvar0.Constant y ->
        if Field.equal x y then Checked.return ()
        else
          failwithf !"assert_equal: %{sexp: Field.t} != %{sexp: Field.t}" x y ()
    | _ ->
        Checked.assert_equal ?label x y

The solution

I just moved the correct logic to checked.ml

It works

the following test runs without issue now!

make snarkyjs
cd src/lib/snarky_js_bindings/test_module 
npm i
node simple-zkapp-mock-apply.js
Screenshot 2023-01-09 at 6 42 47 PM