add L1 msg.sender validation

contracts/Mocks/MockAMB.sol

@@ -26,5 +26,6 @@ contract MockAMB is IAMB {
 
   function execute(address _who, bytes calldata _calldata) external returns (bool success, bytes memory result) {
     (success, result) = _who.call(_calldata);
+    require(success, string(result));
   }
 }

contracts/Mocks/MockOmniBridge.sol

@@ -16,6 +16,7 @@ contract MockOmniBridge is IOmniBridge {
 
   function execute(address _who, bytes calldata _calldata) external returns (bool success, bytes memory result) {
     (success, result) = _who.call(_calldata);
+    require(success, string(result));
   }
 
   event OnTokenTransfer(address contr, address from, address receiver, uint256 value, bytes data);

contracts/TornadoPool.sol

@@ -14,6 +14,7 @@ pragma solidity ^0.7.0;
 pragma experimental ABIEncoderV2;
 
 import "@openzeppelin/contracts/contracts/token/ERC20/IERC20.sol";
+import "@openzeppelin/contracts/contracts/cryptography/ECDSA.sol";
 import "./MerkleTreeWithHistory.sol";
 
 interface IERC6777 is IERC20 {
@@ -41,6 +42,8 @@ interface IERC20Receiver {
 contract TornadoPool is MerkleTreeWithHistory, IERC20Receiver {
   int256 public constant MAX_EXT_AMOUNT = 2**248;
   uint256 public constant MAX_FEE = 2**248;
+  bytes32 public constant ACCOUNT_TYPEHASH = keccak256("TornadoAccount(address owner,bytes publicKey)");
+  uint256 public immutable L1_CHAIN_ID;
 
   IVerifier public immutable verifier2;
   IVerifier public immutable verifier16;
@@ -91,13 +94,15 @@ contract TornadoPool is MerkleTreeWithHistory, IERC20Receiver {
     address _hasher,
     IERC6777 _token,
     address _omniBridge,
-    address _l1Unwrapper
+    address _l1Unwrapper,
+    uint256 _l1ChainId
   ) MerkleTreeWithHistory(_levels, _hasher) {
     verifier2 = _verifier2;
     verifier16 = _verifier16;
     token = _token;
     omniBridge = _omniBridge;
     l1Unwrapper = _l1Unwrapper;
+    L1_CHAIN_ID = _l1ChainId;
   }
 
   function transact(Proof memory _args, ExtData memory _extData) public {
@@ -227,7 +232,11 @@ contract TornadoPool is MerkleTreeWithHistory, IERC20Receiver {
     uint256 _amount,
     bytes calldata _data
   ) external override {
-    (Account memory _account, Proof memory _args, ExtData memory _extData) = abi.decode(_data, (Account, Proof, ExtData));
+    (Account memory _account, Proof memory _args, ExtData memory _extData, bytes memory _signature) = abi.decode(
+      _data,
+      (Account, Proof, ExtData, bytes)
+    );
+    require(isValidSignature(_account, _signature), "Invalid account signature");
     require(_token == token, "provided token is not supported");
     require(msg.sender == omniBridge, "only omni bridge");
     require(_amount == uint256(_extData.extAmount), "amount from bridge is incorrect");
@@ -240,4 +249,25 @@ contract TornadoPool is MerkleTreeWithHistory, IERC20Receiver {
     }
     _transact(_args, _extData);
   }
+
+  function isValidSignature(Account memory _account, bytes memory _signature) public view returns (bool) {
+    bytes32 hashStruct = keccak256(abi.encode(ACCOUNT_TYPEHASH, _account.owner, keccak256(_account.publicKey)));
+    bytes32 hash = keccak256(abi.encodePacked(uint16(0x1901), domainSeparator(), hashStruct));
+
+    address signer = ECDSA.recover(hash, _signature);
+    return signer == _account.owner;
+  }
+
+  function domainSeparator() public view returns (bytes32) {
+    return
+      keccak256(
+        abi.encode(
+          keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"),
+          keccak256(bytes("TornadoPool")),
+          keccak256(bytes("1")), // Version
+          L1_CHAIN_ID,
+          address(this)
+        )
+      );
+  }
 }

package.json

@@ -22,6 +22,7 @@
   "author": "",
   "license": "ISC",
   "dependencies": {
+    "@metamask/eth-sig-util": "^4.0.0",
     "@nomiclabs/hardhat-ethers": "^2.0.2",
     "@nomiclabs/hardhat-waffle": "^2.0.1",
     "@openzeppelin/contracts": "git+https://github.com/tornadocash/openzeppelin-contracts.git#6e46aa6946a7f215e7604169ddf46e1aebea850f",
@@ -36,6 +37,7 @@
     "dotenv": "^10.0.0",
     "eth-sig-util": "^3.0.1",
     "ethereum-waffle": "^3.2.0",
+    "ethereumjs-util": "^7.1.2",
     "ethers": "^5.0.0",
     "ffiasm": "^0.1.3",
     "ffjavascript": "^0.2.36",

test/full.test.js

@@ -4,11 +4,17 @@ const { loadFixture } = waffle
 const { expect } = require('chai')
 const { utils } = ethers
 
+const { toBuffer } = require('ethereumjs-util')
+const { signTypedData, SignTypedDataVersion } = require('@metamask/eth-sig-util')
+
 const Utxo = require('../src/utxo')
 const { transaction, registerAndTransact, prepareTransaction } = require('../src/index')
 const { Keypair } = require('../src/keypair')
+const { EIP721Params, encodeDataForBridge } = require('./utils')
 
 const MERKLE_TREE_HEIGHT = 5
+const L1ChainId = 1
+const SENDER_PRIVATE_KEY = '0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80'
 
 describe('TornadoPool', function () {
   this.timeout(20000)
@@ -26,14 +32,14 @@ describe('TornadoPool', function () {
     const verifier16 = await deploy('Verifier16')
     const hasher = await deploy('Hasher')
 
-    const token = await deploy('PermittableToken', 'Wrapped ETH', 'WETH', 18, 1)
+    const token = await deploy('PermittableToken', 'Wrapped ETH', 'WETH', 18, L1ChainId)
     await token.mint(sender.address, utils.parseEther('10000'))
 
-    const amb = await deploy('MockAMB', gov.address, 1)
+    const amb = await deploy('MockAMB', gov.address, L1ChainId)
     const omniBridge = await deploy('MockOmniBridge', amb.address)
 
     /** @type {TornadoPool} */
-    const tornadoPool = await deploy(
+    const tornadoPoolImpl = await deploy(
       'TornadoPool',
       verifier2.address,
       verifier16.address,
@@ -42,43 +48,38 @@ describe('TornadoPool', function () {
       token.address,
       omniBridge.address,
       l1Unwrapper.address,
+      L1ChainId,
     )
-    await tornadoPool.initialize()
+    await tornadoPoolImpl.initialize() // not necessary
 
-    await token.approve(tornadoPool.address, utils.parseEther('10000'))
-    return { tornadoPool, token, omniBridge, amb }
-  }
-
-  async function fixtureUpgradeable() {
-    const { tornadoPool, omniBridge, amb } = await loadFixture(fixture)
-    const [, gov] = await ethers.getSigners()
     const proxy = await deploy(
       'CrossChainUpgradeableProxy',
-      tornadoPool.address,
+      tornadoPoolImpl.address,
       gov.address,
       [],
       amb.address,
-      1,
+      L1ChainId,
     )
 
-    /** @type {TornadoPool} */
     const TornadoPool = await ethers.getContractFactory('TornadoPool')
-    const tornadoPoolProxied = TornadoPool.attach(proxy.address)
+    const tornadoPool = TornadoPool.attach(proxy.address)
-    await tornadoPoolProxied.initialize()
+    await tornadoPool.initialize()
 
-    return { tornadoPool: tornadoPoolProxied, proxy, gov, omniBridge, amb }
+    await token.approve(tornadoPool.address, utils.parseEther('10000'))
+
+    return { tornadoPool, token, proxy, omniBridge, amb, gov }
   }
 
   describe('Upgradeability tests', () => {
     it('admin should be gov', async () => {
-      const { proxy, amb, gov } = await loadFixture(fixtureUpgradeable)
+      const { proxy, amb, gov } = await loadFixture(fixture)
       const { data } = await proxy.populateTransaction.admin()
       const { result } = await amb.callStatic.execute(proxy.address, data)
       expect('0x' + result.slice(26)).to.be.equal(gov.address.toLowerCase())
     })
 
     it('non admin cannot call', async () => {
-      const { proxy } = await loadFixture(fixtureUpgradeable)
+      const { proxy } = await loadFixture(fixture)
       await expect(proxy.admin()).to.be.revertedWith(
         "Transaction reverted: function selector was not recognized and there's no fallback function",
       )
@@ -202,24 +203,38 @@ describe('TornadoPool', function () {
 
   it('should deposit from L1 and withdraw to L1', async function () {
     const { tornadoPool, token, omniBridge } = await loadFixture(fixture)
-    // console.log('tornadoPool', tornadoPool.interface)
+    const owner = (await ethers.getSigners())[0].address
+    const aliceKeypair = new Keypair() // contains private and public keys
 
     // Alice deposits into tornado pool
     const aliceDepositAmount = 1e7
-    const aliceDepositUtxo = new Utxo({ amount: aliceDepositAmount })
+    const aliceDepositUtxo = new Utxo({ amount: aliceDepositAmount, keypair: aliceKeypair })
     const { args, extData } = await prepareTransaction({
       tornadoPool,
       outputs: [aliceDepositUtxo],
     })
-    const transactTx = await tornadoPool.populateTransaction.registerAndTransact(
+
-      {
+    const signature = signTypedData({
-        owner: '0x0000000000000000000000000000000000000000',
+      privateKey: toBuffer(SENDER_PRIVATE_KEY),
-        publicKey: [],
+      data: EIP721Params({
+        chainId: L1ChainId,
+        verifyingContract: tornadoPool.address,
+        owner,
+        publicKey: aliceKeypair.address(),
+      }),
+      version: SignTypedDataVersion.V4,
+    })
+
+    const onTokenBridgedData = encodeDataForBridge({
+      account: {
+        owner,
+        publicKey: aliceKeypair.address(),
       },
-      args,
+      proof: args,
       extData,
-    )
+      signature,
-    const onTokenBridgedData = '0x' + transactTx.data.slice(10)
+    })
+
     const onTokenBridgedTx = await tornadoPool.populateTransaction.onTokenBridged(
       token.address,
       aliceDepositUtxo.amount,
@@ -230,7 +245,6 @@ describe('TornadoPool', function () {
     await omniBridge.execute(tornadoPool.address, onTokenBridgedTx.data)
 
     // withdraws a part of his funds from the shielded pool
-    const aliceKeypair = new Keypair() // contains private and public keys
     const aliceWithdrawAmount = 2e6
     const recipient = '0xDeaD00000000000000000000000000000000BEEf'
     const aliceChangeUtxo = new Utxo({

test/utils.js

@@ -0,0 +1,45 @@
+const { ethers } = require('hardhat')
+
+const abi = new ethers.utils.AbiCoder()
+
+function encodeDataForBridge({ account, proof, extData, signature }) {
+  return abi.encode(
+    [
+      'tuple(address owner,bytes publicKey)',
+      'tuple(bytes proof,bytes32 root,bytes32[] inputNullifiers,bytes32[2] outputCommitments,uint256 publicAmount,bytes32 extDataHash)',
+      'tuple(address recipient,int256 extAmount,address relayer,uint256 fee,bytes encryptedOutput1,bytes encryptedOutput2,bool isL1Withdrawal)',
+      'bytes',
+    ],
+    [account, proof, extData, signature],
+  )
+}
+
+function EIP721Params({ chainId, verifyingContract, owner, publicKey }) {
+  return {
+    types: {
+      EIP712Domain: [
+        { name: 'name', type: 'string' },
+        { name: 'version', type: 'string' },
+        { name: 'chainId', type: 'uint256' },
+        { name: 'verifyingContract', type: 'address' },
+      ],
+      TornadoAccount: [
+        { name: 'owner', type: 'address' },
+        { name: 'publicKey', type: 'bytes' },
+      ],
+    },
+    primaryType: 'TornadoAccount',
+    domain: {
+      name: 'TornadoPool',
+      version: '1',
+      chainId,
+      verifyingContract,
+    },
+    message: {
+      owner,
+      publicKey,
+    },
+  }
+}
+
+module.exports = { encodeDataForBridge, EIP721Params }

yarn.lock

@@ -656,6 +656,17 @@
     fastfile "0.0.19"
     ffjavascript "^0.2.30"
 
+"@metamask/eth-sig-util@^4.0.0":
+  version "4.0.0"
+  resolved "https://registry.yarnpkg.com/@metamask/eth-sig-util/-/eth-sig-util-4.0.0.tgz#11553ba06de0d1352332c1bde28c8edd00e0dcf6"
+  integrity sha512-LczOjjxY4A7XYloxzyxJIHONELmUxVZncpOLoClpEcTiebiVdM46KRPYXGuULro9oNNR2xdVx3yoKiQjdfWmoA==
+  dependencies:
+    ethereumjs-abi "^0.6.8"
+    ethereumjs-util "^6.2.1"
+    ethjs-util "^0.1.6"
+    tweetnacl "^1.0.3"
+    tweetnacl-util "^0.15.1"
+
 "@nomiclabs/hardhat-ethers@^2.0.2":
   version "2.0.2"
   resolved "https://registry.yarnpkg.com/@nomiclabs/hardhat-ethers/-/hardhat-ethers-2.0.2.tgz#c472abcba0c5185aaa4ad4070146e95213c68511"
@@ -3723,7 +3734,7 @@ ethereumjs-tx@^1.1.1, ethereumjs-tx@^1.2.0, ethereumjs-tx@^1.2.2, ethereumjs-tx@
     ethereum-common "^0.0.18"
     ethereumjs-util "^5.0.0"
 
-ethereumjs-util@6.2.1, ethereumjs-util@^6.0.0, ethereumjs-util@^6.1.0, ethereumjs-util@^6.2.0:
+ethereumjs-util@6.2.1, ethereumjs-util@^6.0.0, ethereumjs-util@^6.1.0, ethereumjs-util@^6.2.0, ethereumjs-util@^6.2.1:
   version "6.2.1"
   resolved "https://registry.yarnpkg.com/ethereumjs-util/-/ethereumjs-util-6.2.1.tgz#fcb4e4dd5ceacb9d2305426ab1a5cd93e3163b69"
   integrity sha512-W2Ktez4L01Vexijrm5EB6w7dg4n/TgpoYU4avuT5T3Vmnw/eCRtiBrJfQYS/DCSvDIOLn2k57GcHdeBcgVxAqw==
@@ -3772,6 +3783,18 @@ ethereumjs-util@^7.0.10, ethereumjs-util@^7.0.2, ethereumjs-util@^7.0.7, ethereu
     ethjs-util "0.1.6"
     rlp "^2.2.4"
 
+ethereumjs-util@^7.1.2:
+  version "7.1.2"
+  resolved "https://registry.yarnpkg.com/ethereumjs-util/-/ethereumjs-util-7.1.2.tgz#cfd79a9a3f5cdc042d1abf29964de9caf10ec238"
+  integrity sha512-xCV3PTAhW8Q2k88XZn9VcO4OrjpeXAlDm5LQTaOLp81SjNSSY6+MwuGXrx6vafOMheWSmZGxIXUbue5e9UvUBw==
+  dependencies:
+    "@types/bn.js" "^5.1.0"
+    bn.js "^5.1.2"
+    create-hash "^1.1.2"
+    ethereum-cryptography "^0.1.3"
+    ethjs-util "0.1.6"
+    rlp "^2.2.4"
+
 ethereumjs-vm@4.2.0:
   version "4.2.0"
   resolved "https://registry.yarnpkg.com/ethereumjs-vm/-/ethereumjs-vm-4.2.0.tgz#e885e861424e373dbc556278f7259ff3fca5edab"
@@ -3869,7 +3892,7 @@ ethjs-unit@0.1.6:
     bn.js "4.11.6"
     number-to-bn "1.7.0"
 
-ethjs-util@0.1.6, ethjs-util@^0.1.3:
+ethjs-util@0.1.6, ethjs-util@^0.1.3, ethjs-util@^0.1.6:
   version "0.1.6"
   resolved "https://registry.yarnpkg.com/ethjs-util/-/ethjs-util-0.1.6.tgz#f308b62f185f9fe6237132fb2a9818866a5cd536"
   integrity sha512-CUnVOQq7gSpDHZVVrQW8ExxUETWrnrvXYvYz55wOU8Uj4VCgw56XC2B/fVqQN+f7gmrnRHSLVnFAwsCuNwji8w==
@@ -8642,7 +8665,7 @@ tunnel-agent@^0.6.0:
   dependencies:
     safe-buffer "^5.0.1"
 
-tweetnacl-util@^0.15.0:
+tweetnacl-util@^0.15.0, tweetnacl-util@^0.15.1:
   version "0.15.1"
   resolved "https://registry.yarnpkg.com/tweetnacl-util/-/tweetnacl-util-0.15.1.tgz#b80fcdb5c97bcc508be18c44a4be50f022eea00b"
   integrity sha512-RKJBIj8lySrShN4w6i/BonWp2Z/uxwC3h4y7xsRrpP59ZboCd0GpEVsOnMDYLMmKBpYhb5TgHzZXy7wTfYFBRw==