diff options
Diffstat (limited to 'packages/contracts')
31 files changed, 1034 insertions, 363 deletions
diff --git a/packages/contracts/compiler.json b/packages/contracts/compiler.json index dba836bde..ad35fc5b3 100644 --- a/packages/contracts/compiler.json +++ b/packages/contracts/compiler.json @@ -42,12 +42,13 @@ "TestConstants", "TestLibBytes", "TestLibs", + "TestExchangeInternals", "TestSignatureValidator", "TokenRegistry", "Validator", "Wallet", - "Whitelist", "WETH9", + "Whitelist", "ZRXToken" ] } diff --git a/packages/contracts/package.json b/packages/contracts/package.json index 9fe8046b1..067a20775 100644 --- a/packages/contracts/package.json +++ b/packages/contracts/package.json @@ -33,7 +33,7 @@ "lint-contracts": "solhint src/2.0.0/**/**/**/**/*.sol" }, "config": { - "abis": "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Receiver|DummyERC721Token|ERC20Proxy|ERC721Proxy|Forwarder|Exchange|ExchangeWrapper|IAssetData|IAssetProxy|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|TestAssetProxyOwner|TestAssetProxyDispatcher|TestConstants|TestLibBytes|TestLibs|TestSignatureValidator|Validator|Wallet|TokenRegistry|Whitelist|WETH9|ZRXToken).json" + "abis": "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Receiver|DummyERC721Token|ERC20Proxy|ERC721Proxy|Forwarder|Exchange|ExchangeWrapper|IAssetData|IAssetProxy|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|TestAssetProxyOwner|TestAssetProxyDispatcher|TestConstants|TestExchangeInternals|TestLibBytes|TestLibs|TestSignatureValidator|Validator|Wallet|TokenRegistry|Whitelist|WETH9|ZRXToken).json" }, "repository": { "type": "git", @@ -62,13 +62,13 @@ "copyfiles": "^1.2.0", "dirty-chai": "^2.0.1", "make-promises-safe": "^1.1.0", - "mocha": "^4.0.1", + "mocha": "^4.1.0", "npm-run-all": "^4.1.2", "shx": "^0.2.2", "solc": "^0.4.24", "solhint": "^1.2.1", "tslint": "5.11.0", - "typescript": "2.7.1", + "typescript": "2.9.2", "yargs": "^10.0.3" }, "dependencies": { @@ -79,11 +79,13 @@ "@0xproject/typescript-typings": "^1.0.3", "@0xproject/utils": "^1.0.4", "@0xproject/web3-wrapper": "^1.1.2", + "@types/js-combinatorics": "^0.5.29", "bn.js": "^4.11.8", "ethereum-types": "^1.0.3", "ethereumjs-abi": "0.6.5", "ethereumjs-util": "^5.1.1", "ethers": "3.0.22", - "lodash": "^4.17.4" + "js-combinatorics": "^0.5.3", + "lodash": "^4.17.5" } } diff --git a/packages/contracts/src/2.0.0/forwarder/MixinAssets.sol b/packages/contracts/src/2.0.0/forwarder/MixinAssets.sol index 5cf5f831b..e06f9a8e3 100644 --- a/packages/contracts/src/2.0.0/forwarder/MixinAssets.sol +++ b/packages/contracts/src/2.0.0/forwarder/MixinAssets.sol @@ -36,28 +36,25 @@ contract MixinAssets is bytes4 constant internal ERC20_TRANSFER_SELECTOR = bytes4(keccak256("transfer(address,uint256)")); - /// @dev Withdraws ERC20 tokens from this contract. The contract requires a ZRX balance in order to + /// @dev Withdraws assets from this contract. The contract requires a ZRX balance in order to /// function optimally, and this function allows the ZRX to be withdrawn by owner. It may also be - /// used to withdraw tokens that were accidentally sent to this contract. - /// @param token Address of ERC20 token to withdraw. + /// used to withdraw assets that were accidentally sent to this contract. + /// @param assetData Byte array encoded for the respective asset proxy. /// @param amount Amount of ERC20 token to withdraw. - function withdrawERC20( - address token, + function withdrawAsset( + bytes assetData, uint256 amount ) external onlyOwner { - require( - IERC20Token(token).transfer(msg.sender, amount), - "WITHDRAWAL_FAILED" - ); + transferAssetToSender(assetData, amount); } /// @dev Transfers given amount of asset to sender. /// @param assetData Byte array encoded for the respective asset proxy. /// @param amount Amount of asset to transfer to sender. - function transferPurchasedAssetToSender( + function transferAssetToSender( bytes memory assetData, uint256 amount ) diff --git a/packages/contracts/src/2.0.0/forwarder/MixinExchangeWrapper.sol b/packages/contracts/src/2.0.0/forwarder/MixinExchangeWrapper.sol index f3aa483c5..4584bb840 100644 --- a/packages/contracts/src/2.0.0/forwarder/MixinExchangeWrapper.sol +++ b/packages/contracts/src/2.0.0/forwarder/MixinExchangeWrapper.sol @@ -139,7 +139,7 @@ contract MixinExchangeWrapper is /// @param makerAssetFillAmount Desired amount of makerAsset to buy. /// @param signatures Proofs that orders have been signed by makers. /// @return Amounts filled and fees paid by makers and taker. - function marketBuyWithWeth( + function marketBuyExactAmountWithWeth( LibOrder.Order[] memory orders, uint256 makerAssetFillAmount, bytes[] memory signatures @@ -180,10 +180,16 @@ contract MixinExchangeWrapper is addFillResults(totalFillResults, singleFillResults); // Stop execution if the entire amount of makerAsset has been bought - if (totalFillResults.makerAssetFilledAmount >= makerAssetFillAmount) { + uint256 makerAssetFilledAmount = totalFillResults.makerAssetFilledAmount; + if (makerAssetFilledAmount >= makerAssetFillAmount) { break; } } + + require( + makerAssetFilledAmount >= makerAssetFillAmount, + "COMPLETE_FILL_FAILED" + ); return totalFillResults; } @@ -196,7 +202,7 @@ contract MixinExchangeWrapper is /// @param zrxBuyAmount Desired amount of ZRX to buy. /// @param signatures Proofs that orders have been created by makers. /// @return totalFillResults Amounts filled and fees paid by maker and taker. - function marketBuyZrxWithWeth( + function marketBuyExactZrxWithWeth( LibOrder.Order[] memory orders, uint256 zrxBuyAmount, bytes[] memory signatures @@ -248,6 +254,10 @@ contract MixinExchangeWrapper is } } + require( + zrxPurchased >= zrxBuyAmount, + "COMPLETE_FILL_FAILED" + ); return totalFillResults; } } diff --git a/packages/contracts/src/2.0.0/forwarder/MixinForwarderCore.sol b/packages/contracts/src/2.0.0/forwarder/MixinForwarderCore.sol index 1164ae919..93cbf79be 100644 --- a/packages/contracts/src/2.0.0/forwarder/MixinForwarderCore.sol +++ b/packages/contracts/src/2.0.0/forwarder/MixinForwarderCore.sol @@ -23,7 +23,7 @@ import "./libs/LibConstants.sol"; import "./mixins/MWeth.sol"; import "./mixins/MAssets.sol"; import "./mixins/MExchangeWrapper.sol"; -import "./mixins/MForwarderCore.sol"; +import "./interfaces/IForwarderCore.sol"; import "../utils/LibBytes/LibBytes.sol"; import "../protocol/Exchange/libs/LibOrder.sol"; import "../protocol/Exchange/libs/LibFillResults.sol"; @@ -37,7 +37,7 @@ contract MixinForwarderCore is MWeth, MAssets, MExchangeWrapper, - MForwarderCore + IForwarderCore { using LibBytes for bytes; @@ -117,7 +117,7 @@ contract MixinForwarderCore is ); // Buy back all ZRX spent on fees. zrxBuyAmount = orderFillResults.takerFeePaid; - feeOrderFillResults = marketBuyZrxWithWeth( + feeOrderFillResults = marketBuyExactZrxWithWeth( feeOrders, zrxBuyAmount, feeSignatures @@ -125,13 +125,6 @@ contract MixinForwarderCore is makerAssetAmountPurchased = orderFillResults.makerAssetFilledAmount; } - // Ensure that all ZRX fees have been repurchased and no extra WETH owned by this contract has been sold. - assertValidFillResults( - orderFillResults, - feeOrderFillResults, - zrxBuyAmount - ); - // Transfer feePercentage of total ETH spent on primary orders to feeRecipient. // Refund remaining ETH to msg.sender. transferEthFeeAndRefund( @@ -142,7 +135,7 @@ contract MixinForwarderCore is ); // Transfer purchased assets to msg.sender. - transferPurchasedAssetToSender(orders[0].makerAssetData, makerAssetAmountPurchased); + transferAssetToSender(orders[0].makerAssetData, makerAssetAmountPurchased); } /// @dev Attempt to purchase makerAssetFillAmount of makerAsset by selling ETH provided with transaction. @@ -180,7 +173,7 @@ contract MixinForwarderCore is if (orders[0].makerAssetData.equals(ZRX_ASSET_DATA)) { // If the makerAsset is ZRX, it is not necessary to pay fees out of this // contracts's ZRX balance because fees are factored into the price of the order. - orderFillResults = marketBuyZrxWithWeth( + orderFillResults = marketBuyExactZrxWithWeth( orders, makerAssetFillAmount, signatures @@ -190,14 +183,14 @@ contract MixinForwarderCore is } else { // Attemp to purchase desired amount of makerAsset. // ZRX fees are payed with this contract's balance. - orderFillResults = marketBuyWithWeth( + orderFillResults = marketBuyExactAmountWithWeth( orders, makerAssetFillAmount, signatures ); // Buy back all ZRX spent on fees. zrxBuyAmount = orderFillResults.takerFeePaid; - feeOrderFillResults = marketBuyZrxWithWeth( + feeOrderFillResults = marketBuyExactZrxWithWeth( feeOrders, zrxBuyAmount, feeSignatures @@ -205,13 +198,6 @@ contract MixinForwarderCore is makerAssetAmountPurchased = orderFillResults.makerAssetFilledAmount; } - // Ensure that all ZRX fees have been repurchased and no extra WETH owned by this contract has been sold. - assertValidFillResults( - orderFillResults, - feeOrderFillResults, - zrxBuyAmount - ); - // Transfer feePercentage of total ETH spent on primary orders to feeRecipient. // Refund remaining ETH to msg.sender. transferEthFeeAndRefund( @@ -222,33 +208,6 @@ contract MixinForwarderCore is ); // Transfer purchased assets to msg.sender. - transferPurchasedAssetToSender(orders[0].makerAssetData, makerAssetAmountPurchased); - } - - /// @dev Ensures that all ZRX fees have been repurchased and no extra WETH owned by this contract has been sold. - /// @param orderFillResults Amounts filled and fees paid for primary orders. - /// @param feeOrderFillResults Amounts filled and fees paid for fee orders. - /// @param zrxBuyAmount The amount of ZRX that needed to be repurchased after filling primary orders. - function assertValidFillResults( - FillResults memory orderFillResults, - FillResults memory feeOrderFillResults, - uint256 zrxBuyAmount - ) - internal - view - { - // Ensure that all ZRX spent while filling primary orders has been repurchased. - uint256 zrxPurchased = safeSub(feeOrderFillResults.makerAssetFilledAmount, feeOrderFillResults.takerFeePaid); - require( - zrxPurchased >= zrxBuyAmount, - "COMPLETE_FILL_FAILED" - ); - - // Ensure that no extra WETH owned by this contract has been sold. - uint256 wethSold = safeAdd(orderFillResults.takerAssetFilledAmount, feeOrderFillResults.takerAssetFilledAmount); - require( - wethSold <= msg.value, - "OVERSOLD_WETH" - ); + transferAssetToSender(orders[0].makerAssetData, makerAssetAmountPurchased); } } diff --git a/packages/contracts/src/2.0.0/forwarder/MixinWeth.sol b/packages/contracts/src/2.0.0/forwarder/MixinWeth.sol index 8ba236e7f..e07940776 100644 --- a/packages/contracts/src/2.0.0/forwarder/MixinWeth.sol +++ b/packages/contracts/src/2.0.0/forwarder/MixinWeth.sol @@ -71,12 +71,16 @@ contract MixinWeth is "FEE_PERCENTAGE_TOO_LARGE" ); - // Calculate amount of WETH that hasn't been sold. - uint256 wethRemaining = safeSub( - msg.value, - safeAdd(wethSoldExcludingFeeOrders, wethSoldForZrx) + // Ensure that no extra WETH owned by this contract has been sold. + uint256 wethSold = safeAdd(wethSoldExcludingFeeOrders, wethSoldForZrx); + require( + wethSold <= msg.value, + "OVERSOLD_WETH" ); + // Calculate amount of WETH that hasn't been sold. + uint256 wethRemaining = safeSub(msg.value, wethSold); + // Calculate ETH fee to pay to feeRecipient. uint256 ethFee = getPartialAmount( feePercentage, diff --git a/packages/contracts/src/2.0.0/forwarder/interfaces/IAssets.sol b/packages/contracts/src/2.0.0/forwarder/interfaces/IAssets.sol index 9b0d995eb..1e034c003 100644 --- a/packages/contracts/src/2.0.0/forwarder/interfaces/IAssets.sol +++ b/packages/contracts/src/2.0.0/forwarder/interfaces/IAssets.sol @@ -21,13 +21,13 @@ pragma solidity 0.4.24; contract IAssets { - /// @dev Withdraws ERC20 tokens from this contract. The contract requires a ZRX balance in order to + /// @dev Withdraws assets from this contract. The contract requires a ZRX balance in order to /// function optimally, and this function allows the ZRX to be withdrawn by owner. It may also be - /// used to withdraw tokens that were accidentally sent to this contract. - /// @param token Address of ERC20 token to withdraw. + /// used to withdraw assets that were accidentally sent to this contract. + /// @param assetData Byte array encoded for the respective asset proxy. /// @param amount Amount of ERC20 token to withdraw. - function withdrawERC20( - address token, + function withdrawAsset( + bytes assetData, uint256 amount ) external; diff --git a/packages/contracts/src/2.0.0/forwarder/mixins/MAssets.sol b/packages/contracts/src/2.0.0/forwarder/mixins/MAssets.sol index 340ee0bcb..83636432a 100644 --- a/packages/contracts/src/2.0.0/forwarder/mixins/MAssets.sol +++ b/packages/contracts/src/2.0.0/forwarder/mixins/MAssets.sol @@ -28,7 +28,7 @@ contract MAssets is /// @dev Transfers given amount of asset to sender. /// @param assetData Byte array encoded for the respective asset proxy. /// @param amount Amount of asset to transfer to sender. - function transferPurchasedAssetToSender( + function transferAssetToSender( bytes memory assetData, uint256 amount ) diff --git a/packages/contracts/src/2.0.0/forwarder/mixins/MExchangeWrapper.sol b/packages/contracts/src/2.0.0/forwarder/mixins/MExchangeWrapper.sol index 5a2def7e5..360dea0e4 100644 --- a/packages/contracts/src/2.0.0/forwarder/mixins/MExchangeWrapper.sol +++ b/packages/contracts/src/2.0.0/forwarder/mixins/MExchangeWrapper.sol @@ -60,7 +60,7 @@ contract MExchangeWrapper { /// @param makerAssetFillAmount Desired amount of makerAsset to buy. /// @param signatures Proofs that orders have been signed by makers. /// @return Amounts filled and fees paid by makers and taker. - function marketBuyWithWeth( + function marketBuyExactAmountWithWeth( LibOrder.Order[] memory orders, uint256 makerAssetFillAmount, bytes[] memory signatures @@ -77,7 +77,7 @@ contract MExchangeWrapper { /// @param zrxBuyAmount Desired amount of ZRX to buy. /// @param signatures Proofs that orders have been created by makers. /// @return totalFillResults Amounts filled and fees paid by maker and taker. - function marketBuyZrxWithWeth( + function marketBuyExactZrxWithWeth( LibOrder.Order[] memory orders, uint256 zrxBuyAmount, bytes[] memory signatures diff --git a/packages/contracts/src/2.0.0/forwarder/mixins/MForwarderCore.sol b/packages/contracts/src/2.0.0/forwarder/mixins/MForwarderCore.sol deleted file mode 100644 index 0f5cd9c66..000000000 --- a/packages/contracts/src/2.0.0/forwarder/mixins/MForwarderCore.sol +++ /dev/null @@ -1,42 +0,0 @@ -/* - - Copyright 2018 ZeroEx Intl. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - -*/ - -pragma solidity 0.4.24; -pragma experimental ABIEncoderV2; - -import "../../protocol/Exchange/libs/LibOrder.sol"; -import "../../protocol/Exchange/libs/LibFillResults.sol"; -import "../interfaces/IForwarderCore.sol"; - - -contract MForwarderCore is - IForwarderCore -{ - - /// @dev Ensures that all ZRX fees have been repurchased and no extra WETH owned by this contract has been sold. - /// @param orderFillResults Amounts filled and fees paid for primary orders. - /// @param feeOrderFillResults Amounts filled and fees paid for fee orders. - /// @param zrxBuyAmount The amount of ZRX that needed to be repurchased after filling primary orders. - function assertValidFillResults( - LibFillResults.FillResults memory orderFillResults, - LibFillResults.FillResults memory feeOrderFillResults, - uint256 zrxBuyAmount - ) - internal - view; -} diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol index 354d0e9c3..ab5c6e507 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol @@ -207,7 +207,6 @@ contract MixinExchangeCore is /// @param order that was filled. /// @param takerAddress Address of taker who filled the order. /// @param orderTakerAssetFilledAmount Amount of order already filled. - /// @return fillResults Amounts filled and fees paid by maker and taker. function updateFilledState( Order memory order, address takerAddress, diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol index ac7382715..44de54817 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol @@ -96,14 +96,15 @@ contract MixinSignatureValidator is "LENGTH_GREATER_THAN_0_REQUIRED" ); - // Ensure signature is supported + // Pop last byte off of signature byte array. uint8 signatureTypeRaw = uint8(signature.popLastByte()); + + // Ensure signature is supported require( signatureTypeRaw < uint8(SignatureType.NSignatureTypes), "SIGNATURE_UNSUPPORTED" ); - // Pop last byte off of signature byte array. SignatureType signatureType = SignatureType(signatureTypeRaw); // Variables are not scoped in Solidity. @@ -141,7 +142,12 @@ contract MixinSignatureValidator is v = uint8(signature[0]); r = signature.readBytes32(1); s = signature.readBytes32(33); - recovered = ecrecover(hash, v, r, s); + recovered = ecrecover( + hash, + v, + r, + s + ); isValid = signerAddress == recovered; return isValid; @@ -197,7 +203,6 @@ contract MixinSignatureValidator is // | 0x14 + x | 1 | Signature type is always "\x06" | } else if (signatureType == SignatureType.Validator) { // Pop last 20 bytes off of signature byte array. - address validatorAddress = signature.popLast20Bytes(); // Ensure signer has approved validator. diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinTransactions.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinTransactions.sol index 88d2da7d7..821d30279 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinTransactions.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinTransactions.sol @@ -123,22 +123,25 @@ contract MixinTransactions is bytes32 dataHash = keccak256(data); // Assembly for more efficiently computing: - // keccak256(abi.encode( + // keccak256(abi.encodePacked( // EIP712_ZEROEX_TRANSACTION_SCHEMA_HASH, // salt, - // signerAddress, + // bytes32(signerAddress), // keccak256(data) // )); assembly { + // Load free memory pointer let memPtr := mload(64) - mstore(memPtr, schemaHash) - mstore(add(memPtr, 32), salt) - mstore(add(memPtr, 64), and(signerAddress, 0xffffffffffffffffffffffffffffffffffffffff)) - mstore(add(memPtr, 96), dataHash) + + mstore(memPtr, schemaHash) // hash of schema + mstore(add(memPtr, 32), salt) // salt + mstore(add(memPtr, 64), and(signerAddress, 0xffffffffffffffffffffffffffffffffffffffff)) // signerAddress + mstore(add(memPtr, 96), dataHash) // hash of data + + // Compute hash result := keccak256(memPtr, 128) } - return result; } diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibAbiEncoder.sol b/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibAbiEncoder.sol index 704c7061c..4aad37709 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibAbiEncoder.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibAbiEncoder.sol @@ -24,20 +24,17 @@ import "./LibOrder.sol"; contract LibAbiEncoder { - /// @dev ABI encodes calldata for `fillOrder` in memory and returns the address range. - /// This range can be passed into `call` or `delegatecall` to invoke an external - /// call to `fillOrder`. + /// @dev ABI encodes calldata for `fillOrder`. /// @param order Order struct containing order specifications. /// @param takerAssetFillAmount Desired amount of takerAsset to sell. /// @param signature Proof that order has been created by maker. - /// @return calldataBegin Memory address of ABI encoded calldata. - /// @return calldataLength Lenfgth of ABI encoded calldata. + /// @return ABI encoded calldata for `fillOrder`. function abiEncodeFillOrder( LibOrder.Order memory order, uint256 takerAssetFillAmount, bytes memory signature ) - public + internal pure returns (bytes memory fillOrderCalldata) { @@ -207,10 +204,10 @@ contract LibAbiEncoder { } // Set length of calldata - mstore( - fillOrderCalldata, - sub(dataAreaEnd, add(fillOrderCalldata, 0x20)) - ) + mstore(fillOrderCalldata, sub(dataAreaEnd, add(fillOrderCalldata, 0x20))) + + // Increment free memory pointer + mstore(0x40, dataAreaEnd) } return fillOrderCalldata; diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibEIP712.sol b/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibEIP712.sol index 1fc41dafd..b02f7632e 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibEIP712.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibEIP712.sol @@ -30,7 +30,7 @@ contract LibEIP712 { string constant internal EIP712_DOMAIN_VERSION = "2"; // Hash of the EIP712 Domain Separator Schema - bytes32 public constant EIP712_DOMAIN_SEPARATOR_SCHEMA_HASH = keccak256(abi.encodePacked( + bytes32 constant internal EIP712_DOMAIN_SEPARATOR_SCHEMA_HASH = keccak256(abi.encodePacked( "EIP712Domain(", "string name,", "string version,", @@ -45,11 +45,11 @@ contract LibEIP712 { constructor () public { - EIP712_DOMAIN_HASH = keccak256(abi.encode( + EIP712_DOMAIN_HASH = keccak256(abi.encodePacked( EIP712_DOMAIN_SEPARATOR_SCHEMA_HASH, keccak256(bytes(EIP712_DOMAIN_NAME)), keccak256(bytes(EIP712_DOMAIN_VERSION)), - address(this) + bytes32(address(this)) )); } @@ -59,8 +59,28 @@ contract LibEIP712 { function hashEIP712Message(bytes32 hashStruct) internal view - returns (bytes32) + returns (bytes32 result) { - return keccak256(abi.encodePacked(EIP191_HEADER, EIP712_DOMAIN_HASH, hashStruct)); + bytes32 eip712DomainHash = EIP712_DOMAIN_HASH; + + // Assembly for more efficient computing: + // keccak256(abi.encodePacked( + // EIP191_HEADER, + // EIP712_DOMAIN_HASH, + // hashStruct + // )); + + assembly { + // Load free memory pointer + let memPtr := mload(64) + + mstore(memPtr, 0x1901000000000000000000000000000000000000000000000000000000000000) // EIP191 header + mstore(add(memPtr, 2), eip712DomainHash) // EIP712 domain hash + mstore(add(memPtr, 34), hashStruct) // Hash of struct + + // Compute hash + result := keccak256(memPtr, 66) + } + return result; } } diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibOrder.sol b/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibOrder.sol index 4031ff26b..68f4f5f1b 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibOrder.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibOrder.sol @@ -103,11 +103,12 @@ contract LibOrder is bytes32 takerAssetDataHash = keccak256(order.takerAssetData); // Assembly for more efficiently computing: - // keccak256(abi.encode( - // order.makerAddress, - // order.takerAddress, - // order.feeRecipientAddress, - // order.senderAddress, + // keccak256(abi.encodePacked( + // EIP712_ORDER_SCHEMA_HASH, + // bytes32(order.makerAddress), + // bytes32(order.takerAddress), + // bytes32(order.feeRecipientAddress), + // bytes32(order.senderAddress), // order.makerAssetAmount, // order.takerAssetAmount, // order.makerFee, @@ -119,24 +120,26 @@ contract LibOrder is // )); assembly { + // Calculate memory addresses that will be swapped out before hashing + let pos1 := sub(order, 32) + let pos2 := add(order, 320) + let pos3 := add(order, 352) + // Backup - // solhint-disable-next-line space-after-comma - let temp1 := mload(sub(order, 32)) - let temp2 := mload(add(order, 320)) - let temp3 := mload(add(order, 352)) + let temp1 := mload(pos1) + let temp2 := mload(pos2) + let temp3 := mload(pos3) // Hash in place - // solhint-disable-next-line space-after-comma - mstore(sub(order, 32), schemaHash) - mstore(add(order, 320), makerAssetDataHash) - mstore(add(order, 352), takerAssetDataHash) - result := keccak256(sub(order, 32), 416) + mstore(pos1, schemaHash) + mstore(pos2, makerAssetDataHash) + mstore(pos3, takerAssetDataHash) + result := keccak256(pos1, 416) // Restore - // solhint-disable-next-line space-after-comma - mstore(sub(order, 32), temp1) - mstore(add(order, 320), temp2) - mstore(add(order, 352), temp3) + mstore(pos1, temp1) + mstore(pos2, temp2) + mstore(pos3, temp3) } return result; } diff --git a/packages/contracts/src/2.0.0/test/TestExchangeInternals/TestExchangeInternals.sol b/packages/contracts/src/2.0.0/test/TestExchangeInternals/TestExchangeInternals.sol new file mode 100644 index 000000000..d9cec9edc --- /dev/null +++ b/packages/contracts/src/2.0.0/test/TestExchangeInternals/TestExchangeInternals.sol @@ -0,0 +1,121 @@ +/* + + Copyright 2018 ZeroEx Intl. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + +*/ + +pragma solidity 0.4.24; +pragma experimental ABIEncoderV2; + +import "../../protocol/Exchange/Exchange.sol"; + + +// solhint-disable no-empty-blocks +contract TestExchangeInternals is + Exchange +{ + constructor () + public + Exchange("") + {} + + /// @dev Adds properties of both FillResults instances. + /// Modifies the first FillResults instance specified. + /// Note that this function has been modified from the original + // internal version to return the FillResults. + /// @param totalFillResults Fill results instance that will be added onto. + /// @param singleFillResults Fill results instance that will be added to totalFillResults. + /// @return newTotalFillResults The result of adding singleFillResults to totalFilResults. + function publicAddFillResults(FillResults memory totalFillResults, FillResults memory singleFillResults) + public + pure + returns (FillResults memory) + { + addFillResults(totalFillResults, singleFillResults); + return totalFillResults; + } + + /// @dev Calculates amounts filled and fees paid by maker and taker. + /// @param order to be filled. + /// @param takerAssetFilledAmount Amount of takerAsset that will be filled. + /// @return fillResults Amounts filled and fees paid by maker and taker. + function publicCalculateFillResults( + Order memory order, + uint256 takerAssetFilledAmount + ) + public + pure + returns (FillResults memory fillResults) + { + return calculateFillResults(order, takerAssetFilledAmount); + } + + /// @dev Calculates partial value given a numerator and denominator. + /// @param numerator Numerator. + /// @param denominator Denominator. + /// @param target Value to calculate partial of. + /// @return Partial value of target. + function publicGetPartialAmount( + uint256 numerator, + uint256 denominator, + uint256 target + ) + public + pure + returns (uint256 partialAmount) + { + return getPartialAmount(numerator, denominator, target); + } + + /// @dev Checks if rounding error > 0.1%. + /// @param numerator Numerator. + /// @param denominator Denominator. + /// @param target Value to multiply with numerator/denominator. + /// @return Rounding error is present. + function publicIsRoundingError( + uint256 numerator, + uint256 denominator, + uint256 target + ) + public + pure + returns (bool isError) + { + return isRoundingError(numerator, denominator, target); + } + + /// @dev Updates state with results of a fill order. + /// @param order that was filled. + /// @param takerAddress Address of taker who filled the order. + /// @param orderTakerAssetFilledAmount Amount of order already filled. + /// @return fillResults Amounts filled and fees paid by maker and taker. + function publicUpdateFilledState( + Order memory order, + address takerAddress, + bytes32 orderHash, + uint256 orderTakerAssetFilledAmount, + FillResults memory fillResults + ) + public + { + updateFilledState( + order, + takerAddress, + orderHash, + orderTakerAssetFilledAmount, + fillResults + ); + } +} diff --git a/packages/contracts/src/2.0.0/test/TestLibs/TestLibs.sol b/packages/contracts/src/2.0.0/test/TestLibs/TestLibs.sol index 5a349527b..4a99dd9c1 100644 --- a/packages/contracts/src/2.0.0/test/TestLibs/TestLibs.sol +++ b/packages/contracts/src/2.0.0/test/TestLibs/TestLibs.sol @@ -22,13 +22,33 @@ pragma experimental ABIEncoderV2; import "../../protocol/Exchange/libs/LibMath.sol"; import "../../protocol/Exchange/libs/LibOrder.sol"; import "../../protocol/Exchange/libs/LibFillResults.sol"; +import "../../protocol/Exchange/libs/LibAbiEncoder.sol"; contract TestLibs is LibMath, LibOrder, - LibFillResults + LibFillResults, + LibAbiEncoder { + + function publicAbiEncodeFillOrder( + Order memory order, + uint256 takerAssetFillAmount, + bytes memory signature + ) + public + pure + returns (bytes memory fillOrderCalldata) + { + fillOrderCalldata = abiEncodeFillOrder( + order, + takerAssetFillAmount, + signature + ); + return fillOrderCalldata; + } + function publicGetPartialAmount( uint256 numerator, uint256 denominator, diff --git a/packages/contracts/test/exchange/fill_order.ts b/packages/contracts/test/exchange/fill_order.ts index 029bd66e2..1494fe093 100644 --- a/packages/contracts/test/exchange/fill_order.ts +++ b/packages/contracts/test/exchange/fill_order.ts @@ -2,7 +2,10 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; import * as _ from 'lodash'; import { chaiSetup } from '../utils/chai_setup'; -import { CoreCombinatorialUtils, coreCombinatorialUtilsFactoryAsync } from '../utils/core_combinatorial_utils'; +import { + FillOrderCombinatorialUtils, + fillOrderCombinatorialUtilsFactoryAsync, +} from '../utils/fill_order_combinatorial_utils'; import { AllowanceAmountScenario, AssetDataScenario, @@ -47,11 +50,11 @@ const defaultFillScenario = { }; describe('FillOrder Tests', () => { - let coreCombinatorialUtils: CoreCombinatorialUtils; + let fillOrderCombinatorialUtils: FillOrderCombinatorialUtils; before(async () => { await blockchainLifecycle.startAsync(); - coreCombinatorialUtils = await coreCombinatorialUtilsFactoryAsync(web3Wrapper, txDefaults); + fillOrderCombinatorialUtils = await fillOrderCombinatorialUtilsFactoryAsync(web3Wrapper, txDefaults); }); after(async () => { await blockchainLifecycle.revertAsync(); @@ -67,19 +70,19 @@ describe('FillOrder Tests', () => { _.forEach(fillScenarios, fillScenario => { const description = `Combinatorial OrderFill: ${JSON.stringify(fillScenario)}`; it(description, async () => { - await coreCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); + await fillOrderCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); }); }); }; - const allFillScenarios = CoreCombinatorialUtils.generateFillOrderCombinations(); + const allFillScenarios = FillOrderCombinatorialUtils.generateFillOrderCombinations(); describe('Combinatorially generated fills orders', () => test(allFillScenarios)); it('should transfer the correct amounts when makerAssetAmount === takerAssetAmount', async () => { const fillScenario = { ...defaultFillScenario, }; - await coreCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); + await fillOrderCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); }); it('should transfer the correct amounts when makerAssetAmount > takerAssetAmount', async () => { const fillScenario = { @@ -89,7 +92,7 @@ describe('FillOrder Tests', () => { takerAssetAmountScenario: OrderAssetAmountScenario.Small, }, }; - await coreCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); + await fillOrderCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); }); it('should transfer the correct amounts when makerAssetAmount < takerAssetAmount', async () => { const fillScenario = { @@ -99,7 +102,7 @@ describe('FillOrder Tests', () => { makerAssetAmountScenario: OrderAssetAmountScenario.Small, }, }; - await coreCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); + await fillOrderCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); }); it('should transfer the correct amounts when taker is specified and order is claimed by taker', async () => { const fillScenario = { @@ -109,14 +112,14 @@ describe('FillOrder Tests', () => { takerScenario: TakerScenario.CorrectlySpecified, }, }; - await coreCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); + await fillOrderCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); }); it('should fill remaining value if takerAssetFillAmount > remaining takerAssetAmount', async () => { const fillScenario = { ...defaultFillScenario, takerAssetFillAmountScenario: TakerAssetFillAmountScenario.GreaterThanRemainingFillableTakerAssetAmount, }; - await coreCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); + await fillOrderCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); }); it('should throw when taker is specified and order is claimed by other', async () => { const fillScenario = { @@ -126,7 +129,7 @@ describe('FillOrder Tests', () => { takerScenario: TakerScenario.IncorrectlySpecified, }, }; - await coreCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); + await fillOrderCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); }); it('should throw if makerAssetAmount is 0', async () => { @@ -138,7 +141,7 @@ describe('FillOrder Tests', () => { }, takerAssetFillAmountScenario: TakerAssetFillAmountScenario.GreaterThanRemainingFillableTakerAssetAmount, }; - await coreCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); + await fillOrderCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); }); it('should throw if takerAssetAmount is 0', async () => { @@ -150,7 +153,7 @@ describe('FillOrder Tests', () => { }, takerAssetFillAmountScenario: TakerAssetFillAmountScenario.GreaterThanRemainingFillableTakerAssetAmount, }; - await coreCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); + await fillOrderCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); }); it('should throw if takerAssetFillAmount is 0', async () => { @@ -158,7 +161,7 @@ describe('FillOrder Tests', () => { ...defaultFillScenario, takerAssetFillAmountScenario: TakerAssetFillAmountScenario.Zero, }; - await coreCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); + await fillOrderCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); }); it('should throw if an order is expired', async () => { @@ -169,7 +172,7 @@ describe('FillOrder Tests', () => { expirationTimeSecondsScenario: ExpirationTimeSecondsScenario.InPast, }, }; - await coreCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); + await fillOrderCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); }); it('should throw if maker erc20Balances are too low to fill order', async () => { @@ -180,7 +183,7 @@ describe('FillOrder Tests', () => { traderAssetBalance: BalanceAmountScenario.TooLow, }, }; - await coreCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); + await fillOrderCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); }); it('should throw if taker erc20Balances are too low to fill order', async () => { @@ -191,7 +194,7 @@ describe('FillOrder Tests', () => { traderAssetBalance: BalanceAmountScenario.TooLow, }, }; - await coreCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); + await fillOrderCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); }); it('should throw if maker allowances are too low to fill order', async () => { @@ -202,7 +205,7 @@ describe('FillOrder Tests', () => { traderAssetAllowance: AllowanceAmountScenario.TooLow, }, }; - await coreCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); + await fillOrderCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); }); it('should throw if taker allowances are too low to fill order', async () => { @@ -213,7 +216,7 @@ describe('FillOrder Tests', () => { traderAssetAllowance: AllowanceAmountScenario.TooLow, }, }; - await coreCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); + await fillOrderCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); }); }); @@ -228,7 +231,7 @@ describe('FillOrder Tests', () => { }, takerAssetFillAmountScenario: TakerAssetFillAmountScenario.ExactlyRemainingFillableTakerAssetAmount, }; - await coreCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); + await fillOrderCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); }); it('should successfully fill order when makerAsset is ERC721 and takerAsset is ERC20', async () => { @@ -241,7 +244,7 @@ describe('FillOrder Tests', () => { }, takerAssetFillAmountScenario: TakerAssetFillAmountScenario.ExactlyRemainingFillableTakerAssetAmount, }; - await coreCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario, true); + await fillOrderCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario, true); }); it('should successfully fill order when makerAsset is ERC20 and takerAsset is ERC721', async () => { @@ -254,7 +257,7 @@ describe('FillOrder Tests', () => { }, takerAssetFillAmountScenario: TakerAssetFillAmountScenario.ExactlyRemainingFillableTakerAssetAmount, }; - await coreCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); + await fillOrderCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); }); it('should successfully fill order when makerAsset is ERC721 and approveAll is set for it', async () => { @@ -271,7 +274,7 @@ describe('FillOrder Tests', () => { traderAssetAllowance: AllowanceAmountScenario.Unlimited, }, }; - await coreCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); + await fillOrderCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); }); it('should successfully fill order when makerAsset and takerAsset are ERC721 and approveAll is set for them', async () => { @@ -292,7 +295,7 @@ describe('FillOrder Tests', () => { traderAssetAllowance: AllowanceAmountScenario.Unlimited, }, }; - await coreCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); + await fillOrderCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario); }); }); }); diff --git a/packages/contracts/test/exchange/internal.ts b/packages/contracts/test/exchange/internal.ts new file mode 100644 index 000000000..67d1d2d2c --- /dev/null +++ b/packages/contracts/test/exchange/internal.ts @@ -0,0 +1,305 @@ +import { BlockchainLifecycle } from '@0xproject/dev-utils'; +import { Order, RevertReason, SignedOrder } from '@0xproject/types'; +import { BigNumber } from '@0xproject/utils'; +import * as _ from 'lodash'; + +import { TestExchangeInternalsContract } from '../../generated_contract_wrappers/test_exchange_internals'; +import { artifacts } from '../utils/artifacts'; +import { + getInvalidOpcodeErrorMessageForCallAsync, + getRevertReasonOrErrorMessageForSendTransactionAsync, +} from '../utils/assertions'; +import { chaiSetup } from '../utils/chai_setup'; +import { bytes32Values, testCombinatoriallyWithReferenceFuncAsync, uint256Values } from '../utils/combinatorial_utils'; +import { constants } from '../utils/constants'; +import { FillResults } from '../utils/types'; +import { provider, txDefaults, web3Wrapper } from '../utils/web3_wrapper'; + +chaiSetup.configure(); +const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); + +const MAX_UINT256 = new BigNumber(2).pow(256).minus(1); + +const emptyOrder: Order = { + senderAddress: constants.NULL_ADDRESS, + makerAddress: constants.NULL_ADDRESS, + takerAddress: constants.NULL_ADDRESS, + makerFee: new BigNumber(0), + takerFee: new BigNumber(0), + makerAssetAmount: new BigNumber(0), + takerAssetAmount: new BigNumber(0), + makerAssetData: '0x', + takerAssetData: '0x', + salt: new BigNumber(0), + exchangeAddress: constants.NULL_ADDRESS, + feeRecipientAddress: constants.NULL_ADDRESS, + expirationTimeSeconds: new BigNumber(0), +}; + +const emptySignedOrder: SignedOrder = { + ...emptyOrder, + signature: '', +}; + +const overflowErrorForCall = new Error(RevertReason.Uint256Overflow); + +async function referenceGetPartialAmountAsync( + numerator: BigNumber, + denominator: BigNumber, + target: BigNumber, +): Promise<BigNumber> { + const invalidOpcodeErrorForCall = new Error(await getInvalidOpcodeErrorMessageForCallAsync()); + const product = numerator.mul(target); + if (product.greaterThan(MAX_UINT256)) { + throw overflowErrorForCall; + } + if (denominator.eq(0)) { + throw invalidOpcodeErrorForCall; + } + return product.dividedToIntegerBy(denominator); +} + +describe('Exchange core internal functions', () => { + let testExchange: TestExchangeInternalsContract; + let invalidOpcodeErrorForCall: Error | undefined; + let overflowErrorForSendTransaction: Error | undefined; + + before(async () => { + await blockchainLifecycle.startAsync(); + }); + after(async () => { + await blockchainLifecycle.revertAsync(); + }); + before(async () => { + testExchange = await TestExchangeInternalsContract.deployFrom0xArtifactAsync( + artifacts.TestExchangeInternals, + provider, + txDefaults, + ); + overflowErrorForSendTransaction = new Error( + await getRevertReasonOrErrorMessageForSendTransactionAsync(RevertReason.Uint256Overflow), + ); + invalidOpcodeErrorForCall = new Error(await getInvalidOpcodeErrorMessageForCallAsync()); + }); + // Note(albrow): Don't forget to add beforeEach and afterEach calls to reset + // the blockchain state for any tests which modify it! + + describe('addFillResults', async () => { + function makeFillResults(value: BigNumber): FillResults { + return { + makerAssetFilledAmount: value, + takerAssetFilledAmount: value, + makerFeePaid: value, + takerFeePaid: value, + }; + } + async function referenceAddFillResultsAsync( + totalValue: BigNumber, + singleValue: BigNumber, + ): Promise<FillResults> { + // Note(albrow): Here, each of totalFillResults and + // singleFillResults will consist of fields with the same values. + // This should be safe because none of the fields in a given + // FillResults are ever used together in a mathemetical operation. + // They are only used with the corresponding field from *the other* + // FillResults, which are different. + const totalFillResults = makeFillResults(totalValue); + const singleFillResults = makeFillResults(singleValue); + // HACK(albrow): _.mergeWith mutates the first argument! To + // workaround this we use _.cloneDeep. + return _.mergeWith( + _.cloneDeep(totalFillResults), + singleFillResults, + (totalVal: BigNumber, singleVal: BigNumber) => { + const newTotal = totalVal.add(singleVal); + if (newTotal.greaterThan(MAX_UINT256)) { + throw overflowErrorForCall; + } + return newTotal; + }, + ); + } + async function testAddFillResultsAsync(totalValue: BigNumber, singleValue: BigNumber): Promise<FillResults> { + const totalFillResults = makeFillResults(totalValue); + const singleFillResults = makeFillResults(singleValue); + return testExchange.publicAddFillResults.callAsync(totalFillResults, singleFillResults); + } + await testCombinatoriallyWithReferenceFuncAsync( + 'addFillResults', + referenceAddFillResultsAsync, + testAddFillResultsAsync, + [uint256Values, uint256Values], + ); + }); + + describe('calculateFillResults', async () => { + function makeOrder( + makerAssetAmount: BigNumber, + takerAssetAmount: BigNumber, + makerFee: BigNumber, + takerFee: BigNumber, + ): Order { + return { + ...emptyOrder, + makerAssetAmount, + takerAssetAmount, + makerFee, + takerFee, + }; + } + async function referenceCalculateFillResultsAsync( + orderTakerAssetAmount: BigNumber, + takerAssetFilledAmount: BigNumber, + otherAmount: BigNumber, + ): Promise<FillResults> { + // Note(albrow): Here we are re-using the same value (otherAmount) + // for order.makerAssetAmount, order.makerFee, and order.takerFee. + // This should be safe because they are never used with each other + // in any mathematical operation in either the reference TypeScript + // implementation or the Solidity implementation of + // calculateFillResults. + return { + makerAssetFilledAmount: await referenceGetPartialAmountAsync( + takerAssetFilledAmount, + orderTakerAssetAmount, + otherAmount, + ), + takerAssetFilledAmount, + makerFeePaid: await referenceGetPartialAmountAsync( + takerAssetFilledAmount, + orderTakerAssetAmount, + otherAmount, + ), + takerFeePaid: await referenceGetPartialAmountAsync( + takerAssetFilledAmount, + orderTakerAssetAmount, + otherAmount, + ), + }; + } + async function testCalculateFillResultsAsync( + orderTakerAssetAmount: BigNumber, + takerAssetFilledAmount: BigNumber, + otherAmount: BigNumber, + ): Promise<FillResults> { + const order = makeOrder(otherAmount, orderTakerAssetAmount, otherAmount, otherAmount); + return testExchange.publicCalculateFillResults.callAsync(order, takerAssetFilledAmount); + } + await testCombinatoriallyWithReferenceFuncAsync( + 'calculateFillResults', + referenceCalculateFillResultsAsync, + testCalculateFillResultsAsync, + [uint256Values, uint256Values, uint256Values], + ); + }); + + describe('getPartialAmount', async () => { + async function testGetPartialAmountAsync( + numerator: BigNumber, + denominator: BigNumber, + target: BigNumber, + ): Promise<BigNumber> { + return testExchange.publicGetPartialAmount.callAsync(numerator, denominator, target); + } + await testCombinatoriallyWithReferenceFuncAsync( + 'getPartialAmount', + referenceGetPartialAmountAsync, + testGetPartialAmountAsync, + [uint256Values, uint256Values, uint256Values], + ); + }); + + describe('isRoundingError', async () => { + async function referenceIsRoundingErrorAsync( + numerator: BigNumber, + denominator: BigNumber, + target: BigNumber, + ): Promise<boolean> { + const product = numerator.mul(target); + if (denominator.eq(0)) { + throw invalidOpcodeErrorForCall; + } + const remainder = product.mod(denominator); + if (remainder.eq(0)) { + return false; + } + if (product.greaterThan(MAX_UINT256)) { + throw overflowErrorForCall; + } + if (product.eq(0)) { + throw invalidOpcodeErrorForCall; + } + const remainderTimes1000000 = remainder.mul('1000000'); + if (remainderTimes1000000.greaterThan(MAX_UINT256)) { + throw overflowErrorForCall; + } + const errPercentageTimes1000000 = remainderTimes1000000.dividedToIntegerBy(product); + return errPercentageTimes1000000.greaterThan('1000'); + } + async function testIsRoundingErrorAsync( + numerator: BigNumber, + denominator: BigNumber, + target: BigNumber, + ): Promise<boolean> { + return testExchange.publicIsRoundingError.callAsync(numerator, denominator, target); + } + await testCombinatoriallyWithReferenceFuncAsync( + 'isRoundingError', + referenceIsRoundingErrorAsync, + testIsRoundingErrorAsync, + [uint256Values, uint256Values, uint256Values], + ); + }); + + describe('updateFilledState', async () => { + // Note(albrow): Since updateFilledState modifies the state by calling + // sendTransaction, we must reset the state after each test. + beforeEach(async () => { + await blockchainLifecycle.startAsync(); + }); + afterEach(async () => { + await blockchainLifecycle.revertAsync(); + }); + async function referenceUpdateFilledStateAsync( + takerAssetFilledAmount: BigNumber, + orderTakerAssetFilledAmount: BigNumber, + // tslint:disable-next-line:no-unused-variable + orderHash: string, + ): Promise<BigNumber> { + const totalFilledAmount = takerAssetFilledAmount.add(orderTakerAssetFilledAmount); + if (totalFilledAmount.greaterThan(MAX_UINT256)) { + throw overflowErrorForSendTransaction; + } + return totalFilledAmount; + } + async function testUpdateFilledStateAsync( + takerAssetFilledAmount: BigNumber, + orderTakerAssetFilledAmount: BigNumber, + orderHash: string, + ): Promise<BigNumber> { + const fillResults = { + makerAssetFilledAmount: new BigNumber(0), + takerAssetFilledAmount, + makerFeePaid: new BigNumber(0), + takerFeePaid: new BigNumber(0), + }; + await web3Wrapper.awaitTransactionSuccessAsync( + await testExchange.publicUpdateFilledState.sendTransactionAsync( + emptySignedOrder, + constants.NULL_ADDRESS, + orderHash, + orderTakerAssetFilledAmount, + fillResults, + ), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + return testExchange.filled.callAsync(orderHash); + } + await testCombinatoriallyWithReferenceFuncAsync( + 'updateFilledState', + referenceUpdateFilledStateAsync, + testUpdateFilledStateAsync, + [uint256Values, uint256Values, bytes32Values], + ); + }); +}); diff --git a/packages/contracts/test/exchange/libs.ts b/packages/contracts/test/exchange/libs.ts index 51794d8a3..5c9f9aac7 100644 --- a/packages/contracts/test/exchange/libs.ts +++ b/packages/contracts/test/exchange/libs.ts @@ -67,6 +67,35 @@ describe('Exchange libs', () => { }); }); }); + // Note(albrow): These tests are designed to be supplemental to the + // combinatorial tests in test/exchange/internal. They test specific edge + // cases that are not covered by the combinatorial tests. + describe('LibMath', () => { + it('should return false if there is a rounding error of 0.1%', async () => { + const numerator = new BigNumber(20); + const denominator = new BigNumber(999); + const target = new BigNumber(50); + // rounding error = ((20*50/999) - floor(20*50/999)) / (20*50/999) = 0.1% + const isRoundingError = await libs.publicIsRoundingError.callAsync(numerator, denominator, target); + expect(isRoundingError).to.be.false(); + }); + it('should return false if there is a rounding of 0.09%', async () => { + const numerator = new BigNumber(20); + const denominator = new BigNumber(9991); + const target = new BigNumber(500); + // rounding error = ((20*500/9991) - floor(20*500/9991)) / (20*500/9991) = 0.09% + const isRoundingError = await libs.publicIsRoundingError.callAsync(numerator, denominator, target); + expect(isRoundingError).to.be.false(); + }); + it('should return true if there is a rounding error of 0.11%', async () => { + const numerator = new BigNumber(20); + const denominator = new BigNumber(9989); + const target = new BigNumber(500); + // rounding error = ((20*500/9989) - floor(20*500/9989)) / (20*500/9989) = 0.011% + const isRoundingError = await libs.publicIsRoundingError.callAsync(numerator, denominator, target); + expect(isRoundingError).to.be.true(); + }); + }); describe('LibOrder', () => { describe('getOrderSchema', () => { @@ -93,96 +122,4 @@ describe('Exchange libs', () => { }); }); }); - - describe('LibMath', () => { - describe('isRoundingError', () => { - it('should return false if there is a rounding error of 0.1%', async () => { - const numerator = new BigNumber(20); - const denominator = new BigNumber(999); - const target = new BigNumber(50); - // rounding error = ((20*50/999) - floor(20*50/999)) / (20*50/999) = 0.1% - const isRoundingError = await libs.publicIsRoundingError.callAsync(numerator, denominator, target); - expect(isRoundingError).to.be.false(); - }); - - it('should return false if there is a rounding of 0.09%', async () => { - const numerator = new BigNumber(20); - const denominator = new BigNumber(9991); - const target = new BigNumber(500); - // rounding error = ((20*500/9991) - floor(20*500/9991)) / (20*500/9991) = 0.09% - const isRoundingError = await libs.publicIsRoundingError.callAsync(numerator, denominator, target); - expect(isRoundingError).to.be.false(); - }); - - it('should return true if there is a rounding error of 0.11%', async () => { - const numerator = new BigNumber(20); - const denominator = new BigNumber(9989); - const target = new BigNumber(500); - // rounding error = ((20*500/9989) - floor(20*500/9989)) / (20*500/9989) = 0.011% - const isRoundingError = await libs.publicIsRoundingError.callAsync(numerator, denominator, target); - expect(isRoundingError).to.be.true(); - }); - - it('should return true if there is a rounding error > 0.1%', async () => { - const numerator = new BigNumber(3); - const denominator = new BigNumber(7); - const target = new BigNumber(10); - // rounding error = ((3*10/7) - floor(3*10/7)) / (3*10/7) = 6.67% - const isRoundingError = await libs.publicIsRoundingError.callAsync(numerator, denominator, target); - expect(isRoundingError).to.be.true(); - }); - - it('should return false when there is no rounding error', async () => { - const numerator = new BigNumber(1); - const denominator = new BigNumber(2); - const target = new BigNumber(10); - - const isRoundingError = await libs.publicIsRoundingError.callAsync(numerator, denominator, target); - expect(isRoundingError).to.be.false(); - }); - - it('should return false when there is rounding error <= 0.1%', async () => { - // randomly generated numbers - const numerator = new BigNumber(76564); - const denominator = new BigNumber(676373677); - const target = new BigNumber(105762562); - // rounding error = ((76564*105762562/676373677) - floor(76564*105762562/676373677)) / - // (76564*105762562/676373677) = 0.0007% - const isRoundingError = await libs.publicIsRoundingError.callAsync(numerator, denominator, target); - expect(isRoundingError).to.be.false(); - }); - }); - - describe('getPartialAmount', () => { - it('should return the numerator/denominator*target', async () => { - const numerator = new BigNumber(1); - const denominator = new BigNumber(2); - const target = new BigNumber(10); - - const partialAmount = await libs.publicGetPartialAmount.callAsync(numerator, denominator, target); - const expectedPartialAmount = 5; - expect(partialAmount).to.be.bignumber.equal(expectedPartialAmount); - }); - - it('should round down', async () => { - const numerator = new BigNumber(2); - const denominator = new BigNumber(3); - const target = new BigNumber(10); - - const partialAmount = await libs.publicGetPartialAmount.callAsync(numerator, denominator, target); - const expectedPartialAmount = 6; - expect(partialAmount).to.be.bignumber.equal(expectedPartialAmount); - }); - - it('should round .5 down', async () => { - const numerator = new BigNumber(1); - const denominator = new BigNumber(20); - const target = new BigNumber(10); - - const partialAmount = await libs.publicGetPartialAmount.callAsync(numerator, denominator, target); - const expectedPartialAmount = 0; - expect(partialAmount).to.be.bignumber.equal(expectedPartialAmount); - }); - }); - }); }); diff --git a/packages/contracts/test/exchange/match_orders.ts b/packages/contracts/test/exchange/match_orders.ts index 440097562..46b3569bd 100644 --- a/packages/contracts/test/exchange/match_orders.ts +++ b/packages/contracts/test/exchange/match_orders.ts @@ -69,13 +69,22 @@ describe('matchOrders', () => { before(async () => { // Create accounts const accounts = await web3Wrapper.getAvailableAddressesAsync(); + // Hack(albrow): Both Prettier and TSLint insert a trailing comma below + // but that is invalid syntax as of TypeScript version >= 2.8. We don't + // have the right fine-grained configuration options in TSLint, + // Prettier, or TypeScript, to reconcile this, so we will just have to + // wait for them to sort it out. We disable TSLint and Prettier for + // this part of the code for now. This occurs several times in this + // file. See https://github.com/prettier/prettier/issues/4624. + // prettier-ignore const usedAddresses = ([ owner, makerAddressLeft, makerAddressRight, takerAddress, feeRecipientAddressLeft, - feeRecipientAddressRight, + // tslint:disable-next-line:trailing-comma + feeRecipientAddressRight ] = _.slice(accounts, 0, 6)); // Create wrappers erc20Wrapper = new ERC20Wrapper(provider, usedAddresses, owner); @@ -201,9 +210,11 @@ describe('matchOrders', () => { // Match signedOrderLeft with signedOrderRight let newERC20BalancesByOwner: ERC20BalancesByOwner; let newERC721TokenIdsByOwner: ERC721TokenIdsByOwner; + // prettier-ignore [ newERC20BalancesByOwner, - newERC721TokenIdsByOwner, + // tslint:disable-next-line:trailing-comma + newERC721TokenIdsByOwner ] = await matchOrderTester.matchOrdersAndVerifyBalancesAsync( signedOrderLeft, signedOrderRight, @@ -306,9 +317,11 @@ describe('matchOrders', () => { // Match orders let newERC20BalancesByOwner: ERC20BalancesByOwner; let newERC721TokenIdsByOwner: ERC721TokenIdsByOwner; + // prettier-ignore [ newERC20BalancesByOwner, - newERC721TokenIdsByOwner, + // tslint:disable-next-line:trailing-comma + newERC721TokenIdsByOwner ] = await matchOrderTester.matchOrdersAndVerifyBalancesAsync( signedOrderLeft, signedOrderRight, @@ -374,9 +387,11 @@ describe('matchOrders', () => { // Match orders let newERC20BalancesByOwner: ERC20BalancesByOwner; let newERC721TokenIdsByOwner: ERC721TokenIdsByOwner; + // prettier-ignore [ newERC20BalancesByOwner, - newERC721TokenIdsByOwner, + // tslint:disable-next-line:trailing-comma + newERC721TokenIdsByOwner ] = await matchOrderTester.matchOrdersAndVerifyBalancesAsync( signedOrderLeft, signedOrderRight, diff --git a/packages/contracts/test/exchange/signature_validator.ts b/packages/contracts/test/exchange/signature_validator.ts index f2bb42c75..56a06c247 100644 --- a/packages/contracts/test/exchange/signature_validator.ts +++ b/packages/contracts/test/exchange/signature_validator.ts @@ -1,6 +1,6 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; -import { addSignedMessagePrefix, assetDataUtils, MessagePrefixType, orderHashUtils } from '@0xproject/order-utils'; -import { RevertReason, SignatureType, SignedOrder } from '@0xproject/types'; +import { addSignedMessagePrefix, assetDataUtils, orderHashUtils } from '@0xproject/order-utils'; +import { RevertReason, SignatureType, SignedOrder, SignerType } from '@0xproject/types'; import * as chai from 'chai'; import { LogWithDecodedArgs } from 'ethereum-types'; import ethUtil = require('ethereumjs-util'); @@ -113,7 +113,7 @@ describe('MixinSignatureValidator', () => { it('should revert when signature type is unsupported', async () => { const unsupportedSignatureType = SignatureType.NSignatureTypes; - const unsupportedSignatureHex = `0x${unsupportedSignatureType}`; + const unsupportedSignatureHex = '0x' + Buffer.from([unsupportedSignatureType]).toString('hex'); const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); return expectContractCallFailed( signatureValidator.publicIsValidSignature.callAsync( @@ -126,7 +126,7 @@ describe('MixinSignatureValidator', () => { }); it('should revert when SignatureType=Illegal', async () => { - const unsupportedSignatureHex = `0x${SignatureType.Illegal}`; + const unsupportedSignatureHex = '0x' + Buffer.from([SignatureType.Illegal]).toString('hex'); const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); return expectContractCallFailed( signatureValidator.publicIsValidSignature.callAsync( @@ -139,7 +139,7 @@ describe('MixinSignatureValidator', () => { }); it('should return false when SignatureType=Invalid and signature has a length of zero', async () => { - const signatureHex = `0x${SignatureType.Invalid}`; + const signatureHex = '0x' + Buffer.from([SignatureType.Invalid]).toString('hex'); const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( orderHashHex, @@ -213,7 +213,7 @@ describe('MixinSignatureValidator', () => { it('should return true when SignatureType=EthSign and signature is valid', async () => { // Create EthSign signature const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - const orderHashWithEthSignPrefixHex = addSignedMessagePrefix(orderHashHex, MessagePrefixType.EthSign); + const orderHashWithEthSignPrefixHex = addSignedMessagePrefix(orderHashHex, SignerType.Default); const orderHashWithEthSignPrefixBuffer = ethUtil.toBuffer(orderHashWithEthSignPrefixHex); const ecSignature = ethUtil.ecsign(orderHashWithEthSignPrefixBuffer, signerPrivateKey); // Create 0x signature from EthSign signature @@ -236,7 +236,7 @@ describe('MixinSignatureValidator', () => { it('should return false when SignatureType=EthSign and signature is invalid', async () => { // Create EthSign signature const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - const orderHashWithEthSignPrefixHex = addSignedMessagePrefix(orderHashHex, MessagePrefixType.EthSign); + const orderHashWithEthSignPrefixHex = addSignedMessagePrefix(orderHashHex, SignerType.Default); const orderHashWithEthSignPrefixBuffer = ethUtil.toBuffer(orderHashWithEthSignPrefixHex); const ecSignature = ethUtil.ecsign(orderHashWithEthSignPrefixBuffer, signerPrivateKey); // Create 0x signature from EthSign signature @@ -385,7 +385,7 @@ describe('MixinSignatureValidator', () => { it('should return true when SignatureType=Trezor and signature is valid', async () => { // Create Trezor signature const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - const orderHashWithTrezorPrefixHex = addSignedMessagePrefix(orderHashHex, MessagePrefixType.Trezor); + const orderHashWithTrezorPrefixHex = addSignedMessagePrefix(orderHashHex, SignerType.Trezor); const orderHashWithTrezorPrefixBuffer = ethUtil.toBuffer(orderHashWithTrezorPrefixHex); const ecSignature = ethUtil.ecsign(orderHashWithTrezorPrefixBuffer, signerPrivateKey); // Create 0x signature from Trezor signature @@ -408,7 +408,7 @@ describe('MixinSignatureValidator', () => { it('should return false when SignatureType=Trezor and signature is invalid', async () => { // Create Trezor signature const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - const orderHashWithTrezorPrefixHex = addSignedMessagePrefix(orderHashHex, MessagePrefixType.Trezor); + const orderHashWithTrezorPrefixHex = addSignedMessagePrefix(orderHashHex, SignerType.Trezor); const orderHashWithTrezorPrefixBuffer = ethUtil.toBuffer(orderHashWithTrezorPrefixHex); const ecSignature = ethUtil.ecsign(orderHashWithTrezorPrefixBuffer, signerPrivateKey); // Create 0x signature from Trezor signature diff --git a/packages/contracts/test/forwarder/forwarder.ts b/packages/contracts/test/forwarder/forwarder.ts index 19639d3aa..28ffdeabe 100644 --- a/packages/contracts/test/forwarder/forwarder.ts +++ b/packages/contracts/test/forwarder/forwarder.ts @@ -36,6 +36,7 @@ describe(ContractName.Forwarder, () => { let feeRecipientAddress: string; let otherAddress: string; let defaultMakerAssetAddress: string; + let zrxAssetData: string; let weth: DummyERC20TokenContract; let zrxToken: DummyERC20TokenContract; @@ -90,7 +91,7 @@ describe(ContractName.Forwarder, () => { erc20Wrapper.addDummyTokenContract(weth); const wethAssetData = assetDataUtils.encodeERC20AssetData(wethContract.address); - const zrxAssetData = assetDataUtils.encodeERC20AssetData(zrxToken.address); + zrxAssetData = assetDataUtils.encodeERC20AssetData(zrxToken.address); const exchangeInstance = await ExchangeContract.deployFrom0xArtifactAsync( artifacts.Exchange, provider, @@ -722,25 +723,18 @@ describe(ContractName.Forwarder, () => { ); expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); }); - it('should not change balances if the amount of ETH sent is too low to fill the makerAssetAmount', async () => { + it('should revert if the amount of ETH sent is too low to fill the makerAssetAmount', async () => { const ordersWithoutFee = [orderWithoutFee]; const feeOrders: SignedOrder[] = []; const makerAssetFillAmount = orderWithoutFee.makerAssetAmount.dividedToIntegerBy(2); const ethValue = orderWithoutFee.takerAssetAmount.dividedToIntegerBy(4); - - tx = await forwarderWrapper.marketBuyOrdersWithEthAsync(ordersWithoutFee, feeOrders, makerAssetFillAmount, { - value: ethValue, - from: takerAddress, - }); - const takerEthBalanceAfter = await web3Wrapper.getBalanceInWeiAsync(takerAddress); - const forwarderEthBalance = await web3Wrapper.getBalanceInWeiAsync(forwarderContract.address); - const newBalances = await erc20Wrapper.getBalancesAsync(); - - const totalEthSpent = gasPrice.times(tx.gasUsed); - - expect(takerEthBalanceAfter).to.be.bignumber.equal(takerEthBalanceBefore.minus(totalEthSpent)); - expect(newBalances).to.deep.equal(erc20Balances); - expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); + return expectTransactionFailedAsync( + forwarderWrapper.marketBuyOrdersWithEthAsync(ordersWithoutFee, feeOrders, makerAssetFillAmount, { + value: ethValue, + from: takerAddress, + }), + RevertReason.CompleteFillFailed, + ); }); it('should buy an ERC721 asset from a single order', async () => { const makerAssetId = erc721MakerAssetIds[0]; @@ -775,7 +769,7 @@ describe(ContractName.Forwarder, () => { ); expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); }); - it('should buy an ERC721 asset and ignore later orders with different makerAssetData', async () => { + it('should revert if buying an ERC721 asset when later orders contain different makerAssetData', async () => { const makerAssetId = erc721MakerAssetIds[0]; orderWithoutFee = await orderFactory.newSignedOrderAsync({ makerAssetAmount: new BigNumber(1), @@ -786,33 +780,12 @@ describe(ContractName.Forwarder, () => { const feeOrders: SignedOrder[] = []; const makerAssetFillAmount = new BigNumber(1).plus(differentMakerAssetDataOrder.makerAssetAmount); const ethValue = orderWithFee.takerAssetAmount; - - tx = await forwarderWrapper.marketBuyOrdersWithEthAsync(ordersWithoutFee, feeOrders, makerAssetFillAmount, { - from: takerAddress, - value: ethValue, - }); - const takerEthBalanceAfter = await web3Wrapper.getBalanceInWeiAsync(takerAddress); - const forwarderEthBalance = await web3Wrapper.getBalanceInWeiAsync(forwarderContract.address); - const newOwner = await erc721Token.ownerOf.callAsync(makerAssetId); - const newBalances = await erc20Wrapper.getBalancesAsync(); - - const primaryTakerAssetFillAmount = ethValue; - const totalEthSpent = primaryTakerAssetFillAmount.plus(gasPrice.times(tx.gasUsed)); - expect(newOwner).to.be.bignumber.equal(takerAddress); - expect(takerEthBalanceAfter).to.be.bignumber.equal(takerEthBalanceBefore.minus(totalEthSpent)); - expect(newBalances[makerAddress][weth.address]).to.be.bignumber.equal( - erc20Balances[makerAddress][weth.address].plus(primaryTakerAssetFillAmount), - ); - expect(newBalances[forwarderContract.address][weth.address]).to.be.bignumber.equal(constants.ZERO_AMOUNT); - expect(newBalances[forwarderContract.address][defaultMakerAssetAddress]).to.be.bignumber.equal( - constants.ZERO_AMOUNT, - ); - expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); - expect(newBalances[makerAddress][defaultMakerAssetAddress]).to.be.bignumber.equal( - erc20Balances[makerAddress][defaultMakerAssetAddress], - ); - expect(newBalances[takerAddress][defaultMakerAssetAddress]).to.be.bignumber.equal( - erc20Balances[takerAddress][defaultMakerAssetAddress], + return expectTransactionFailedAsync( + forwarderWrapper.marketBuyOrdersWithEthAsync(ordersWithoutFee, feeOrders, makerAssetFillAmount, { + value: ethValue, + from: takerAddress, + }), + RevertReason.CompleteFillFailed, ); }); it('should buy an ERC721 asset and pay ZRX fees from a single fee order', async () => { @@ -998,6 +971,26 @@ describe(ContractName.Forwarder, () => { ); }); }); + describe('withdrawAsset', () => { + it('should allow owner to withdraw ERC20 tokens', async () => { + const zrxWithdrawAmount = erc20Balances[forwarderContract.address][zrxToken.address]; + await forwarderWrapper.withdrawAssetAsync(zrxAssetData, zrxWithdrawAmount, { from: owner }); + const newBalances = await erc20Wrapper.getBalancesAsync(); + expect(newBalances[owner][zrxToken.address]).to.be.bignumber.equal( + erc20Balances[owner][zrxToken.address].plus(zrxWithdrawAmount), + ); + expect(newBalances[forwarderContract.address][zrxToken.address]).to.be.bignumber.equal( + erc20Balances[forwarderContract.address][zrxToken.address].minus(zrxWithdrawAmount), + ); + }); + it('should revert if not called by owner', async () => { + const zrxWithdrawAmount = erc20Balances[forwarderContract.address][zrxToken.address]; + await expectTransactionFailedAsync( + forwarderWrapper.withdrawAssetAsync(zrxAssetData, zrxWithdrawAmount, { from: makerAddress }), + RevertReason.OnlyContractOwner, + ); + }); + }); }); // tslint:disable:max-file-line-count // tslint:enable:no-unnecessary-type-assertion diff --git a/packages/contracts/test/utils/artifacts.ts b/packages/contracts/test/utils/artifacts.ts index 63bd555a4..e608ee174 100644 --- a/packages/contracts/test/utils/artifacts.ts +++ b/packages/contracts/test/utils/artifacts.ts @@ -16,6 +16,7 @@ import * as MultiSigWalletWithTimeLock from '../../artifacts/MultiSigWalletWithT import * as TestAssetProxyDispatcher from '../../artifacts/TestAssetProxyDispatcher.json'; import * as TestAssetProxyOwner from '../../artifacts/TestAssetProxyOwner.json'; import * as TestConstants from '../../artifacts/TestConstants.json'; +import * as TestExchangeInternals from '../../artifacts/TestExchangeInternals.json'; import * as TestLibBytes from '../../artifacts/TestLibBytes.json'; import * as TestLibs from '../../artifacts/TestLibs.json'; import * as TestSignatureValidator from '../../artifacts/TestSignatureValidator.json'; @@ -46,6 +47,7 @@ export const artifacts = { TestConstants: (TestConstants as any) as ContractArtifact, TestLibBytes: (TestLibBytes as any) as ContractArtifact, TestLibs: (TestLibs as any) as ContractArtifact, + TestExchangeInternals: (TestExchangeInternals as any) as ContractArtifact, TestSignatureValidator: (TestSignatureValidator as any) as ContractArtifact, Validator: (Validator as any) as ContractArtifact, Wallet: (Wallet as any) as ContractArtifact, diff --git a/packages/contracts/test/utils/assertions.ts b/packages/contracts/test/utils/assertions.ts index 112a470f6..61df800c8 100644 --- a/packages/contracts/test/utils/assertions.ts +++ b/packages/contracts/test/utils/assertions.ts @@ -15,6 +15,14 @@ let nodeType: NodeType | undefined; // resolve with either a transaction receipt or a transaction hash. export type sendTransactionResult = Promise<TransactionReceipt | TransactionReceiptWithDecodedLogs | string>; +/** + * Returns ganacheError if the backing Ethereum node is Ganache and gethError + * if it is Geth. + * @param ganacheError the error to be returned if the backing node is Ganache. + * @param gethError the error to be returned if the backing node is Geth. + * @returns either the given ganacheError or gethError depending on the backing + * node. + */ async function _getGanacheOrGethError(ganacheError: string, gethError: string): Promise<string> { if (_.isUndefined(nodeType)) { nodeType = await web3Wrapper.getNodeTypeAsync(); @@ -42,6 +50,25 @@ async function _getContractCallFailedErrorMessageAsync(): Promise<string> { } /** + * Returns the expected error message for an 'invalid opcode' resulting from a + * contract call. The exact error message depends on the backing Ethereum node. + */ +export async function getInvalidOpcodeErrorMessageForCallAsync(): Promise<string> { + return _getGanacheOrGethError('invalid opcode', 'Contract call failed'); +} + +/** + * Returns the expected error message for the given revert reason resulting from + * a sendTransaction call. The exact error message depends on the backing + * Ethereum node and whether it supports revert reasons. + * @param reason a specific revert reason. + * @returns the expected error message. + */ +export async function getRevertReasonOrErrorMessageForSendTransactionAsync(reason: RevertReason): Promise<string> { + return _getGanacheOrGethError(reason, 'always failing transaction'); +} + +/** * Rejects if the given Promise does not reject with an error indicating * insufficient funds. * @param p a promise resulting from a contract call or sendTransaction call. diff --git a/packages/contracts/test/utils/combinatorial_utils.ts b/packages/contracts/test/utils/combinatorial_utils.ts new file mode 100644 index 000000000..d72b41f8a --- /dev/null +++ b/packages/contracts/test/utils/combinatorial_utils.ts @@ -0,0 +1,113 @@ +import { BigNumber } from '@0xproject/utils'; +import * as combinatorics from 'js-combinatorics'; + +import { testWithReferenceFuncAsync } from './test_with_reference'; + +// A set of values corresponding to the uint256 type in Solidity. This set +// contains some notable edge cases, including some values which will overflow +// the uint256 type when used in different mathematical operations. +export const uint256Values = [ + new BigNumber(0), + new BigNumber(1), + new BigNumber(2), + // Non-trivial big number. + new BigNumber(2).pow(64), + // Max that does not overflow when squared. + new BigNumber(2).pow(128).minus(1), + // Min that does overflow when squared. + new BigNumber(2).pow(128), + // Max that does not overflow when doubled. + new BigNumber(2).pow(255).minus(1), + // Min that does overflow when doubled. + new BigNumber(2).pow(255), + // Max that does not overflow. + new BigNumber(2).pow(256).minus(1), +]; + +// A set of values corresponding to the bytes32 type in Solidity. +export const bytes32Values = [ + // Min + '0x0000000000000000000000000000000000000000000000000000000000000000', + '0x0000000000000000000000000000000000000000000000000000000000000001', + '0x0000000000000000000000000000000000000000000000000000000000000002', + // Non-trivial big number. + '0x000000000000f000000000000000000000000000000000000000000000000000', + // Max + '0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff', +]; + +export async function testCombinatoriallyWithReferenceFuncAsync<P0, P1, R>( + name: string, + referenceFunc: (p0: P0, p1: P1) => Promise<R>, + testFunc: (p0: P0, p1: P1) => Promise<R>, + allValues: [P0[], P1[]], +): Promise<void>; +export async function testCombinatoriallyWithReferenceFuncAsync<P0, P1, P2, R>( + name: string, + referenceFunc: (p0: P0, p1: P1, p2: P2) => Promise<R>, + testFunc: (p0: P0, p1: P1, p2: P2) => Promise<R>, + allValues: [P0[], P1[], P2[]], +): Promise<void>; +export async function testCombinatoriallyWithReferenceFuncAsync<P0, P1, P2, P3, R>( + name: string, + referenceFunc: (p0: P0, p1: P1, p2: P2, p3: P3) => Promise<R>, + testFunc: (p0: P0, p1: P1, p2: P2, p3: P3) => Promise<R>, + allValues: [P0[], P1[], P2[], P3[]], +): Promise<void>; +export async function testCombinatoriallyWithReferenceFuncAsync<P0, P1, P2, P3, P4, R>( + name: string, + referenceFunc: (p0: P0, p1: P1, p2: P2, p3: P3, p4: P4) => Promise<R>, + testFunc: (p0: P0, p1: P1, p2: P2, p3: P3, p4: P4) => Promise<R>, + allValues: [P0[], P1[], P2[], P3[], P4[]], +): Promise<void>; + +/** + * Uses combinatorics to test the behavior of a test function by comparing it to + * the expected behavior (defined by a reference function) for a large number of + * possible input values. + * + * First generates test cases by taking the cartesian product of the given + * values. Each test case is a set of N values corresponding to the N arguments + * for the test func and the reference func. For each test case, first the + * reference function will be called to obtain an "expected result", or if the + * reference function throws/rejects, an "expected error". Next, the test + * function will be called to obtain an "actual result", or if the test function + * throws/rejects, an "actual error". Each test case passes if at least one of + * the following conditions is met: + * + * 1) Neither the reference function or the test function throw and the + * "expected result" equals the "actual result". + * + * 2) Both the reference function and the test function throw and the "actual + * error" message *contains* the "expected error" message. + * + * The first test case which does not meet one of these conditions will cause + * the entire test to fail and this function will throw/reject. + * + * @param referenceFuncAsync a reference function implemented in pure + * JavaScript/TypeScript which accepts N arguments and returns the "expected + * result" or "expected error" for a given test case. + * @param testFuncAsync a test function which, e.g., makes a call or sends a + * transaction to a contract. It accepts the same N arguments returns the + * "actual result" or "actual error" for a given test case. + * @param values an array of N arrays. Each inner array is a set of possible + * values which are passed into both the reference function and the test + * function. + * @return A Promise that resolves if the test passes and rejects if the test + * fails, according to the rules described above. + */ +export async function testCombinatoriallyWithReferenceFuncAsync( + name: string, + referenceFuncAsync: (...args: any[]) => Promise<any>, + testFuncAsync: (...args: any[]) => Promise<any>, + allValues: any[], +): Promise<void> { + const testCases = combinatorics.cartesianProduct(...allValues); + let counter = 0; + testCases.forEach(async testCase => { + counter += 1; + it(`${name} ${counter}/${testCases.length}`, async () => { + await testWithReferenceFuncAsync(referenceFuncAsync, testFuncAsync, testCase as any); + }); + }); +} diff --git a/packages/contracts/test/utils/exchange_wrapper.ts b/packages/contracts/test/utils/exchange_wrapper.ts index 490ea959b..d57592d6d 100644 --- a/packages/contracts/test/utils/exchange_wrapper.ts +++ b/packages/contracts/test/utils/exchange_wrapper.ts @@ -8,7 +8,7 @@ import { ExchangeContract } from '../../generated_contract_wrappers/exchange'; import { formatters } from './formatters'; import { LogDecoder } from './log_decoder'; import { orderUtils } from './order_utils'; -import { OrderInfo, SignedTransaction } from './types'; +import { FillResults, OrderInfo, SignedTransaction } from './types'; export class ExchangeWrapper { private readonly _exchange: ExchangeContract; @@ -243,4 +243,27 @@ export class ExchangeWrapper { const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; } + public async getFillOrderResultsAsync( + signedOrder: SignedOrder, + from: string, + opts: { takerAssetFillAmount?: BigNumber } = {}, + ): Promise<FillResults> { + const params = orderUtils.createFill(signedOrder, opts.takerAssetFillAmount); + const fillResults = await this._exchange.fillOrder.callAsync( + params.order, + params.takerAssetFillAmount, + params.signature, + { from }, + ); + return fillResults; + } + public abiEncodeFillOrder(signedOrder: SignedOrder, opts: { takerAssetFillAmount?: BigNumber } = {}): string { + const params = orderUtils.createFill(signedOrder, opts.takerAssetFillAmount); + const data = this._exchange.fillOrder.getABIEncodedTransactionData( + params.order, + params.takerAssetFillAmount, + params.signature, + ); + return data; + } } diff --git a/packages/contracts/test/utils/core_combinatorial_utils.ts b/packages/contracts/test/utils/fill_order_combinatorial_utils.ts index 44a5199c0..284c4a2db 100644 --- a/packages/contracts/test/utils/core_combinatorial_utils.ts +++ b/packages/contracts/test/utils/fill_order_combinatorial_utils.ts @@ -15,6 +15,7 @@ import * as _ from 'lodash'; import 'make-promises-safe'; import { ExchangeContract, ExchangeFillEventArgs } from '../../generated_contract_wrappers/exchange'; +import { TestLibsContract } from '../../generated_contract_wrappers/test_libs'; import { artifacts } from './artifacts'; import { expectTransactionFailedAsync } from './assertions'; @@ -46,16 +47,16 @@ chaiSetup.configure(); const expect = chai.expect; /** - * Instantiates a new instance of CoreCombinatorialUtils. Since this method has some + * Instantiates a new instance of FillOrderCombinatorialUtils. Since this method has some * required async setup, a factory method is required. * @param web3Wrapper Web3Wrapper instance * @param txDefaults Default Ethereum tx options - * @return CoreCombinatorialUtils instance + * @return FillOrderCombinatorialUtils instance */ -export async function coreCombinatorialUtilsFactoryAsync( +export async function fillOrderCombinatorialUtilsFactoryAsync( web3Wrapper: Web3Wrapper, txDefaults: Partial<TxData>, -): Promise<CoreCombinatorialUtils> { +): Promise<FillOrderCombinatorialUtils> { const accounts = await web3Wrapper.getAvailableAddressesAsync(); const userAddresses = _.slice(accounts, 0, 5); const [ownerAddress, makerAddress, takerAddress] = userAddresses; @@ -123,7 +124,9 @@ export async function coreCombinatorialUtilsFactoryAsync( exchangeContract.address, ); - const coreCombinatorialUtils = new CoreCombinatorialUtils( + const testLibsContract = await TestLibsContract.deployFrom0xArtifactAsync(artifacts.TestLibs, provider, txDefaults); + + const fillOrderCombinatorialUtils = new FillOrderCombinatorialUtils( orderFactory, ownerAddress, makerAddress, @@ -132,11 +135,12 @@ export async function coreCombinatorialUtilsFactoryAsync( zrxAssetData, exchangeWrapper, assetWrapper, + testLibsContract, ); - return coreCombinatorialUtils; + return fillOrderCombinatorialUtils; } -export class CoreCombinatorialUtils { +export class FillOrderCombinatorialUtils { public orderFactory: OrderFactoryFromScenario; public ownerAddress: string; public makerAddress: string; @@ -145,6 +149,7 @@ export class CoreCombinatorialUtils { public zrxAssetData: string; public exchangeWrapper: ExchangeWrapper; public assetWrapper: AssetWrapper; + public testLibsContract: TestLibsContract; public static generateFillOrderCombinations(): FillScenario[] { const takerScenarios = [ TakerScenario.Unspecified, @@ -240,7 +245,7 @@ export class CoreCombinatorialUtils { // AllowanceAmountScenario.TooLow, // AllowanceAmountScenario.Unlimited, ]; - const fillScenarioArrays = CoreCombinatorialUtils._getAllCombinations([ + const fillScenarioArrays = FillOrderCombinatorialUtils._getAllCombinations([ takerScenarios, feeRecipientScenarios, makerAssetAmountScenario, @@ -309,7 +314,7 @@ export class CoreCombinatorialUtils { } else { const result = []; const restOfArrays = arrays.slice(1); - const allCombinationsOfRemaining = CoreCombinatorialUtils._getAllCombinations(restOfArrays); // recur with the rest of array + const allCombinationsOfRemaining = FillOrderCombinatorialUtils._getAllCombinations(restOfArrays); // recur with the rest of array // tslint:disable:prefer-for-of for (let i = 0; i < allCombinationsOfRemaining.length; i++) { for (let j = 0; j < arrays[0].length; j++) { @@ -329,6 +334,7 @@ export class CoreCombinatorialUtils { zrxAssetData: string, exchangeWrapper: ExchangeWrapper, assetWrapper: AssetWrapper, + testLibsContract: TestLibsContract, ) { this.orderFactory = orderFactory; this.ownerAddress = ownerAddress; @@ -338,6 +344,7 @@ export class CoreCombinatorialUtils { this.zrxAssetData = zrxAssetData; this.exchangeWrapper = exchangeWrapper; this.assetWrapper = assetWrapper; + this.testLibsContract = testLibsContract; } public async testFillOrderScenarioAsync( provider: Provider, @@ -410,6 +417,8 @@ export class CoreCombinatorialUtils { lazyStore, fillRevertReasonIfExists, ); + + await this._abiEncodeFillOrderAndAssertOutcomeAsync(signedOrder, takerAssetFillAmount); } private async _fillOrderAndAssertOutcomeAsync( signedOrder: SignedOrder, @@ -456,6 +465,29 @@ export class CoreCombinatorialUtils { signedOrder.takerAssetAmount, signedOrder.makerAssetAmount, ); + const expMakerFeePaid = orderUtils.getPartialAmount( + expFilledTakerAmount, + signedOrder.takerAssetAmount, + signedOrder.makerFee, + ); + const expTakerFeePaid = orderUtils.getPartialAmount( + expFilledTakerAmount, + signedOrder.takerAssetAmount, + signedOrder.takerFee, + ); + const fillResults = await this.exchangeWrapper.getFillOrderResultsAsync(signedOrder, this.takerAddress, { + takerAssetFillAmount, + }); + expect(fillResults.takerAssetFilledAmount).to.be.bignumber.equal( + expFilledTakerAmount, + 'takerAssetFilledAmount', + ); + expect(fillResults.makerAssetFilledAmount).to.be.bignumber.equal( + expFilledMakerAmount, + 'makerAssetFilledAmount', + ); + expect(fillResults.takerFeePaid).to.be.bignumber.equal(expTakerFeePaid, 'takerFeePaid'); + expect(fillResults.makerFeePaid).to.be.bignumber.equal(expMakerFeePaid, 'makerFeePaid'); // - Let's fill the order! const txReceipt = await this.exchangeWrapper.fillOrderAsync(signedOrder, this.takerAddress, { @@ -479,17 +511,7 @@ export class CoreCombinatorialUtils { expFilledTakerAmount, 'log.args.takerAssetFilledAmount', ); - const expMakerFeePaid = orderUtils.getPartialAmount( - expFilledTakerAmount, - signedOrder.takerAssetAmount, - signedOrder.makerFee, - ); expect(log.args.makerFeePaid).to.be.bignumber.equal(expMakerFeePaid, 'log.args.makerFeePaid'); - const expTakerFeePaid = orderUtils.getPartialAmount( - expFilledTakerAmount, - signedOrder.takerAssetAmount, - signedOrder.takerFee, - ); expect(log.args.takerFeePaid).to.be.bignumber.equal(expTakerFeePaid, 'logs.args.takerFeePaid'); expect(log.args.orderHash).to.be.equal(orderHash, 'log.args.orderHash'); expect(log.args.makerAssetData).to.be.equal(makerAssetData, 'log.args.makerAssetData'); @@ -571,6 +593,19 @@ export class CoreCombinatorialUtils { 'ZRXAssetBalanceOfFeeRecipient', ); } + private async _abiEncodeFillOrderAndAssertOutcomeAsync( + signedOrder: SignedOrder, + takerAssetFillAmount: BigNumber, + ): Promise<void> { + const params = orderUtils.createFill(signedOrder, takerAssetFillAmount); + const expectedAbiEncodedData = this.exchangeWrapper.abiEncodeFillOrder(signedOrder, { takerAssetFillAmount }); + const libsAbiEncodedData = await this.testLibsContract.publicAbiEncodeFillOrder.callAsync( + params.order, + params.takerAssetFillAmount, + params.signature, + ); + expect(libsAbiEncodedData).to.be.equal(expectedAbiEncodedData, 'ABIEncodedFillOrderData'); + } private async _getTakerAssetFillAmountAsync( signedOrder: SignedOrder, takerAssetFillAmountScenario: TakerAssetFillAmountScenario, diff --git a/packages/contracts/test/utils/forwarder_wrapper.ts b/packages/contracts/test/utils/forwarder_wrapper.ts index ef7476e36..5b9a63ddf 100644 --- a/packages/contracts/test/utils/forwarder_wrapper.ts +++ b/packages/contracts/test/utils/forwarder_wrapper.ts @@ -106,12 +106,12 @@ export class ForwarderWrapper { const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; } - public async withdrawERC20Async( - tokenAddress: string, + public async withdrawAssetAsync( + assetData: string, amount: BigNumber, txData: TxDataPayable, ): Promise<TransactionReceiptWithDecodedLogs> { - const txHash = await this._forwarderContract.withdrawERC20.sendTransactionAsync(tokenAddress, amount, txData); + const txHash = await this._forwarderContract.withdrawAsset.sendTransactionAsync(assetData, amount, txData); const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; } diff --git a/packages/contracts/test/utils/test_with_reference.ts b/packages/contracts/test/utils/test_with_reference.ts new file mode 100644 index 000000000..599b1eed4 --- /dev/null +++ b/packages/contracts/test/utils/test_with_reference.ts @@ -0,0 +1,119 @@ +import * as chai from 'chai'; +import * as _ from 'lodash'; + +import { chaiSetup } from './chai_setup'; + +chaiSetup.configure(); +const expect = chai.expect; + +export async function testWithReferenceFuncAsync<P0, R>( + referenceFunc: (p0: P0) => Promise<R>, + testFunc: (p0: P0) => Promise<R>, + values: [P0], +): Promise<void>; +export async function testWithReferenceFuncAsync<P0, P1, R>( + referenceFunc: (p0: P0, p1: P1) => Promise<R>, + testFunc: (p0: P0, p1: P1) => Promise<R>, + values: [P0, P1], +): Promise<void>; +export async function testWithReferenceFuncAsync<P0, P1, P2, R>( + referenceFunc: (p0: P0, p1: P1, p2: P2) => Promise<R>, + testFunc: (p0: P0, p1: P1, p2: P2) => Promise<R>, + values: [P0, P1, P2], +): Promise<void>; +export async function testWithReferenceFuncAsync<P0, P1, P2, P3, R>( + referenceFunc: (p0: P0, p1: P1, p2: P2, p3: P3) => Promise<R>, + testFunc: (p0: P0, p1: P1, p2: P2, p3: P3) => Promise<R>, + values: [P0, P1, P2, P3], +): Promise<void>; +export async function testWithReferenceFuncAsync<P0, P1, P2, P3, P4, R>( + referenceFunc: (p0: P0, p1: P1, p2: P2, p3: P3, p4: P4) => Promise<R>, + testFunc: (p0: P0, p1: P1, p2: P2, p3: P3, p4: P4) => Promise<R>, + values: [P0, P1, P2, P3, P4], +): Promise<void>; + +/** + * Tests the behavior of a test function by comparing it to the expected + * behavior (defined by a reference function). + * + * First the reference function will be called to obtain an "expected result", + * or if the reference function throws/rejects, an "expected error". Next, the + * test function will be called to obtain an "actual result", or if the test + * function throws/rejects, an "actual error". The test passes if at least one + * of the following conditions is met: + * + * 1) Neither the reference function or the test function throw and the + * "expected result" equals the "actual result". + * + * 2) Both the reference function and the test function throw and the "actual + * error" message *contains* the "expected error" message. + * + * @param referenceFuncAsync a reference function implemented in pure + * JavaScript/TypeScript which accepts N arguments and returns the "expected + * result" or throws/rejects with the "expected error". + * @param testFuncAsync a test function which, e.g., makes a call or sends a + * transaction to a contract. It accepts the same N arguments returns the + * "actual result" or throws/rejects with the "actual error". + * @param values an array of N values, where each value corresponds in-order to + * an argument to both the test function and the reference function. + * @return A Promise that resolves if the test passes and rejects if the test + * fails, according to the rules described above. + */ +export async function testWithReferenceFuncAsync( + referenceFuncAsync: (...args: any[]) => Promise<any>, + testFuncAsync: (...args: any[]) => Promise<any>, + values: any[], +): Promise<void> { + let expectedResult: any; + let expectedErr: string | undefined; + try { + expectedResult = await referenceFuncAsync(...values); + } catch (e) { + expectedErr = e.message; + } + let actualResult: any | undefined; + try { + actualResult = await testFuncAsync(...values); + if (!_.isUndefined(expectedErr)) { + throw new Error( + `Expected error containing ${expectedErr} but got no error\n\tTest case: ${_getTestCaseString( + referenceFuncAsync, + values, + )}`, + ); + } + } catch (e) { + if (_.isUndefined(expectedErr)) { + throw new Error(`${e.message}\n\tTest case: ${_getTestCaseString(referenceFuncAsync, values)}`); + } else { + expect(e.message).to.contain( + expectedErr, + `${e.message}\n\tTest case: ${_getTestCaseString(referenceFuncAsync, values)}`, + ); + } + } + if (!_.isUndefined(actualResult) && !_.isUndefined(expectedResult)) { + expect(actualResult).to.deep.equal( + expectedResult, + `Test case: ${_getTestCaseString(referenceFuncAsync, values)}`, + ); + } +} + +function _getTestCaseString(referenceFuncAsync: (...args: any[]) => Promise<any>, values: any[]): string { + const paramNames = _getParameterNames(referenceFuncAsync); + return JSON.stringify(_.zipObject(paramNames, values)); +} + +// Source: https://stackoverflow.com/questions/1007981/how-to-get-function-parameter-names-values-dynamically +function _getParameterNames(func: (...args: any[]) => any): string[] { + return _.toString(func) + .replace(/[/][/].*$/gm, '') // strip single-line comments + .replace(/\s+/g, '') // strip white space + .replace(/[/][*][^/*]*[*][/]/g, '') // strip multi-line comments + .split('){', 1)[0] + .replace(/^[^(]*[(]/, '') // extract the parameters + .replace(/=[^,]+/g, '') // strip any ES6 defaults + .split(',') + .filter(Boolean); // split & filter [""] +} |