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

Expand cases for generating a witness in prepare inputs #452

Merged
merged 7 commits into from
Nov 9, 2022

Conversation

vezenovm
Copy link
Contributor

@vezenovm vezenovm commented Nov 8, 2022

Related issue(s)

Resolves issue with not being able to use inputs to gadgets that had been set through a conditional statement.

Description

While writing the account circuit I was performing this conditional:

let mut signer: [Field; 2] = [signing_pub_key[0], signing_pub_key[1]];
    if create == 1 {
        signer[0] = account_public_key[0];
        signer[1] = account_public_key[1];
    }

Which was later used by this intrinsic gadget:

 let sig_res = std::schnorr::verify_signature(signer[0], signer[1], signature, message_byte_array);

When compiling this would lead to the error below:

Screen Shot 2022-11-08 at 4 40 18 PM

We require that inputs to intrinsic gadgets be transformed into witnesses, however, we were not handling the case where the inputs to the function are a result of a conditional instruction. The result of this conditional does not have a witness associated with it.

Summary of changes

I simply added extra logic to prepare_inputs for cases other than a NodeObj::Obj. In the case that the NodeId is in the arithmetic cache, but does not yet have a witness generate, we simply generate a new witness for this instruction with generate_witness.

Dependency additions / changes

(If applicable.)

Test additions / changes

I added a little extra logic to our scalar_mul test. If you run nargo compile on this test using a nargo build from the current master you should see the error that I mentioned above. The test passes as expected with the current changes. You can also change the input of a for the scalar_mul test to see how the inputs to std::fixed_based::scalar_mul are changed.

Checklist

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt with default settings.
  • I have linked this PR to the issue(s) that it resolves.
  • I have reviewed the changes on GitHub, line by line.
  • I have ensured all changes are covered in the description.

Additional context

(If applicable.)

crates/noirc_evaluator/src/ssa/node.rs Outdated Show resolved Hide resolved
crates/noirc_evaluator/src/ssa/acir_gen.rs Outdated Show resolved Hide resolved
crates/noirc_evaluator/src/ssa/acir_gen.rs Outdated Show resolved Hide resolved
@vezenovm vezenovm merged commit 1d1b592 into master Nov 9, 2022
@vezenovm vezenovm deleted the mv/expand-ssa-prep-inputs branch November 9, 2022 15:23
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.

3 participants