From e46e104872ae8e078705bfbe7cb231179d6c5de1 Mon Sep 17 00:00:00 2001 From: poma Date: Wed, 16 Jun 2021 14:01:29 +0300 Subject: [PATCH] ensure no holes in merkle tree leaves --- circuits/transaction.circom | 6 +++--- contracts/TornadoPool.sol | 28 ++++++++++++++++------------ src/index.js | 6 ++++-- 3 files changed, 23 insertions(+), 17 deletions(-) diff --git a/circuits/transaction.circom b/circuits/transaction.circom index 40cf27d..29b9486 100644 --- a/circuits/transaction.circom +++ b/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]; diff --git a/contracts/TornadoPool.sol b/contracts/TornadoPool.sol index 39501f1..6e24c07 100644 --- a/contracts/TornadoPool.sol +++ b/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 { diff --git a/src/index.js b/src/index.js index ff215fb..26d8beb 100644 --- a/src/index.js +++ b/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,