Merge branch 'master' into optimism

README.md

@@ -7,3 +7,7 @@ yarn
 yarn build
 yarn test
 ```
+
+TODO
+
+1. deposit from mainnet to the pool on optimism in one tx

circuits/transaction.circom

@@ -18,10 +18,10 @@ nullifier = hash(commitment, privKey, merklePath)
 template Transaction(levels, nIns, nOuts, zeroLeaf) {
     signal input root;
     signal input newRoot;
-    // external amount used for deposits and withdrawals
+    // extAmount = external amount used for deposits and withdrawals
     // correct extAmount range is enforced on the smart contract
-    signal input extAmount;
+    // publicAmount = fee - extAmount
-    signal input fee;
+    signal input publicAmount;
     signal input extDataHash;
 
     // data for transaction inputs
@@ -103,10 +103,6 @@ template Transaction(levels, nIns, nOuts, zeroLeaf) {
         sumOuts += outAmount[tx];
     }
 
-    // Check that fee fits into 248 bits to prevent overflow
-    component feeCheck = Num2Bits(248);
-    feeCheck.in <== fee;
-
     // check that there are no same nullifiers among all inputs
     component sameNullifiers[nIns * (nIns - 1) / 2];
     var index = 0;
@@ -121,10 +117,10 @@ template Transaction(levels, nIns, nOuts, zeroLeaf) {
     }
 
     // verify amount invariant
-    sumIns + extAmount === sumOuts + fee;
+    sumIns + publicAmount === sumOuts;
 
     // Check merkle tree update with inserted transaction outputs
-    component treeUpdater = TreeUpdater(levels, zeroLeaf);
+    component treeUpdater = TreeUpdater(levels, 1 /* log2(nOuts) */, zeroLeaf);
     treeUpdater.oldRoot <== root;
     treeUpdater.newRoot <== newRoot;
     for (var i = 0; i < nOuts; i++) {

circuits/treeUpdater.circom

@@ -1,29 +1,36 @@
 include "./merkleTree.circom";
 
-// inserts a pair of leaves into a tree
+// inserts a subtree into a merkle tree
-// checks that tree previously contained zeroes is same positions
+// checks that tree previously contained zeroes is the same positions
-// zeroLeaf is a second level leaf: `hash(0, 0)`
+// zeroSubtreeRoot is a root of a subtree that contains only zeroes
-template TreeUpdater(n, zeroLeaf) {
+template TreeUpdater(levels, subtreeLevels, zeroSubtreeRoot) {
+    // currently it works only with 1-level subtrees
+    assert(subtreeLevels == 1);
+    var remainingLevels = levels - subtreeLevels;
+
     signal input oldRoot;
     signal input newRoot;
-    signal input leaf[2];
+    signal input leaf[1 << subtreeLevels];
     signal input pathIndices;
-    signal private input pathElements[n - 1];
+    signal private input pathElements[remainingLevels];
 
+    // calculate subtree root
+    // todo: make it work with arbitrary subtree levels
+    // currently it works only with 1-level subtrees
     component leafPair = HashLeftRight();
     leafPair.left <== leaf[0];
     leafPair.right <== leaf[1];
 
-    component treeBefore = MerkleTree(n - 1);
+    component treeBefore = MerkleTree(remainingLevels);
-    for(var i = 0; i < n - 1; i++) {
+    for(var i = 0; i < remainingLevels; i++) {
         treeBefore.pathElements[i] <== pathElements[i];
     }
     treeBefore.pathIndices <== pathIndices;
-    treeBefore.leaf <== zeroLeaf;
+    treeBefore.leaf <== zeroSubtreeRoot;
     treeBefore.root === oldRoot;
 
-    component treeAfter = MerkleTree(n - 1);
+    component treeAfter = MerkleTree(remainingLevels);
-    for(var i = 0; i < n - 1; i++) {
+    for(var i = 0; i < remainingLevels; i++) {
         treeAfter.pathElements[i] <== pathElements[i];
     }
     treeAfter.pathIndices <== pathIndices;

contracts/TornadoPool.sol

@@ -15,9 +15,9 @@ pragma experimental ABIEncoderV2;
 import "@openzeppelin/contracts-upgradeable/proxy/Initializable.sol";
 
 interface IVerifier {
-  function verifyProof(bytes memory _proof, uint256[10] memory _input) external view returns (bool);
+  function verifyProof(bytes memory _proof, uint256[9] memory _input) external view returns (bool);
 
-  function verifyProof(bytes memory _proof, uint256[24] memory _input) external view returns (bool);
+  function verifyProof(bytes memory _proof, uint256[23] memory _input) external view returns (bool);
 }
 
 interface ERC20 {
@@ -26,7 +26,8 @@ interface ERC20 {
 
 contract TornadoPool is Initializable {
   uint256 public constant FIELD_SIZE = 21888242871839275222246405745257275088548364400416034343698204186575808495617;
-  uint256 public constant MAX_EXT_AMOUNT = 2**248 - 1;
+  int256 public constant MAX_EXT_AMOUNT = 2**248;
+  uint256 public constant MAX_FEE = 2**248;
 
   mapping(bytes32 => bool) public nullifierHashes;
   bytes32 public currentRoot;
@@ -36,7 +37,9 @@ contract TornadoPool is Initializable {
 
   struct ExtData {
     address payable recipient;
+    int256 extAmount;
     address payable relayer;
+    uint256 fee;
     bytes encryptedOutput1;
     bytes encryptedOutput2;
   }
@@ -48,8 +51,7 @@ contract TornadoPool is Initializable {
     bytes32[] inputNullifiers;
     bytes32[2] outputCommitments;
     uint256 outPathIndices;
-    uint256 extAmount;
+    uint256 publicAmount;
-    uint256 fee;
     bytes32 extDataHash;
   }
 
@@ -83,33 +85,35 @@ contract TornadoPool is Initializable {
       require(!isSpent(_args.inputNullifiers[i]), "Input is already spent");
     }
     require(uint256(_args.extDataHash) == uint256(keccak256(abi.encode(_extData))) % FIELD_SIZE, "Incorrect external data hash");
-    require(_args.outPathIndices == currentCommitmentIndex >> 1, "Invalid merkle tree insert position");
+    uint256 cachedCommitmentIndex = currentCommitmentIndex;
+    require(_args.outPathIndices == cachedCommitmentIndex >> 1, "Invalid merkle tree insert position");
+    require(_args.publicAmount == calculatePublicAmount(_extData.extAmount, _extData.fee), "Invalid public amount");
     require(verifyProof(_args), "Invalid transaction proof");
 
     currentRoot = _args.newRoot;
+    currentCommitmentIndex = cachedCommitmentIndex + 2;
     for (uint256 i = 0; i < _args.inputNullifiers.length; i++) {
       nullifierHashes[_args.inputNullifiers[i]] = true;
     }
 
-    int256 extAmount = calculateExternalAmount(_args.extAmount);
+    if (_extData.extAmount > 0) {
-    if (extAmount > 0) {
+      require(msg.value == uint256(_extData.extAmount), "Incorrect amount of ETH sent on deposit");
-      require(msg.value == uint256(_args.extAmount), "Incorrect amount of ETH sent on deposit");
+    } else if (_extData.extAmount < 0) {
-    } else if (extAmount < 0) {
       require(msg.value == 0, "Sent ETH amount should be 0 for withdrawal");
       require(_extData.recipient != address(0), "Can't withdraw to zero address");
       // _extData.recipient.transfer(uint256(-extAmount));
-      _transfer(_extData.recipient, uint256(-extAmount));
+      _transfer(_extData.recipient, uint256(-_extData.extAmount));
     } else {
       require(msg.value == 0, "Sent ETH amount should be 0 for transaction");
     }
 
-    if (_args.fee > 0) {
+    if (_extData.fee > 0) {
       // _extData.relayer.transfer(_args.fee);
-      _transfer(_extData.relayer, _args.fee);
+      _transfer(_extData.relayer, _extData.fee);
     }
 
-    emit NewCommitment(_args.outputCommitments[0], currentCommitmentIndex++, _extData.encryptedOutput1);
+    emit NewCommitment(_args.outputCommitments[0], cachedCommitmentIndex, _extData.encryptedOutput1);
-    emit NewCommitment(_args.outputCommitments[1], currentCommitmentIndex++, _extData.encryptedOutput2);
+    emit NewCommitment(_args.outputCommitments[1], cachedCommitmentIndex + 1, _extData.encryptedOutput2);
     for (uint256 i = 0; i < _args.inputNullifiers.length; i++) {
       emit NewNullifier(_args.inputNullifiers[i]);
     }
@@ -127,16 +131,11 @@ contract TornadoPool is Initializable {
     }
   }
 
-  function calculateExternalAmount(uint256 _extAmount) public pure returns (int256) {
+  function calculatePublicAmount(int256 _extAmount, uint256 _fee) public pure returns(uint256) {
-    // -MAX_EXT_AMOUNT < extAmount < MAX_EXT_AMOUNT
+    require(_fee < MAX_FEE, "Invalid fee");
-    if (_extAmount < MAX_EXT_AMOUNT) {
+    require(_extAmount > -MAX_EXT_AMOUNT && _extAmount < MAX_EXT_AMOUNT, "Invalid ext amount");
-      return int256(_extAmount);
+    int256 publicAmount = _extAmount - int256(_fee);
-    } else if (_extAmount > FIELD_SIZE - MAX_EXT_AMOUNT) {
+    return (publicAmount >= 0) ? uint256(publicAmount) : FIELD_SIZE - uint256(-publicAmount);
-      // FIELD_SIZE - MAX_EXT_AMOUNT < _extAmount < FIELD_SIZE
-      return -(int256(FIELD_SIZE) - int256(_extAmount));
-    } else {
-      revert("Invalid extAmount value");
-    }
   }
 
   /** @dev whether a note is already spent */
@@ -152,8 +151,7 @@ contract TornadoPool is Initializable {
           [
             uint256(_args.root),
             uint256(_args.newRoot),
-            _args.extAmount,
+            _args.publicAmount,
-            _args.fee,
             uint256(_args.extDataHash),
             uint256(_args.inputNullifiers[0]),
             uint256(_args.inputNullifiers[1]),
@@ -169,8 +167,7 @@ contract TornadoPool is Initializable {
           [
             uint256(_args.root),
             uint256(_args.newRoot),
-            _args.extAmount,
+            _args.publicAmount,
-            _args.fee,
             uint256(_args.extDataHash),
             uint256(_args.inputNullifiers[0]),
             uint256(_args.inputNullifiers[1]),

src/index.js

@@ -48,7 +48,9 @@ async function getProof({ inputs, outputs, tree, extAmount, fee, recipient, rela
 
   const extData = {
     recipient: toFixedHex(recipient, 20),
+    extAmount: toFixedHex(extAmount),
     relayer: toFixedHex(relayer, 20),
+    fee: toFixedHex(fee),
     encryptedOutput1: outputs[0].encrypt(),
     encryptedOutput2: outputs[1].encrypt(),
   }
@@ -59,8 +61,7 @@ async function getProof({ inputs, outputs, tree, extAmount, fee, recipient, rela
     newRoot: tree.root(),
     inputNullifier: inputs.map((x) => x.getNullifier()),
     outputCommitment: outputs.map((x) => x.getCommitment()),
-    extAmount,
+    publicAmount: BigNumber.from(extAmount).sub(fee).add(FIELD_SIZE).mod(FIELD_SIZE).toString(),
-    fee,
     extDataHash,
 
     // data for 2 transaction inputs
@@ -87,8 +88,7 @@ async function getProof({ inputs, outputs, tree, extAmount, fee, recipient, rela
     inputNullifiers: inputs.map((x) => toFixedHex(x.getNullifier())),
     outputCommitments: outputs.map((x) => toFixedHex(x.getCommitment())),
     outPathIndices: toFixedHex(outputIndex >> outputBatchBits),
-    extAmount: toFixedHex(extAmount),
+    publicAmount: toFixedHex(input.publicAmount),
-    fee: toFixedHex(fee),
     extDataHash: toFixedHex(extDataHash),
   }
   // console.log('Solidity args', args)
@@ -121,10 +121,7 @@ async function prepareTransaction({
     .add(outputs.reduce((sum, x) => sum.add(x.amount), BigNumber.from(0)))
     .sub(inputs.reduce((sum, x) => sum.add(x.amount), BigNumber.from(0)))
 
-  const amount = extAmount > 0 ? extAmount : 0 // extAmount will be positive for a deposit, zero for a transact and negative for withdraw
+  const amount = extAmount > 0 ? extAmount : 0
-  if (extAmount < 0) {
-    extAmount = FIELD_SIZE.add(extAmount)
-  }
 
   const { args, extData } = await getProof({
     inputs,

src/utils.js

@@ -14,15 +14,19 @@ const FIELD_SIZE = BigNumber.from(
 /** Generate random number of specified byte length */
 const randomBN = (nbytes = 31) => BigNumber.from(crypto.randomBytes(nbytes))
 
-function getExtDataHash({ recipient, relayer, encryptedOutput1, encryptedOutput2 }) {
+function getExtDataHash({ recipient, extAmount, relayer, fee, encryptedOutput1, encryptedOutput2 }) {
   const abi = new ethers.utils.AbiCoder()
 
   const encodedData = abi.encode(
-    ['tuple(address recipient,address relayer,bytes encryptedOutput1,bytes encryptedOutput2)'],
+    [
+      'tuple(address recipient,int256 extAmount,address relayer,uint256 fee,bytes encryptedOutput1,bytes encryptedOutput2)',
+    ],
     [
       {
         recipient: toFixedHex(recipient, 20),
+        extAmount: toFixedHex(extAmount),
         relayer: toFixedHex(relayer, 20),
+        fee: toFixedHex(fee),
         encryptedOutput1: encryptedOutput1,
         encryptedOutput2: encryptedOutput2,
       },
@@ -33,12 +37,18 @@ function getExtDataHash({ recipient, relayer, encryptedOutput1, encryptedOutput2
 }
 
 /** BigNumber to hex string of specified length */
-const toFixedHex = (number, length = 32) =>
+function toFixedHex(number, length = 32) {
+  let result =
     '0x' +
     (number instanceof Buffer
       ? number.toString('hex')
-    : BigNumber.from(number).toHexString().slice(2)
+      : BigNumber.from(number).toHexString().replace('0x', '')
     ).padStart(length * 2, '0')
+  if (result.indexOf('-') > -1) {
+    result = '-' + result.replace('-', '')
+  }
+  return result
+}
 
 /** Convert value into buffer of specified byte length */
 const toBuffer = (value, length) =>

test/full.test.js

@@ -71,6 +71,14 @@ describe('TornadoPool', () => {
     expect(result).to.be.deep.equal(data)
   })
 
+  it('constants check', async () => {
+    const maxFee = await tornadoPool.MAX_FEE()
+    const maxExtAmount = await tornadoPool.MAX_EXT_AMOUNT()
+    const fieldSize = await tornadoPool.FIELD_SIZE()
+
+    expect(maxExtAmount.add(maxFee)).to.be.lt(fieldSize)
+  })
+
   it('should register and deposit', async function () {
     // Alice deposits into tornado pool
     const aliceDepositAmount = 1e7