circomkit icon indicating copy to clipboard operation
circomkit copied to clipboard

[draft] Add C tester

Open erhant opened this issue 1 year ago • 6 comments

Wonder how far we can go with simply adding tester: 'wasm' | 'c' = 'wasm' parameter to WitnessTester (defaults so to not be breaking change) and then do:

    const circomTester: CircomTester = await (tester === 'wasm' ? wasm_tester : c_tester)(targetPath, {
      output: undefined, // this makes tests to be created under /tmp
      prime: this.config.prime,
      verbose: this.config.verbose,
      O: Math.min(this.config.optimization, 1), // tester doesnt have O2
      json: false,
      include: this.config.include,
      wasm: true,
      sym: true,
      recompile: circuitConfig.recompile ?? true,
    });

using the same interface for both (circomWasmTester has been renamed).

Sadly, I could not test it because im on Mac M2 and I cant get nasm to output arm64, the compiler fails during the linking step due to:

 ld: symbol(s) not found for architecture arm64

erhant avatar Aug 05 '24 21:08 erhant

heloo @CPerezz , tagging you as per the Discord message! 🙏🏻

erhant avatar Aug 05 '24 21:08 erhant

Hey @erhant

Let me try to run this. Wil likely do so later today or tomorrow. Hope it's ok. Is there any particular test or command I can run to test this easily?

CPerezz avatar Aug 06 '24 08:08 CPerezz

Let me try to run this. Wil likely do so later today or tomorrow. Hope it's ok. Is there any particular test or command I can run to test this easily?

yea that would be awesome ❤️ you can run the witnessTester.test.ts one: https://github.com/erhant/circomkit/blob/erhant/c-tester/tests/witnessTester.test.ts#L4

its configured for C tester in this branch, didnt add separate tests yet for wasm & c together.

i get compiler errors within beforeAll here, and then I play around with the Makefile at the created directory to see if i can get it to compile; circom_tester creates a tmp folder for each run (perhaps one may configure that at a circomkit-specified path, topic for later though!)

erhant avatar Aug 06 '24 09:08 erhant

I tested on x86_64 (x86_64 GNU/Linux) and encountered the following error. However, I can see in the /tmp directory, created by circom_tester, that it was successfully built.

● witness tester › should NOT assert for maliciously edited witness

  assert.strictEqual(received, expected)

  Expected value to strictly be equal to:
    undefined
  Received:
    null
  
  Message:
    error building the executable C program
  /usr/bin/ld: warning: fr_asm.o: missing .note.GNU-stack section implies executable stack
  /usr/bin/ld: NOTE: This behaviour is deprecated and will be removed in a future version of the linker
  

  Difference:

    Comparing two different types of values. Expected undefined but received null.

    496 |
    497 |     const targetPath = this.instantiate(circuit, circuitConfig);
  > 498 |     const circomWasmTester: CircomTester = await (tester === 'wasm' ? wasm_tester : c_tester)(targetPath, {
        |                                            ^
    499 |       output: undefined, // this makes tests to be created under /tmp
    500 |       prime: this.config.prime,
    501 |       verbose: this.config.verbose,

    at compile (../node_modules/circom_tester/c/tester.js:92:5)
    at c_tester (../node_modules/circom_tester/c/tester.js:49:2)
    at Circomkit.WitnessTester (../src/core/index.ts:498:44)
    at Object.<anonymous> (witnessTester.test.ts:21:15)

Test Suites: 1 failed, 1 total
Tests:       7 failed, 7 total
Snapshots:   0 total
Time:        4.439 s
Ran all test suites matching /tests\/witnessTester.test.ts/i.
/tmp/circom_-36168-HiK0qmXr308J/multiplier_4_cpp$ ls
calcwit.cpp  calcwit.hpp  calcwit.o  circom.hpp  fr.asm  fr_asm.o  fr.cpp  fr.hpp  fr.o  main.cpp  main.o  Makefile  multiplier_4  multiplier_4.cpp  multiplier_4.dat  multiplier_4.o

elmol avatar Aug 07 '24 17:08 elmol

hmm somehow the compile function fails nevertheless, could you try to use the Makefile within the tmp directory as well? maybe we can get a more descriptive error that way

erhant avatar Aug 08 '24 08:08 erhant

Compiling and building seem correct; however, a warning message is shown that could be related to the error.

/tmp/circom_-10295-VKaZ9KhG9p11/multiplier_4_cpp$ make
g++ -c main.cpp -std=c++11 -O3 -I.
g++ -c calcwit.cpp -std=c++11 -O3 -I.
g++ -c fr.cpp -std=c++11 -O3 -I.
nasm -felf64 fr.asm -o fr_asm.o
g++ -c multiplier_4.cpp -std=c++11 -O3 -I.
g++ -o multiplier_4 *.o -lgmp 
/usr/bin/ld: warning: fr_asm.o: missing .note.GNU-stack section implies executable stack
/usr/bin/ld: NOTE: This behaviour is deprecated and will be removed in a future version of the linker
$ ld --version
GNU ld (GNU Binutils for Ubuntu) 2.42

https://stackoverflow.com/questions/73435637/how-can-i-fix-usr-bin-ld-warning-trap-o-missing-note-gnu-stack-section-imp

elmol avatar Aug 08 '24 11:08 elmol

On my Fedora 40 x86_64 machine, I was able to install the dependencies and get most of the tests to pass but I had to modify the circom_tester dependency. I think there's a bug in the code. I don't know how it could ever work the way it is.

https://github.com/iden3/circom_tester/blob/v0.0.19/c/tester.js#L58

This third argument run goes into the CTester constructor as the witnessCalculator but is never used so I just removed it from node_modules/circom_tester/c/tester.js and it seems to have fixed it. I guess you could try making a PR on that repo.

With that change, I get all but one passing:

ben@fedora:~/circomkit$ pnpm test

> [email protected] test /home/ben/circomkit
> jest

 FAIL  tests/witnessTester.test.ts (7.035 s)
  witness tester
    ✓ should have correct number of constraints (49 ms)
    ✓ should assert correctly (31 ms)
    ✕ should fail for bad witness (116 ms)
    ✓ should compute correctly (26 ms)
    ✓ should read witness correctly (26 ms)
    ✓ should assert for correct witness (19 ms)
    ✓ should NOT assert for maliciously edited witness (24 ms)

  ● witness tester › should fail for bad witness

    Command failed: /tmp/circom_-1011380-rbsWnXmUCS0K/multiplier_4_cpp/multiplier_4 /tmp/circom_-1011380-rbsWnXmUCS0K/multiplier_4_cpp/multiplier_4.json /tmp/circom_-1011380-rbsWnXmUCS0K/multiplier_4_cpp/multiplier_4.wtns
    multiplier_4: multiplier_4.cpp:194: void Multiplier_1_run(uint, Circom_CalcWit*): Assertion `Fr_isTrue(&expaux[0])' failed.

That command that should fail succeeds and produces this witness (inside the zip) multiplier_4.zip

I don't know if that helps you. If you want, it's probably easiest to try debugging on an EC2 instance or something. I can make a cloudformation file to do this with one click.

numtel avatar Aug 30 '24 01:08 numtel

hmmm the bug within circom_tester is quite weird, wonder how there is no issue on that so far; nice find!

I wil try to setup an ubuntu machine and try this myself as well

P.S. sorry for assign @numtel that was a misclick hahah

erhant avatar Sep 01 '24 12:09 erhant

I think I will merge this one, it wont break anything existing & I will keep the issue open and pinned. We should also describe the prerequisites on what is needed on the host for the C tester to run.

erhant avatar Sep 09 '24 21:09 erhant