From 0e3544e1f997d57089023e65ea9030270914e8eb Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Tue, 17 Apr 2018 12:01:33 -0700 Subject: Miscellaneous style changes to the contracts package; specifically tests --- packages/0x.js/package.json | 3 +- packages/contracts/package.json | 2 +- .../contracts/current/utils/LibBytes/LibBytes.sol | 2 +- packages/contracts/src/utils/types.ts | 4 +-- .../contracts/test/asset_proxy_dispatcher/auth.ts | 2 +- .../test/asset_proxy_dispatcher/dispatcher.ts | 37 ++++++++++++++-------- packages/contracts/test/exchange/core.ts | 6 ++-- packages/contracts/test/exchange/wrapper.ts | 6 ++-- ...i_sig_with_time_lock_except_remove_auth_addr.ts | 2 +- 9 files changed, 40 insertions(+), 24 deletions(-) (limited to 'packages') diff --git a/packages/0x.js/package.json b/packages/0x.js/package.json index b9a4c1ec5..79053e033 100644 --- a/packages/0x.js/package.json +++ b/packages/0x.js/package.json @@ -26,7 +26,8 @@ "build:umd:prod": "NODE_ENV=production webpack", "build:commonjs": "tsc && yarn update_artifacts && copyfiles -u 2 './src/compact_artifacts/**/*.json' ./lib/src/compact_artifacts && copyfiles -u 3 './lib/src/monorepo_scripts/**/*' ./scripts", "test:commonjs": "run-s build:commonjs run_mocha", - "_comment": {"run_mocha": "mocha lib/test/**/*_test.js lib/test/global_hooks.js --timeout 10000 --bail --exit"}, + "_comment": {"run_mocha": "mocha lib/test/**/*_test.js lib/test/global_hooks.js --timeout 10000 --bail --exit", + "note": "The `run_mocha` test has been commented out by @hysz (greg@0xproject.com) until the migration package and 0x.js have been updated for V2."}, "run_mocha": "", "manual:postpublish": "yarn build; node ./scripts/postpublish.js", "docs:stage": "yarn build && node ./scripts/stage_docs.js", diff --git a/packages/contracts/package.json b/packages/contracts/package.json index d4bab4dca..8be09c9f8 100644 --- a/packages/contracts/package.json +++ b/packages/contracts/package.json @@ -28,7 +28,7 @@ "config": { "abis": "../migrations/src/artifacts/@(DummyToken|Exchange|TokenRegistry|MultiSigWallet|MultiSigWalletWithTimeLock|MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress|TokenRegistry|ZRXToken|AssetProxyDispatcher|ERC20Proxy|ERC721Proxy|DummyERC721Token).json", "contracts": "Exchange,DummyToken,ZRXToken,Token,WETH9,MultiSigWallet,MultiSigWalletWithTimeLock,MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress,MaliciousToken,TokenRegistry,AssetProxyDispatcher,ERC20Proxy,ERC721Proxy,DummyERC721Token", - "dirs": "src/contracts,zeppelin:../../node_modules/zeppelin-solidity" + "contract_dirs": "src/contracts,zeppelin:../../node_modules/zeppelin-solidity" }, "repository": { "type": "git", diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index 90f3b32d7..f78d9baec 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -70,7 +70,7 @@ contract LibBytes { // Read address from array memory assembly { - // 1. Add index to to address of bytes array + // 1. Add index to address of bytes array // 2. Load 32-byte word from memory // 3. Apply 20-byte mask to obtain address result := and(mload(add(b, index)), 0xffffffffffffffffffffffffffffffffffffffff) diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts index 5bb1a36d4..0e30ddaee 100644 --- a/packages/contracts/src/utils/types.ts +++ b/packages/contracts/src/utils/types.ts @@ -51,8 +51,8 @@ export interface DefaultOrderParams { takerTokenAddress: string; makerTokenAmount: BigNumber; takerTokenAmount: BigNumber; - makerFeeAmount: BigNumber; - takerFeeAmount: BigNumber; + makerFee: BigNumber; + takerFee: BigNumber; makerAssetData: string; takerAssetData: string; } diff --git a/packages/contracts/test/asset_proxy_dispatcher/auth.ts b/packages/contracts/test/asset_proxy_dispatcher/auth.ts index ded5ff287..9d8645ab7 100644 --- a/packages/contracts/test/asset_proxy_dispatcher/auth.ts +++ b/packages/contracts/test/asset_proxy_dispatcher/auth.ts @@ -14,7 +14,7 @@ chaiSetup.configure(); const expect = chai.expect; const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); -describe('AssetProxyDispatcher - Auth', () => { +describe('AssetProxyDispatcher (Authorization Methods)', () => { let owner: string; let notOwner: string; let address: string; diff --git a/packages/contracts/test/asset_proxy_dispatcher/dispatcher.ts b/packages/contracts/test/asset_proxy_dispatcher/dispatcher.ts index eb23a988f..44b3ac6fb 100644 --- a/packages/contracts/test/asset_proxy_dispatcher/dispatcher.ts +++ b/packages/contracts/test/asset_proxy_dispatcher/dispatcher.ts @@ -88,10 +88,11 @@ describe('AssetProxyDispatcher', () => { }); describe('addAssetProxy', () => { it('should record proxy upon registration', async () => { + const prevProxyAddress = ZeroEx.NULL_ADDRESS; await assetProxyDispatcher.addAssetProxy.sendTransactionAsync( AssetProxyId.ERC20, erc20Proxy.address, - ZeroEx.NULL_ADDRESS, + prevProxyAddress, { from: owner }, ); const proxyAddress = await assetProxyDispatcher.getAssetProxy.callAsync(AssetProxyId.ERC20); @@ -100,19 +101,21 @@ describe('AssetProxyDispatcher', () => { it('should be able to record multiple proxies', async () => { // Record first proxy + const prevERC20ProxyAddress = ZeroEx.NULL_ADDRESS; await assetProxyDispatcher.addAssetProxy.sendTransactionAsync( AssetProxyId.ERC20, erc20Proxy.address, - ZeroEx.NULL_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.addAssetProxy.sendTransactionAsync( AssetProxyId.ERC721, erc721Proxy.address, - ZeroEx.NULL_ADDRESS, + prevERC721ProxyAddress, { from: owner }, ); proxyAddress = await assetProxyDispatcher.getAssetProxy.callAsync(AssetProxyId.ERC721); @@ -121,10 +124,11 @@ describe('AssetProxyDispatcher', () => { it('should replace proxy address upon re-registration', async () => { // Initial registration + const prevProxyAddress = ZeroEx.NULL_ADDRESS; await assetProxyDispatcher.addAssetProxy.sendTransactionAsync( AssetProxyId.ERC20, erc20Proxy.address, - ZeroEx.NULL_ADDRESS, + prevProxyAddress, { from: owner }, ); let proxyAddress = await assetProxyDispatcher.getAssetProxy.callAsync(AssetProxyId.ERC20); @@ -152,10 +156,11 @@ describe('AssetProxyDispatcher', () => { it('should throw if registering with incorrect "currentAssetProxyAddress" field', async () => { // Initial registration + const prevProxyAddress = ZeroEx.NULL_ADDRESS; await assetProxyDispatcher.addAssetProxy.sendTransactionAsync( AssetProxyId.ERC20, erc20Proxy.address, - ZeroEx.NULL_ADDRESS, + prevProxyAddress, { from: owner }, ); const proxyAddress = await assetProxyDispatcher.getAssetProxy.callAsync(AssetProxyId.ERC20); @@ -173,31 +178,34 @@ describe('AssetProxyDispatcher', () => { it('should be able to reset proxy address to NULL', async () => { // Initial registration + const prevProxyAddress = ZeroEx.NULL_ADDRESS; await assetProxyDispatcher.addAssetProxy.sendTransactionAsync( AssetProxyId.ERC20, erc20Proxy.address, - ZeroEx.NULL_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.addAssetProxy.sendTransactionAsync( AssetProxyId.ERC20, - ZeroEx.NULL_ADDRESS, + newProxyAddress, erc20Proxy.address, { from: owner }, ); - const newProxyAddress = await assetProxyDispatcher.getAssetProxy.callAsync(AssetProxyId.ERC20); - expect(newProxyAddress).to.be.equal(ZeroEx.NULL_ADDRESS); + 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.addAssetProxy.sendTransactionAsync( AssetProxyId.ERC20, erc20Proxy.address, - ZeroEx.NULL_ADDRESS, + prevProxyAddress, { from: notOwner }, ), ).to.be.rejectedWith(constants.REVERT); @@ -206,10 +214,11 @@ describe('AssetProxyDispatcher', () => { describe('getAssetProxy', () => { it('should return correct address of registered proxy', async () => { + const prevProxyAddress = ZeroEx.NULL_ADDRESS; await assetProxyDispatcher.addAssetProxy.sendTransactionAsync( AssetProxyId.ERC20, erc20Proxy.address, - ZeroEx.NULL_ADDRESS, + prevProxyAddress, { from: owner }, ); const proxyAddress = await assetProxyDispatcher.getAssetProxy.callAsync(AssetProxyId.ERC20); @@ -225,10 +234,11 @@ describe('AssetProxyDispatcher', () => { describe('transferFrom', () => { it('should dispatch transfer to registered proxy', async () => { // Register ERC20 proxy + const prevProxyAddress = ZeroEx.NULL_ADDRESS; await assetProxyDispatcher.addAssetProxy.sendTransactionAsync( AssetProxyId.ERC20, erc20Proxy.address, - ZeroEx.NULL_ADDRESS, + prevProxyAddress, { from: owner }, ); // Construct metadata for ERC20 proxy @@ -272,10 +282,11 @@ describe('AssetProxyDispatcher', () => { it('should throw on transfer if requesting address is not authorized', async () => { // Register ERC20 proxy + const prevProxyAddress = ZeroEx.NULL_ADDRESS; await assetProxyDispatcher.addAssetProxy.sendTransactionAsync( AssetProxyId.ERC20, erc20Proxy.address, - ZeroEx.NULL_ADDRESS, + prevProxyAddress, { from: owner }, ); // Construct metadata for ERC20 proxy diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts index 6971ac170..4e5ab4031 100644 --- a/packages/contracts/test/exchange/core.ts +++ b/packages/contracts/test/exchange/core.ts @@ -107,10 +107,11 @@ describe('Exchange', () => { await erc20Proxy.addAuthorizedAddress.sendTransactionAsync(assetProxyDispatcher.address, { from: owner, }); + const prevERC20ProxyAddress = ZeroEx.NULL_ADDRESS; await assetProxyDispatcher.addAssetProxy.sendTransactionAsync( AssetProxyId.ERC20, erc20Proxy.address, - ZeroEx.NULL_ADDRESS, + prevERC20ProxyAddress, { from: owner }, ); // Deploy ERC721 Proxy @@ -119,10 +120,11 @@ describe('Exchange', () => { await erc721Proxy.addAuthorizedAddress.sendTransactionAsync(assetProxyDispatcher.address, { from: owner, }); + const prevERC721ProxyAddress = ZeroEx.NULL_ADDRESS; await assetProxyDispatcher.addAssetProxy.sendTransactionAsync( AssetProxyId.ERC721, erc721Proxy.address, - ZeroEx.NULL_ADDRESS, + prevERC721ProxyAddress, { from: owner }, ); // Deploy and configure Exchange diff --git a/packages/contracts/test/exchange/wrapper.ts b/packages/contracts/test/exchange/wrapper.ts index fe754c679..5f1d3111b 100644 --- a/packages/contracts/test/exchange/wrapper.ts +++ b/packages/contracts/test/exchange/wrapper.ts @@ -88,10 +88,11 @@ describe('Exchange', () => { await erc20Proxy.addAuthorizedAddress.sendTransactionAsync(assetProxyDispatcher.address, { from: owner, }); + const prevERC20ProxyAddress = ZeroEx.NULL_ADDRESS; await assetProxyDispatcher.addAssetProxy.sendTransactionAsync( AssetProxyId.ERC20, erc20Proxy.address, - ZeroEx.NULL_ADDRESS, + prevERC20ProxyAddress, { from: owner }, ); // Deploy ERC721 Proxy @@ -100,10 +101,11 @@ describe('Exchange', () => { await erc721Proxy.addAuthorizedAddress.sendTransactionAsync(assetProxyDispatcher.address, { from: owner, }); + const prevERC721ProxyAddress = ZeroEx.NULL_ADDRESS; await assetProxyDispatcher.addAssetProxy.sendTransactionAsync( AssetProxyId.ERC721, erc721Proxy.address, - ZeroEx.NULL_ADDRESS, + prevERC721ProxyAddress, { from: owner }, ); // Deploy and configure Exchange diff --git a/packages/contracts/test/multi_sig_with_time_lock_except_remove_auth_addr.ts b/packages/contracts/test/multi_sig_with_time_lock_except_remove_auth_addr.ts index 762292c9f..74475c3db 100644 --- a/packages/contracts/test/multi_sig_with_time_lock_except_remove_auth_addr.ts +++ b/packages/contracts/test/multi_sig_with_time_lock_except_remove_auth_addr.ts @@ -1,7 +1,7 @@ /* * * @TODO: Before deploying, the MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress contract must be updated - * to have a mapping of all approved addresses. These tests muts be updated appropriately. + * to have a mapping of all approved addresses. These tests must be updated appropriately. * For now, these tests have been commented out by @hysz (greg@0xproject.com). * import { LogWithDecodedArgs, ZeroEx } from '0x.js'; -- cgit v1.2.3