aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAmir Bandeali <abandeali1@gmail.com>2018-08-31 04:07:19 +0800
committerGitHub <noreply@github.com>2018-08-31 04:07:19 +0800
commita6e8b28da56c94cb7548ca29ed478dc803237efc (patch)
tree3102132b655a5b351182a2e63fdaca3b5a93fc0c
parenteb4517d7379128b18c830b704742154c8b2f3c8d (diff)
parentd0f693398074eaa8b08f76d18a594e86ad81b0ae (diff)
downloaddexon-sol-tools-a6e8b28da56c94cb7548ca29ed478dc803237efc.tar
dexon-sol-tools-a6e8b28da56c94cb7548ca29ed478dc803237efc.tar.gz
dexon-sol-tools-a6e8b28da56c94cb7548ca29ed478dc803237efc.tar.bz2
dexon-sol-tools-a6e8b28da56c94cb7548ca29ed478dc803237efc.tar.lz
dexon-sol-tools-a6e8b28da56c94cb7548ca29ed478dc803237efc.tar.xz
dexon-sol-tools-a6e8b28da56c94cb7548ca29ed478dc803237efc.tar.zst
dexon-sol-tools-a6e8b28da56c94cb7548ca29ed478dc803237efc.zip
Merge pull request #1043 from 0xProject/feature/contracts/edgeCaseTests
Add edge case tests
-rw-r--r--packages/contracts/compiler.json1
-rw-r--r--packages/contracts/package.json12
-rw-r--r--packages/contracts/src/2.0.0/test/DummyERC20Token/DummyMultipleReturnERC20Token.sol70
-rw-r--r--packages/contracts/test/asset_proxy/authorizable.ts4
-rw-r--r--packages/contracts/test/asset_proxy/proxies.ts237
-rw-r--r--packages/contracts/test/exchange/core.ts39
-rw-r--r--packages/contracts/test/exchange/dispatcher.ts54
-rw-r--r--packages/contracts/test/utils/artifacts.ts2
8 files changed, 404 insertions, 15 deletions
diff --git a/packages/contracts/compiler.json b/packages/contracts/compiler.json
index f66114e87..cdf49dcf6 100644
--- a/packages/contracts/compiler.json
+++ b/packages/contracts/compiler.json
@@ -23,6 +23,7 @@
"DummyERC20Token",
"DummyERC721Receiver",
"DummyERC721Token",
+ "DummyMultipleReturnERC20Token",
"DummyNoReturnERC20Token",
"ERC20Proxy",
"ERC20Token",
diff --git a/packages/contracts/package.json b/packages/contracts/package.json
index d3351e6cf..283991cee 100644
--- a/packages/contracts/package.json
+++ b/packages/contracts/package.json
@@ -19,11 +19,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",
@@ -32,7 +35,8 @@
"lint-contracts": "solhint src/2.0.0/**/**/**/**/*.sol"
},
"config": {
- "abis": "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Receiver|DummyERC721Token|DummyNoReturnERC20Token|ERC20Proxy|ERC721Proxy|Forwarder|Exchange|ExchangeWrapper|IAssetData|IAssetProxy|InvalidERC721Receiver|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|OrderValidator|ReentrantERC20Token|TestAssetProxyOwner|TestAssetProxyDispatcher|TestConstants|TestExchangeInternals|TestLibBytes|TestLibs|TestSignatureValidator|TestStaticCallReceiver|Validator|Wallet|TokenRegistry|Whitelist|WETH9|ZRXToken).json"
+ "abis":
+ "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Receiver|DummyERC721Token|DummyMultipleReturnERC20Token|DummyNoReturnERC20Token|ERC20Proxy|ERC721Proxy|Forwarder|Exchange|ExchangeWrapper|IAssetData|IAssetProxy|InvalidERC721Receiver|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|OrderValidator|ReentrantERC20Token|TestAssetProxyOwner|TestAssetProxyDispatcher|TestConstants|TestExchangeInternals|TestLibBytes|TestLibs|TestSignatureValidator|TestStaticCallReceiver|Validator|Wallet|TokenRegistry|Whitelist|WETH9|ZRXToken).json"
},
"repository": {
"type": "git",
diff --git a/packages/contracts/src/2.0.0/test/DummyERC20Token/DummyMultipleReturnERC20Token.sol b/packages/contracts/src/2.0.0/test/DummyERC20Token/DummyMultipleReturnERC20Token.sol
new file mode 100644
index 000000000..8a8aecae8
--- /dev/null
+++ b/packages/contracts/src/2.0.0/test/DummyERC20Token/DummyMultipleReturnERC20Token.sol
@@ -0,0 +1,70 @@
+/*
+
+ 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 DummyMultipleReturnERC20Token is
+ DummyERC20Token
+{
+
+ constructor (
+ string _name,
+ string _symbol,
+ uint256 _decimals,
+ uint256 _totalSupply
+ )
+ public
+ DummyERC20Token(
+ _name,
+ _symbol,
+ _decimals,
+ _totalSupply
+ )
+ {}
+
+ /// @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)
+ {
+ emit Transfer(
+ _from,
+ _to,
+ _value
+ );
+
+ // HACK: This contract will not compile if we remove `returns (bool)`, so we manually return 64 bytes (equiavalent to true, true)
+ assembly {
+ mstore(0, 1)
+ mstore(32, 1)
+ return(0, 64)
+ }
+ }
+}
+
diff --git a/packages/contracts/test/asset_proxy/authorizable.ts b/packages/contracts/test/asset_proxy/authorizable.ts
index e99c6cee3..0c0da46b3 100644
--- a/packages/contracts/test/asset_proxy/authorizable.ts
+++ b/packages/contracts/test/asset_proxy/authorizable.ts
@@ -2,6 +2,7 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils';
import { RevertReason } from '@0xproject/types';
import { BigNumber } from '@0xproject/utils';
import * as chai from 'chai';
+import * as _ from 'lodash';
import { MixinAuthorizableContract } from '../../generated_contract_wrappers/mixin_authorizable';
import { artifacts } from '../utils/artifacts';
@@ -28,8 +29,7 @@ describe('Authorizable', () => {
});
before(async () => {
const accounts = await web3Wrapper.getAvailableAddressesAsync();
- owner = address = accounts[0];
- notOwner = accounts[1];
+ [owner, address, notOwner] = _.slice(accounts, 0, 3);
authorizable = await MixinAuthorizableContract.deployFrom0xArtifactAsync(
artifacts.MixinAuthorizable,
provider,
diff --git a/packages/contracts/test/asset_proxy/proxies.ts b/packages/contracts/test/asset_proxy/proxies.ts
index 6031e554d..4d70031d1 100644
--- a/packages/contracts/test/asset_proxy/proxies.ts
+++ b/packages/contracts/test/asset_proxy/proxies.ts
@@ -8,6 +8,8 @@ import * as _ from 'lodash';
import { DummyERC20TokenContract } from '../../generated_contract_wrappers/dummy_erc20_token';
import { DummyERC721ReceiverContract } from '../../generated_contract_wrappers/dummy_erc721_receiver';
import { DummyERC721TokenContract } from '../../generated_contract_wrappers/dummy_erc721_token';
+import { DummyMultipleReturnERC20TokenContract } from '../../generated_contract_wrappers/dummy_multiple_return_erc20_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 { IAssetProxyContract } from '../../generated_contract_wrappers/i_asset_proxy';
@@ -42,6 +44,8 @@ describe('Asset Transfer Proxies', () => {
let erc721Receiver: DummyERC721ReceiverContract;
let erc20Proxy: ERC20ProxyContract;
let erc721Proxy: ERC721ProxyContract;
+ let noReturnErc20Token: DummyNoReturnERC20TokenContract;
+ let multipleReturnErc20Token: DummyMultipleReturnERC20TokenContract;
let erc20Wrapper: ERC20Wrapper;
let erc721Wrapper: ERC721Wrapper;
@@ -91,6 +95,51 @@ describe('Asset Transfer Proxies', () => {
provider,
txDefaults,
);
+ 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,
+ );
+ multipleReturnErc20Token = await DummyMultipleReturnERC20TokenContract.deployFrom0xArtifactAsync(
+ artifacts.DummyMultipleReturnERC20Token,
+ provider,
+ txDefaults,
+ constants.DUMMY_TOKEN_NAME,
+ constants.DUMMY_TOKEN_SYMBOL,
+ constants.DUMMY_TOKEN_DECIMALS,
+ constants.DUMMY_TOKEN_TOTAL_SUPPLY,
+ );
+ await web3Wrapper.awaitTransactionSuccessAsync(
+ await multipleReturnErc20Token.setBalance.sendTransactionAsync(
+ makerAddress,
+ constants.INITIAL_ERC20_BALANCE,
+ ),
+ constants.AWAIT_TRANSACTION_MINED_MS,
+ );
+ await web3Wrapper.awaitTransactionSuccessAsync(
+ await multipleReturnErc20Token.approve.sendTransactionAsync(
+ erc20Proxy.address,
+ constants.INITIAL_ERC20_ALLOWANCE,
+ { from: makerAddress },
+ ),
+ constants.AWAIT_TRANSACTION_MINED_MS,
+ );
});
beforeEach(async () => {
await blockchainLifecycle.startAsync();
@@ -141,6 +190,65 @@ describe('Asset Transfer Proxies', () => {
);
});
+ it('should successfully transfer tokens that do not return a value', async () => {
+ // Construct ERC20 asset data
+ const encodedAssetData = assetDataUtils.encodeERC20AssetData(noReturnErc20Token.address);
+ // Perform a transfer from makerAddress to takerAddress
+ const initialMakerBalance = await noReturnErc20Token.balanceOf.callAsync(makerAddress);
+ const initialTakerBalance = await noReturnErc20Token.balanceOf.callAsync(takerAddress);
+ const amount = new BigNumber(10);
+ const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData(
+ encodedAssetData,
+ makerAddress,
+ takerAddress,
+ amount,
+ );
+ await web3Wrapper.awaitTransactionSuccessAsync(
+ await web3Wrapper.sendTransactionAsync({
+ to: erc20Proxy.address,
+ data,
+ from: exchangeAddress,
+ }),
+ constants.AWAIT_TRANSACTION_MINED_MS,
+ );
+ // Verify transfer was successful
+ const newMakerBalance = await noReturnErc20Token.balanceOf.callAsync(makerAddress);
+ const newTakerBalance = await noReturnErc20Token.balanceOf.callAsync(takerAddress);
+ expect(newMakerBalance).to.be.bignumber.equal(initialMakerBalance.minus(amount));
+ expect(newTakerBalance).to.be.bignumber.equal(initialTakerBalance.plus(amount));
+ });
+
+ it('should successfully transfer tokens and ignore extra assetData', async () => {
+ // Construct ERC20 asset data
+ const extraData = '0102030405060708';
+ const encodedAssetData = `${assetDataUtils.encodeERC20AssetData(zrxToken.address)}${extraData}`;
+ // Perform a transfer from makerAddress to takerAddress
+ const erc20Balances = await erc20Wrapper.getBalancesAsync();
+ const amount = new BigNumber(10);
+ const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData(
+ encodedAssetData,
+ makerAddress,
+ takerAddress,
+ amount,
+ );
+ await web3Wrapper.awaitTransactionSuccessAsync(
+ await web3Wrapper.sendTransactionAsync({
+ to: erc20Proxy.address,
+ data,
+ from: exchangeAddress,
+ }),
+ constants.AWAIT_TRANSACTION_MINED_MS,
+ );
+ // Verify transfer was successful
+ const newBalances = await erc20Wrapper.getBalancesAsync();
+ expect(newBalances[makerAddress][zrxToken.address]).to.be.bignumber.equal(
+ erc20Balances[makerAddress][zrxToken.address].minus(amount),
+ );
+ expect(newBalances[takerAddress][zrxToken.address]).to.be.bignumber.equal(
+ erc20Balances[takerAddress][zrxToken.address].add(amount),
+ );
+ });
+
it('should do nothing if transferring 0 amount of a token', async () => {
// Construct ERC20 asset data
const encodedAssetData = assetDataUtils.encodeERC20AssetData(zrxToken.address);
@@ -189,6 +297,7 @@ describe('Asset Transfer Proxies', () => {
}),
constants.AWAIT_TRANSACTION_MINED_MS,
);
+ const erc20Balances = await erc20Wrapper.getBalancesAsync();
// Perform a transfer; expect this to fail.
await expectTransactionFailedAsync(
web3Wrapper.sendTransactionAsync({
@@ -198,6 +307,43 @@ describe('Asset Transfer Proxies', () => {
}),
RevertReason.TransferFailed,
);
+ const newBalances = await erc20Wrapper.getBalancesAsync();
+ expect(newBalances).to.deep.equal(erc20Balances);
+ });
+
+ it('should throw if allowances are too low and token does not return a value', async () => {
+ // Construct ERC20 asset data
+ const encodedAssetData = assetDataUtils.encodeERC20AssetData(noReturnErc20Token.address);
+ // Create allowance less than transfer amount. Set allowance on proxy.
+ const allowance = new BigNumber(0);
+ const amount = new BigNumber(10);
+ const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData(
+ encodedAssetData,
+ makerAddress,
+ takerAddress,
+ amount,
+ );
+ await web3Wrapper.awaitTransactionSuccessAsync(
+ await noReturnErc20Token.approve.sendTransactionAsync(erc20Proxy.address, allowance, {
+ from: makerAddress,
+ }),
+ constants.AWAIT_TRANSACTION_MINED_MS,
+ );
+ const initialMakerBalance = await noReturnErc20Token.balanceOf.callAsync(makerAddress);
+ const initialTakerBalance = await noReturnErc20Token.balanceOf.callAsync(takerAddress);
+ // Perform a transfer; expect this to fail.
+ await expectTransactionFailedAsync(
+ web3Wrapper.sendTransactionAsync({
+ to: erc20Proxy.address,
+ data,
+ from: exchangeAddress,
+ }),
+ RevertReason.TransferFailed,
+ );
+ const newMakerBalance = await noReturnErc20Token.balanceOf.callAsync(makerAddress);
+ const newTakerBalance = await noReturnErc20Token.balanceOf.callAsync(takerAddress);
+ expect(newMakerBalance).to.be.bignumber.equal(initialMakerBalance);
+ expect(newTakerBalance).to.be.bignumber.equal(initialTakerBalance);
});
it('should throw if requesting address is not authorized', async () => {
@@ -211,6 +357,7 @@ describe('Asset Transfer Proxies', () => {
takerAddress,
amount,
);
+ const erc20Balances = await erc20Wrapper.getBalancesAsync();
await expectTransactionFailedAsync(
web3Wrapper.sendTransactionAsync({
to: erc20Proxy.address,
@@ -219,6 +366,35 @@ describe('Asset Transfer Proxies', () => {
}),
RevertReason.SenderNotAuthorized,
);
+ const newBalances = await erc20Wrapper.getBalancesAsync();
+ expect(newBalances).to.deep.equal(erc20Balances);
+ });
+
+ it('should throw if token returns more than 32 bytes', async () => {
+ // Construct ERC20 asset data
+ const encodedAssetData = assetDataUtils.encodeERC20AssetData(multipleReturnErc20Token.address);
+ const amount = new BigNumber(10);
+ const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData(
+ encodedAssetData,
+ makerAddress,
+ takerAddress,
+ amount,
+ );
+ const initialMakerBalance = await multipleReturnErc20Token.balanceOf.callAsync(makerAddress);
+ const initialTakerBalance = await multipleReturnErc20Token.balanceOf.callAsync(takerAddress);
+ // Perform a transfer; expect this to fail.
+ await expectTransactionFailedAsync(
+ web3Wrapper.sendTransactionAsync({
+ to: erc20Proxy.address,
+ data,
+ from: exchangeAddress,
+ }),
+ RevertReason.TransferFailed,
+ );
+ const newMakerBalance = await multipleReturnErc20Token.balanceOf.callAsync(makerAddress);
+ const newTakerBalance = await multipleReturnErc20Token.balanceOf.callAsync(takerAddress);
+ expect(newMakerBalance).to.be.bignumber.equal(initialMakerBalance);
+ expect(newTakerBalance).to.be.bignumber.equal(initialTakerBalance);
});
});
@@ -247,7 +423,38 @@ describe('Asset Transfer Proxies', () => {
const encodedAssetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
// Verify pre-condition
const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
- expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress);
+ expect(ownerMakerAsset).to.be.equal(makerAddress);
+ // Perform a transfer from makerAddress to takerAddress
+ const amount = new BigNumber(1);
+ const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData(
+ encodedAssetData,
+ makerAddress,
+ takerAddress,
+ amount,
+ );
+ await web3Wrapper.awaitTransactionSuccessAsync(
+ await web3Wrapper.sendTransactionAsync({
+ to: erc721Proxy.address,
+ data,
+ from: exchangeAddress,
+ }),
+ constants.AWAIT_TRANSACTION_MINED_MS,
+ );
+ // Verify transfer was successful
+ const newOwnerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
+ expect(newOwnerMakerAsset).to.be.bignumber.equal(takerAddress);
+ });
+
+ it('should successfully transfer tokens and ignore extra assetData', async () => {
+ // Construct ERC721 asset data
+ const extraData = '0102030405060708';
+ const encodedAssetData = `${assetDataUtils.encodeERC721AssetData(
+ erc721Token.address,
+ erc721MakerTokenId,
+ )}${extraData}`;
+ // Verify pre-condition
+ const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
+ expect(ownerMakerAsset).to.be.equal(makerAddress);
// Perform a transfer from makerAddress to takerAddress
const amount = new BigNumber(1);
const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData(
@@ -274,7 +481,7 @@ describe('Asset Transfer Proxies', () => {
const encodedAssetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
// Verify pre-condition
const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
- expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress);
+ expect(ownerMakerAsset).to.be.equal(makerAddress);
// Perform a transfer from makerAddress to takerAddress
const amount = new BigNumber(1);
const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData(
@@ -304,7 +511,7 @@ describe('Asset Transfer Proxies', () => {
const encodedAssetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
// Verify pre-condition
const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
- expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress);
+ expect(ownerMakerAsset).to.be.equal(makerAddress);
// Perform a transfer from makerAddress to takerAddress
const amount = new BigNumber(0);
const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData(
@@ -313,7 +520,7 @@ describe('Asset Transfer Proxies', () => {
takerAddress,
amount,
);
- return expectTransactionFailedAsync(
+ await expectTransactionFailedAsync(
web3Wrapper.sendTransactionAsync({
to: erc721Proxy.address,
data,
@@ -321,6 +528,8 @@ describe('Asset Transfer Proxies', () => {
}),
RevertReason.InvalidAmount,
);
+ const newOwner = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
+ expect(newOwner).to.be.equal(ownerMakerAsset);
});
it('should throw if transferring > 1 amount of a token', async () => {
@@ -328,7 +537,7 @@ describe('Asset Transfer Proxies', () => {
const encodedAssetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
// Verify pre-condition
const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
- expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress);
+ expect(ownerMakerAsset).to.be.equal(makerAddress);
// Perform a transfer from makerAddress to takerAddress
const amount = new BigNumber(500);
const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData(
@@ -337,7 +546,7 @@ describe('Asset Transfer Proxies', () => {
takerAddress,
amount,
);
- return expectTransactionFailedAsync(
+ await expectTransactionFailedAsync(
web3Wrapper.sendTransactionAsync({
to: erc721Proxy.address,
data,
@@ -345,11 +554,16 @@ describe('Asset Transfer Proxies', () => {
}),
RevertReason.InvalidAmount,
);
+ const newOwner = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
+ expect(newOwner).to.be.equal(ownerMakerAsset);
});
it('should throw if allowances are too low', async () => {
// Construct ERC721 asset data
const encodedAssetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
+ // Verify pre-condition
+ const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
+ expect(ownerMakerAsset).to.be.equal(makerAddress);
// Remove transfer approval for makerAddress.
await web3Wrapper.awaitTransactionSuccessAsync(
await erc721Token.approve.sendTransactionAsync(constants.NULL_ADDRESS, erc721MakerTokenId, {
@@ -365,7 +579,7 @@ describe('Asset Transfer Proxies', () => {
takerAddress,
amount,
);
- return expectTransactionFailedAsync(
+ await expectTransactionFailedAsync(
web3Wrapper.sendTransactionAsync({
to: erc721Proxy.address,
data,
@@ -373,11 +587,16 @@ describe('Asset Transfer Proxies', () => {
}),
RevertReason.TransferFailed,
);
+ const newOwner = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
+ expect(newOwner).to.be.equal(ownerMakerAsset);
});
it('should throw if requesting address is not authorized', async () => {
// Construct ERC721 asset data
const encodedAssetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
+ // Verify pre-condition
+ const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
+ expect(ownerMakerAsset).to.be.equal(makerAddress);
// Perform a transfer from makerAddress to takerAddress
const amount = new BigNumber(1);
const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData(
@@ -386,7 +605,7 @@ describe('Asset Transfer Proxies', () => {
takerAddress,
amount,
);
- return expectTransactionFailedAsync(
+ await expectTransactionFailedAsync(
web3Wrapper.sendTransactionAsync({
to: erc721Proxy.address,
data,
@@ -394,6 +613,8 @@ describe('Asset Transfer Proxies', () => {
}),
RevertReason.SenderNotAuthorized,
);
+ const newOwner = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
+ expect(newOwner).to.be.equal(ownerMakerAsset);
});
});
diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts
index 3bb71b58f..acb99eed1 100644
--- a/packages/contracts/test/exchange/core.ts
+++ b/packages/contracts/test/exchange/core.ts
@@ -8,7 +8,10 @@ import { LogWithDecodedArgs } from 'ethereum-types';
import ethUtil = require('ethereumjs-util');
import * as _ from 'lodash';
-import { DummyERC20TokenContract } from '../../generated_contract_wrappers/dummy_erc20_token';
+import {
+ DummyERC20TokenContract,
+ DummyERC20TokenTransferEventArgs,
+} 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';
@@ -243,6 +246,40 @@ describe('Exchange core', () => {
RevertReason.ValidatorError,
);
});
+
+ it('should not emit transfer events for transfers where from == to', async () => {
+ const txReceipt = await exchangeWrapper.fillOrderAsync(signedOrder, makerAddress);
+ const logs = txReceipt.logs;
+ const transferLogs = _.filter(
+ logs,
+ log => (log as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).event === 'Transfer',
+ );
+ expect(transferLogs.length).to.be.equal(2);
+ expect((transferLogs[0] as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).address).to.be.equal(
+ zrxToken.address,
+ );
+ expect((transferLogs[0] as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).args._from).to.be.equal(
+ makerAddress,
+ );
+ expect((transferLogs[0] as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).args._to).to.be.equal(
+ feeRecipientAddress,
+ );
+ expect(
+ (transferLogs[0] as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).args._value,
+ ).to.be.bignumber.equal(signedOrder.makerFee);
+ expect((transferLogs[1] as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).address).to.be.equal(
+ zrxToken.address,
+ );
+ expect((transferLogs[1] as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).args._from).to.be.equal(
+ makerAddress,
+ );
+ expect((transferLogs[1] as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).args._to).to.be.equal(
+ feeRecipientAddress,
+ );
+ expect(
+ (transferLogs[1] as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).args._value,
+ ).to.be.bignumber.equal(signedOrder.takerFee);
+ });
});
describe('Testing exchange of ERC20 tokens with no return values', () => {
diff --git a/packages/contracts/test/exchange/dispatcher.ts b/packages/contracts/test/exchange/dispatcher.ts
index 81871a680..a8ae897a8 100644
--- a/packages/contracts/test/exchange/dispatcher.ts
+++ b/packages/contracts/test/exchange/dispatcher.ts
@@ -205,6 +205,60 @@ describe('AssetProxyDispatcher', () => {
);
});
+ it('should not dispatch a transfer if amount == 0', async () => {
+ // Register ERC20 proxy
+ await web3Wrapper.awaitTransactionSuccessAsync(
+ await assetProxyDispatcher.registerAssetProxy.sendTransactionAsync(erc20Proxy.address, { from: owner }),
+ constants.AWAIT_TRANSACTION_MINED_MS,
+ );
+ // Construct metadata for ERC20 proxy
+ const encodedAssetData = assetDataUtils.encodeERC20AssetData(zrxToken.address);
+
+ // Perform a transfer from makerAddress to takerAddress
+ const erc20Balances = await erc20Wrapper.getBalancesAsync();
+ const amount = constants.ZERO_AMOUNT;
+ const txReceipt = await web3Wrapper.awaitTransactionSuccessAsync(
+ await assetProxyDispatcher.publicDispatchTransferFrom.sendTransactionAsync(
+ encodedAssetData,
+ makerAddress,
+ takerAddress,
+ amount,
+ { from: owner },
+ ),
+ constants.AWAIT_TRANSACTION_MINED_MS,
+ );
+ expect(txReceipt.logs.length).to.be.equal(0);
+ const newBalances = await erc20Wrapper.getBalancesAsync();
+ expect(newBalances).to.deep.equal(erc20Balances);
+ });
+
+ it('should not dispatch a transfer if from == to', async () => {
+ // Register ERC20 proxy
+ await web3Wrapper.awaitTransactionSuccessAsync(
+ await assetProxyDispatcher.registerAssetProxy.sendTransactionAsync(erc20Proxy.address, { from: owner }),
+ constants.AWAIT_TRANSACTION_MINED_MS,
+ );
+ // Construct metadata for ERC20 proxy
+ const encodedAssetData = assetDataUtils.encodeERC20AssetData(zrxToken.address);
+
+ // Perform a transfer from makerAddress to takerAddress
+ const erc20Balances = await erc20Wrapper.getBalancesAsync();
+ const amount = new BigNumber(10);
+ const txReceipt = await web3Wrapper.awaitTransactionSuccessAsync(
+ await assetProxyDispatcher.publicDispatchTransferFrom.sendTransactionAsync(
+ encodedAssetData,
+ makerAddress,
+ makerAddress,
+ amount,
+ { from: owner },
+ ),
+ constants.AWAIT_TRANSACTION_MINED_MS,
+ );
+ expect(txReceipt.logs.length).to.be.equal(0);
+ const newBalances = await erc20Wrapper.getBalancesAsync();
+ expect(newBalances).to.deep.equal(erc20Balances);
+ });
+
it('should throw if dispatching to unregistered proxy', async () => {
// Construct metadata for ERC20 proxy
const encodedAssetData = assetDataUtils.encodeERC20AssetData(zrxToken.address);
diff --git a/packages/contracts/test/utils/artifacts.ts b/packages/contracts/test/utils/artifacts.ts
index 5ddb5cc7f..53f2a4e4e 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 DummyMultipleReturnERC20Token from '../../artifacts/DummyMultipleReturnERC20Token.json';
import * as DummyNoReturnERC20Token from '../../artifacts/DummyNoReturnERC20Token.json';
import * as ERC20Proxy from '../../artifacts/ERC20Proxy.json';
import * as ERC721Proxy from '../../artifacts/ERC721Proxy.json';
@@ -37,6 +38,7 @@ export const artifacts = {
DummyERC20Token: (DummyERC20Token as any) as ContractArtifact,
DummyERC721Receiver: (DummyERC721Receiver as any) as ContractArtifact,
DummyERC721Token: (DummyERC721Token as any) as ContractArtifact,
+ DummyMultipleReturnERC20Token: (DummyMultipleReturnERC20Token as any) as ContractArtifact,
DummyNoReturnERC20Token: (DummyNoReturnERC20Token as any) as ContractArtifact,
ERC20Proxy: (ERC20Proxy as any) as ContractArtifact,
ERC721Proxy: (ERC721Proxy as any) as ContractArtifact,