Fix tests

4 files changed, 286 insertions, 95 deletions

diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts
index 9065c5e3f..4d462826a 100644
--- a/packages/contracts/test/exchange/core.ts
+++ b/packages/contracts/test/exchange/core.ts
@@ -5,7 +5,6 @@ import * as chai from 'chai';
 import ethUtil = require('ethereumjs-util');
 import * as _ from 'lodash';
 
-import { AssetProxyDispatcherContract } from '../../src/contract_wrappers/generated/asset_proxy_dispatcher';
 import { DummyERC20TokenContract } from '../../src/contract_wrappers/generated/dummy_e_r_c20_token';
 import { DummyERC721TokenContract } from '../../src/contract_wrappers/generated/dummy_e_r_c721_token';
 import { ERC20ProxyContract } from '../../src/contract_wrappers/generated/e_r_c20_proxy';
@@ -51,7 +50,6 @@ describe('Exchange core', () => {
     let zrx: DummyERC20TokenContract;
     let erc721Token: DummyERC721TokenContract;
     let exchange: ExchangeContract;
-    let assetProxyDispatcher: AssetProxyDispatcherContract;
     let erc20Proxy: ERC20ProxyContract;
     let erc721Proxy: ERC721ProxyContract;
 
@@ -88,44 +86,24 @@ describe('Exchange core', () => {
         erc721MakerAssetIds = erc721Balances[makerAddress][erc721Token.address];
         erc721TakerAssetIds = erc721Balances[takerAddress][erc721Token.address];
 
-        const assetProxyDispatcherInstance = await deployer.deployAsync(ContractName.AssetProxyDispatcher);
-        assetProxyDispatcher = new AssetProxyDispatcherContract(
-            assetProxyDispatcherInstance.abi,
-            assetProxyDispatcherInstance.address,
-            provider,
-        );
-        const prevERC20ProxyAddress = ZeroEx.NULL_ADDRESS;
-        await assetProxyDispatcher.registerAssetProxy.sendTransactionAsync(
-            AssetProxyId.ERC20,
-            erc20Proxy.address,
-            prevERC20ProxyAddress,
-            { from: owner },
-        );
-        const prevERC721ProxyAddress = ZeroEx.NULL_ADDRESS;
-        await assetProxyDispatcher.registerAssetProxy.sendTransactionAsync(
-            AssetProxyId.ERC721,
-            erc721Proxy.address,
-            prevERC721ProxyAddress,
-            { from: owner },
-        );
-        await erc20Proxy.addAuthorizedAddress.sendTransactionAsync(assetProxyDispatcher.address, {
-            from: owner,
-        });
-        await erc721Proxy.addAuthorizedAddress.sendTransactionAsync(assetProxyDispatcher.address, {
-            from: owner,
-        });
-
         const exchangeInstance = await deployer.deployAsync(ContractName.Exchange, [
-            assetProxyDispatcher.address,
             assetProxyUtils.encodeERC20ProxyData(zrx.address),
         ]);
         exchange = new ExchangeContract(exchangeInstance.abi, exchangeInstance.address, provider);
-        await assetProxyDispatcher.addAuthorizedAddress.sendTransactionAsync(exchange.address, { from: owner });
         zeroEx = new ZeroEx(provider, {
             exchangeContractAddress: exchange.address,
             networkId: constants.TESTRPC_NETWORK_ID,
         });
         exchangeWrapper = new ExchangeWrapper(exchange, zeroEx);
+        await exchangeWrapper.registerAssetProxyAsync(AssetProxyId.ERC20, erc20Proxy.address, owner);
+        await exchangeWrapper.registerAssetProxyAsync(AssetProxyId.ERC721, erc721Proxy.address, owner);
+
+        await erc20Proxy.addAuthorizedAddress.sendTransactionAsync(exchange.address, {
+            from: owner,
+        });
+        await erc721Proxy.addAuthorizedAddress.sendTransactionAsync(exchange.address, {
+            from: owner,
+        });
 
         defaultMakerAssetAddress = erc20TokenA.address;
         defaultTakerAssetAddress = erc20TokenB.address;
@@ -467,35 +445,6 @@ describe('Exchange core', () => {
             expect(orderUtils.getOrderHashHex(signedOrder)).to.be.equal(logArgs.orderHash);
         });
 
-        it('should log 1 event with the correct arguments when order has no feeRecipient', async () => {
-            signedOrder = orderFactory.newSignedOrder({
-                feeRecipientAddress: ZeroEx.NULL_ADDRESS,
-            });
-            const divisor = 2;
-            const res = await exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, {
-                takerAssetFillAmount: signedOrder.takerAssetAmount.div(divisor),
-            });
-            expect(res.logs).to.have.length(1);
-
-            const log = res.logs[0] as LogWithDecodedArgs<FillContractEventArgs>;
-            const logArgs = log.args;
-            const expectedFilledMakerAssetAmount = signedOrder.makerAssetAmount.div(divisor);
-            const expectedFilledTakerAssetAmount = signedOrder.takerAssetAmount.div(divisor);
-            const expectedFeeMPaid = new BigNumber(0);
-            const expectedFeeTPaid = new BigNumber(0);
-
-            expect(signedOrder.makerAddress).to.be.equal(logArgs.makerAddress);
-            expect(takerAddress).to.be.equal(logArgs.takerAddress);
-            expect(signedOrder.feeRecipientAddress).to.be.equal(logArgs.feeRecipientAddress);
-            expect(signedOrder.makerAssetData).to.be.equal(logArgs.makerAssetData);
-            expect(signedOrder.takerAssetData).to.be.equal(logArgs.takerAssetData);
-            expect(expectedFilledMakerAssetAmount).to.be.bignumber.equal(logArgs.makerAssetFilledAmount);
-            expect(expectedFilledTakerAssetAmount).to.be.bignumber.equal(logArgs.takerAssetFilledAmount);
-            expect(expectedFeeMPaid).to.be.bignumber.equal(logArgs.makerFeePaid);
-            expect(expectedFeeTPaid).to.be.bignumber.equal(logArgs.takerFeePaid);
-            expect(orderUtils.getOrderHashHex(signedOrder)).to.be.equal(logArgs.orderHash);
-        });
-
         it('should throw when taker is specified and order is claimed by other', async () => {
             signedOrder = orderFactory.newSignedOrder({
                 takerAddress: feeRecipientAddress,
diff --git a/packages/contracts/test/exchange/dispatcher.ts b/packages/contracts/test/exchange/dispatcher.ts
new file mode 100644
index 000000000..7d40bcfe3
--- /dev/null
+++ b/packages/contracts/test/exchange/dispatcher.ts
@@ -0,0 +1,267 @@
+import { ZeroEx } from '0x.js';
+import { BlockchainLifecycle } from '@0xproject/dev-utils';
+import { BigNumber } from '@0xproject/utils';
+import * as chai from 'chai';
+import * as Web3 from 'web3';
+
+import { DummyERC20TokenContract } from '../../src/contract_wrappers/generated/dummy_e_r_c20_token';
+import { ERC20ProxyContract } from '../../src/contract_wrappers/generated/e_r_c20_proxy';
+import { ERC721ProxyContract } from '../../src/contract_wrappers/generated/e_r_c721_proxy';
+import { TestAssetProxyDispatcherContract } from '../../src/contract_wrappers/generated/test_asset_proxy_dispatcher';
+import { assetProxyUtils } from '../../src/utils/asset_proxy_utils';
+import { constants } from '../../src/utils/constants';
+import { ERC20Wrapper } from '../../src/utils/erc20_wrapper';
+import { ERC721Wrapper } from '../../src/utils/erc721_wrapper';
+import { AssetProxyId, ContractName } from '../../src/utils/types';
+import { chaiSetup } from '../utils/chai_setup';
+import { deployer } from '../utils/deployer';
+import { provider, web3Wrapper } from '../utils/web3_wrapper';
+
+chaiSetup.configure();
+const expect = chai.expect;
+const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper);
+
+describe('AssetProxyDispatcher', () => {
+    let owner: string;
+    let notOwner: string;
+    let notAuthorized: string;
+    let makerAddress: string;
+    let takerAddress: string;
+
+    let zrx: DummyERC20TokenContract;
+    let erc20Proxy: ERC20ProxyContract;
+    let erc721Proxy: ERC721ProxyContract;
+    let assetProxyDispatcher: TestAssetProxyDispatcherContract;
+
+    let erc20Wrapper: ERC20Wrapper;
+    let erc721Wrapper: ERC721Wrapper;
+
+    before(async () => {
+        // Setup accounts & addresses
+        const accounts = await web3Wrapper.getAvailableAddressesAsync();
+        const usedAddresses = ([owner, notOwner, makerAddress, takerAddress] = accounts);
+        notAuthorized = notOwner;
+
+        erc20Wrapper = new ERC20Wrapper(deployer, provider, usedAddresses, owner);
+        erc721Wrapper = new ERC721Wrapper(deployer, provider, usedAddresses, owner);
+
+        [zrx] = await erc20Wrapper.deployDummyERC20TokensAsync();
+        erc20Proxy = await erc20Wrapper.deployERC20ProxyAsync();
+        await erc20Wrapper.setBalancesAndAllowancesAsync();
+
+        erc721Proxy = await erc721Wrapper.deployERC721ProxyAsync();
+
+        const assetProxyDispatcherInstance = await deployer.deployAsync(ContractName.TestAssetProxyDispatcher);
+        assetProxyDispatcher = new TestAssetProxyDispatcherContract(
+            assetProxyDispatcherInstance.abi,
+            assetProxyDispatcherInstance.address,
+            provider,
+        );
+        await erc20Proxy.addAuthorizedAddress.sendTransactionAsync(assetProxyDispatcher.address, {
+            from: owner,
+        });
+        await erc721Proxy.addAuthorizedAddress.sendTransactionAsync(assetProxyDispatcher.address, {
+            from: owner,
+        });
+    });
+    beforeEach(async () => {
+        await blockchainLifecycle.startAsync();
+    });
+    afterEach(async () => {
+        await blockchainLifecycle.revertAsync();
+    });
+    describe('registerAssetProxy', () => {
+        it('should record proxy upon registration', async () => {
+            const prevProxyAddress = ZeroEx.NULL_ADDRESS;
+            await assetProxyDispatcher.registerAssetProxy.sendTransactionAsync(
+                AssetProxyId.ERC20,
+                erc20Proxy.address,
+                prevProxyAddress,
+                { from: owner },
+            );
+            const proxyAddress = await assetProxyDispatcher.getAssetProxy.callAsync(AssetProxyId.ERC20);
+            expect(proxyAddress).to.be.equal(erc20Proxy.address);
+        });
+
+        it('should be able to record multiple proxies', async () => {
+            // Record first proxy
+            const prevERC20ProxyAddress = ZeroEx.NULL_ADDRESS;
+            await assetProxyDispatcher.registerAssetProxy.sendTransactionAsync(
+                AssetProxyId.ERC20,
+                erc20Proxy.address,
+                prevERC20ProxyAddress,
+                { from: owner },
+            );
+            let proxyAddress = await assetProxyDispatcher.getAssetProxy.callAsync(AssetProxyId.ERC20);
+            expect(proxyAddress).to.be.equal(erc20Proxy.address);
+            // Record another proxy
+            const prevERC721ProxyAddress = ZeroEx.NULL_ADDRESS;
+            await assetProxyDispatcher.registerAssetProxy.sendTransactionAsync(
+                AssetProxyId.ERC721,
+                erc721Proxy.address,
+                prevERC721ProxyAddress,
+                { from: owner },
+            );
+            proxyAddress = await assetProxyDispatcher.getAssetProxy.callAsync(AssetProxyId.ERC721);
+            expect(proxyAddress).to.be.equal(erc721Proxy.address);
+        });
+
+        it('should replace proxy address upon re-registration', async () => {
+            // Initial registration
+            const prevProxyAddress = ZeroEx.NULL_ADDRESS;
+            await assetProxyDispatcher.registerAssetProxy.sendTransactionAsync(
+                AssetProxyId.ERC20,
+                erc20Proxy.address,
+                prevProxyAddress,
+                { from: owner },
+            );
+            let proxyAddress = await assetProxyDispatcher.getAssetProxy.callAsync(AssetProxyId.ERC20);
+            expect(proxyAddress).to.be.equal(erc20Proxy.address);
+            // Deploy a new version of the ERC20 Transfer Proxy contract
+            const newErc20TransferProxyInstance = await deployer.deployAsync(ContractName.ERC20Proxy);
+            const newErc20TransferProxy = new ERC20ProxyContract(
+                newErc20TransferProxyInstance.abi,
+                newErc20TransferProxyInstance.address,
+                provider,
+            );
+            // Register new ERC20 Transfer Proxy contract
+            const newAddress = newErc20TransferProxy.address;
+            const currentAddress = erc20Proxy.address;
+            await assetProxyDispatcher.registerAssetProxy.sendTransactionAsync(
+                AssetProxyId.ERC20,
+                newAddress,
+                currentAddress,
+                { from: owner },
+            );
+            // Verify new asset proxy has replaced initial version
+            proxyAddress = await assetProxyDispatcher.getAssetProxy.callAsync(AssetProxyId.ERC20);
+            expect(proxyAddress).to.be.equal(newAddress);
+        });
+
+        it('should throw if registering with incorrect "currentAssetProxyAddress" field', async () => {
+            // Initial registration
+            const prevProxyAddress = ZeroEx.NULL_ADDRESS;
+            await assetProxyDispatcher.registerAssetProxy.sendTransactionAsync(
+                AssetProxyId.ERC20,
+                erc20Proxy.address,
+                prevProxyAddress,
+                { from: owner },
+            );
+            const proxyAddress = await assetProxyDispatcher.getAssetProxy.callAsync(AssetProxyId.ERC20);
+            expect(proxyAddress).to.be.equal(erc20Proxy.address);
+            // The following transaction will throw because the currentAddress is no longer ZeroEx.NULL_ADDRESS
+            return expect(
+                assetProxyDispatcher.registerAssetProxy.sendTransactionAsync(
+                    AssetProxyId.ERC20,
+                    erc20Proxy.address,
+                    ZeroEx.NULL_ADDRESS,
+                    { from: owner },
+                ),
+            ).to.be.rejectedWith(constants.REVERT);
+        });
+
+        it('should be able to reset proxy address to NULL', async () => {
+            // Initial registration
+            const prevProxyAddress = ZeroEx.NULL_ADDRESS;
+            await assetProxyDispatcher.registerAssetProxy.sendTransactionAsync(
+                AssetProxyId.ERC20,
+                erc20Proxy.address,
+                prevProxyAddress,
+                { from: owner },
+            );
+            const proxyAddress = await assetProxyDispatcher.getAssetProxy.callAsync(AssetProxyId.ERC20);
+            expect(proxyAddress).to.be.equal(erc20Proxy.address);
+            // The following transaction will reset the proxy address
+            const newProxyAddress = ZeroEx.NULL_ADDRESS;
+            await assetProxyDispatcher.registerAssetProxy.sendTransactionAsync(
+                AssetProxyId.ERC20,
+                newProxyAddress,
+                erc20Proxy.address,
+                { from: owner },
+            );
+            const finalProxyAddress = await assetProxyDispatcher.getAssetProxy.callAsync(AssetProxyId.ERC20);
+            expect(finalProxyAddress).to.be.equal(newProxyAddress);
+        });
+
+        it('should throw if requesting address is not owner', async () => {
+            const prevProxyAddress = ZeroEx.NULL_ADDRESS;
+            return expect(
+                assetProxyDispatcher.registerAssetProxy.sendTransactionAsync(
+                    AssetProxyId.ERC20,
+                    erc20Proxy.address,
+                    prevProxyAddress,
+                    { from: notOwner },
+                ),
+            ).to.be.rejectedWith(constants.REVERT);
+        });
+    });
+
+    describe('getAssetProxy', () => {
+        it('should return correct address of registered proxy', async () => {
+            const prevProxyAddress = ZeroEx.NULL_ADDRESS;
+            await assetProxyDispatcher.registerAssetProxy.sendTransactionAsync(
+                AssetProxyId.ERC20,
+                erc20Proxy.address,
+                prevProxyAddress,
+                { from: owner },
+            );
+            const proxyAddress = await assetProxyDispatcher.getAssetProxy.callAsync(AssetProxyId.ERC20);
+            expect(proxyAddress).to.be.equal(erc20Proxy.address);
+        });
+
+        it('should return NULL address if requesting non-existent proxy', async () => {
+            const proxyAddress = await assetProxyDispatcher.getAssetProxy.callAsync(AssetProxyId.ERC20);
+            expect(proxyAddress).to.be.equal(ZeroEx.NULL_ADDRESS);
+        });
+    });
+
+    describe('dispatchTransferFrom', () => {
+        it('should dispatch transfer to registered proxy', async () => {
+            // Register ERC20 proxy
+            const prevProxyAddress = ZeroEx.NULL_ADDRESS;
+            await assetProxyDispatcher.registerAssetProxy.sendTransactionAsync(
+                AssetProxyId.ERC20,
+                erc20Proxy.address,
+                prevProxyAddress,
+                { from: owner },
+            );
+            // Construct metadata for ERC20 proxy
+            const encodedProxyMetadata = assetProxyUtils.encodeERC20ProxyData(zrx.address);
+            // Perform a transfer from makerAddress to takerAddress
+            const erc20Balances = await erc20Wrapper.getBalancesAsync();
+            const amount = new BigNumber(10);
+            await assetProxyDispatcher.publicDispatchTransferFrom.sendTransactionAsync(
+                encodedProxyMetadata,
+                makerAddress,
+                takerAddress,
+                amount,
+                { from: owner },
+            );
+            // Verify transfer was successful
+            const newBalances = await erc20Wrapper.getBalancesAsync();
+            expect(newBalances[makerAddress][zrx.address]).to.be.bignumber.equal(
+                erc20Balances[makerAddress][zrx.address].minus(amount),
+            );
+            expect(newBalances[takerAddress][zrx.address]).to.be.bignumber.equal(
+                erc20Balances[takerAddress][zrx.address].add(amount),
+            );
+        });
+
+        it('should throw if dispatching to unregistered proxy', async () => {
+            // Construct metadata for ERC20 proxy
+            const encodedProxyMetadata = assetProxyUtils.encodeERC20ProxyData(zrx.address);
+            // Perform a transfer from makerAddress to takerAddress
+            const erc20Balances = await erc20Wrapper.getBalancesAsync();
+            const amount = new BigNumber(10);
+            return expect(
+                assetProxyDispatcher.publicDispatchTransferFrom.sendTransactionAsync(
+                    encodedProxyMetadata,
+                    makerAddress,
+                    takerAddress,
+                    amount,
+                    { from: owner },
+                ),
+            ).to.be.rejectedWith(constants.REVERT);
+        });
+    });
+});
diff --git a/packages/contracts/test/exchange/helpers.ts b/packages/contracts/test/exchange/helpers.ts
index a5c2ca8d5..f986665ff 100644
--- a/packages/contracts/test/exchange/helpers.ts
+++ b/packages/contracts/test/exchange/helpers.ts
@@ -29,10 +29,7 @@ describe('Exchange helpers', () => {
     before(async () => {
         const accounts = await web3Wrapper.getAvailableAddressesAsync();
         const makerAddress = accounts[0];
-        const exchangeInstance = await deployer.deployAsync(ContractName.Exchange, [
-            ZeroEx.NULL_ADDRESS,
-            AssetProxyId.ERC20,
-        ]);
+        const exchangeInstance = await deployer.deployAsync(ContractName.Exchange, [AssetProxyId.ERC20]);
         const exchange = new ExchangeContract(exchangeInstance.abi, exchangeInstance.address, provider);
         const zeroEx = new ZeroEx(provider, { networkId: constants.TESTRPC_NETWORK_ID });
         exchangeWrapper = new ExchangeWrapper(exchange, zeroEx);
diff --git a/packages/contracts/test/exchange/wrapper.ts b/packages/contracts/test/exchange/wrapper.ts
index 60f47373e..1d40bd3d9 100644
--- a/packages/contracts/test/exchange/wrapper.ts
+++ b/packages/contracts/test/exchange/wrapper.ts
@@ -6,7 +6,6 @@ import * as chai from 'chai';
 import * as _ from 'lodash';
 import * as Web3 from 'web3';
 
-import { AssetProxyDispatcherContract } from '../../src/contract_wrappers/generated/asset_proxy_dispatcher';
 import { DummyERC20TokenContract } from '../../src/contract_wrappers/generated/dummy_e_r_c20_token';
 import { DummyERC721TokenContract } from '../../src/contract_wrappers/generated/dummy_e_r_c721_token';
 import { ERC20ProxyContract } from '../../src/contract_wrappers/generated/e_r_c20_proxy';
@@ -45,7 +44,6 @@ describe('Exchange wrappers', () => {
     let zrx: DummyERC20TokenContract;
     let erc721Token: DummyERC721TokenContract;
     let exchange: ExchangeContract;
-    let assetProxyDispatcher: AssetProxyDispatcherContract;
     let erc20Proxy: ERC20ProxyContract;
     let erc721Proxy: ERC721ProxyContract;
 
@@ -81,44 +79,24 @@ describe('Exchange wrappers', () => {
         erc721MakerAssetId = erc721Balances[makerAddress][erc721Token.address][0];
         erc721TakerAssetId = erc721Balances[takerAddress][erc721Token.address][0];
 
-        const assetProxyDispatcherInstance = await deployer.deployAsync(ContractName.AssetProxyDispatcher);
-        assetProxyDispatcher = new AssetProxyDispatcherContract(
-            assetProxyDispatcherInstance.abi,
-            assetProxyDispatcherInstance.address,
-            provider,
-        );
-        const prevERC20ProxyAddress = ZeroEx.NULL_ADDRESS;
-        await assetProxyDispatcher.registerAssetProxy.sendTransactionAsync(
-            AssetProxyId.ERC20,
-            erc20Proxy.address,
-            prevERC20ProxyAddress,
-            { from: owner },
-        );
-        const prevERC721ProxyAddress = ZeroEx.NULL_ADDRESS;
-        await assetProxyDispatcher.registerAssetProxy.sendTransactionAsync(
-            AssetProxyId.ERC721,
-            erc721Proxy.address,
-            prevERC721ProxyAddress,
-            { from: owner },
-        );
-        await erc20Proxy.addAuthorizedAddress.sendTransactionAsync(assetProxyDispatcher.address, {
-            from: owner,
-        });
-        await erc721Proxy.addAuthorizedAddress.sendTransactionAsync(assetProxyDispatcher.address, {
-            from: owner,
-        });
-
         const exchangeInstance = await deployer.deployAsync(ContractName.Exchange, [
-            assetProxyDispatcher.address,
             assetProxyUtils.encodeERC20ProxyData(zrx.address),
         ]);
         exchange = new ExchangeContract(exchangeInstance.abi, exchangeInstance.address, provider);
-        await assetProxyDispatcher.addAuthorizedAddress.sendTransactionAsync(exchange.address, { from: owner });
         zeroEx = new ZeroEx(provider, {
             exchangeContractAddress: exchange.address,
             networkId: constants.TESTRPC_NETWORK_ID,
         });
         exchangeWrapper = new ExchangeWrapper(exchange, zeroEx);
+        await exchangeWrapper.registerAssetProxyAsync(AssetProxyId.ERC20, erc20Proxy.address, owner);
+        await exchangeWrapper.registerAssetProxyAsync(AssetProxyId.ERC721, erc721Proxy.address, owner);
+
+        await erc20Proxy.addAuthorizedAddress.sendTransactionAsync(exchange.address, {
+            from: owner,
+        });
+        await erc721Proxy.addAuthorizedAddress.sendTransactionAsync(exchange.address, {
+            from: owner,
+        });
 
         defaultMakerAssetAddress = erc20TokenA.address;
         defaultTakerAssetAddress = erc20TokenB.address;