Merge pull request #15 from peppersec/audit-circuit

Circuit audit fixes

circuits/merkleTree.circom

@@ -1,72 +1,49 @@
 include "../node_modules/circomlib/circuits/mimcsponge.circom";
 
-// Computes MiMC(left + right)
-template HashLeftRight(rounds) {
+// Computes MiMC([left, right])
+template HashLeftRight() {
     signal input left;
     signal input right;
-
     signal output hash;
 
-    component hasher = MiMCSponge(2, rounds, 1);
+    component hasher = MiMCSponge(2, 220, 1);
     hasher.ins[0] <== left;
     hasher.ins[1] <== right;
     hasher.k <== 0;
-
     hash <== hasher.outs[0];
 }
 
-// if pathIndex == 0 returns (left = inputElement, right = pathElement)
-// if pathIndex == 1 returns (left = pathElement, right = inputElement)
-template Selector() {
-    signal input inputElement;
-    signal input pathElement;
-    signal input pathIndex;
+// if s == 0 returns [in[0], in[1]]
+// if s == 1 returns [in[1], in[0]]
+template Mux() {
+    signal input in[2];
+    signal input s;
+    signal output out[2];
 
-    signal output left;
-    signal output right;
-
-    signal leftSelector1;
-    signal leftSelector2;
-    signal rightSelector1;
-    signal rightSelector2;
-
-    pathIndex * (1-pathIndex) === 0
-
-    leftSelector1 <== (1 - pathIndex) * inputElement;
-    leftSelector2 <== (pathIndex) * pathElement;
-    rightSelector1 <== (pathIndex) * inputElement;
-    rightSelector2 <== (1 - pathIndex) * pathElement;
-
-    left <== leftSelector1 + leftSelector2;
-    right <== rightSelector1 + rightSelector2;
+    out[0] <== (in[1] - in[0])*s + in[0];
+    out[1] <== (in[0] - in[1])*s + in[1];
 }
 
 // Verifies that merkle proof is correct for given merkle root and a leaf
-// pathIndex input is an array of 0/1 selectors telling whether given pathElement is on the left or right side of merkle path
-template MerkleTree(levels, rounds) {
+// pathIndices input is an array of 0/1 selectors telling whether given pathElement is on the left or right side of merkle path
+template MerkleTree(levels) {
     signal input leaf;
     signal input root;
     signal private input pathElements[levels];
-    signal private input pathIndex[levels];
+    signal private input pathIndices[levels];
 
     component selectors[levels];
     component hashers[levels];
 
     for (var i = 0; i < levels; i++) {
-        selectors[i] = Selector();
-        hashers[i] = HashLeftRight(rounds);
+        selectors[i] = Mux();
+        selectors[i].in[0] <== i == 0 ? leaf : hashers[i - 1].hash;
+        selectors[i].in[1] <== pathElements[i];
+        selectors[i].s <== pathIndices[i];
 
-        selectors[i].pathElement <== pathElements[i];
-        selectors[i].pathIndex <== pathIndex[i];
-
-        hashers[i].left <== selectors[i].left;
-        hashers[i].right <== selectors[i].right;
-    }
-
-    selectors[0].inputElement <== leaf;
-
-    for (var i = 1; i < levels; i++) {
-        selectors[i].inputElement <== hashers[i-1].hash;
+        hashers[i] = HashLeftRight();
+        hashers[i].left <== selectors[i].out[0];
+        hashers[i].right <== selectors[i].out[1];
     }
 
     root === hashers[levels - 1].hash;

circuits/withdraw.circom

@@ -6,7 +6,6 @@ include "merkleTree.circom";
 template CommitmentHasher() {
     signal private input nullifier;
     signal private input secret;
-
     signal output commitment;
     signal output nullifierHash;
 
@@ -27,7 +26,7 @@ template CommitmentHasher() {
 }
 
 // Verifies that commitment that corresponds to given secret and nullifier is included in the merkle tree of deposits
-template Withdraw(levels, rounds) {
+template Withdraw(levels) {
     signal input root;
     signal input nullifierHash;
     signal input receiver; // not taking part in any computations
@@ -37,33 +36,20 @@ template Withdraw(levels, rounds) {
     signal private input nullifier;
     signal private input secret;
     signal private input pathElements[levels];
-    signal private input pathIndex[levels];
+    signal private input pathIndices[levels];
 
     component hasher = CommitmentHasher();
     hasher.nullifier <== nullifier;
     hasher.secret <== secret;
+    hasher.nullifierHash === nullifierHash;
 
-    nullifierHash === hasher.nullifierHash;
-
-    component tree = MerkleTree(levels, rounds);
+    component tree = MerkleTree(levels);
     tree.leaf <== hasher.commitment;
     tree.root <== root;
     for (var i = 0; i < levels; i++) {
         tree.pathElements[i] <== pathElements[i];
-        tree.pathIndex[i] <== pathIndex[i];
+        tree.pathIndices[i] <== pathIndices[i];
+    }
 }
 
-    // Add hidden signals to make sure that tampering with receiver or fee will invalidate the snark proof
-    // Most likely it is not required, but it's better to stay on the safe side and it only takes 2 constraints
-    // Squares are used to prevent optimizer from removing those constraints
-    signal receiverSquare;
-    signal feeSquare;
-    signal relayerSquare;
-    signal refundSquare;
-    receiverSquare <== receiver * receiver;
-    feeSquare <== fee * fee;
-    relayerSquare <== relayer * relayer;
-    refundSquare <== refund * refund;
-}
-
-component main = Withdraw(16, 220);
+component main = Withdraw(16);

cli.js

@@ -107,7 +107,7 @@ async function withdrawErc20(note, receiver, relayer) {
     nullifier: deposit.nullifier,
     secret: deposit.secret,
     pathElements: path_elements,
-    pathIndex: path_index,
+    pathIndices: path_index,
   }
 
   console.log('Generating SNARK proof')
@@ -182,7 +182,7 @@ async function withdraw(note, receiver) {
     nullifier: deposit.nullifier,
     secret: deposit.secret,
     pathElements: path_elements,
-    pathIndex: path_index,
+    pathIndices: path_index,
   }
 
   console.log('Generating SNARK proof')

test/ERC20Mixer.test.js

@@ -142,7 +142,7 @@ contract('ERC20Mixer', accounts => {
         nullifier: deposit.nullifier,
         secret: deposit.secret,
         pathElements: path_elements,
-        pathIndex: path_index,
+        pathIndices: path_index,
       })
 
 
@@ -210,7 +210,7 @@ contract('ERC20Mixer', accounts => {
         nullifier: deposit.nullifier,
         secret: deposit.secret,
         pathElements: path_elements,
-        pathIndex: path_index,
+        pathIndices: path_index,
       })
 
 
@@ -269,7 +269,7 @@ contract('ERC20Mixer', accounts => {
         nullifier: deposit.nullifier,
         secret: deposit.secret,
         pathElements: path_elements,
-        pathIndex: path_index,
+        pathIndices: path_index,
       })
 
 
@@ -350,7 +350,7 @@ contract('ERC20Mixer', accounts => {
         nullifier: deposit.nullifier,
         secret: deposit.secret,
         pathElements: path_elements,
-        pathIndex: path_index,
+        pathIndices: path_index,
       })
 
 

test/ETHMixer.test.js

@@ -150,7 +150,7 @@ contract('ETHMixer', accounts => {
         refund,
         secret: deposit.secret,
         pathElements: path_elements,
-        pathIndex: path_index,
+        pathIndices: path_index,
       })
 
       let proofData = await websnarkUtils.genWitnessAndProve(groth16, input, circuit, proving_key)
@@ -210,7 +210,7 @@ contract('ETHMixer', accounts => {
         nullifier: deposit.nullifier,
         secret: deposit.secret,
         pathElements: path_elements,
-        pathIndex: path_index,
+        pathIndices: path_index,
       })
 
 
@@ -265,7 +265,7 @@ contract('ETHMixer', accounts => {
         refund,
         secret: deposit.secret,
         pathElements: path_elements,
-        pathIndex: path_index,
+        pathIndices: path_index,
       })
       const proofData = await websnarkUtils.genWitnessAndProve(groth16, input, circuit, proving_key)
       const { proof, publicSignals } = websnarkUtils.toSolidityInput(proofData)
@@ -291,7 +291,7 @@ contract('ETHMixer', accounts => {
         refund,
         secret: deposit.secret,
         pathElements: path_elements,
-        pathIndex: path_index,
+        pathIndices: path_index,
       })
       const proofData = await websnarkUtils.genWitnessAndProve(groth16, input, circuit, proving_key)
       const { proof, publicSignals } = websnarkUtils.toSolidityInput(proofData)
@@ -317,7 +317,7 @@ contract('ETHMixer', accounts => {
         refund,
         secret: deposit.secret,
         pathElements: path_elements,
-        pathIndex: path_index,
+        pathIndices: path_index,
       })
 
       const proofData = await websnarkUtils.genWitnessAndProve(groth16, input, circuit, proving_key)
@@ -343,7 +343,7 @@ contract('ETHMixer', accounts => {
         refund,
         secret: deposit.secret,
         pathElements: path_elements,
-        pathIndex: path_index,
+        pathIndices: path_index,
       })
 
       const dummyRoot = randomHex(32)
@@ -372,7 +372,7 @@ contract('ETHMixer', accounts => {
         refund,
         secret: deposit.secret,
         pathElements: path_elements,
-        pathIndex: path_index,
+        pathIndices: path_index,
       })
       const proofData = await websnarkUtils.genWitnessAndProve(groth16, input, circuit, proving_key)
       let { proof, publicSignals } = websnarkUtils.toSolidityInput(proofData)
@@ -424,7 +424,7 @@ contract('ETHMixer', accounts => {
         refund: bigInt(1),
         secret: deposit.secret,
         pathElements: path_elements,
-        pathIndex: path_index,
+        pathIndices: path_index,
       })
 
       const proofData = await websnarkUtils.genWitnessAndProve(groth16, input, circuit, proving_key)