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

Failing tests with NoirHelper #1

Open
critesjosh opened this issue Dec 20, 2024 · 14 comments
Open

Failing tests with NoirHelper #1

critesjosh opened this issue Dec 20, 2024 · 14 comments

Comments

@critesjosh
Copy link

I am trying to integrate this tool with the with-foundry template in the noir-starter here: https://github.com/AztecProtocol/noir-starter/tree/main/with-foundry

This may be user error, but would love some help 😅 You can see my update on this branch

I've been running tests with forge test --optimize --optimizer-runs 5000 --evm-version cancun since the compiler doesn't like when I use london

If I try to log some of the returned publicInputs I see
[FAIL: panic: array out-of-bounds access (0x32)] test_dynamicProof() (gas: 116848)

Without logging, I see [FAIL: PUBLIC_INPUT_COUNT_INVALID(2, 0)] test_dynamicProof() (gas: 127192) as part of this test

Do I need to pass the circuit return value to NoirHelper via withInput?

@critesjosh
Copy link
Author

critesjosh commented Dec 20, 2024

Here are the test logs, there is only 1 public input, but the y and the return value should be public inputs? maybe its improperly handling the return value? 🤔 Also the x value (1) is a private input, the public inputs should both be 5.

image

@0xnonso
Copy link
Owner

0xnonso commented Dec 20, 2024

hi hi @critesjosh i think the inputs you are passing into the circuit 1, 5 does not satisfy the constraint :- fn main(x: Field, y: pub Field) -> pub Field { assert(x == y); y } in main.nr
hence the event FailedConstraintWithError() and empty outputs, maybe it should explicitly revert in such situations 🤔

@0xnonso
Copy link
Owner

0xnonso commented Dec 20, 2024

I am trying to integrate this tool with the with-foundry template in the noir-starter here: https://github.com/AztecProtocol/noir-starter/tree/main/with-foundry

This may be user error, but would love some help 😅 You can see my update on this branch

I've been running tests with forge test --optimize --optimizer-runs 5000 --evm-version cancun since the compiler doesn't like when I use london

If I try to log some of the returned publicInputs I see [FAIL: panic: array out-of-bounds access (0x32)] test_dynamicProof() (gas: 116848)

Without logging, I see [FAIL: PUBLIC_INPUT_COUNT_INVALID(2, 0)] test_dynamicProof() (gas: 127192) as part of this test

Do I need to pass the circuit return value to NoirHelper via withInput?

the reason for this is also related to the above, because the public input array returned is empty trying to access an index fails

@critesjosh
Copy link
Author

lol derp 🤦‍♂️

@critesjosh
Copy link
Author

I am still seeing some weird behavior. When I run the tests repeatedly, sometimes they fail, and sometimes they pass. I updated my branch. I also added withInput("return", 1) because without it, it doesn't seem to be included in the prover.toml file, which it should be.

Sometimes I the test_wrongProof fails with:
image

Other times test_verifyProof fails with:

image

But they never both pass.

Sometimes this:

image

It's different every time

Perhaps noirHelper.clean should also delete the Prover.toml file?

@critesjosh critesjosh reopened this Dec 20, 2024
@0xnonso
Copy link
Owner

0xnonso commented Dec 21, 2024

hmm, strange. let me look at it. also noticed that some data is cached between test runs so we might have to cleanup Prover.toml each time.

@critesjosh
Copy link
Author

critesjosh commented Dec 21, 2024

I think foundry tests also run in parallel, which likely contributes to the problem. I'll try running in sequence and see if this changes things

@critesjosh
Copy link
Author

critesjosh commented Dec 21, 2024

Ok I am pretty sure this is the issue. Running one test at a time causes them to always succeed. I am not sure how to force forge to run tests sequentially though

This seems to work, wdyt?

    function verifyProof() public {
        noirHelper.withInput("x", 1).withInput("y", 1).withInput("return", 1);
        (bytes32[] memory publicInputs, bytes memory proof) = noirHelper.generateProofAndClean(2);
        starter.verifyEqual(proof, publicInputs);
    }

    function wrongProof() public {
        noirHelper.clean();
        noirHelper.withInput("x", 1).withInput("y", 5).withInput("return", 5);
        (bytes32[] memory publicInputs, bytes memory proof) = noirHelper.generateProofAndClean(2);
        vm.expectRevert();
        starter.verifyEqual(proof, publicInputs);
    }

    function test_all() public {
        // Run tests in wrapper to make them run sequentially
        verifyProof();
        wrongProof();
    }

@0xnonso
Copy link
Owner

0xnonso commented Dec 21, 2024

I think foundry tests also run in parallel, which likely contributes to the problem. I'll try running in sequence and see if this changes things

yes yes this seems to be the bulk of the issue.

@0xnonso
Copy link
Owner

0xnonso commented Dec 21, 2024

Ok I am pretty sure this is the issue. Running one test at a time causes them to always succeed. I am not sure how to force forge to run tests sequentially though

This seems to work, wdyt?

    function verifyProof() public {
        noirHelper.withInput("x", 1).withInput("y", 1).withInput("return", 1);
        (bytes32[] memory publicInputs, bytes memory proof) = noirHelper.generateProofAndClean(2);
        starter.verifyEqual(proof, publicInputs);
    }

    function wrongProof() public {
        noirHelper.clean();
        noirHelper.withInput("x", 1).withInput("y", 5).withInput("return", 5);
        (bytes32[] memory publicInputs, bytes memory proof) = noirHelper.generateProofAndClean(2);
        vm.expectRevert();
        starter.verifyEqual(proof, publicInputs);
    }

    function test_all() public {
        // Run tests in wrapper to make them run sequentially
        verifyProof();
        wrongProof();
    }

you are right, i created a pr #3 to try and address the issue. if it works completely then we might not need to run the tests sequentially.
it also cleans up by default so no need to handle cleaning except you really want to.

@critesjosh
Copy link
Author

This seems to work most of the time.

    function test_verifyProof() public {
        noirHelper.withInput("x", 1).withInput("y", 1).withInput("return", 1);
        (bytes32[] memory publicInputs, bytes memory proof) = noirHelper.generateProof("test_verifyProof", 2);
        starter.verifyEqual(proof, publicInputs);
    }

    function test_wrongProof() public {
        noirHelper.clean();
        noirHelper.withInput("x", 1).withInput("y", 5).withInput("return", 5);
        (bytes32[] memory publicInputs, bytes memory proof) = noirHelper.generateProof("test_wrongProof", 2);
        vm.expectRevert();
        starter.verifyEqual(proof, publicInputs);
    }

I occasionally see this though

image

@0xnonso
Copy link
Owner

0xnonso commented Dec 21, 2024

i get the same error sometimes too, might be foundry related - foundry-rs/book#949
its hard to debug because i really dont know how to reproduce this particular bug.
also most of the tests here run fine - https://github.com/0xnonso/noir-starter/blob/n/update-with-foundry/with-foundry/test/Starter.t.sol

@0xnonso
Copy link
Owner

0xnonso commented Dec 21, 2024

looks like it is actually related to noir - noir-lang/noir#6445

@0xnonso
Copy link
Owner

0xnonso commented Dec 21, 2024

do you know if there is any way we can specify the output of the compiled artefact JSON file like we do for the prover file?

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

No branches or pull requests

2 participants