From b58bf8259db7378b841e6695d99580027b4e2969 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Tue, 5 Dec 2017 12:45:08 -0800 Subject: Add tests for ERC20Token and EtherToken_v2 --- packages/contracts/contracts/base/ERC20Token.sol | 51 --------- packages/contracts/contracts/test/Mintable.sol | 4 +- packages/contracts/contracts/tokens/ERC20Token.sol | 51 +++++++++ .../contracts/contracts/tokens/EtherToken_v2.sol | 2 +- .../migrations/2_deploy_independent_contracts.ts | 9 +- packages/contracts/package.json | 1 - packages/contracts/test/ts/erc20Token.ts | 123 +++++++++++++++++++++ packages/contracts/test/ts/ether_token_v2.ts | 52 ++++++++- packages/contracts/util/artifacts.ts | 4 +- 9 files changed, 233 insertions(+), 64 deletions(-) delete mode 100644 packages/contracts/contracts/base/ERC20Token.sol create mode 100644 packages/contracts/contracts/tokens/ERC20Token.sol create mode 100644 packages/contracts/test/ts/erc20Token.ts diff --git a/packages/contracts/contracts/base/ERC20Token.sol b/packages/contracts/contracts/base/ERC20Token.sol deleted file mode 100644 index f0117894d..000000000 --- a/packages/contracts/contracts/base/ERC20Token.sol +++ /dev/null @@ -1,51 +0,0 @@ -pragma solidity 0.4.11; - -import "./Token.sol"; - -contract ERC20Token is Token { - - uint constant MAX_UINT = 2**256 - 1; - - function transfer(address _to, uint _value) returns (bool) { - require(balances[msg.sender] >= _value && balances[_to] + _value >= balances[_to]); - balances[msg.sender] -= _value; - balances[_to] += _value; - Transfer(msg.sender, _to, _value); - return true; - } - - /// @dev ERC20 transferFrom, modified such that an allowance of MAX_UINT represents an unlimited allowance. See https://github.com/ethereum/EIPs/issues/717 - /// @param _from Address to transfer from. - /// @param _to Address to transfer to. - /// @param _value Amount to transfer. - /// @return Success of transfer. - function transferFrom(address _from, address _to, uint _value) returns (bool) { - uint allowance = allowed[_from][msg.sender]; - require(balances[_from] >= _value && allowance >= _value && balances[_to] + _value >= balances[_to]); - balances[_to] += _value; - balances[_from] -= _value; - if (allowance < MAX_UINT) { - allowed[_from][msg.sender] -= _value; - } - Transfer(_from, _to, _value); - return true; - } - - function balanceOf(address _owner) constant returns (uint) { - return balances[_owner]; - } - - function approve(address _spender, uint _value) returns (bool) { - allowed[msg.sender][_spender] = _value; - Approval(msg.sender, _spender, _value); - return true; - } - - function allowance(address _owner, address _spender) constant returns (uint) { - return allowed[_owner][_spender]; - } - - mapping (address => uint) balances; - mapping (address => mapping (address => uint)) allowed; - uint public totalSupply; -} \ No newline at end of file diff --git a/packages/contracts/contracts/test/Mintable.sol b/packages/contracts/contracts/test/Mintable.sol index c0438f304..257dca705 100644 --- a/packages/contracts/contracts/test/Mintable.sol +++ b/packages/contracts/contracts/test/Mintable.sol @@ -1,13 +1,13 @@ pragma solidity 0.4.11; -import "./../tokens/UnlimitedAllowanceToken.sol"; +import "./../tokens/ERC20Token.sol"; import "./../base/SafeMath.sol"; /* * Mintable * Base contract that creates a mintable UnlimitedAllowanceToken */ -contract Mintable is UnlimitedAllowanceToken, SafeMath { +contract Mintable is ERC20Token, SafeMath { function mint(uint _value) { require(_value <= 100000000000000000000); balances[msg.sender] = safeAdd(_value, balances[msg.sender]); diff --git a/packages/contracts/contracts/tokens/ERC20Token.sol b/packages/contracts/contracts/tokens/ERC20Token.sol new file mode 100644 index 000000000..8db58c707 --- /dev/null +++ b/packages/contracts/contracts/tokens/ERC20Token.sol @@ -0,0 +1,51 @@ +pragma solidity 0.4.11; + +import "./../base/Token.sol"; + +contract ERC20Token is Token { + + uint constant MAX_UINT = 2**256 - 1; + + function transfer(address _to, uint _value) returns (bool) { + require(balances[msg.sender] >= _value && balances[_to] + _value >= balances[_to]); + balances[msg.sender] -= _value; + balances[_to] += _value; + Transfer(msg.sender, _to, _value); + return true; + } + + /// @dev ERC20 transferFrom, modified such that an allowance of MAX_UINT represents an unlimited allowance. See https://github.com/ethereum/EIPs/issues/717 + /// @param _from Address to transfer from. + /// @param _to Address to transfer to. + /// @param _value Amount to transfer. + /// @return Success of transfer. + function transferFrom(address _from, address _to, uint _value) returns (bool) { + uint allowance = allowed[_from][msg.sender]; + require(balances[_from] >= _value && allowance >= _value && balances[_to] + _value >= balances[_to]); + balances[_to] += _value; + balances[_from] -= _value; + if (allowance < MAX_UINT) { + allowed[_from][msg.sender] -= _value; + } + Transfer(_from, _to, _value); + return true; + } + + function balanceOf(address _owner) constant returns (uint) { + return balances[_owner]; + } + + function approve(address _spender, uint _value) returns (bool) { + allowed[msg.sender][_spender] = _value; + Approval(msg.sender, _spender, _value); + return true; + } + + function allowance(address _owner, address _spender) constant returns (uint) { + return allowed[_owner][_spender]; + } + + mapping (address => uint) balances; + mapping (address => mapping (address => uint)) allowed; + uint public totalSupply; +} \ No newline at end of file diff --git a/packages/contracts/contracts/tokens/EtherToken_v2.sol b/packages/contracts/contracts/tokens/EtherToken_v2.sol index a75a2aa7c..0c7acf8a0 100644 --- a/packages/contracts/contracts/tokens/EtherToken_v2.sol +++ b/packages/contracts/contracts/tokens/EtherToken_v2.sol @@ -18,7 +18,7 @@ pragma solidity 0.4.11; -import "./../base/ERC20Token.sol"; +import "./../tokens/ERC20Token.sol"; import "./../base/SafeMath.sol"; contract EtherToken_v2 is ERC20Token, SafeMath { diff --git a/packages/contracts/migrations/2_deploy_independent_contracts.ts b/packages/contracts/migrations/2_deploy_independent_contracts.ts index 878860eb6..db9fc370d 100644 --- a/packages/contracts/migrations/2_deploy_independent_contracts.ts +++ b/packages/contracts/migrations/2_deploy_independent_contracts.ts @@ -4,6 +4,7 @@ const { MultiSigWalletWithTimeLock, TokenTransferProxy, EtherToken, + EtherTokenV2, TokenRegistry, } = new Artifacts(artifacts); @@ -28,11 +29,13 @@ module.exports = (deployer: any, network: string, accounts: string[]) => { deployer.deploy(MultiSigWalletWithTimeLock, config.owners, config.confirmationsRequired, config.secondsRequired) .then(() => { - return deployer.deploy(TokenTransferProxy); + return deployer.deploy(TokenTransferProxy); }).then(() => { - return deployer.deploy(TokenRegistry); + return deployer.deploy(TokenRegistry); }).then(() => { - return deployer.deploy(EtherToken); + return deployer.deploy(EtherToken); + }).then(() => { + return deployer.deploy(EtherTokenV2); }); } else { deployer.deploy([ diff --git a/packages/contracts/package.json b/packages/contracts/package.json index d2f30933a..46bf4f731 100644 --- a/packages/contracts/package.json +++ b/packages/contracts/package.json @@ -31,7 +31,6 @@ "@0xproject/tslint-config": "^0.2.1", "@0xproject/types": "^0.1.0", "@types/bluebird": "^3.5.3", - "@types/isomorphic-fetch": "^0.0.34", "@types/lodash": "^4.14.86", "@types/node": "^8.0.53", "@types/request-promise-native": "^1.0.2", diff --git a/packages/contracts/test/ts/erc20Token.ts b/packages/contracts/test/ts/erc20Token.ts new file mode 100644 index 000000000..e2282cbf3 --- /dev/null +++ b/packages/contracts/test/ts/erc20Token.ts @@ -0,0 +1,123 @@ +import {ZeroEx} from '0x.js'; +import {BigNumber} from 'bignumber.js'; +import * as chai from 'chai'; +import * as Web3 from 'web3'; + +import {Artifacts} from '../../util/artifacts'; +import {constants} from '../../util/constants'; +import {ContractInstance} from '../../util/types'; + +import {chaiSetup} from './utils/chai_setup'; + +const {DummyToken} = new Artifacts(artifacts); +const web3: Web3 = (global as any).web3; +chaiSetup.configure(); +const expect = chai.expect; + +contract('ERC20Token', (accounts: string[]) => { + const zeroEx = new ZeroEx(web3.currentProvider); + const owner = accounts[0]; + const spender = accounts[1]; + + const MAX_MINT_VALUE = new BigNumber(100000000000000000000); + let tokenAddress: string; + let token: ContractInstance; + + beforeEach(async () => { + token = await DummyToken.new({from: owner}); + await token.mint(MAX_MINT_VALUE, {from: owner}); + tokenAddress = token.address; + }); + + describe('transfer', () => { + it('should throw if owner has insufficient balance', async () => { + const ownerBalance = await zeroEx.token.getBalanceAsync(tokenAddress, owner); + const amountToTransfer = ownerBalance.plus(1); + return expect(token.transfer.call(owner, spender, amountToTransfer, {from: spender})) + .to.be.rejectedWith(constants.REVERT); + }); + + it('should transfer balance from sender to receiver', async () => { + const receiver = spender; + const initOwnerBalance = await zeroEx.token.getBalanceAsync(tokenAddress, owner); + const amountToTransfer = new BigNumber(1); + await zeroEx.token.transferAsync(tokenAddress, owner, receiver, amountToTransfer); + const finalOwnerBalance = await zeroEx.token.getBalanceAsync(tokenAddress, owner); + const finalReceiverBalance = await zeroEx.token.getBalanceAsync(tokenAddress, receiver); + + const expectedFinalOwnerBalance = initOwnerBalance.minus(amountToTransfer); + const expectedFinalReceiverBalance = amountToTransfer; + expect(finalOwnerBalance).to.be.bignumber.equal(expectedFinalOwnerBalance); + expect(finalReceiverBalance).to.be.bignumber.equal(expectedFinalReceiverBalance); + }); + + it('should return true on a 0 value transfer', async () => { + const didReturnTrue = await token.transfer.call(spender, 0, {from: owner}); + expect(didReturnTrue).to.be.true(); + }); + }); + + describe('transferFrom', () => { + it('should throw if owner has insufficient balance', async () => { + const ownerBalance = await zeroEx.token.getBalanceAsync(tokenAddress, owner); + const amountToTransfer = ownerBalance.plus(1); + await zeroEx.token.setAllowanceAsync(tokenAddress, owner, spender, amountToTransfer); + return expect(token.transferFrom.call(owner, spender, amountToTransfer, {from: spender})) + .to.be.rejectedWith(constants.REVERT); + }); + + it('should throw if spender has insufficient allowance', async () => { + const ownerBalance = await zeroEx.token.getBalanceAsync(tokenAddress, owner); + const amountToTransfer = ownerBalance; + + const spenderAllowance = await zeroEx.token.getAllowanceAsync(tokenAddress, owner, spender); + const spenderAllowanceIsInsufficient = spenderAllowance.cmp(amountToTransfer) < 0; + expect(spenderAllowanceIsInsufficient).to.be.true(); + + return expect(token.transferFrom.call(owner, spender, amountToTransfer, {from: spender})) + .to.be.rejectedWith(constants.REVERT); + }); + + it('should return true on a 0 value transfer', async () => { + const amountToTransfer = 0; + const didReturnTrue = await token.transferFrom.call(owner, spender, amountToTransfer, {from: spender}); + expect(didReturnTrue).to.be.true(); + }); + + it('should not modify spender allowance if spender allowance is 2^256 - 1', async () => { + const initOwnerBalance = await zeroEx.token.getBalanceAsync(tokenAddress, owner); + const amountToTransfer = initOwnerBalance; + const initSpenderAllowance = zeroEx.token.UNLIMITED_ALLOWANCE_IN_BASE_UNITS; + await zeroEx.token.setAllowanceAsync(tokenAddress, owner, spender, initSpenderAllowance); + await zeroEx.token.transferFromAsync(tokenAddress, owner, spender, spender, amountToTransfer); + + const newSpenderAllowance = await zeroEx.token.getAllowanceAsync(tokenAddress, owner, spender); + expect(initSpenderAllowance).to.be.bignumber.equal(newSpenderAllowance); + }); + + it('should transfer the correct balances if spender has sufficient allowance', async () => { + const initOwnerBalance = await zeroEx.token.getBalanceAsync(tokenAddress, owner); + const amountToTransfer = initOwnerBalance; + const initSpenderAllowance = initOwnerBalance; + await zeroEx.token.setAllowanceAsync(tokenAddress, owner, spender, initSpenderAllowance); + await zeroEx.token.transferFromAsync(tokenAddress, owner, spender, spender, amountToTransfer); + + const newOwnerBalance = await zeroEx.token.getBalanceAsync(tokenAddress, owner); + const newSpenderBalance = await zeroEx.token.getBalanceAsync(tokenAddress, spender); + + expect(newOwnerBalance).to.be.bignumber.equal(0); + expect(newSpenderBalance).to.be.bignumber.equal(initOwnerBalance); + }); + + it('should modify allowance if spender has sufficient allowance less than 2^256 - 1', async () => { + const initOwnerBalance = await zeroEx.token.getBalanceAsync(tokenAddress, owner); + const amountToTransfer = initOwnerBalance; + const initSpenderAllowance = initOwnerBalance; + await zeroEx.token.setAllowanceAsync(tokenAddress, owner, spender, initSpenderAllowance); + await zeroEx.token.transferFromAsync(tokenAddress, owner, spender, spender, amountToTransfer); + + const newSpenderAllowance = await zeroEx.token.getAllowanceAsync(tokenAddress, owner, spender); + expect(newSpenderAllowance).to.be.bignumber.equal(0); + }); + }); +}); diff --git a/packages/contracts/test/ts/ether_token_v2.ts b/packages/contracts/test/ts/ether_token_v2.ts index 857371578..0c352b522 100644 --- a/packages/contracts/test/ts/ether_token_v2.ts +++ b/packages/contracts/test/ts/ether_token_v2.ts @@ -8,7 +8,7 @@ import {Artifacts} from '../../util/artifacts'; import {chaiSetup} from './utils/chai_setup'; -const {EtherToken} = new Artifacts(artifacts); +const {EtherTokenV2} = new Artifacts(artifacts); chaiSetup.configure(); const expect = chai.expect; @@ -17,13 +17,14 @@ const expect = chai.expect; // with type `any` to a variable of type `Web3`. const web3: Web3 = (global as any).web3; -contract('EtherToken', (accounts: string[]) => { +contract('EtherTokenV2', (accounts: string[]) => { const account = accounts[0]; const gasPrice = ZeroEx.toBaseUnitAmount(new BigNumber(20), 9); let zeroEx: ZeroEx; let etherTokenAddress: string; - before(async () => { - etherTokenAddress = EtherToken.address; + beforeEach(async () => { + const etherToken = await EtherTokenV2.new(); + etherTokenAddress = etherToken.address; zeroEx = new ZeroEx(web3.currentProvider, { gasPrice, etherTokenContractAddress: etherTokenAddress, @@ -62,9 +63,32 @@ contract('EtherToken', (accounts: string[]) => { expect(finalEthBalance).to.be.bignumber.equal(initEthBalance.minus(ethToDeposit.plus(ethSpentOnGas))); expect(finalEthTokenBalance).to.be.bignumber.equal(initEthTokenBalance.plus(ethToDeposit)); }); + + it('should log 1 event with correct arguments', async () => { + const ethToDeposit = new BigNumber(web3.toWei(1, 'ether')); + + const txHash = await zeroEx.etherToken.depositAsync(ethToDeposit, account); + const receipt = await zeroEx.awaitTransactionMinedAsync(txHash); + + const logs = receipt.logs; + expect(logs.length).to.equal(1); + + const expectedFrom = ZeroEx.NULL_ADDRESS; + const expectedTo = account; + const expectedValue = ethToDeposit; + const logArgs = logs[0].args; + expect(logArgs._from).to.equal(expectedFrom); + expect(logArgs._to).to.equal(expectedTo); + expect(logArgs._value).to.be.bignumber.equal(ethToDeposit); + }); }); describe('withdraw', () => { + beforeEach(async () => { + const ethToDeposit = new BigNumber(web3.toWei(1, 'ether')); + await zeroEx.etherToken.depositAsync(ethToDeposit, account); + }); + it('should throw if caller attempts to withdraw greater than caller balance', async () => { const initEthTokenBalance = await zeroEx.token.getBalanceAsync(etherTokenAddress, account); const ethTokensToWithdraw = initEthTokenBalance.plus(1); @@ -89,6 +113,26 @@ contract('EtherToken', (accounts: string[]) => { .equal(initEthBalance.plus(ethTokensToWithdraw.minus(ethSpentOnGas))); expect(finalEthTokenBalance).to.be.bignumber.equal(initEthTokenBalance.minus(ethTokensToWithdraw)); }); + + it('should log 1 event with correct arguments', async () => { + const initEthTokenBalance = await zeroEx.token.getBalanceAsync(etherTokenAddress, account); + const initEthBalance = await getEthBalanceAsync(account); + const ethTokensToWithdraw = initEthTokenBalance; + expect(ethTokensToWithdraw).to.not.be.bignumber.equal(0); + const txHash = await zeroEx.etherToken.withdrawAsync(ethTokensToWithdraw, account); + const receipt = await zeroEx.awaitTransactionMinedAsync(txHash); + + const logs = receipt.logs; + expect(logs.length).to.equal(1); + + const expectedFrom = account; + const expectedTo = ZeroEx.NULL_ADDRESS; + const expectedValue = ethTokensToWithdraw; + const logArgs = logs[0].args; + expect(logArgs._from).to.equal(expectedFrom); + expect(logArgs._to).to.equal(expectedTo); + expect(logArgs._value).to.be.bignumber.equal(ethTokensToWithdraw); + }); }); describe('fallback', () => { diff --git a/packages/contracts/util/artifacts.ts b/packages/contracts/util/artifacts.ts index 12321ac66..cb06739af 100644 --- a/packages/contracts/util/artifacts.ts +++ b/packages/contracts/util/artifacts.ts @@ -7,7 +7,7 @@ export class Artifacts { public ZRXToken: any; public DummyToken: any; public EtherToken: any; - public EtherToken_v2: any; + public EtherTokenV2: any; public MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress: any; public MaliciousToken: any; constructor(artifacts: any) { @@ -19,7 +19,7 @@ export class Artifacts { this.ZRXToken = artifacts.require('ZRXToken'); this.DummyToken = artifacts.require('DummyToken'); this.EtherToken = artifacts.require('EtherToken'); - this.EtherToken_v2 = artifacts.require('EtherToken_v2'); + this.EtherTokenV2 = artifacts.require('EtherToken_v2'); this.MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress = artifacts.require( 'MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress'); this.MaliciousToken = artifacts.require('MaliciousToken'); -- cgit