AztecProtocol / aztec-verifier-contracts

28 stars 8 forks source link

compiling plonk_vk.sol throws YulException: too deep in the stack & then fails to verify proof format #22

Open Jovonni opened 1 year ago

Jovonni commented 1 year ago

Linked from issue in noir repo: https://github.com/noir-lang/noir/issues/1140

Here is the most updated issue:

Update:

I still got the "proof failed" error. Here I am doing it again to show you the results.

I am passing the following format:

0x [function signature] + [proof data pointer] + [length of proof data] + [public input 1] + [public input 2] + [proof data array]

Details

Function Signature

0x8e760afe

Proof Data Pointer

0000000000000000000000000000000000000000000000000000000000000020

Length of proof data array

00000000000000000000000000000000000000000000000000000000000004e0

Public Input 1

0bff8247aa94b08d1c680d7a3e10831bd8c8cf2ea2c756b0d1d89acdcad877ad

Public Input 2

2a5d7253a6ed48462fedb2d350cc768d13956310f54e73a8a47914f34a34c5c4

Proof Data Array

2abfb...

Doing this, I no longer get the G1 point not on curve, or is malformed] error, but I get the proof failed error. This seems like it indicates the formatting is correct, but the proof input is wrong...

If I don't prepend my proof data with the two public inputs, then I get the G1 point not on curve, or is malformed] error again, so it seems like prepending the two public inputs works, but I cannot get past this proof failed error..

LHerskind commented 1 year ago

From the noir issue it seems to be turbo verifier, which is not in here. Noir is in progress to use Ultraplonk instead which should switch to the verifiers in here, which also support an interface where you don't have to prepend data but can just pass it in as a list of bytes32

I think there was an issue with challenge computation (@Globallager, @kevaundray) where the backend computed different challenges (different hashfunction) than the verifier which sohuld make it fail consistently.

Related, we are working on moving the verifiers from here into Barretenberg (https://github.com/AztecProtocol/barretenberg/pull/363),

kevaundray commented 1 year ago

Yeah this issue would be TurboPlonk related -- Please wait for us to switch to UP, as Lasse said, it has a much better interface and support :)

Jovonni commented 1 year ago

@LHerskind @kevaundray this sounds like a plan. Do you both have any rough timelines on when this can be accomplished? Our project heavily depends on being able to provide a valid proof via smart contract.

Also, will there be some type of tutorial on actually submitting a proof to the smart contract and encoding it correctly?

It was kind of hard to find the exact steps to get this far on our end. We had to rely on different sources of information to get this far towards an end-to-end workflow.

Thanks in advance team! πŸš€

LHerskind commented 1 year ago

Also, will there be some type of tutorial on actually submitting a proof to the smart contract and encoding it correctly? It was kind of hard to find the exact steps to get this far on our end. We had to rely on different sources of information to get this far towards an end-to-end workflow.

This should be much easier when this work is in. The new Interface looks like this:

function verify(bytes calldata _proof, bytes32[] calldata _publicInputs) external view returns (bool);

So no more prepending or dealing with the bytes directly, just passing in a list of the public inputs.

Savio-Sou commented 1 year ago

@Jovonni There were some unexpected delays and our latest outlook timeline-wise is roughly the end of month. You can subscribe to https://github.com/noir-lang/noir/issues/991 for notification when it's out from the oven πŸ™Œ

Jovonni commented 1 year ago

Awesome team thanks. Am I waiting for noir 0.4.1? Just want to make sure I know what to watch out for.

Thanks again for the explanation!

@LHerskind @Globallager

Update: Nevermind, I see the PR, thanks πŸš€

Jovonni commented 1 year ago

Hey team @Globallager @LHerskind @kevaundray ,

I am trying to update noir to the most recent PR that implements this fix, via this command:

noirup --pr 1114

But i get the following error:

noirup % noirup --pr 1114                             
HEAD is now at ab9e8c39 Update transcript comment
error: no packages found with binaries or examples
noirup: command failed: cargo install --path ./crates/nargo --bins --locked --force --root /Users/test/.nargo

When I change the noirup script, from this:

RUSTFLAGS="-C target-cpu=native" ensure cargo install --path ./crates/nargo --bins --locked --force --root $NARGO_HOME

To this (since ./crates/nargo folder doesn't exist):

RUSTFLAGS="-C target-cpu=native" ensure cargo install --path ./crates/nargo_cli --bins --locked --force --root $NARGO_HOME

I get the following output:

noirup --pr 1114        
HEAD is now at ab9e8c39 Update transcript comment
  Installing nargo_cli v0.4.1 (/Users/test/.nargo/noir-lang/noir/crates/nargo_cli)
    Updating crates.io index
warning: package `crossbeam-channel v0.5.7` in Cargo.lock is yanked in registry `crates-io`, consider running without --locked
   Compiling hyper-rustls v0.23.2
   Compiling noirc_frontend v0.4.1 (/Users/test/.nargo/noir-lang/noir/crates/noirc_frontend)
   Compiling barretenberg-sys v0.1.2
   Compiling const_format v0.2.30
   Compiling nargo_cli v0.4.1 (/Users/test/.nargo/noir-lang/noir/crates/nargo_cli)
error: failed to run custom build command for `barretenberg-sys v0.1.2`

Caused by:
  process didn't exit successfully: `/Users/test/.nargo/noir-lang/noir/target/release/build/barretenberg-sys-387d190866d3bc98/build-script-build` (exit status: 1)
  --- stdout
  cargo:rerun-if-changed=build.rs
  cargo:rerun-if-env-changed=BARRETENBERG_NO_PKG_CONFIG
  cargo:rerun-if-env-changed=PKG_CONFIG_aarch64-apple-darwin
  cargo:rerun-if-env-changed=PKG_CONFIG_aarch64_apple_darwin
  cargo:rerun-if-env-changed=HOST_PKG_CONFIG
  cargo:rerun-if-env-changed=PKG_CONFIG
  cargo:rerun-if-env-changed=BARRETENBERG_STATIC
  cargo:rerun-if-env-changed=BARRETENBERG_DYNAMIC
  cargo:rerun-if-env-changed=PKG_CONFIG_ALL_STATIC
  cargo:rerun-if-env-changed=PKG_CONFIG_ALL_DYNAMIC
  cargo:rerun-if-env-changed=PKG_CONFIG_PATH_aarch64-apple-darwin
  cargo:rerun-if-env-changed=PKG_CONFIG_PATH_aarch64_apple_darwin
  cargo:rerun-if-env-changed=HOST_PKG_CONFIG_PATH
  cargo:rerun-if-env-changed=PKG_CONFIG_PATH
  cargo:rerun-if-env-changed=PKG_CONFIG_LIBDIR_aarch64-apple-darwin
  cargo:rerun-if-env-changed=PKG_CONFIG_LIBDIR_aarch64_apple_darwin
  cargo:rerun-if-env-changed=HOST_PKG_CONFIG_LIBDIR
  cargo:rerun-if-env-changed=PKG_CONFIG_LIBDIR
  cargo:rerun-if-env-changed=PKG_CONFIG_SYSROOT_DIR_aarch64-apple-darwin
  cargo:rerun-if-env-changed=PKG_CONFIG_SYSROOT_DIR_aarch64_apple_darwin
  cargo:rerun-if-env-changed=HOST_PKG_CONFIG_SYSROOT_DIR
  cargo:rerun-if-env-changed=PKG_CONFIG_SYSROOT_DIR

  --- stderr
  Error: 
     0: Failed to locate correct Barretenberg. Package barretenberg was not found in the pkg-config search path.
        Perhaps you should add the directory containing `barretenberg.pc'
        to the PKG_CONFIG_PATH environment variable
        No package 'barretenberg' found
        Package barretenberg was not found in the pkg-config search path.
        Perhaps you should add the directory containing `barretenberg.pc'
        to the PKG_CONFIG_PATH environment variable
        No package 'barretenberg' found
        Package barretenberg was not found in the pkg-config search path.
        Perhaps you should add the directory containing `barretenberg.pc'
        to the PKG_CONFIG_PATH environment variable
        No package 'barretenberg' found.

I see the released version of noir is still 0.4.1, but how do I upgrade my noir version to take advantage of the newly finished PRs you linked above? Is there a step I am missing with brattenburg or something else?

Thanks for working speedy team.

Jovonni commented 1 year ago

Hey team, just checking in on this. I have been trying to use noirup to install PR 991, and 1114 but to no avail due to the above issue. I see @Globallager to wait until #991 is done, and I see 1114 marked it as complete.

Any insight on this?

kevaundray commented 1 year ago

Hey team @Globallager @LHerskind @kevaundray ,

I am trying to update noir to the most recent PR that implements this fix, via this command:


noirup --pr 1114

But i get the following error:


noirup % noirup --pr 1114                             

HEAD is now at ab9e8c39 Update transcript comment

error: no packages found with binaries or examples

noirup: command failed: cargo install --path ./crates/nargo --bins --locked --force --root /Users/test/.nargo

When I change the noirup script, from this:


RUSTFLAGS="-C target-cpu=native" ensure cargo install --path ./crates/nargo --bins --locked --force --root $NARGO_HOME

To this (since ./crates/nargo folder doesn't exist):


RUSTFLAGS="-C target-cpu=native" ensure cargo install --path ./crates/nargo_cli --bins --locked --force --root $NARGO_HOME

I get the following output:


noirup --pr 1114        

HEAD is now at ab9e8c39 Update transcript comment

  Installing nargo_cli v0.4.1 (/Users/test/.nargo/noir-lang/noir/crates/nargo_cli)

    Updating crates.io index

warning: package `crossbeam-channel v0.5.7` in Cargo.lock is yanked in registry `crates-io`, consider running without --locked

   Compiling hyper-rustls v0.23.2

   Compiling noirc_frontend v0.4.1 (/Users/test/.nargo/noir-lang/noir/crates/noirc_frontend)

   Compiling barretenberg-sys v0.1.2

   Compiling const_format v0.2.30

   Compiling nargo_cli v0.4.1 (/Users/test/.nargo/noir-lang/noir/crates/nargo_cli)

error: failed to run custom build command for `barretenberg-sys v0.1.2`

Caused by:

  process didn't exit successfully: `/Users/test/.nargo/noir-lang/noir/target/release/build/barretenberg-sys-387d190866d3bc98/build-script-build` (exit status: 1)

  --- stdout

  cargo:rerun-if-changed=build.rs

  cargo:rerun-if-env-changed=BARRETENBERG_NO_PKG_CONFIG

  cargo:rerun-if-env-changed=PKG_CONFIG_aarch64-apple-darwin

  cargo:rerun-if-env-changed=PKG_CONFIG_aarch64_apple_darwin

  cargo:rerun-if-env-changed=HOST_PKG_CONFIG

  cargo:rerun-if-env-changed=PKG_CONFIG

  cargo:rerun-if-env-changed=BARRETENBERG_STATIC

  cargo:rerun-if-env-changed=BARRETENBERG_DYNAMIC

  cargo:rerun-if-env-changed=PKG_CONFIG_ALL_STATIC

  cargo:rerun-if-env-changed=PKG_CONFIG_ALL_DYNAMIC

  cargo:rerun-if-env-changed=PKG_CONFIG_PATH_aarch64-apple-darwin

  cargo:rerun-if-env-changed=PKG_CONFIG_PATH_aarch64_apple_darwin

  cargo:rerun-if-env-changed=HOST_PKG_CONFIG_PATH

  cargo:rerun-if-env-changed=PKG_CONFIG_PATH

  cargo:rerun-if-env-changed=PKG_CONFIG_LIBDIR_aarch64-apple-darwin

  cargo:rerun-if-env-changed=PKG_CONFIG_LIBDIR_aarch64_apple_darwin

  cargo:rerun-if-env-changed=HOST_PKG_CONFIG_LIBDIR

  cargo:rerun-if-env-changed=PKG_CONFIG_LIBDIR

  cargo:rerun-if-env-changed=PKG_CONFIG_SYSROOT_DIR_aarch64-apple-darwin

  cargo:rerun-if-env-changed=PKG_CONFIG_SYSROOT_DIR_aarch64_apple_darwin

  cargo:rerun-if-env-changed=HOST_PKG_CONFIG_SYSROOT_DIR

  cargo:rerun-if-env-changed=PKG_CONFIG_SYSROOT_DIR

  --- stderr

  Error: 

     0: Failed to locate correct Barretenberg. Package barretenberg was not found in the pkg-config search path.

        Perhaps you should add the directory containing `barretenberg.pc'

        to the PKG_CONFIG_PATH environment variable

        No package 'barretenberg' found

        Package barretenberg was not found in the pkg-config search path.

        Perhaps you should add the directory containing `barretenberg.pc'

        to the PKG_CONFIG_PATH environment variable

        No package 'barretenberg' found

        Package barretenberg was not found in the pkg-config search path.

        Perhaps you should add the directory containing `barretenberg.pc'

        to the PKG_CONFIG_PATH environment variable

        No package 'barretenberg' found.

I see the released version of noir is still 0.4.1, but how do I upgrade my noir version to take advantage of the newly finished PRs you linked above? Is there a step I am missing with brattenburg or something else?

Thanks for working speedy team.

Cc @TomAFrench for this noirup issue

Jovonni commented 1 year ago

I am working on noir v0.5.1 now, good work on this so far on this implementation.

I am seeing an issue that I also experienced in previous versions.

My plonk_vk.sol file is showing a number of public inputs to be wrong, and the verification transaction is reverting with:

 Reason: PUBLIC_INPUT_COUNT_INVALID(3, 2)

My circuit is:

fn main(
    // Public inputs
    pubkey_x: pub Field,
    pubkey_y: pub Field,
    // Private inputs
    priv_key: Field,

) -> pub Field {

Why would the UltraVerifier expect 3 public inputs, when I only define 2?

I am passing a bytes32[] with a size of 2, but if I change the bytes32[] array to be a size of 3 (with a zero address as the 3rd element), then the transaction reverts with:

Reason: PROOF_FAILURE()

I am just calling the verify function:

function verify(bytes calldata _proof, bytes32[] calldata _publicInputs) external view returns (bool) {

Is that expected behavior? I would assume my public inputs in the generated contract should be two with my noir script shown here.

I see in the plonk_vk.sol the

uint256 internal constant NUM_INPUTS_LOC = 0x3a0;

declaration. Wondering if the issue is here:

assembly {
            requiredPublicInputCount := mload(NUM_INPUTS_LOC)
        }
        if (requiredPublicInputCount != _publicInputs.length) {
            revert PUBLIC_INPUT_COUNT_INVALID(requiredPublicInputCount, _publicInputs.length);
        }

@kevaundray @Globallager @LHerskind @TomAFrench

LHerskind commented 1 year ago

The output of your main function is also a public input to the verifier so you have 3 public inputs [pubkey_x, pubkey_y, return]. Should probably be clarified in the docs @Globallager @critesjosh @signorecello.

Jovonni commented 1 year ago

It works. Thank you kindly team. Great work, and great response effort. @LHerskind @Globallager @kevaundray

Also wondering if this was the cause of my issue in the TurboVerifier as well. Not passing the return value in to the verify function call!

πŸ™ƒ

LHerskind commented 1 year ago

It works. Thank you kindly team. Great work, and great response effort. @LHerskind @Globallager @kevaundray

Also wondering if this was the cause of my issue in the TurboVerifier as well. Not passing the return value in to the verify function call!

πŸ™ƒ

On the bright side your issue made an improvement to the docs! πŸŽ‰ https://noir-lang.org/nargo/solidity_verifier/#public-inputs