Create assertions for non-strict comparison.

[BlakeMScurr]

assert(claimedValue<=sourceValue); does not produce a constraint, it just stops witness generation. So an adversarial prover could remove the assert line and generate an invalid proof that the verifier would accept. assert can't produce constraints because all constraints in circom are quadratic, and <= is not quadratic.

Note that LessThan(n) can fail for values greater than 2^n due to overflow, hence that compconstant components.

[BlakeMScurr]

Also, several of the tests failed for me, but my changes didn't make any additional tests fail.

My test output after importing `kv-merkle-tree` to fix compile error

yarn run test                                                   
yarn run v1.22.10
$ hardhat test
No need to generate any newer typings.

  Hydra S1 Circuits
    Generating proof
      ✓ Snark proof of simple value in a merkleTree with simple userTicket
    Verifying accountsTree and registryTree are good
      ✓ Should throw when using an Accounts tree with wrong height
      ✓ Should throw when using an Registry Merkle tree with wrong height
    Verifying source address constraints are good
      ✓ Should throw with wrong source address
      ✓ Should throw with wrong sourceSecret
      ✓ Should throw with wrong sourceCommitmentReceipt
    Verifying destination address constraints are good
      ✓ Should throw with wrong destination address
      ✓ Should throw with wrong destinationSecret
      ✓ Should throw with wrong destinationCommitmentReceipt
      ✓ Should throw with wrong commitmentMapperPubKey
    Verify externalDataMerkleTree constraint against the globalSismoTree
      ✓ Should verify the global sismo tree root constraint
      ✓ Should verify the accountsTreeRoot constraint along the global sismo merkle tree
    Verify data merkle tree constraint is good
      ✓ Should throw when having a bad data merkle tree root or a bad data merkle tree height
      ✓ Should throw when using a bad Merkle path
      ✓ Should throw when using a good Merkle path but for an other source address than the specified one
    Verify the value selected by the user
      ✓ Should throw when using isStrict < 0
      ✓ Should throw when using isStrict > 1
      ✓ Should throw when using a value superior of the Merkle tree value for isStrict == 1
      ✓ Should throw when using a value superior of the Merkle tree value for isStrict == 0
      ✓ Should throw when using negative value for isStrict == 1
      ✓ Should throw when using negative value for isStrict == 0
      ✓ Should throw when using a value inferior of the Merkle tree value for isStrict == 1
      ✓ Should generate a Snark proof when using a value inferior of the Merkle tree value for isStrict == 0
    Verify userTicket validity
      ✓ Should throw when the ticketIdentifier does not corresponds to the userTicket 
      ✓ Should throw when the userTicket does not corresponds to the ticketIdentifier and sourceNullifier

  Hydra S1 Prover

    1) Should generate a snark proof with correct inputs

    2) Should export the proof in Bytes
    ✓ Should throw with Invalid Accounts Merkle tree height
    ✓ Should throw with invalid Registry tree height
    ✓ Should throw when the external nullifier overflow the snark field
    ✓ Should throw with invalid source secret
    ✓ Should throw with invalid source commitment receipt
    ✓ Should throw with invalid destination secret
    ✓ Should throw with invalid destination commitment receipt
    ✓ Should throw when sending claimedValue > sourceValue
    ✓ Should throw when sending claimedValue is not equal to sourceValue and isStrict == 1
    ✓ Should throw when sending claimedValue negative
    ✓ Should throw when sending Accounts tree which is not in the Registry tree

    3) Should throw when sending a source which is not in the accountsTree

  Hydra S1 Verifier contract

    4) Should be able to generate the proof using the prover package

    5) Should be able to verify the proof using the verifier

    6) Should change a public input and expect the verifier to revert

  Hydra S1 Verifier

    7) Should be able to generate the proof using the prover package

    8) Should be able to verify the proof using the verifier

    9) Should change a public input and expect the verifier to revert

·-----------------------|----------------------------|-------------|-----------------------------·
|  Solc version: 0.8.4  ·  Optimizer enabled: false  ·  Runs: 200  ·  Block limit: 30000000 gas  │
························|····························|·············|······························
|  Methods                                                                                       │
·············|··········|··············|·············|·············|···············|··············
|  Contract  ·  Method  ·  Min         ·  Max        ·  Avg        ·  # calls      ·  eur (avg)  │
·············|··········|··············|·············|·············|···············|··············
|  Deployments          ·                                          ·  % of limit   ·             │
························|··············|·············|·············|···············|··············
|  HydraS1Verifier      ·           -  ·          -  ·    2248865  ·        7.5 %  ·          -  │
·-----------------------|--------------|-------------|-------------|---------------|-------------·

  36 passing (27s)
  9 failing

  1) Hydra S1 Prover
       Should generate a snark proof with correct inputs:
     LinkError: WebAssembly.instantiate(): Import #0 module="runtime" function="exceptionHandler" error: function import requires a callable
      at Object.builder [as WitnessCalculatorBuilder] (node_modules/circom_runtime/build/main.cjs:76:40)
      at async wtnsCalculate (node_modules/snarkjs/build/main.cjs:1110:16)
      at async Object.groth16FullProve [as fullProve] (node_modules/snarkjs/build/main.cjs:1143:5)

  2) Hydra S1 Prover
       Should export the proof in Bytes:
     TypeError: Cannot read property 'toBytes' of undefined
      at /Users/blakemcalevey-scurr/Documents/projects/hydra-s1-zkps/test/prover.test.ts:102:25
      at step (test/prover.test.ts:44:23)
      at Object.next (test/prover.test.ts:25:53)
      at /Users/blakemcalevey-scurr/Documents/projects/hydra-s1-zkps/test/prover.test.ts:19:71
      at new Promise (<anonymous>)
      at __awaiter (test/prover.test.ts:15:12)
      at Context.<anonymous> (test/prover.test.ts:101:44)

  3) Hydra S1 Prover
       Should throw when sending a source which is not in the accountsTree:

      AssertionError: expected 'WebAssembly.instantiate(): Import #0 …' to equal 'Could not find the source 0x75e480db5…'
      + expected - actual

      -WebAssembly.instantiate(): Import #0 module="runtime" function="exceptionHandler" error: function import requires a callable
      +Could not find the source 0x75e480db528101a381ce68544611c169ad7eb342 in the Accounts tree
      
      at /Users/blakemcalevey-scurr/Documents/projects/hydra-s1-zkps/test/prover.test.ts:336:30
      at step (test/prover.test.ts:44:23)
      at Object.throw (test/prover.test.ts:25:53)
      at rejected (test/prover.test.ts:17:65)

  4) Hydra S1 Verifier contract
       Should be able to generate the proof using the prover package:
     LinkError: WebAssembly.instantiate(): Import #0 module="runtime" function="exceptionHandler" error: function import requires a callable
      at Object.builder [as WitnessCalculatorBuilder] (node_modules/circom_runtime/build/main.cjs:76:40)
      at async wtnsCalculate (node_modules/snarkjs/build/main.cjs:1110:16)
      at async Object.groth16FullProve [as fullProve] (node_modules/snarkjs/build/main.cjs:1143:5)

  5) Hydra S1 Verifier contract
       Should be able to verify the proof using the verifier:
     TypeError: Cannot read property 'a' of undefined
      at /Users/blakemcalevey-scurr/Documents/projects/hydra-s1-zkps/test/verifier-contract.test.ts:90:77
      at step (test/verifier-contract.test.ts:33:23)
      at Object.next (test/verifier-contract.test.ts:14:53)
      at /Users/blakemcalevey-scurr/Documents/projects/hydra-s1-zkps/test/verifier-contract.test.ts:8:71
      at new Promise (<anonymous>)
      at __awaiter (test/verifier-contract.test.ts:4:12)
      at Context.<anonymous> (test/verifier-contract.test.ts:89:63)

  6) Hydra S1 Verifier contract
       Should change a public input and expect the verifier to revert:
     TypeError: Cannot read property 'input' of undefined
      at /Users/blakemcalevey-scurr/Documents/projects/hydra-s1-zkps/test/verifier-contract.test.ts:95:32
      at step (test/verifier-contract.test.ts:33:23)
      at Object.next (test/verifier-contract.test.ts:14:53)
      at /Users/blakemcalevey-scurr/Documents/projects/hydra-s1-zkps/test/verifier-contract.test.ts:8:71
      at new Promise (<anonymous>)
      at __awaiter (test/verifier-contract.test.ts:4:12)
      at Context.<anonymous> (test/verifier-contract.test.ts:94:72)

  7) Hydra S1 Verifier
       Should be able to generate the proof using the prover package:
     LinkError: WebAssembly.instantiate(): Import #0 module="runtime" function="exceptionHandler" error: function import requires a callable
      at Object.builder [as WitnessCalculatorBuilder] (node_modules/circom_runtime/build/main.cjs:76:40)
      at async wtnsCalculate (node_modules/snarkjs/build/main.cjs:1110:16)
      at async Object.groth16FullProve [as fullProve] (node_modules/snarkjs/build/main.cjs:1143:5)

  8) Hydra S1 Verifier
       Should be able to verify the proof using the verifier:
     TypeError: Cannot read property 'a' of undefined
      at /Users/blakemcalevey-scurr/Documents/projects/hydra-s1-zkps/test/verifier.test.ts:78:69
      at step (test/verifier.test.ts:33:23)
      at Object.next (test/verifier.test.ts:14:53)
      at /Users/blakemcalevey-scurr/Documents/projects/hydra-s1-zkps/test/verifier.test.ts:8:71
      at new Promise (<anonymous>)
      at __awaiter (test/verifier.test.ts:4:12)
      at Context.<anonymous> (test/verifier.test.ts:77:63)

  9) Hydra S1 Verifier
       Should change a public input and expect the verifier to revert:
     TypeError: Cannot read property 'input' of undefined
      at /Users/blakemcalevey-scurr/Documents/projects/hydra-s1-zkps/test/verifier.test.ts:83:32
      at step (test/verifier.test.ts:33:23)
      at Object.next (test/verifier.test.ts:14:53)
      at /Users/blakemcalevey-scurr/Documents/projects/hydra-s1-zkps/test/verifier.test.ts:8:71
      at new Promise (<anonymous>)
      at __awaiter (test/verifier.test.ts:4:12)
      at Context.<anonymous> (test/verifier.test.ts:82:72)



error Command failed with exit code 9.

[gabin54]

Hey @BlakeMScurr !

Thanks for the pr ! :)

For the tests did you install the node modules in the "./package" folder ? KV-Merkle-tree and other tests dependencies are imported from this folder. (I will add in the prepare script "cd ./package && yarn)

[BlakeMScurr]

Ah thanks @gabin54 it works now! I hadn't installed the node modules in ./package.
I'll remove the change I made to package.json.

I fixed up a little mistake I made and now all the tests pass!

[leosayous21]

Hey @BlakeMScurr, thank you for your PR! 🙏

Are you sure the check of the overflow is necessary ?
It seems to be already used in the Num2Bits inside the LessThan template here https://github.com/iden3/circomlib/blob/master/circuits/comparators.circom#L94.

For example in the Query system of iden3 they don't check the overflow https://github.com/iden3/circuits/blob/master/circuits/lib/query/query.circom#L29

But i asked the question here https://t.me/iden3io/7348

[BlakeMScurr]

Hi @leosayous21!

I'm not so sure about the overflow check. From my reading of the code it should be necessary, but circom can be subtle and I may be missing something. You're right, the iden3 library doesn't seem to do an overflow check.

I will try to demonstrate an overflow, and if I can't, I'll revert the check!

[BlakeMScurr]

@leosayous21 I think I've demonstrated that the overflow check is necessary. Have a look at this repository and see if you agree and whether you can reproduce it. https://github.com/BlakeMScurr/comparator-overflow
I'll try to get the iden3 people to have a look too!

[leosayous21]

I loved so much this repo and what you did to test it ! 😍
I agree with you, it's needed! Thanks a lot 🙏

The is252Bits can be remove to use directly Num2Bits because this works:

template Is252Bits() {
    signal input in;
    component bits = Num2Bits(252);
    bits.in <== in;
}

The Num2Bits reconstructs the number using the number of bits used and compares it with the original value with a hard constraint here! https://github.com/iden3/circomlib/blob/master/circuits/bitify.circom#L38
I tried it with your repository and it works well

[BlakeMScurr]

Very good point! I've changed it like you suggested and I'll update my repo too.

[leosayous21]

Thank you for your help on this! @BlakeMScurr i sent you some msg on twitter 🙂