Add tests for ERC20 token with no return values

6 files changed, 261 insertions, 5 deletions

diff --git a/packages/contracts/compiler.json b/packages/contracts/compiler.json
index ad35fc5b3..0d938b8ea 100644
--- a/packages/contracts/compiler.json
+++ b/packages/contracts/compiler.json
@@ -23,6 +23,7 @@
         "DummyERC20Token",
         "DummyERC721Receiver",
         "DummyERC721Token",
+        "DummyNoReturnERC20Token",
         "ERC20Proxy",
         "ERC20Token",
         "ERC721Token",
diff --git a/packages/contracts/package.json b/packages/contracts/package.json
index 8bd808a27..29a440bcc 100644
--- a/packages/contracts/package.json
+++ b/packages/contracts/package.json
@@ -20,11 +20,14 @@
         "test:coverage": "SOLIDITY_COVERAGE=true run-s build run_mocha coverage:report:text coverage:report:lcov",
         "test:profiler": "SOLIDITY_PROFILER=true run-s build run_mocha profiler:report:html",
         "test:trace": "SOLIDITY_REVERT_TRACE=true run-s build run_mocha",
-        "run_mocha": "mocha --require source-map-support/register --require make-promises-safe 'lib/test/**/*.js' --timeout 100000 --bail --exit",
+        "run_mocha":
+            "mocha --require source-map-support/register --require make-promises-safe 'lib/test/**/*.js' --timeout 100000 --bail --exit",
         "compile": "sol-compiler --contracts-dir src",
         "clean": "shx rm -rf lib generated_contract_wrappers",
-        "generate_contract_wrappers": "abi-gen --abis  ${npm_package_config_abis} --template ../contract_templates/contract.handlebars --partials '../contract_templates/partials/**/*.handlebars' --output generated_contract_wrappers --backend ethers",
-        "lint": "tslint --project . --exclude **/src/generated_contract_wrappers/**/* --exclude **/lib/**/* && yarn lint-contracts",
+        "generate_contract_wrappers":
+            "abi-gen --abis  ${npm_package_config_abis} --template ../contract_templates/contract.handlebars --partials '../contract_templates/partials/**/*.handlebars' --output generated_contract_wrappers --backend ethers",
+        "lint":
+            "tslint --project . --exclude **/src/generated_contract_wrappers/**/* --exclude **/lib/**/* && yarn lint-contracts",
         "coverage:report:text": "istanbul report text",
         "coverage:report:html": "istanbul report html && open coverage/index.html",
         "profiler:report:html": "istanbul report html && open coverage/index.html",
@@ -33,7 +36,8 @@
         "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|TestExchangeInternals|TestLibBytes|TestLibs|TestSignatureValidator|Validator|Wallet|TokenRegistry|Whitelist|WETH9|ZRXToken).json"
+        "abis":
+            "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Receiver|DummyERC721Token|DummyNoReturnERC20Token|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",
diff --git a/packages/contracts/src/2.0.0/test/DummyERC20Token/DummyNoReturnERC20Token.sol b/packages/contracts/src/2.0.0/test/DummyERC20Token/DummyNoReturnERC20Token.sol
new file mode 100644
index 000000000..79156d3dd
--- /dev/null
+++ b/packages/contracts/src/2.0.0/test/DummyERC20Token/DummyNoReturnERC20Token.sol
@@ -0,0 +1,116 @@
+/*
+
+  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;
+
+import "./DummyERC20Token.sol";
+
+
+// solhint-disable no-empty-blocks
+contract DummyNoReturnERC20Token is
+    DummyERC20Token
+{
+
+    constructor (
+        string _name,
+        string _symbol,
+        uint256 _decimals,
+        uint256 _totalSupply
+    )
+        public
+        DummyERC20Token(
+            _name,
+            _symbol,
+            _decimals,
+            _totalSupply
+        )
+    {}
+
+    /// @dev send `value` token to `to` from `msg.sender`
+    /// @param _to The address of the recipient
+    /// @param _value The amount of token to be transferred
+    function transfer(address _to, uint256 _value)
+        external
+        returns (bool)
+    {
+        require(
+            balances[msg.sender] >= _value,
+            "ERC20_INSUFFICIENT_BALANCE"
+        );
+        require(
+            balances[_to] + _value >= balances[_to],
+            "UINT256_OVERFLOW"
+        );
+
+        balances[msg.sender] -= _value;
+        balances[_to] += _value;
+
+        emit Transfer(
+            msg.sender,
+            _to,
+            _value
+        );
+
+        // HACK: This contract will not compile if we remove `returns (bool)`, so we manually return no data
+        assembly {
+            return(0, 0)
+        }
+    }
+
+    /// @dev send `value` token to `to` from `from` on the condition it is approved by `from`
+    /// @param _from The address of the sender
+    /// @param _to The address of the recipient
+    /// @param _value The amount of token to be transferred
+    function transferFrom(
+        address _from,
+        address _to,
+        uint256 _value
+    )
+        external
+        returns (bool)
+    {
+        require(
+            balances[_from] >= _value,
+            "ERC20_INSUFFICIENT_BALANCE"
+        );
+        require(
+            allowed[_from][msg.sender] >= _value,
+            "ERC20_INSUFFICIENT_ALLOWANCE"
+        );
+        require(
+            balances[_to] + _value >= balances[_to],
+            "UINT256_OVERFLOW"
+        );
+
+        balances[_to] += _value;
+        balances[_from] -= _value;
+        allowed[_from][msg.sender] -= _value;
+    
+        emit Transfer(
+            _from,
+            _to,
+            _value
+        );
+
+        // HACK: This contract will not compile if we remove `returns (bool)`, so we manually return no data
+        assembly {
+            return(0, 0)
+        }
+    }
+}
+
diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts
index b324988cc..bc2bad749 100644
--- a/packages/contracts/test/exchange/core.ts
+++ b/packages/contracts/test/exchange/core.ts
@@ -10,6 +10,7 @@ import * as _ from 'lodash';
 
 import { DummyERC20TokenContract } from '../../generated_contract_wrappers/dummy_erc20_token';
 import { DummyERC721TokenContract } from '../../generated_contract_wrappers/dummy_erc721_token';
+import { DummyNoReturnERC20TokenContract } from '../../generated_contract_wrappers/dummy_no_return_erc20_token';
 import { ERC20ProxyContract } from '../../generated_contract_wrappers/erc20_proxy';
 import { ERC721ProxyContract } from '../../generated_contract_wrappers/erc721_proxy';
 import { ExchangeCancelEventArgs, ExchangeContract } from '../../generated_contract_wrappers/exchange';
@@ -39,6 +40,7 @@ describe('Exchange core', () => {
     let erc20TokenB: DummyERC20TokenContract;
     let zrxToken: DummyERC20TokenContract;
     let erc721Token: DummyERC721TokenContract;
+    let noReturnErc20Token: DummyNoReturnERC20TokenContract;
     let exchange: ExchangeContract;
     let erc20Proxy: ERC20ProxyContract;
     let erc721Proxy: ERC721ProxyContract;
@@ -161,6 +163,137 @@ describe('Exchange core', () => {
         });
     });
 
+    describe('Testing exchange of ERC20 tokens with no return values', () => {
+        before(async () => {
+            noReturnErc20Token = await DummyNoReturnERC20TokenContract.deployFrom0xArtifactAsync(
+                artifacts.DummyNoReturnERC20Token,
+                provider,
+                txDefaults,
+                constants.DUMMY_TOKEN_NAME,
+                constants.DUMMY_TOKEN_SYMBOL,
+                constants.DUMMY_TOKEN_DECIMALS,
+                constants.DUMMY_TOKEN_TOTAL_SUPPLY,
+            );
+            await web3Wrapper.awaitTransactionSuccessAsync(
+                await noReturnErc20Token.setBalance.sendTransactionAsync(makerAddress, constants.INITIAL_ERC20_BALANCE),
+                constants.AWAIT_TRANSACTION_MINED_MS,
+            );
+            await web3Wrapper.awaitTransactionSuccessAsync(
+                await noReturnErc20Token.approve.sendTransactionAsync(
+                    erc20Proxy.address,
+                    constants.INITIAL_ERC20_ALLOWANCE,
+                    { from: makerAddress },
+                ),
+                constants.AWAIT_TRANSACTION_MINED_MS,
+            );
+        });
+        it('should transfer the correct amounts when makerAssetAmount === takerAssetAmount', async () => {
+            signedOrder = await orderFactory.newSignedOrderAsync({
+                makerAssetData: assetDataUtils.encodeERC20AssetData(noReturnErc20Token.address),
+                makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18),
+                takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18),
+            });
+
+            const initialMakerBalanceA = await noReturnErc20Token.balanceOf.callAsync(makerAddress);
+            const initialMakerBalanceB = await erc20TokenB.balanceOf.callAsync(makerAddress);
+            const initialMakerZrxBalance = await zrxToken.balanceOf.callAsync(makerAddress);
+            const initialTakerBalanceA = await noReturnErc20Token.balanceOf.callAsync(takerAddress);
+            const initialTakerBalanceB = await erc20TokenB.balanceOf.callAsync(takerAddress);
+            const initialTakerZrxBalance = await zrxToken.balanceOf.callAsync(takerAddress);
+            const initialFeeRecipientZrxBalance = await zrxToken.balanceOf.callAsync(feeRecipientAddress);
+
+            await exchangeWrapper.fillOrderAsync(signedOrder, takerAddress);
+
+            const finalMakerBalanceA = await noReturnErc20Token.balanceOf.callAsync(makerAddress);
+            const finalMakerBalanceB = await erc20TokenB.balanceOf.callAsync(makerAddress);
+            const finalMakerZrxBalance = await zrxToken.balanceOf.callAsync(makerAddress);
+            const finalTakerBalanceA = await noReturnErc20Token.balanceOf.callAsync(takerAddress);
+            const finalTakerBalanceB = await erc20TokenB.balanceOf.callAsync(takerAddress);
+            const finalTakerZrxBalance = await zrxToken.balanceOf.callAsync(takerAddress);
+            const finalFeeRecipientZrxBalance = await zrxToken.balanceOf.callAsync(feeRecipientAddress);
+
+            expect(finalMakerBalanceA).to.be.bignumber.equal(initialMakerBalanceA.minus(signedOrder.makerAssetAmount));
+            expect(finalMakerBalanceB).to.be.bignumber.equal(initialMakerBalanceB.plus(signedOrder.takerAssetAmount));
+            expect(finalTakerBalanceA).to.be.bignumber.equal(initialTakerBalanceA.plus(signedOrder.makerAssetAmount));
+            expect(finalTakerBalanceB).to.be.bignumber.equal(initialTakerBalanceB.minus(signedOrder.takerAssetAmount));
+            expect(finalMakerZrxBalance).to.be.bignumber.equal(initialMakerZrxBalance.minus(signedOrder.makerFee));
+            expect(finalTakerZrxBalance).to.be.bignumber.equal(initialTakerZrxBalance.minus(signedOrder.takerFee));
+            expect(finalFeeRecipientZrxBalance).to.be.bignumber.equal(
+                initialFeeRecipientZrxBalance.plus(signedOrder.makerFee.plus(signedOrder.takerFee)),
+            );
+        });
+        it('should transfer the correct amounts when makerAssetAmount > takerAssetAmount', async () => {
+            signedOrder = await orderFactory.newSignedOrderAsync({
+                makerAssetData: assetDataUtils.encodeERC20AssetData(noReturnErc20Token.address),
+                makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(200), 18),
+                takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18),
+            });
+
+            const initialMakerBalanceA = await noReturnErc20Token.balanceOf.callAsync(makerAddress);
+            const initialMakerBalanceB = await erc20TokenB.balanceOf.callAsync(makerAddress);
+            const initialMakerZrxBalance = await zrxToken.balanceOf.callAsync(makerAddress);
+            const initialTakerBalanceA = await noReturnErc20Token.balanceOf.callAsync(takerAddress);
+            const initialTakerBalanceB = await erc20TokenB.balanceOf.callAsync(takerAddress);
+            const initialTakerZrxBalance = await zrxToken.balanceOf.callAsync(takerAddress);
+            const initialFeeRecipientZrxBalance = await zrxToken.balanceOf.callAsync(feeRecipientAddress);
+
+            await exchangeWrapper.fillOrderAsync(signedOrder, takerAddress);
+
+            const finalMakerBalanceA = await noReturnErc20Token.balanceOf.callAsync(makerAddress);
+            const finalMakerBalanceB = await erc20TokenB.balanceOf.callAsync(makerAddress);
+            const finalMakerZrxBalance = await zrxToken.balanceOf.callAsync(makerAddress);
+            const finalTakerBalanceA = await noReturnErc20Token.balanceOf.callAsync(takerAddress);
+            const finalTakerBalanceB = await erc20TokenB.balanceOf.callAsync(takerAddress);
+            const finalTakerZrxBalance = await zrxToken.balanceOf.callAsync(takerAddress);
+            const finalFeeRecipientZrxBalance = await zrxToken.balanceOf.callAsync(feeRecipientAddress);
+
+            expect(finalMakerBalanceA).to.be.bignumber.equal(initialMakerBalanceA.minus(signedOrder.makerAssetAmount));
+            expect(finalMakerBalanceB).to.be.bignumber.equal(initialMakerBalanceB.plus(signedOrder.takerAssetAmount));
+            expect(finalTakerBalanceA).to.be.bignumber.equal(initialTakerBalanceA.plus(signedOrder.makerAssetAmount));
+            expect(finalTakerBalanceB).to.be.bignumber.equal(initialTakerBalanceB.minus(signedOrder.takerAssetAmount));
+            expect(finalMakerZrxBalance).to.be.bignumber.equal(initialMakerZrxBalance.minus(signedOrder.makerFee));
+            expect(finalTakerZrxBalance).to.be.bignumber.equal(initialTakerZrxBalance.minus(signedOrder.takerFee));
+            expect(finalFeeRecipientZrxBalance).to.be.bignumber.equal(
+                initialFeeRecipientZrxBalance.plus(signedOrder.makerFee.plus(signedOrder.takerFee)),
+            );
+        });
+        it('should transfer the correct amounts when makerAssetAmount < takerAssetAmount', async () => {
+            signedOrder = await orderFactory.newSignedOrderAsync({
+                makerAssetData: assetDataUtils.encodeERC20AssetData(noReturnErc20Token.address),
+                makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18),
+                takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(200), 18),
+            });
+
+            const initialMakerBalanceA = await noReturnErc20Token.balanceOf.callAsync(makerAddress);
+            const initialMakerBalanceB = await erc20TokenB.balanceOf.callAsync(makerAddress);
+            const initialMakerZrxBalance = await zrxToken.balanceOf.callAsync(makerAddress);
+            const initialTakerBalanceA = await noReturnErc20Token.balanceOf.callAsync(takerAddress);
+            const initialTakerBalanceB = await erc20TokenB.balanceOf.callAsync(takerAddress);
+            const initialTakerZrxBalance = await zrxToken.balanceOf.callAsync(takerAddress);
+            const initialFeeRecipientZrxBalance = await zrxToken.balanceOf.callAsync(feeRecipientAddress);
+
+            await exchangeWrapper.fillOrderAsync(signedOrder, takerAddress);
+
+            const finalMakerBalanceA = await noReturnErc20Token.balanceOf.callAsync(makerAddress);
+            const finalMakerBalanceB = await erc20TokenB.balanceOf.callAsync(makerAddress);
+            const finalMakerZrxBalance = await zrxToken.balanceOf.callAsync(makerAddress);
+            const finalTakerBalanceA = await noReturnErc20Token.balanceOf.callAsync(takerAddress);
+            const finalTakerBalanceB = await erc20TokenB.balanceOf.callAsync(takerAddress);
+            const finalTakerZrxBalance = await zrxToken.balanceOf.callAsync(takerAddress);
+            const finalFeeRecipientZrxBalance = await zrxToken.balanceOf.callAsync(feeRecipientAddress);
+
+            expect(finalMakerBalanceA).to.be.bignumber.equal(initialMakerBalanceA.minus(signedOrder.makerAssetAmount));
+            expect(finalMakerBalanceB).to.be.bignumber.equal(initialMakerBalanceB.plus(signedOrder.takerAssetAmount));
+            expect(finalTakerBalanceA).to.be.bignumber.equal(initialTakerBalanceA.plus(signedOrder.makerAssetAmount));
+            expect(finalTakerBalanceB).to.be.bignumber.equal(initialTakerBalanceB.minus(signedOrder.takerAssetAmount));
+            expect(finalMakerZrxBalance).to.be.bignumber.equal(initialMakerZrxBalance.minus(signedOrder.makerFee));
+            expect(finalTakerZrxBalance).to.be.bignumber.equal(initialTakerZrxBalance.minus(signedOrder.takerFee));
+            expect(finalFeeRecipientZrxBalance).to.be.bignumber.equal(
+                initialFeeRecipientZrxBalance.plus(signedOrder.makerFee.plus(signedOrder.takerFee)),
+            );
+        });
+    });
+
     describe('cancelOrder', () => {
         beforeEach(async () => {
             erc20Balances = await erc20Wrapper.getBalancesAsync();
diff --git a/packages/contracts/test/exchange/fill_order.ts b/packages/contracts/test/exchange/fill_order.ts
index e79e2239e..b1e08324f 100644
--- a/packages/contracts/test/exchange/fill_order.ts
+++ b/packages/contracts/test/exchange/fill_order.ts
@@ -231,7 +231,7 @@ describe('FillOrder Tests', () => {
         });
     });
 
-    describe('Testing Exchange of ERC721 Tokens', () => {
+    describe('Testing exchange of ERC721 Tokens', () => {
         it('should successfully exchange a single token between the maker and taker (via fillOrder)', async () => {
             const fillScenario = {
                 ...defaultFillScenario,
diff --git a/packages/contracts/test/utils/artifacts.ts b/packages/contracts/test/utils/artifacts.ts
index e608ee174..65f72015f 100644
--- a/packages/contracts/test/utils/artifacts.ts
+++ b/packages/contracts/test/utils/artifacts.ts
@@ -4,6 +4,7 @@ import * as AssetProxyOwner from '../../artifacts/AssetProxyOwner.json';
 import * as DummyERC20Token from '../../artifacts/DummyERC20Token.json';
 import * as DummyERC721Receiver from '../../artifacts/DummyERC721Receiver.json';
 import * as DummyERC721Token from '../../artifacts/DummyERC721Token.json';
+import * as DummyNoReturnERC20Token from '../../artifacts/DummyNoReturnERC20Token.json';
 import * as ERC20Proxy from '../../artifacts/ERC20Proxy.json';
 import * as ERC721Proxy from '../../artifacts/ERC721Proxy.json';
 import * as Exchange from '../../artifacts/Exchange.json';
@@ -32,6 +33,7 @@ export const artifacts = {
     DummyERC20Token: (DummyERC20Token as any) as ContractArtifact,
     DummyERC721Receiver: (DummyERC721Receiver as any) as ContractArtifact,
     DummyERC721Token: (DummyERC721Token as any) as ContractArtifact,
+    DummyNoReturnERC20Token: (DummyNoReturnERC20Token as any) as ContractArtifact,
     ERC20Proxy: (ERC20Proxy as any) as ContractArtifact,
     ERC721Proxy: (ERC721Proxy as any) as ContractArtifact,
     Exchange: (Exchange as any) as ContractArtifact,