From 1c106caa9a3ab19426af3dcbf1d78228e7d41550 Mon Sep 17 00:00:00 2001 From: Agustin Aguilar Date: Sun, 15 Sep 2019 20:37:32 +0200 Subject: [PATCH 1/2] Add frontrun protection using witness and secret --- contracts/UniswapEX.sol | 106 ++++++-- contracts/commons/SigUtils.sol | 35 +++ package-lock.json | 287 ++++++++++++++++++++ package.json | 3 + test/UniswapEX.spec.js | 473 ++++++++++++++++++--------------- test/helpers/assertRevert.js | 12 +- test/helpers/balanceSnap.js | 119 +++++---- 7 files changed, 730 insertions(+), 305 deletions(-) create mode 100644 contracts/commons/SigUtils.sol diff --git a/contracts/UniswapEX.sol b/contracts/UniswapEX.sol index 67dd587..c1f7a3a 100644 --- a/contracts/UniswapEX.sol +++ b/contracts/UniswapEX.sol @@ -2,6 +2,7 @@ pragma solidity ^0.5.11; import "./commons/SafeMath.sol"; +import "./commons/SigUtils.sol"; import "./interfaces/IERC20.sol"; import "./interfaces/UniswapExchange.sol"; import "./interfaces/UniswapFactory.sol"; @@ -26,7 +27,7 @@ contract UniswapEX { uint256 _minReturn, uint256 _fee, address _owner, - bytes32 _salt, + address _witness, address _relayer, uint256 _amount, uint256 _bought @@ -39,7 +40,7 @@ contract UniswapEX { uint256 _minReturn, uint256 _fee, address _owner, - bytes32 _salt, + address _witness, uint256 _amount ); @@ -61,7 +62,27 @@ contract UniswapEX { ) external payable { require(msg.value > 0, "No value provided"); - bytes32 key = keccak256(_data); + ( + address fromToken, + address toToken, + uint256 minReturn, + uint256 fee, + address payable owner, + , + address witness + ) = decodeOrder(_data); + + require(fromToken == ETH_ADDRESS, "order is not from ETH"); + + bytes32 key = _keyOf( + IERC20(ETH_ADDRESS), + IERC20(toToken), + minReturn, + fee, + owner, + witness + ); + ethDeposits[key] = ethDeposits[key].add(msg.value); emit DepositETH(key, msg.sender, msg.value, _data); } @@ -72,7 +93,7 @@ contract UniswapEX { uint256 _minReturn, uint256 _fee, address payable _owner, - bytes32 _salt + address _witness ) external { require(msg.sender == _owner, "Only the owner of the order can cancel it"); bytes32 key = _keyOf( @@ -81,7 +102,7 @@ contract UniswapEX { _minReturn, _fee, _owner, - _salt + _witness ); uint256 amount; @@ -100,26 +121,43 @@ contract UniswapEX { _minReturn, _fee, _owner, - _salt, + _witness, amount ); } + function readWitnesses( + bytes calldata _witnesses + ) external view returns (address) { + return SigUtils.ecrecover2( + keccak256(abi.encodePacked(msg.sender)), + _witnesses + ); + } + function executeOrder( IERC20 _fromToken, IERC20 _toToken, uint256 _minReturn, uint256 _fee, address payable _owner, - bytes32 _salt + bytes calldata _witnesses ) external { + // Calculate witness using signature + // avoid front-run by requiring msg.sender to know + // the secret + address witness = SigUtils.ecrecover2( + keccak256(abi.encodePacked(msg.sender)), + _witnesses + ); + bytes32 key = _keyOf( _fromToken, _toToken, _minReturn, _fee, _owner, - _salt + witness ); // Pull amount @@ -159,7 +197,7 @@ contract UniswapEX { _minReturn, _fee, _owner, - _salt, + witness, msg.sender, amount, bought @@ -173,7 +211,8 @@ contract UniswapEX { uint256 _minReturn, uint256 _fee, address payable _owner, - bytes32 _salt + bytes32 _secret, + address _witness ) external view returns (bytes memory) { return abi.encodeWithSelector( _fromToken.transfer.selector, @@ -183,7 +222,7 @@ contract UniswapEX { _minReturn, _fee, _owner, - _salt + _witness ), _amount, abi.encode( @@ -192,7 +231,8 @@ contract UniswapEX { _minReturn, _fee, _owner, - _salt + _secret, + _witness ) ); } @@ -203,7 +243,8 @@ contract UniswapEX { uint256 _minReturn, uint256 _fee, address payable _owner, - bytes32 _salt + bytes32 _secret, + address _witness ) external pure returns (bytes memory) { return abi.encode( _fromToken, @@ -211,19 +252,21 @@ contract UniswapEX { _minReturn, _fee, _owner, - _salt + _secret, + _witness ); } function decodeOrder( - bytes calldata _data - ) external pure returns ( + bytes memory _data + ) public pure returns ( address fromToken, address toToken, uint256 minReturn, uint256 fee, address payable owner, - bytes32 salt + bytes32 secret, + address witness ) { ( fromToken, @@ -231,10 +274,19 @@ contract UniswapEX { minReturn, fee, owner, - salt + secret, + witness ) = abi.decode( _data, - (address, address, uint256, uint256, address, bytes32) + ( + address, + address, + uint256, + uint256, + address, + bytes32, + address + ) ); } @@ -244,7 +296,7 @@ contract UniswapEX { uint256 _minReturn, uint256 _fee, address payable _owner, - bytes32 _salt + address _witness ) external view returns (bool) { bytes32 key = _keyOf( _fromToken, @@ -252,7 +304,7 @@ contract UniswapEX { _minReturn, _fee, _owner, - _salt + _witness ); if (address(_fromToken) == ETH_ADDRESS) { @@ -268,7 +320,7 @@ contract UniswapEX { uint256 _minReturn, uint256 _fee, address payable _owner, - bytes32 _salt + address _witness ) external view returns (bool) { bytes32 key = _keyOf( _fromToken, @@ -276,7 +328,7 @@ contract UniswapEX { _minReturn, _fee, _owner, - _salt + _witness ); // Pull amount @@ -321,7 +373,7 @@ contract UniswapEX { uint256 _minReturn, uint256 _fee, address payable _owner, - bytes32 _salt + address _witness ) public view returns (address) { return _keyOf( _fromToken, @@ -329,7 +381,7 @@ contract UniswapEX { _minReturn, _fee, _owner, - _salt + _witness ).getVault(); } @@ -393,7 +445,7 @@ contract UniswapEX { uint256 _minReturn, uint256 _fee, address payable _owner, - bytes32 _salt + address _witness ) private pure returns (bytes32) { return keccak256( abi.encode( @@ -402,7 +454,7 @@ contract UniswapEX { _minReturn, _fee, _owner, - _salt + _witness ) ); } diff --git a/contracts/commons/SigUtils.sol b/contracts/commons/SigUtils.sol new file mode 100644 index 0000000..185afda --- /dev/null +++ b/contracts/commons/SigUtils.sol @@ -0,0 +1,35 @@ +pragma solidity ^0.5.5; + + +library SigUtils { + /** + @dev Recovers address who signed the message + @param _hash operation ethereum signed message hash + @param _signature message `hash` signature + */ + function ecrecover2 ( + bytes32 _hash, + bytes memory _signature + ) internal pure returns (address) { + bytes32 r; + bytes32 s; + uint8 v; + + assembly { + r := mload(add(_signature, 32)) + s := mload(add(_signature, 64)) + v := and(mload(add(_signature, 65)), 255) + } + + if (v < 27) { + v += 27; + } + + return ecrecover( + _hash, + v, + r, + s + ); + } +} diff --git a/package-lock.json b/package-lock.json index 78a08f6..e5c8cb5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -909,6 +909,24 @@ "tweetnacl": "^0.14.3" } }, + "bindings": { + "version": "1.5.0", + "resolved": "https://registry.npmjs.org/bindings/-/bindings-1.5.0.tgz", + "integrity": "sha512-p2q/t/mhvuOj/UeLlV6566GD/guowlr0hHxClI0W9m7MWYkL1F0hLo+0Aexs9HSPCtR1SXQ0TD3MMKrXZajbiQ==", + "dev": true, + "requires": { + "file-uri-to-path": "1.0.0" + } + }, + "bip66": { + "version": "1.1.5", + "resolved": "https://registry.npmjs.org/bip66/-/bip66-1.1.5.tgz", + "integrity": "sha1-AfqHSHhcpwlV1QESF9GzE5lpyiI=", + "dev": true, + "requires": { + "safe-buffer": "^5.0.1" + } + }, "bluebird": { "version": "3.5.5", "resolved": "https://registry.npmjs.org/bluebird/-/bluebird-3.5.5.tgz", @@ -919,6 +937,12 @@ "resolved": "https://registry.npmjs.org/bn-chai/-/bn-chai-1.0.1.tgz", "integrity": "sha512-7rJXt21DwYiLLpvzLaACixBBoUGkRV1iuFD3wElEhw8Ji9IiY/QsJRtvW+c7ChRgEOyLQkGaSGFUUqBKm21SNA==" }, + "bn.js": { + "version": "4.11.8", + "resolved": "https://registry.npmjs.org/bn.js/-/bn.js-4.11.8.tgz", + "integrity": "sha512-ItfYfPLkWHUjckQCk8xC+LwxgK8NYcXywGigJgSwOP8Y2iyWT4f2vsZnoOXTTbo+o5yXmIUJ4gn5538SO5S3gA==", + "dev": true + }, "boxen": { "version": "3.2.0", "resolved": "https://registry.npmjs.org/boxen/-/boxen-3.2.0.tgz", @@ -968,11 +992,31 @@ "concat-map": "0.0.1" } }, + "brorand": { + "version": "1.1.0", + "resolved": "https://registry.npmjs.org/brorand/-/brorand-1.1.0.tgz", + "integrity": "sha1-EsJe/kCkXjwyPrhnWgoM5XsiNx8=", + "dev": true + }, "browser-stdout": { "version": "1.3.1", "resolved": "https://registry.npmjs.org/browser-stdout/-/browser-stdout-1.3.1.tgz", "integrity": "sha512-qhAVI1+Av2X7qelOfAIYwXONood6XlZE/fXaBSmW/T5SzLAmCgzi+eiWE7fUvbHaeNBQH13UftjpXxsfLkMpgw==" }, + "browserify-aes": { + "version": "1.2.0", + "resolved": "https://registry.npmjs.org/browserify-aes/-/browserify-aes-1.2.0.tgz", + "integrity": "sha512-+7CHXqGuspUn/Sl5aO7Ea0xWGAtETPXNSAjHo48JfLdPWcMng33Xe4znFvQweqc/uzk5zSOI3H52CYnjCfb5hA==", + "dev": true, + "requires": { + "buffer-xor": "^1.0.3", + "cipher-base": "^1.0.0", + "create-hash": "^1.1.0", + "evp_bytestokey": "^1.0.3", + "inherits": "^2.0.1", + "safe-buffer": "^5.0.1" + } + }, "browserslist": { "version": "3.2.8", "resolved": "https://registry.npmjs.org/browserslist/-/browserslist-3.2.8.tgz", @@ -987,6 +1031,12 @@ "resolved": "https://registry.npmjs.org/buffer-from/-/buffer-from-1.1.1.tgz", "integrity": "sha512-MQcXEUbCKtEo7bhqEs6560Hyd4XaovZlO/k9V3hjVUF/zwW7KBVdSK4gIt/bzwS9MbR5qob+F5jusZsb0YQK2A==" }, + "buffer-xor": { + "version": "1.0.3", + "resolved": "https://registry.npmjs.org/buffer-xor/-/buffer-xor-1.0.3.tgz", + "integrity": "sha1-JuYe0UIvtw3ULm42cp7VHYVf6Nk=", + "dev": true + }, "builtins": { "version": "1.0.3", "resolved": "https://registry.npmjs.org/builtins/-/builtins-1.0.3.tgz", @@ -1144,6 +1194,16 @@ "resolved": "https://registry.npmjs.org/cint/-/cint-8.2.1.tgz", "integrity": "sha1-cDhrG0jidz0NYxZqVa/5TvRFahI=" }, + "cipher-base": { + "version": "1.0.4", + "resolved": "https://registry.npmjs.org/cipher-base/-/cipher-base-1.0.4.tgz", + "integrity": "sha512-Kkht5ye6ZGmwv40uUDZztayT2ThLQGfnj/T71N/XzeZeo3nf8foyW7zGTsPYkEya3m5f3cAypH+qe7YOrM1U2Q==", + "dev": true, + "requires": { + "inherits": "^2.0.1", + "safe-buffer": "^5.0.1" + } + }, "cli-boxes": { "version": "2.2.0", "resolved": "https://registry.npmjs.org/cli-boxes/-/cli-boxes-2.2.0.tgz", @@ -1278,6 +1338,33 @@ "resolved": "https://registry.npmjs.org/core-util-is/-/core-util-is-1.0.2.tgz", "integrity": "sha1-tf1UIgqivFq1eqtxQMlAdUUDwac=" }, + "create-hash": { + "version": "1.2.0", + "resolved": "https://registry.npmjs.org/create-hash/-/create-hash-1.2.0.tgz", + "integrity": "sha512-z00bCGNHDG8mHAkP7CtT1qVu+bFQUPjYq/4Iv3C3kWjTFV10zIjfSoeqXo9Asws8gwSHDGj/hl2u4OGIjapeCg==", + "dev": true, + "requires": { + "cipher-base": "^1.0.1", + "inherits": "^2.0.1", + "md5.js": "^1.3.4", + "ripemd160": "^2.0.1", + "sha.js": "^2.4.0" + } + }, + "create-hmac": { + "version": "1.1.7", + "resolved": "https://registry.npmjs.org/create-hmac/-/create-hmac-1.1.7.tgz", + "integrity": "sha512-MJG9liiZ+ogc4TzUwuvbER1JRdgvUFSB5+VR/g5h82fGaIRWMWddtKBHi7/sVhfjQZ6SehlyhvQYrcYkaUIpLg==", + "dev": true, + "requires": { + "cipher-base": "^1.0.3", + "create-hash": "^1.1.0", + "inherits": "^2.0.1", + "ripemd160": "^2.0.0", + "safe-buffer": "^5.0.1", + "sha.js": "^2.4.8" + } + }, "cross-spawn": { "version": "6.0.5", "resolved": "https://registry.npmjs.org/cross-spawn/-/cross-spawn-6.0.5.tgz", @@ -1373,6 +1460,17 @@ "is-obj": "^1.0.0" } }, + "drbg.js": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/drbg.js/-/drbg.js-1.0.1.tgz", + "integrity": "sha1-Pja2xCs3BDgjzbwzLVjzHiRFSAs=", + "dev": true, + "requires": { + "browserify-aes": "^1.0.6", + "create-hash": "^1.1.2", + "create-hmac": "^1.1.4" + } + }, "duplexer3": { "version": "0.1.4", "resolved": "https://registry.npmjs.org/duplexer3/-/duplexer3-0.1.4.tgz", @@ -1403,6 +1501,21 @@ "resolved": "https://registry.npmjs.org/electron-to-chromium/-/electron-to-chromium-1.3.238.tgz", "integrity": "sha512-k+s6EIiSTgfOm7WiMlGBMMMtBQXSui8OfLN1sXU3RohJOuLGVq0lVm7hXyDIDBcbPM0MeZabAdIPQOSEZT3ZLg==" }, + "elliptic": { + "version": "6.5.1", + "resolved": "https://registry.npmjs.org/elliptic/-/elliptic-6.5.1.tgz", + "integrity": "sha512-xvJINNLbTeWQjrl6X+7eQCrIy/YPv5XCpKW6kB5mKvtnGILoLDcySuwomfdzt0BMdLNVnuRNTuzKNHj0bva1Cg==", + "dev": true, + "requires": { + "bn.js": "^4.4.0", + "brorand": "^1.0.1", + "hash.js": "^1.0.0", + "hmac-drbg": "^1.0.0", + "inherits": "^2.0.1", + "minimalistic-assert": "^1.0.0", + "minimalistic-crypto-utils": "^1.0.0" + } + }, "emoji-regex": { "version": "7.0.3", "resolved": "https://registry.npmjs.org/emoji-regex/-/emoji-regex-7.0.3.tgz", @@ -1480,6 +1593,41 @@ "resolved": "https://registry.npmjs.org/esutils/-/esutils-2.0.3.tgz", "integrity": "sha512-kVscqXk4OCp68SZ0dkgEKVi6/8ij300KBWTJq32P/dYeWTSwK41WyTxalN1eRmA5Z9UU/LX9D7FWSmV9SAYx6g==" }, + "ethereumjs-util": { + "version": "6.1.0", + "resolved": "https://registry.npmjs.org/ethereumjs-util/-/ethereumjs-util-6.1.0.tgz", + "integrity": "sha512-URESKMFbDeJxnAxPppnk2fN6Y3BIatn9fwn76Lm8bQlt+s52TpG8dN9M66MLPuRAiAOIqL3dfwqWJf0sd0fL0Q==", + "dev": true, + "requires": { + "bn.js": "^4.11.0", + "create-hash": "^1.1.2", + "ethjs-util": "0.1.6", + "keccak": "^1.0.2", + "rlp": "^2.0.0", + "safe-buffer": "^5.1.1", + "secp256k1": "^3.0.1" + } + }, + "ethjs-util": { + "version": "0.1.6", + "resolved": "https://registry.npmjs.org/ethjs-util/-/ethjs-util-0.1.6.tgz", + "integrity": "sha512-CUnVOQq7gSpDHZVVrQW8ExxUETWrnrvXYvYz55wOU8Uj4VCgw56XC2B/fVqQN+f7gmrnRHSLVnFAwsCuNwji8w==", + "dev": true, + "requires": { + "is-hex-prefixed": "1.0.0", + "strip-hex-prefix": "1.0.0" + } + }, + "evp_bytestokey": { + "version": "1.0.3", + "resolved": "https://registry.npmjs.org/evp_bytestokey/-/evp_bytestokey-1.0.3.tgz", + "integrity": "sha512-/f2Go4TognH/KvCISP7OUsHn85hT9nUkxxA9BEWxFn+Oj9o8ZNLm/40hdlgSLyuOimsrTKLUMEorQexp/aPQeA==", + "dev": true, + "requires": { + "md5.js": "^1.3.4", + "safe-buffer": "^5.1.1" + } + }, "execa": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/execa/-/execa-1.0.0.tgz", @@ -1524,6 +1672,12 @@ "resolved": "https://registry.npmjs.org/figgy-pudding/-/figgy-pudding-3.5.1.tgz", "integrity": "sha512-vNKxJHTEKNThjfrdJwHc7brvM6eVevuO5nTj6ez8ZQ1qbXTvGthucRF7S4vf2cr71QVnT70V34v0S1DyQsti0w==" }, + "file-uri-to-path": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/file-uri-to-path/-/file-uri-to-path-1.0.0.tgz", + "integrity": "sha512-0Zt+s3L7Vf1biwWZ29aARiVYLx7iMGnEUl9x33fbB/j3jR81u/O2LbqK+Bm1CDSNDKVtJ/YjwY7TUd5SkeLQLw==", + "dev": true + }, "find-up": { "version": "3.0.0", "resolved": "https://registry.npmjs.org/find-up/-/find-up-3.0.0.tgz", @@ -2416,11 +2570,42 @@ "resolved": "https://registry.npmjs.org/has-yarn/-/has-yarn-2.1.0.tgz", "integrity": "sha512-UqBRqi4ju7T+TqGNdqAO0PaSVGsDGJUBQvk9eUWNGRY1CFGDzYhLWoM7JQEemnlvVcv/YEmc2wNW8BC24EnUsw==" }, + "hash-base": { + "version": "3.0.4", + "resolved": "https://registry.npmjs.org/hash-base/-/hash-base-3.0.4.tgz", + "integrity": "sha1-X8hoaEfs1zSZQDMZprCj8/auSRg=", + "dev": true, + "requires": { + "inherits": "^2.0.1", + "safe-buffer": "^5.0.1" + } + }, + "hash.js": { + "version": "1.1.7", + "resolved": "https://registry.npmjs.org/hash.js/-/hash.js-1.1.7.tgz", + "integrity": "sha512-taOaskGt4z4SOANNseOviYDvjEJinIkRgmp7LbKP2YTTmVxWBl87s/uzK9r+44BclBSp2X7K1hqeNfz9JbBeXA==", + "dev": true, + "requires": { + "inherits": "^2.0.3", + "minimalistic-assert": "^1.0.1" + } + }, "he": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/he/-/he-1.2.0.tgz", "integrity": "sha512-F/1DnUGPopORZi0ni+CvrCgHQ5FyEAHRLSApuYWMmrbSwoN2Mn/7k+Gl38gJnR7yyDZk6WLXwiGod1JOWNDKGw==" }, + "hmac-drbg": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/hmac-drbg/-/hmac-drbg-1.0.1.tgz", + "integrity": "sha1-0nRXAQJabHdabFRXk+1QL8DGSaE=", + "dev": true, + "requires": { + "hash.js": "^1.0.3", + "minimalistic-assert": "^1.0.0", + "minimalistic-crypto-utils": "^1.0.1" + } + }, "home-or-tmp": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/home-or-tmp/-/home-or-tmp-2.0.0.tgz", @@ -2610,6 +2795,12 @@ "resolved": "https://registry.npmjs.org/is-fullwidth-code-point/-/is-fullwidth-code-point-2.0.0.tgz", "integrity": "sha1-o7MKXE8ZkYMWeqq5O+764937ZU8=" }, + "is-hex-prefixed": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/is-hex-prefixed/-/is-hex-prefixed-1.0.0.tgz", + "integrity": "sha1-fY035q135dEnFIkTxXPggtd39VQ=", + "dev": true + }, "is-installed-globally": { "version": "0.1.0", "resolved": "https://registry.npmjs.org/is-installed-globally/-/is-installed-globally-0.1.0.tgz", @@ -2776,6 +2967,18 @@ "verror": "1.10.0" } }, + "keccak": { + "version": "1.4.0", + "resolved": "https://registry.npmjs.org/keccak/-/keccak-1.4.0.tgz", + "integrity": "sha512-eZVaCpblK5formjPjeTBik7TAg+pqnDrMHIffSvi9Lh7PQgM1+hSzakUeZFCk9DVVG0dacZJuaz2ntwlzZUIBw==", + "dev": true, + "requires": { + "bindings": "^1.2.1", + "inherits": "^2.0.3", + "nan": "^2.2.1", + "safe-buffer": "^5.1.0" + } + }, "keyv": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/keyv/-/keyv-3.1.0.tgz", @@ -2892,6 +3095,17 @@ "p-defer": "^1.0.0" } }, + "md5.js": { + "version": "1.3.5", + "resolved": "https://registry.npmjs.org/md5.js/-/md5.js-1.3.5.tgz", + "integrity": "sha512-xitP+WxNPcTTOgnTJcrhM0xvdPepipPSf3I8EIpGKeFLjt3PlJLIDG3u8EX53ZIubkb+5U2+3rELYpEhHhzdkg==", + "dev": true, + "requires": { + "hash-base": "^3.0.0", + "inherits": "^2.0.1", + "safe-buffer": "^5.1.2" + } + }, "mem": { "version": "4.3.0", "resolved": "https://registry.npmjs.org/mem/-/mem-4.3.0.tgz", @@ -2925,6 +3139,18 @@ "resolved": "https://registry.npmjs.org/mimic-response/-/mimic-response-1.0.1.tgz", "integrity": "sha512-j5EctnkH7amfV/q5Hgmoal1g2QHFJRraOtmx0JpIqkxhBhI/lJSl1nMpQ45hVarwNETOoWEimndZ4QK0RHxuxQ==" }, + "minimalistic-assert": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/minimalistic-assert/-/minimalistic-assert-1.0.1.tgz", + "integrity": "sha512-UtJcAD4yEaGtjPezWuO9wC4nwUnVH/8/Im3yEHQP4b67cXlD/Qr9hdITCU1xDbSEXg2XKNaP8jsReV7vQd00/A==", + "dev": true + }, + "minimalistic-crypto-utils": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/minimalistic-crypto-utils/-/minimalistic-crypto-utils-1.0.1.tgz", + "integrity": "sha1-9sAMHAsIIkblxNmd+4x8CDsrWCo=", + "dev": true + }, "minimatch": { "version": "3.0.4", "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.0.4.tgz", @@ -3038,6 +3264,12 @@ "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.1.tgz", "integrity": "sha512-tgp+dl5cGk28utYktBsrFqA7HKgrhgPsg6Z/EfhWI4gl1Hwq8B/GmY/0oXZ6nF8hDVesS/FpnYaD/kOWhYQvyg==" }, + "nan": { + "version": "2.14.0", + "resolved": "https://registry.npmjs.org/nan/-/nan-2.14.0.tgz", + "integrity": "sha512-INOFj37C7k3AfaNTtX8RhsTw7qRy7eLET14cROi9+5HAVbbHuIWUHEauBv5qT4Av2tWasiTY1Jw6puUNqRJXQg==", + "dev": true + }, "nested-error-stacks": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/nested-error-stacks/-/nested-error-stacks-2.0.1.tgz", @@ -3816,6 +4048,26 @@ "glob": "^7.1.3" } }, + "ripemd160": { + "version": "2.0.2", + "resolved": "https://registry.npmjs.org/ripemd160/-/ripemd160-2.0.2.tgz", + "integrity": "sha512-ii4iagi25WusVoiC4B4lq7pbXfAp3D9v5CwfkY33vffw2+pkDjY1D8GaN7spsxvCSx8dkPqOZCEZyfxcmJG2IA==", + "dev": true, + "requires": { + "hash-base": "^3.0.0", + "inherits": "^2.0.1" + } + }, + "rlp": { + "version": "2.2.3", + "resolved": "https://registry.npmjs.org/rlp/-/rlp-2.2.3.tgz", + "integrity": "sha512-l6YVrI7+d2vpW6D6rS05x2Xrmq8oW7v3pieZOJKBEdjuTF4Kz/iwk55Zyh1Zaz+KOB2kC8+2jZlp2u9L4tTzCQ==", + "dev": true, + "requires": { + "bn.js": "^4.11.1", + "safe-buffer": "^5.1.1" + } + }, "run-queue": { "version": "1.0.3", "resolved": "https://registry.npmjs.org/run-queue/-/run-queue-1.0.3.tgz", @@ -3834,6 +4086,22 @@ "resolved": "https://registry.npmjs.org/safer-buffer/-/safer-buffer-2.1.2.tgz", "integrity": "sha512-YZo3K82SD7Riyi0E1EQPojLz7kpepnSQI9IyPbHHg1XXXevb5dJI7tpyN2ADxGcQbHG7vcyRHk0cbwqcQriUtg==" }, + "secp256k1": { + "version": "3.7.1", + "resolved": "https://registry.npmjs.org/secp256k1/-/secp256k1-3.7.1.tgz", + "integrity": "sha512-1cf8sbnRreXrQFdH6qsg2H71Xw91fCCS9Yp021GnUNJzWJS/py96fS4lHbnTnouLp08Xj6jBoBB6V78Tdbdu5g==", + "dev": true, + "requires": { + "bindings": "^1.5.0", + "bip66": "^1.1.5", + "bn.js": "^4.11.8", + "create-hash": "^1.2.0", + "drbg.js": "^1.0.1", + "elliptic": "^6.4.1", + "nan": "^2.14.0", + "safe-buffer": "^5.1.2" + } + }, "semver": { "version": "5.7.1", "resolved": "https://registry.npmjs.org/semver/-/semver-5.7.1.tgz", @@ -3857,6 +4125,16 @@ "resolved": "https://registry.npmjs.org/set-blocking/-/set-blocking-2.0.0.tgz", "integrity": "sha1-BF+XgtARrppoA93TgrJDkrPYkPc=" }, + "sha.js": { + "version": "2.4.11", + "resolved": "https://registry.npmjs.org/sha.js/-/sha.js-2.4.11.tgz", + "integrity": "sha512-QMEp5B7cftE7APOjk5Y6xgrbWu+WkLVQwk8JNjZ8nKRciZaByEW6MubieAiToS7+dwvrjGhH8jRXz3MVd0AYqQ==", + "dev": true, + "requires": { + "inherits": "^2.0.1", + "safe-buffer": "^5.0.1" + } + }, "shebang-command": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/shebang-command/-/shebang-command-1.2.0.tgz", @@ -4052,6 +4330,15 @@ "resolved": "https://registry.npmjs.org/strip-eof/-/strip-eof-1.0.0.tgz", "integrity": "sha1-u0P/VZim6wXYm1n80SnJgzE2Br8=" }, + "strip-hex-prefix": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/strip-hex-prefix/-/strip-hex-prefix-1.0.0.tgz", + "integrity": "sha1-DF8VX+8RUTczd96du1iNoFUA428=", + "dev": true, + "requires": { + "is-hex-prefixed": "1.0.0" + } + }, "strip-json-comments": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/strip-json-comments/-/strip-json-comments-2.0.1.tgz", diff --git a/package.json b/package.json index 1a7e429..0d52610 100644 --- a/package.json +++ b/package.json @@ -31,5 +31,8 @@ "now": "^16.2.0", "truffle": "^5.0.33", "truffle-flattener": "^1.2.4" + }, + "devDependencies": { + "ethereumjs-util": "^6.1.0" } } diff --git a/test/UniswapEX.spec.js b/test/UniswapEX.spec.js index 48cd462..934675f 100644 --- a/test/UniswapEX.spec.js +++ b/test/UniswapEX.spec.js @@ -1,189 +1,218 @@ -import assertRevert from "./helpers/assertRevert"; -import { balanceSnap, etherSnap } from "./helpers/balanceSnap"; +import assertRevert from './helpers/assertRevert' +import { balanceSnap, etherSnap } from './helpers/balanceSnap' -const BN = web3.utils.BN; -const expect = require("chai").use(require("bn-chai")(BN)).expect; +const eutils = require('ethereumjs-util') -const UniswapEx = artifacts.require("UniswapEX"); -const ERC20 = artifacts.require("FakeERC20"); -const FakeUniswapFactory = artifacts.require("FakeUniswapFactory"); -const UniswapFactory = artifacts.require("UniswapFactory"); -const UniswapExchange = artifacts.require("UniswapExchange"); -const VaultFactory = artifacts.require("VaultFactory"); +const BN = web3.utils.BN +const expect = require('chai').use(require('bn-chai')(BN)).expect + +const UniswapEx = artifacts.require('UniswapEX') +const ERC20 = artifacts.require('FakeERC20') +const FakeUniswapFactory = artifacts.require('FakeUniswapFactory') +const UniswapFactory = artifacts.require('UniswapFactory') +const UniswapExchange = artifacts.require('UniswapExchange') +const VaultFactory = artifacts.require('VaultFactory') function buildCreate2Address(creatorAddress, saltHex, byteCode) { return `0x${web3.utils .soliditySha3( - { t: "bytes1", v: "0xff" }, - { t: "address", v: creatorAddress }, - { t: "bytes32", v: saltHex }, + { t: 'bytes1', v: '0xff' }, + { t: 'address', v: creatorAddress }, + { t: 'bytes32', v: saltHex }, { - t: "bytes32", - v: web3.utils.soliditySha3({ t: "bytes", v: byteCode }) + t: 'bytes32', + v: web3.utils.soliditySha3({ t: 'bytes', v: byteCode }) } ) - .slice(-40)}`.toLowerCase(); + .slice(-40)}`.toLowerCase() +} + +function toAddress(pk) { + return eutils.toChecksumAddress(eutils.bufferToHex(eutils.privateToAddress(eutils.toBuffer(pk)))) +} + +function sign(address, priv) { + const hash = web3.utils.soliditySha3( + { t: 'address', v: address } + ) + const sig = eutils.ecsign( + eutils.toBuffer(hash), + eutils.toBuffer(priv) + ) + + return eutils.bufferToHex(Buffer.concat([sig.r, sig.s, eutils.toBuffer(sig.v)])) } -contract("UniswapEx", function([_, owner, user, anotherUser, hacker]) { +contract('UniswapEx', function ([_, owner, user, anotherUser, hacker]) { // globals - const zeroAddress = "0x0000000000000000000000000000000000000000"; - const ethAddress = "0xeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee"; - const maxBn = new BN(2).pow(new BN(256)).sub(new BN(1)); - const fromOwner = { from: owner }; - const fromUser = { from: user }; - const fromAnotherUser = { from: anotherUser }; - const fromHacker = { from: hacker }; + const zeroAddress = '0x0000000000000000000000000000000000000000' + const ethAddress = '0xeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee' + const maxBn = new BN(2).pow(new BN(256)).sub(new BN(1)) + const fromOwner = { from: owner } + const fromUser = { from: user } + const fromAnotherUser = { from: anotherUser } + const fromHacker = { from: hacker } - const never = maxBn; + const never = maxBn const creationParams = { ...fromOwner, gas: 6e6, gasPrice: 21e9 - }; + } - const fakeKey = web3.utils.sha3("0x01"); - const anotherFakeKey = web3.utils.sha3("0x02"); - const ONE_ETH = new BN(1); + const fakeKey = web3.utils.sha3('0x01') + const anotherFakeKey = web3.utils.sha3('0x02') + const ONE_ETH = new BN(1) const FIXED_SALT = - "0xf9fea21bcccd3d13caa0d7f67bc4bd0776a06c420e932ee5add8f3affb3f354b"; + '0xf9fea21bcccd3d13caa0d7f67bc4bd0776a06c420e932ee5add8f3affb3f354b' const CRATIONCODE_VAULT = - "6012600081600A8239F360008060448082803781806038355AF132FF"; + '6012600081600A8239F360008060448082803781806038355AF132FF' // Contracts - let token1; - let token2; - let vaultFactory; - let uniswapEx; - let uniswapFactory; - let uniswapToken1; - let uniswapToken2; - - beforeEach(async function() { + let token1 + let token2 + let vaultFactory + let uniswapEx + let uniswapFactory + let uniswapToken1 + let uniswapToken2 + + beforeEach(async function () { // Create tokens - token1 = await ERC20.new(creationParams); - token2 = await ERC20.new(creationParams); + token1 = await ERC20.new(creationParams) + token2 = await ERC20.new(creationParams) // Deploy Uniswap uniswapFactory = await UniswapFactory.at( (await FakeUniswapFactory.new()).address - ); - await uniswapFactory.createExchange(token1.address); - await uniswapFactory.createExchange(token2.address); + ) + await uniswapFactory.createExchange(token1.address) + await uniswapFactory.createExchange(token2.address) uniswapToken1 = await UniswapExchange.at( await uniswapFactory.getExchange(token1.address) - ); + ) uniswapToken2 = await UniswapExchange.at( await uniswapFactory.getExchange(token2.address) - ); + ) // Deploy exchange - uniswapEx = await UniswapEx.new(uniswapFactory.address, { from: owner }); + uniswapEx = await UniswapEx.new(uniswapFactory.address, { from: owner }) // Deploy vault - vaultFactory = await VaultFactory.new(creationParams); + vaultFactory = await VaultFactory.new(creationParams) // Add liquidity to Uniswap exchange 1 - await token1.setBalance(new BN(1000000000), owner); - await token1.approve(uniswapToken1.address, maxBn, { from: owner }); + await token1.setBalance(new BN(1000000000), owner) + await token1.approve(uniswapToken1.address, maxBn, { from: owner }) await uniswapToken1.addLiquidity(0, new BN(1000000000), never, { from: owner, value: new BN(5000000000) - }); + }) // Add liquidity to Uniswap exchange 2 - await token2.setBalance(new BN(1000000000), owner); - await token2.approve(uniswapToken2.address, maxBn, { from: owner }); + await token2.setBalance(new BN(1000000000), owner) + await token2.approve(uniswapToken2.address, maxBn, { from: owner }) await uniswapToken2.addLiquidity(0, new BN(1000000000), never, { from: owner, value: new BN(5000000000) - }); - }); - - describe("Constructor", function() { - it("should be depoyed", async function() { - const contract = await UniswapEx.new(uniswapFactory.address); - - expect(contract).to.not.be.equal(zeroAddress); - }); - }); - describe("It should trade on Uniswap", async function() { - it("should execute buy tokens with ETH", async () => { + }) + }) + + describe('Constructor', function () { + it('should be depoyed', async function () { + const contract = await UniswapEx.new(uniswapFactory.address) + + expect(contract).to.not.be.equal(zeroAddress) + }) + }) + describe('It should trade on Uniswap', async function () { + it('should execute buy tokens with ETH', async () => { + const secret = web3.utils.randomHex(32) + const witness = toAddress(secret); + // Create order const encodedOrder = await uniswapEx.encodeEthOrder( - ethAddress, // Sell ETH - token1.address, // Buy TOKEN 1 - new BN(300), // Get at least 300 Tokens - new BN(10), // Pay 10 WEI to sender - user, // Owner of the order - FIXED_SALT - ); - - await uniswapEx.depositEth(encodedOrder, { - value: new BN(10000), - from: user - }); + ethAddress, // ETH Address + token1.address, // Buy TOKEN 1 + new BN(300), // Get at least 300 Tokens + new BN(10), // Pay 10 WEI to sender + user, // Owner of the order + secret, // Witness secret + witness // Witness public address + ) + + await uniswapEx.depositEth( + encodedOrder, + { + value: new BN(10000), + from: user + } + ) // Take balance snapshots - const exEtherSnap = await etherSnap(uniswapEx.address, "Uniswap EX"); - const executerEtherSnap = await etherSnap(anotherUser, "executer"); - const uniswapEtherSnap = await etherSnap( - uniswapToken1.address, - "uniswap" - ); - const userTokenSnap = await balanceSnap(token1, user, "user"); + const exEtherSnap = await etherSnap(uniswapEx.address, 'Uniswap EX') + const executerEtherSnap = await etherSnap(anotherUser, 'executer') + const uniswapEtherSnap = await etherSnap(uniswapToken1.address, 'uniswap') + const userTokenSnap = await balanceSnap(token1, user, 'user') const uniswapTokenSnap = await balanceSnap( token1, uniswapToken1.address, - "uniswap" - ); + 'uniswap' + ) + + // Sign witnesses using the secret + const witnesses = sign(anotherUser, secret) // Execute order const tx = await uniswapEx.executeOrder( - ethAddress, // Sell ETH + ethAddress, // Sell ETH token1.address, // Buy TOKEN 1 - new BN(300), // Get at least 300 Tokens - new BN(10), // Pay 10 WEI to sender - user, // Owner of the order - FIXED_SALT, + new BN(300), // Get at least 300 Tokens + new BN(10), // Pay 10 WEI to sender + user, // Owner of the order + witnesses, // Witnesses of the secret { from: anotherUser, gasPrice: 0 } - ); + ) - const bought = tx.logs[0].args._bought; + const bought = tx.logs[0].args._bought // Validate balances - await exEtherSnap.requireDecrease(new BN(10000)); - await executerEtherSnap.requireIncrease(new BN(10)); - await uniswapEtherSnap.requireIncrease(new BN(9990)); - await userTokenSnap.requireIncrease(bought); - await uniswapTokenSnap.requireDecrease(bought); - }); - it("should execute sell tokens for ETH", async () => { + await exEtherSnap.requireDecrease(new BN(10000)) + await executerEtherSnap.requireIncrease(new BN(10)) + await uniswapEtherSnap.requireIncrease(new BN(9990)) + await userTokenSnap.requireIncrease(bought) + await uniswapTokenSnap.requireDecrease(bought) + }) + it('should execute sell tokens for ETH', async () => { + const secret = web3.utils.randomHex(32) + const witness = toAddress(secret); + // Encode order transfer const orderTx = await uniswapEx.encodeTokenOrder( token1.address, // Sell token 1 - ethAddress, // Buy ETH - new BN(10000), // Tokens to sell - new BN(50), // Get at least 50 ETH Wei - new BN(15), // Pay 15 WEI to sender - user, // Owner of the order - FIXED_SALT - ); + ethAddress, // Buy ETH + new BN(10000), // Tokens to sell + new BN(50), // Get at least 50 ETH Wei + new BN(15), // Pay 15 WEI to sender + user, // Owner of the order + secret, // Witness secret + witness // Witness address + ) const vaultAddress = await uniswapEx.vaultOfOrder( token1.address, // Sell token 1 - ethAddress, // Buy ETH - new BN(50), // Get at least 50 ETH Wei - new BN(15), // Pay 15 WEI to sender - user, // Owner of the order - FIXED_SALT - ); + ethAddress, // Buy ETH + new BN(50), // Get at least 50 ETH Wei + new BN(15), // Pay 15 WEI to sender + user, // Owner of the order + witness // Witness address + ) - const vaultSnap = await balanceSnap(token1, vaultAddress, "token vault"); + const vaultSnap = await balanceSnap(token1, vaultAddress, 'token vault') - await token1.setBalance(new BN(10000), user); + await token1.setBalance(new BN(10000), user) // Send tokens tx await web3.eth.sendTransaction({ @@ -191,81 +220,85 @@ contract("UniswapEx", function([_, owner, user, anotherUser, hacker]) { to: token1.address, data: orderTx, gasPrice: 0 - }); + }) - await vaultSnap.requireIncrease(new BN(10000)); + await vaultSnap.requireIncrease(new BN(10000)) // Take balance snapshots const exTokenSnap = await balanceSnap( token1, uniswapEx.address, - "Uniswap EX" - ); + 'Uniswap EX' + ) const exEtherSnap = await balanceSnap( token1, uniswapEx.address, - "Uniswap EX" - ); - const executerEtherSnap = await etherSnap(anotherUser, "executer"); + 'Uniswap EX' + ) + const executerEtherSnap = await etherSnap(anotherUser, 'executer') const uniswapTokenSnap = await balanceSnap( token1, uniswapToken1.address, - "uniswap" - ); - const uniswapEtherSnap = await etherSnap( - uniswapToken1.address, - "uniswap" - ); - const userTokenSnap = await etherSnap(user, "user"); + 'uniswap' + ) + const uniswapEtherSnap = await etherSnap(uniswapToken1.address, 'uniswap') + const userTokenSnap = await etherSnap(user, 'user') + + // Sign witnesses using the secret + const witnesses = sign(anotherUser, secret) // Execute order const tx = await uniswapEx.executeOrder( token1.address, // Sell token 1 - ethAddress, // Buy ETH - new BN(50), // Get at least 50 ETH Wei - new BN(15), // Pay 15 WEI to sender - user, // Owner of the order - FIXED_SALT, + ethAddress, // Buy ETH + new BN(50), // Get at least 50 ETH Wei + new BN(15), // Pay 15 WEI to sender + user, // Owner of the order + witnesses, // Witnesses, sender signed using the secret { from: anotherUser, gasPrice: 0 } - ); + ) - const bought = tx.logs[0].args._bought; + const bought = tx.logs[0].args._bought // Validate balances - await exEtherSnap.requireConstant(); - await exTokenSnap.requireConstant(); - await executerEtherSnap.requireIncrease(new BN(15)); - await uniswapTokenSnap.requireIncrease(new BN(10000)); - await uniswapEtherSnap.requireDecrease(bought.add(new BN(15))); - await userTokenSnap.requireIncrease(bought); - }); - it("Should exchange tokens for tokens", async function() { + await exEtherSnap.requireConstant() + await exTokenSnap.requireConstant() + await executerEtherSnap.requireIncrease(new BN(15)) + await uniswapTokenSnap.requireIncrease(new BN(10000)) + await uniswapEtherSnap.requireDecrease(bought.add(new BN(15))) + await userTokenSnap.requireIncrease(bought) + }) + it('Should exchange tokens for tokens', async function () { + const secret = web3.utils.randomHex(32) + const witness = toAddress(secret); + // Encode order transfer const orderTx = await uniswapEx.encodeTokenOrder( token1.address, // Sell token 1 token2.address, // Buy TOKEN 2 - new BN(1000), // Tokens to sell - new BN(50), // Get at least 50 ETH Wei - new BN(9), // Pay WEI to sender - user, // Owner of the order - FIXED_SALT - ); + new BN(1000), // Tokens to sell + new BN(50), // Get at least 50 ETH Wei + new BN(9), // Pay WEI to sender + user, // Owner of the order + secret, // Witness secret + witness // Witness address + ) const vaultAddress = await uniswapEx.vaultOfOrder( token1.address, // Sell token 1 token2.address, // Buy ETH - new BN(50), // Get at least 50 ETH Wei - new BN(9), // Pay WEI to sender - user, // Owner of the order - FIXED_SALT - ); + new BN(50), // Get at least 50 ETH Wei + new BN(9), // Pay WEI to sender + user, // Owner of the order + witness // Wirness address + ) - const vaultSnap = await balanceSnap(token1, vaultAddress, "token vault"); + const vaultSnap = await balanceSnap(token1, vaultAddress, 'token vault') - await token1.setBalance(new BN(1000), user); + await token1.setBalance(new BN(1000), user) // Send tokens tx await web3.eth.sendTransaction({ @@ -273,98 +306,100 @@ contract("UniswapEx", function([_, owner, user, anotherUser, hacker]) { to: token1.address, data: orderTx, gasPrice: 0 - }); + }) - await vaultSnap.requireIncrease(new BN(1000)); + await vaultSnap.requireIncrease(new BN(1000)) // Take balance snapshots const exToken1Snap = await balanceSnap( token1, uniswapEx.address, - "Uniswap EX" - ); + 'Uniswap EX' + ) const exToken2Snap = await balanceSnap( token1, uniswapEx.address, - "Uniswap EX" - ); + 'Uniswap EX' + ) const exEtherSnap = await balanceSnap( token1, uniswapEx.address, - "Uniswap EX" - ); - const executerEtherSnap = await etherSnap(anotherUser, "executer"); + 'Uniswap EX' + ) + const executerEtherSnap = await etherSnap(anotherUser, 'executer') const uniswap1TokenSnap = await balanceSnap( token1, uniswapToken1.address, - "uniswap" - ); + 'uniswap' + ) const uniswap2TokenSnap = await balanceSnap( token2, uniswapToken2.address, - "uniswap" - ); - const userToken2Snap = await balanceSnap(token2, user, "user"); + 'uniswap' + ) + const userToken2Snap = await balanceSnap(token2, user, 'user') + + const witnesses = sign(anotherUser, secret) // Execute order const tx = await uniswapEx.executeOrder( token1.address, // Sell token 1 token2.address, // Buy ETH - new BN(50), // Get at least 50 ETH Wei - new BN(9), // Pay 9 WEI to sender - user, // Owner of the order - FIXED_SALT, + new BN(50), // Get at least 50 ETH Wei + new BN(9), // Pay 9 WEI to sender + user, // Owner of the order + witnesses, // Signature of the sender using the secret { from: anotherUser, gasPrice: 0 } - ); + ) - const bought = tx.logs[0].args._bought; + const bought = tx.logs[0].args._bought // Validate balances - await exEtherSnap.requireConstant(); - await exToken1Snap.requireConstant(); - await exToken2Snap.requireConstant(); - await vaultSnap.requireConstant(); - await executerEtherSnap.requireIncrease(new BN(9)); - await uniswap1TokenSnap.requireIncrease(new BN(1000)); - await uniswap2TokenSnap.requireDecrease(bought); - await userToken2Snap.requireIncrease(bought); - }); - }); - describe("Get vault", function() { - it("should return correct vault", async function() { - const address = (await vaultFactory.getVault(fakeKey)).toLowerCase(); + await exEtherSnap.requireConstant() + await exToken1Snap.requireConstant() + await exToken2Snap.requireConstant() + await vaultSnap.requireConstant() + await executerEtherSnap.requireIncrease(new BN(9)) + await uniswap1TokenSnap.requireIncrease(new BN(1000)) + await uniswap2TokenSnap.requireDecrease(bought) + await userToken2Snap.requireIncrease(bought) + }) + }) + describe('Get vault', function () { + it('should return correct vault', async function () { + const address = (await vaultFactory.getVault(fakeKey)).toLowerCase() const expectedAddress = buildCreate2Address( vaultFactory.address, fakeKey, CRATIONCODE_VAULT - ); - expect(address).to.not.be.equal(zeroAddress); - expect(address).to.be.equal(expectedAddress); - }); - it("should return same vault for the same key", async function() { - const address = await vaultFactory.getVault(fakeKey); - const expectedAddress = await vaultFactory.getVault(fakeKey); - expect(address).to.be.equal(expectedAddress); - }); - it("should return a different vault for a different key", async function() { - const address = await vaultFactory.getVault(fakeKey); - const expectedAddress = await vaultFactory.getVault(anotherFakeKey); - expect(address).to.not.be.equal(zeroAddress); - expect(expectedAddress).to.not.be.equal(zeroAddress); - expect(address).to.not.be.equal(expectedAddress); - }); - }); - describe("Create vault", function() { - it("should return correct vault", async function() { - const address = await vaultFactory.getVault(fakeKey); - await token1.setBalance(ONE_ETH, address); - await vaultFactory.executeVault(fakeKey, token1.address, user); - }); - it("not revert if vault has no balance", async function() { - await vaultFactory.executeVault(fakeKey, token1.address, user); - }); - }); -}); + ) + expect(address).to.not.be.equal(zeroAddress) + expect(address).to.be.equal(expectedAddress) + }) + it('should return same vault for the same key', async function () { + const address = await vaultFactory.getVault(fakeKey) + const expectedAddress = await vaultFactory.getVault(fakeKey) + expect(address).to.be.equal(expectedAddress) + }) + it('should return a different vault for a different key', async function () { + const address = await vaultFactory.getVault(fakeKey) + const expectedAddress = await vaultFactory.getVault(anotherFakeKey) + expect(address).to.not.be.equal(zeroAddress) + expect(expectedAddress).to.not.be.equal(zeroAddress) + expect(address).to.not.be.equal(expectedAddress) + }) + }) + describe('Create vault', function () { + it('should return correct vault', async function () { + const address = await vaultFactory.getVault(fakeKey) + await token1.setBalance(ONE_ETH, address) + await vaultFactory.executeVault(fakeKey, token1.address, user) + }) + it('not revert if vault has no balance', async function () { + await vaultFactory.executeVault(fakeKey, token1.address, user) + }) + }) +}) diff --git a/test/helpers/assertRevert.js b/test/helpers/assertRevert.js index 2ae40f8..6fba24a 100644 --- a/test/helpers/assertRevert.js +++ b/test/helpers/assertRevert.js @@ -1,16 +1,16 @@ -const should = require("chai").should(); +const should = require('chai').should() export default async function assertRevert(promise, message) { try { - await promise; + await promise } catch (error) { error.message.should.include( message ? `VM Exception while processing transaction: revert ${message}` - : "revert", + : 'revert', `Expected "revert", got ${error} instead` - ); - return; + ) + return } - should.fail("Expected revert not received"); + should.fail('Expected revert not received') } diff --git a/test/helpers/balanceSnap.js b/test/helpers/balanceSnap.js index e740525..a0e263a 100644 --- a/test/helpers/balanceSnap.js +++ b/test/helpers/balanceSnap.js @@ -1,59 +1,72 @@ -const BN = web3.utils.BN; -const expect = require("chai").use(require("bn-chai")(BN)).expect; + +const BN = web3.utils.BN +const expect = require('chai') + .use(require('bn-chai')(BN)) + .expect; module.exports.balanceSnap = async (token, address, account = "") => { - const snapBalance = await token.balanceOf(address); - return { - requireConstant: async function() { - expect(snapBalance, `${account} balance should remain constant`).to.eq.BN( - await token.balanceOf(address) - ); - }, - requireIncrease: async function(delta) { - const realincrease = (await token.balanceOf(address)).sub(snapBalance); - expect( - snapBalance.add(delta), - `${account} should increase by ${delta} - but increased by ${realincrease}` - ).to.eq.BN(await token.balanceOf(address)); - }, - requireDecrease: async function(delta) { - const realdecrease = snapBalance.sub(await token.balanceOf(address)); - expect( - snapBalance.sub(delta), - `${account} should decrease by ${delta} - but decreased by ${realdecrease}` - ).to.eq.BN(await token.balanceOf(address)); - }, - restore: async function() { - await token.setBalance(snapBalance, address); + const snapBalance = await token.balanceOf(address) + return { + requireConstant: async function () { + expect( + snapBalance, + `${account} balance should remain constant` + ).to.eq.BN( + await token.balanceOf(address) + ) + }, + requireIncrease: async function (delta) { + const realincrease = (await token.balanceOf(address)).sub(snapBalance) + expect( + snapBalance.add(delta), + `${account} should increase by ${delta} - but increased by ${realincrease}` + ).to.eq.BN( + await token.balanceOf(address) + ) + }, + requireDecrease: async function (delta) { + const realdecrease = snapBalance.sub(await token.balanceOf(address)) + expect( + snapBalance.sub(delta), + `${account} should decrease by ${delta} - but decreased by ${realdecrease}` + ).to.eq.BN( + await token.balanceOf(address) + ) + }, + restore: async function () { + await token.setBalance(snapBalance, address) + } } - }; -}; +} module.exports.etherSnap = async (address, account = "") => { - const snapBalance = new BN(await web3.eth.getBalance(address)); - return { - requireConstant: async function() { - expect(snapBalance, `${account} balance should remain constant`).to.eq.BN( - await web3.eth.getBalance(address) - ); - }, - requireIncrease: async function(delta) { - const realincrease = new BN(await web3.eth.getBalance(address)).sub( - snapBalance - ); - expect( - snapBalance.add(delta), - `${account} should increase by ${delta} - but increased by ${realincrease}` - ).to.eq.BN(new BN(await web3.eth.getBalance(address))); - }, - requireDecrease: async function(delta) { - const realdecrease = snapBalance.sub( - new BN(await web3.eth.getBalance(address)) - ); - expect( - snapBalance.sub(delta), - `${account} should decrease by ${delta} - but decreased by ${realdecrease}` - ).to.eq.BN(new BN(await web3.eth.getBalance(address))); + const snapBalance = new BN(await web3.eth.getBalance(address)) + return { + requireConstant: async function () { + expect( + snapBalance, + `${account} balance should remain constant` + ).to.eq.BN( + await web3.eth.getBalance(address) + ) + }, + requireIncrease: async function (delta) { + const realincrease = new BN(await web3.eth.getBalance(address)).sub(snapBalance) + expect( + snapBalance.add(delta), + `${account} should increase by ${delta} - but increased by ${realincrease}` + ).to.eq.BN( + new BN(await web3.eth.getBalance(address)) + ) + }, + requireDecrease: async function (delta) { + const realdecrease = snapBalance.sub(new BN(await web3.eth.getBalance(address))) + expect( + snapBalance.sub(delta), + `${account} should decrease by ${delta} - but decreased by ${realdecrease}` + ).to.eq.BN( + new BN(await web3.eth.getBalance(address)) + ) + } } - }; -}; +} From 7925ab1ffa7a19a3d11cf09149cbf98b07975698 Mon Sep 17 00:00:00 2001 From: Agustin Aguilar Date: Thu, 19 Sep 2019 12:13:34 +0200 Subject: [PATCH 2/2] Remove readWitness and use fromToken on depositEth --- contracts/UniswapEX.sol | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/contracts/UniswapEX.sol b/contracts/UniswapEX.sol index c1f7a3a..149e4b4 100644 --- a/contracts/UniswapEX.sol +++ b/contracts/UniswapEX.sol @@ -75,7 +75,7 @@ contract UniswapEX { require(fromToken == ETH_ADDRESS, "order is not from ETH"); bytes32 key = _keyOf( - IERC20(ETH_ADDRESS), + IERC20(fromToken), IERC20(toToken), minReturn, fee, @@ -126,15 +126,6 @@ contract UniswapEX { ); } - function readWitnesses( - bytes calldata _witnesses - ) external view returns (address) { - return SigUtils.ecrecover2( - keccak256(abi.encodePacked(msg.sender)), - _witnesses - ); - } - function executeOrder( IERC20 _fromToken, IERC20 _toToken,