ensure no holes in merkle tree leaves

circuits/transaction.circom

@@ -18,8 +18,6 @@ nullifier = hash(commitment, privKey, merklePath)
 template Transaction(levels, nIns, nOuts, zeroLeaf) {
     signal input root;
     signal input newRoot;
-    signal input inputNullifier[nIns];
-    signal input outputCommitment[nOuts];
     // external amount used for deposits and withdrawals
     // correct extAmount range is enforced on the smart contract
     signal input extAmount;
@@ -27,6 +25,7 @@ template Transaction(levels, nIns, nOuts, zeroLeaf) {
     signal input extDataHash;
 
     // data for transaction inputs
+    signal         input inputNullifier[nIns];
     signal private input inAmount[nIns];
     signal private input inBlinding[nIns];
     signal private input inPrivateKey[nIns];
@@ -34,10 +33,11 @@ template Transaction(levels, nIns, nOuts, zeroLeaf) {
     signal private input inPathElements[nIns][levels];
 
     // data for transaction outputs
+    signal         input outputCommitment[nOuts];
     signal private input outAmount[nOuts];
     signal private input outBlinding[nOuts];
     signal private input outPubkey[nOuts];
-    signal private input outPathIndices;
+    signal         input outPathIndices;
     signal private input outPathElements[levels - 1];
 
     component inKeypair[nIns];

contracts/TornadoPool.sol

@@ -16,9 +16,9 @@ pragma experimental ABIEncoderV2;
 import "@openzeppelin/contracts/utils/ReentrancyGuard.sol"; // todo: maybe remove?
 
 interface IVerifier {
-  function verifyProof(bytes memory _proof, uint256[9] memory _input) external view returns (bool);
+  function verifyProof(bytes memory _proof, uint256[10] memory _input) external view returns (bool);
 
-  function verifyProof(bytes memory _proof, uint256[23] memory _input) external view returns (bool);
+  function verifyProof(bytes memory _proof, uint256[24] memory _input) external view returns (bool);
 }
 
 contract TornadoPool is ReentrancyGuard {
@@ -28,8 +28,8 @@ contract TornadoPool is ReentrancyGuard {
   mapping(bytes32 => bool) public nullifierHashes;
   bytes32 public currentRoot;
   uint256 public currentCommitmentIndex;
-  IVerifier public verifier2;
-  IVerifier public verifier16;
+  IVerifier public immutable verifier2;
+  IVerifier public immutable verifier16;
 
   struct ExtData {
     address payable recipient;
@@ -62,6 +62,7 @@ contract TornadoPool is ReentrancyGuard {
     bytes32 _newRoot,
     bytes32[] calldata _inputNullifiers,
     bytes32[2] calldata _outputCommitments,
+    uint256 _outPathIndices,
     uint256 _extAmount,
     uint256 _fee,
     ExtData calldata _extData,
@@ -72,8 +73,9 @@ contract TornadoPool is ReentrancyGuard {
       require(!isSpent(_inputNullifiers[i]), "Input is already spent");
     }
     require(uint256(_extDataHash) == uint256(keccak256(abi.encode(_extData))) % FIELD_SIZE, "Incorrect external data hash");
+    require(_outPathIndices == currentCommitmentIndex >> 1, "Invalid merkle tree insert position");
     require(
-      verifyProof(_proof, _root, _newRoot, _inputNullifiers, _outputCommitments, _extAmount, _fee, _extDataHash),
+      verifyProof(_proof, _root, _newRoot, _inputNullifiers, _outputCommitments, _outPathIndices, _extAmount, _fee, _extDataHash),
       "Invalid transaction proof"
     );
 
@@ -97,7 +99,6 @@ contract TornadoPool is ReentrancyGuard {
       _extData.relayer.transfer(_fee);
     }
 
-    // todo enforce currentCommitmentIndex value in snark
     emit NewCommitment(_outputCommitments[0], currentCommitmentIndex++, _extData.encryptedOutput1);
     emit NewCommitment(_outputCommitments[1], currentCommitmentIndex++, _extData.encryptedOutput2);
     for (uint256 i = 0; i < _inputNullifiers.length; i++) {
@@ -128,6 +129,7 @@ contract TornadoPool is ReentrancyGuard {
     bytes32 _newRoot,
     bytes32[] memory _inputNullifiers,
     bytes32[2] memory _outputCommitments,
+    uint256 _outPathIndices,
     uint256 _extAmount,
     uint256 _fee,
     bytes32 _extDataHash
@@ -139,13 +141,14 @@ contract TornadoPool is ReentrancyGuard {
           [
             uint256(_root),
             uint256(_newRoot),
+            _extAmount,
+            _fee,
+            uint256(_extDataHash),
             uint256(_inputNullifiers[0]),
             uint256(_inputNullifiers[1]),
             uint256(_outputCommitments[0]),
             uint256(_outputCommitments[1]),
-            _extAmount,
-            _fee,
-            uint256(_extDataHash)
+            _outPathIndices
           ]
         );
     } else if (_inputNullifiers.length == 16) {
@@ -155,6 +158,9 @@ contract TornadoPool is ReentrancyGuard {
           [
             uint256(_root),
             uint256(_newRoot),
+            _extAmount,
+            _fee,
+            uint256(_extDataHash),
             uint256(_inputNullifiers[0]),
             uint256(_inputNullifiers[1]),
             uint256(_inputNullifiers[2]),
@@ -173,9 +179,7 @@ contract TornadoPool is ReentrancyGuard {
             uint256(_inputNullifiers[15]),
             uint256(_outputCommitments[0]),
             uint256(_outputCommitments[1]),
-            _extAmount,
-            _fee,
-            uint256(_extDataHash)
+            _outPathIndices
           ]
         );
     } else {

src/index.js

@@ -46,6 +46,7 @@ async function getProof({ inputs, outputs, tree, extAmount, fee, recipient, rela
   }
   const outputIndex = tree.elements().length - 1
   const outputPath = tree.path(outputIndex).pathElements
+  const outputBatchBits = Math.log2(outputs.length)
 
   const extData = {
     recipient: toFixedHex(recipient, 20),
@@ -75,8 +76,8 @@ async function getProof({ inputs, outputs, tree, extAmount, fee, recipient, rela
     outAmount: outputs.map((x) => x.amount),
     outBlinding: outputs.map((x) => x.blinding),
     outPubkey: outputs.map((x) => x.keypair.pubkey),
-    outPathIndices: outputIndex >> Math.log2(outputs.length),
-    outPathElements: outputPath.slice(Math.log2(outputs.length)),
+    outPathIndices: outputIndex >> outputBatchBits,
+    outPathElements: outputPath.slice(outputBatchBits),
   }
 
   const proof = await prove(input, `./artifacts/circuits/transaction${inputs.length}`)
@@ -86,6 +87,7 @@ async function getProof({ inputs, outputs, tree, extAmount, fee, recipient, rela
     toFixedHex(input.newRoot),
     inputs.map((x) => toFixedHex(x.getNullifier())),
     outputs.map((x) => toFixedHex(x.getCommitment())),
+    toFixedHex(outputIndex >> outputBatchBits),
     toFixedHex(extAmount),
     toFixedHex(fee),
     extData,