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

Invalid witness generated on Android #4

Closed
nasko25 opened this issue Nov 4, 2022 · 11 comments
Closed

Invalid witness generated on Android #4

nasko25 opened this issue Nov 4, 2022 · 11 comments

Comments

@nasko25
Copy link

nasko25 commented Nov 4, 2022

Hello,

I am trying to generate a witness of the following circuit on android.
I am using the feature/arm-asm branch, and in order to compile it I change the code in the src/auth.cpp file:

namespace CIRCUIT_NAME {
	// add the code from the cpp compiled poc.circom circuit (from poc_cpp/poc.cpp)
}

Then, I execute

./build_gmp.sh android
make -j 8 android

and I copy the generated package_android/bin/auth executable together with the .dat file generated from circom (poc_cpp/poc.dat) as auth.dat to a physical android device.

Please let me know if I am doing something incorrectly up to now.

Then, if I execute ./auth poc.json w.wtns on android, it calculates a witness, but when I try to verify a proof, generated from this witness file, the proof is not valid.
I viewed the w.wtns file, and it looks like it is mostly empty to me.

Please, let me know if I have made a mistake somewhere, or this is a bug in the witness calculator.

It is probably worth mentioning that this specific circuit did not work for the circom generated c++ witness calculator up until very recently, but there the witness generation crashed without generating a witness file, while witnesscalc generates an invalid witness file.

@OBrezhniev
Copy link
Contributor

OBrezhniev commented Nov 21, 2022

@nasko25 We have applied bug fixes from circom 9 days ago. You can test if this fixes your issue. Also check if USE_ASM flag in cmake changes result.
In general we haven't tested executables on Android (but should work as you describe), we are using compiled libraries.
Also I've just merged feature/arm-asm branch into main.

@nasko25
Copy link
Author

nasko25 commented Nov 22, 2022

I tried the latest changes and setting the USE_ASM flag, but the witness is still not correct.
I also tried generating a witness from the host, and it is not valid either.

@OBrezhniev
Copy link
Contributor

@nasko25 we will update this witness calc to support cpp files generated by the latest circom in about a week, maybe it will fix your issue.

@nasko25
Copy link
Author

nasko25 commented Nov 28, 2022

Thanks, I will wait for the update. I already tried several previous releases of circom to generate a cpp file (most notably v2.0.7 and v2.0.5), and none of them produced a correct witness. Do you think there is a specific version of circom which might generate cpp files that should work?

@nasko25
Copy link
Author

nasko25 commented Dec 6, 2022

@OBrezhniev I tried the latest changes on an x86_64 host machine and the witness is still not correct. Is there anything else I can try?

@nasko25
Copy link
Author

nasko25 commented Dec 17, 2022

I tried the feature/update-authV2 branch with a cpp file from the latest circom, but the proof calculated with the generated witness file is still not valid. Circom's witness calculator still calculates a correct witness.

I also tried a different circuit with these versions of witnesscalc, circom, and snarkjs and it produced a valid proof. So something goes wrong only for the circuit I linked above.

@nasko25
Copy link
Author

nasko25 commented Dec 18, 2022

I also noticed that the logs in this circuit are not printed when generating a witness with witnesscalc. But if I add a log to the very beginning of this circuit (in the main component) it is printed. So something with the use of multiple components probably goes wrong.

@sanketsaagar
Copy link

Hi @nasko25 , Did your issue resolve?

@nasko25
Copy link
Author

nasko25 commented May 24, 2023

Hi @sanketsaagar

Unfortunately, it is not yet resolved. Generating a witness with witnesscalc for the circuit linked above still results in an invalid proof.
However, I think I was able to isolate the issue further.
This circuit also generates an invalid witness:

pragma circom 2.0.0;

template Add2(n) {
    signal input a[n];
    signal input b[n];
    signal output out[n];

    log("add");

    for (var k=0; k<n; k++) {
        out[k] <== a[k] + b[k];
    }
}

template Main() {
    signal input a[2];
    signal input b[2];
    signal output out[2];

    component add = Add2(2);

    for (var i = 0; i < 2; i++) {
        add.a[i] <== a[i];
        add.b[i] <== b[i];
    }

    for (var j = 0; j < 2; j++) {
        log("out[", j, "] = ", add.out[j]);
        out[j] <== add.out[j];
    }

}

component main = Main();

And I believe the source of the issue is an implementation of assert(expr) somewhere withing witnesscalc that does not execute its parameter.

@OBrezhniev
Copy link
Contributor

OBrezhniev commented Jun 10, 2023

@nasko25 try generating witnessccalc with this fix in circom compiler: https://github.com/iden3/circom/tree/fix-counter-decrease-in-assert
It should fix (at least for me it works with your code), but in general check that you don't have NDEBUG defined somewhere, which disables asserts, as asserts are used instead of exceptions in the code, which is not right way of exiting on failed condition checks.

@nasko25
Copy link
Author

nasko25 commented Jun 12, 2023

Thank you. This version of circom fixes the issue.

I did check whether NDEBUG was defined anywhere while I was debugging it, but I could not find a manual definition anywhere, so I assumed it was automatically enabled in the cmake/compiler's release mode.

@nasko25 nasko25 closed this as completed Jun 12, 2023
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

3 participants