@0x:contracts-integrations Addressed review comments

contracts/integrations/test/examples/simple.ts

@@ -1,10 +1,8 @@
-import { blockchainTests, constants, expect } from '@0x/contracts-test-utils';
+import { blockchainTests, expect } from '@0x/contracts-test-utils';
 import { BigNumber } from '@0x/utils';
-import { TxData, TransactionReceiptWithDecodedLogs } from 'ethereum-types';
 
 import { artifacts, TestFrameworkContract } from '../../src';
-import { Condition, FunctionAssertion, Result } from '../utils/function_assertions';
+import { FunctionAssertion, Result } from '../utils/function_assertions';
-import { GetterCache } from '../utils/cache';
 
 // These tests provide examples for how to use the "FunctionAssertion" class to write
 // tests for "payable" and "nonpayable" Solidity functions as well as "pure" and "view" functions.

contracts/integrations/test/framework-unit-tests/function_assertion_test.ts

@@ -1,5 +1,5 @@
 import { blockchainTests, constants, expect, filterLogsToArguments, hexRandom } from '@0x/contracts-test-utils';
-import { BigNumber } from '@0x/utils';
+import { BigNumber, generatePseudoRandom256BitNumber } from '@0x/utils';
 import { TransactionReceiptWithDecodedLogs } from 'ethereum-types';
 
 import { artifacts, TestFrameworkContract, TestFrameworkSomeEventEventArgs, TestFrameworkEvents } from '../../src';
@@ -8,10 +8,6 @@ import { FunctionAssertion, Result } from '../utils/function_assertions';
 blockchainTests.resets('FunctionAssertion Unit Tests', env => {
     let exampleContract: TestFrameworkContract;
 
-    function randomBigNumber(): BigNumber {
-        return new BigNumber(hexRandom(constants.WORD_LENGTH));
-    }
-
     before(async () => {
         exampleContract = await TestFrameworkContract.deployFrom0xArtifactAsync(
             artifacts.TestFramework,
@@ -23,107 +19,107 @@ blockchainTests.resets('FunctionAssertion Unit Tests', env => {
 
     describe('runAsync', () => {
         it('should call the before function with the provided arguments', async () => {
-            let beforeIntrospectionObject = constants.ZERO_AMOUNT;
+            let sideEffectTarget = constants.ZERO_AMOUNT;
             const assertion = new FunctionAssertion(exampleContract.noEffect, {
-                before: async (returnValue: BigNumber) => {
+                before: async (inputValue: BigNumber) => {
-                    beforeIntrospectionObject = returnValue;
+                    sideEffectTarget = inputValue;
                 },
                 after: async (beforeInfo: any, result: Result, returnValue: BigNumber) => {},
             });
 
-            const randomInput = randomBigNumber();
+            const randomInput = generatePseudoRandom256BitNumber();
             await assertion.runAsync(randomInput);
-            expect(beforeIntrospectionObject).bignumber.to.be.eq(randomInput);
+            expect(sideEffectTarget).bignumber.to.be.eq(randomInput);
         });
 
         it('should call the after function with the provided arguments', async () => {
-            let afterIntrospectionObject = constants.ZERO_AMOUNT;
+            let sideEffectTarget = constants.ZERO_AMOUNT;
             const assertion = new FunctionAssertion(exampleContract.noEffect, {
-                before: async (returnValue: BigNumber) => {},
+                before: async (inputValue: BigNumber) => {},
                 after: async (beforeInfo: any, result: Result, returnValue: BigNumber) => {
-                    afterIntrospectionObject = returnValue;
+                    sideEffectTarget = returnValue;
                 },
             });
 
-            const randomInput = randomBigNumber();
+            const randomInput = generatePseudoRandom256BitNumber();
             await assertion.runAsync(randomInput);
-            expect(afterIntrospectionObject).bignumber.to.be.eq(randomInput);
+            expect(sideEffectTarget).bignumber.to.be.eq(randomInput);
         });
 
         it('should not fail immediately if the wrapped function fails', async () => {
             await exampleContract.setCounter.awaitTransactionSuccessAsync(new BigNumber(1));
             const assertion = new FunctionAssertion(exampleContract.revertSideEffect, {
-                before: async (returnValue: BigNumber) => {},
+                before: async (inputValue: BigNumber) => {},
                 after: async (beforeInfo: any, result: Result, returnValue: BigNumber) => {},
             });
-            const randomInput = randomBigNumber();
+            const randomInput = generatePseudoRandom256BitNumber();
             await assertion.runAsync(randomInput);
         });
 
         it('should pass the return value from "before" to "after"', async () => {
-            const randomInput = randomBigNumber();
+            const randomInput = generatePseudoRandom256BitNumber();
-            let afterIntrospectionObject = constants.ZERO_AMOUNT;
+            let sideEffectTarget = constants.ZERO_AMOUNT;
             const assertion = new FunctionAssertion(exampleContract.noEffect, {
-                before: async (returnValue: BigNumber) => {
+                before: async (inputValue: BigNumber) => {
                     return randomInput;
                 },
                 after: async (beforeInfo: any, result: Result, returnValue: BigNumber) => {
-                    afterIntrospectionObject = beforeInfo;
+                    sideEffectTarget = beforeInfo;
                 },
             });
             await assertion.runAsync(randomInput);
-            expect(afterIntrospectionObject).bignumber.to.be.eq(randomInput);
+            expect(sideEffectTarget).bignumber.to.be.eq(randomInput);
         });
 
         it('should pass the result from the function call to "after"', async () => {
-            let afterIntrospectionObject = constants.ZERO_AMOUNT;
+            let sideEffectTarget = constants.ZERO_AMOUNT;
             const assertion = new FunctionAssertion(exampleContract.revertSideEffect, {
-                before: async (returnValue: BigNumber) => {},
+                before: async (inputValue: BigNumber) => {},
                 after: async (beforeInfo: any, result: Result, returnValue: BigNumber) => {
-                    afterIntrospectionObject = result.data;
+                    sideEffectTarget = result.data;
                 },
             });
-            const randomInput = randomBigNumber();
+            const randomInput = generatePseudoRandom256BitNumber();
             await assertion.runAsync(randomInput);
-            expect(afterIntrospectionObject).bignumber.to.be.eq(randomInput);
+            expect(sideEffectTarget).bignumber.to.be.eq(randomInput);
         });
 
         it('should pass the receipt from the function call to "after"', async () => {
-            let afterIntrospectionObject: TransactionReceiptWithDecodedLogs = {} as TransactionReceiptWithDecodedLogs;
+            let sideEffectTarget: TransactionReceiptWithDecodedLogs = {} as TransactionReceiptWithDecodedLogs;
             const assertion = new FunctionAssertion(exampleContract.revertSideEffect, {
-                before: async (returnValue: BigNumber) => {},
+                before: async (inputValue: BigNumber) => {},
                 after: async (beforeInfo: any, result: Result, returnValue: BigNumber) => {
                     if (result.receipt) {
-                        afterIntrospectionObject = result.receipt;
+                        sideEffectTarget = result.receipt;
                     } else {
                         throw new Error('No receipt received.');
                     }
                 },
             });
-            const randomInput = randomBigNumber();
+            const randomInput = generatePseudoRandom256BitNumber();
             await assertion.runAsync(randomInput);
 
             // Ensure that the correct events were emitted.
             const [event] = filterLogsToArguments<TestFrameworkSomeEventEventArgs>(
-                afterIntrospectionObject.logs,
+                sideEffectTarget.logs,
                 TestFrameworkEvents.SomeEvent,
             );
             expect(event).to.be.deep.eq({ someNumber: randomInput });
         });
 
         it('should pass the error to "after" if the function call fails', async () => {
-            let afterIntrospectionObject: Error = new Error('');
+            let sideEffectTarget: Error = new Error('');
             await exampleContract.setCounter.awaitTransactionSuccessAsync(new BigNumber(1));
             const assertion = new FunctionAssertion(exampleContract.revertSideEffect, {
-                before: async (returnValue: BigNumber) => {},
+                before: async (inputValue: BigNumber) => {},
                 after: async (beforeInfo: any, result: Result, returnValue: BigNumber) => {
-                    afterIntrospectionObject = result.data;
+                    sideEffectTarget = result.data;
                 },
             });
-            const randomInput = randomBigNumber();
+            const randomInput = generatePseudoRandom256BitNumber();
             await assertion.runAsync(randomInput);
             const errorMessage = 'VM Exception while processing transaction: revert Revert';
-            expect(afterIntrospectionObject.message).to.be.eq(errorMessage);
+            expect(sideEffectTarget.message).to.be.eq(errorMessage);
         });
     });
 });

contracts/integrations/test/framework-unit-tests/getter_cache_test.ts

@@ -45,7 +45,7 @@ blockchainTests.resets('Cache Tests', env => {
                 // Update the counter to 1.
                 await exampleContract.setCounter.awaitTransactionSuccessAsync(new BigNumber(1));
 
-                // Ensure that the returned value is the cached counter.
+                // This should return "0" because a value was cached by the first call to "callAsync"
                 expect(await cache.callAsync()).bignumber.to.be.eq(constants.ZERO_AMOUNT);
             });
         });
@@ -74,7 +74,7 @@ blockchainTests.resets('Cache Tests', env => {
                 // Update "counter" to "1", which will cause all calls of "isZeroOrFalse" to return "false".
                 await exampleContract.setCounter.awaitTransactionSuccessAsync(new BigNumber(1));
 
-                // This should return "true" because a value was cached.
+                // This should return "true" because a value was by the first call to "callAsync"
                 expect(await cache.callAsync(constants.ZERO_AMOUNT)).to.be.true();
             });
         });
@@ -115,19 +115,16 @@ blockchainTests.resets('Cache Tests', env => {
                     new BigNumber(1),
                     ethUtil.bufferToHex(ethUtil.sha3(0)),
                 );
+                const hash = ethUtil.bufferToHex(ethUtil.sha3(hexSlice(hashData, 4)));
 
                 // Ensure that the cache returns the correct value.
-                expect(await cache.callAsync(new BigNumber(1), ethUtil.bufferToHex(ethUtil.sha3(0)))).to.be.eq(
+                expect(await cache.callAsync(new BigNumber(1), ethUtil.bufferToHex(ethUtil.sha3(0)))).to.be.eq(hash);
-                    ethUtil.bufferToHex(ethUtil.sha3(hexSlice(hashData, 4))),
-                );
 
                 // Update "counter" to "1", which will cause all calls to return the null hash.
                 await exampleContract.setCounter.awaitTransactionSuccessAsync(new BigNumber(1));
 
-                // Ensure that the cache returns the correct value.
+                // The cache should return the same value as the first call to `callAsync` since a value was cached.
-                expect(await cache.callAsync(new BigNumber(1), ethUtil.bufferToHex(ethUtil.sha3(0)))).to.be.eq(
+                expect(await cache.callAsync(new BigNumber(1), ethUtil.bufferToHex(ethUtil.sha3(0)))).to.be.eq(hash);
-                    ethUtil.bufferToHex(ethUtil.sha3(hexSlice(hashData, 4))),
-                );
             });
         });
     });

contracts/integrations/test/internal-integration-tests/exchange_integration_test.ts

@@ -11,8 +11,8 @@ import { DeploymentManager } from '../utils/deployment_manager';
 blockchainTests('Exchange & Staking', env => {
     let accounts: string[];
     let makerAddress: string;
-    let takers: string[];
+    let takers: string[] = [];
-    let delegators: string[];
+    let delegators: string[] = [];
     let feeRecipientAddress: string;
     let addressManager: AddressManager;
     let deploymentManager: DeploymentManager;
@@ -21,15 +21,12 @@ blockchainTests('Exchange & Staking', env => {
     let takerAsset: DummyERC20TokenContract;
     let feeAsset: DummyERC20TokenContract;
 
-    const gasPrice = 1e9;
+    const GAS_PRICE = 1e9;
 
     before(async () => {
         const chainId = await env.getChainIdAsync();
         accounts = await env.getAccountAddressesAsync();
-        makerAddress = accounts[1];
+        [makerAddress, feeRecipientAddress, takers[0], takers[1], ...delegators] = accounts.slice(1);
-        feeRecipientAddress = accounts[2];
-        takers = [accounts[3], accounts[4]];
-        delegators = [accounts[5], accounts[6], accounts[7]];
         deploymentManager = await DeploymentManager.deployAsync(env);
 
         // Create a staking pool with the operator as a maker address.
@@ -55,21 +52,20 @@ blockchainTests('Exchange & Staking', env => {
         );
 
         // Set up two addresses for taking orders.
-        await addressManager.batchAddTakerAsync(
+        await Promise.all(
-            deploymentManager,
+            takers.map(taker =>
-            takers.map(takerAddress => {
+                addressManager.addTakerAsync(deploymentManager, {
-                return {
+                    address: taker,
-                    address: takerAddress,
                     mainToken: deploymentManager.tokens.erc20[1],
                     feeToken: deploymentManager.tokens.erc20[2],
-                };
+                }),
-            }),
+            ),
         );
     });
 
     describe('fillOrder', () => {
         it('should be able to fill an order', async () => {
-            const order = await addressManager.makerAddresses[0].orderFactory.newSignedOrderAsync({
+            const order = await addressManager.makers[0].orderFactory.newSignedOrderAsync({
                 makerAddress,
                 makerAssetAmount: new BigNumber(1),
                 takerAssetAmount: new BigNumber(1),
@@ -84,8 +80,8 @@ blockchainTests('Exchange & Staking', env => {
                 order.signature,
                 {
                     from: takers[0],
-                    gasPrice,
+                    gasPrice: GAS_PRICE,
-                    value: DeploymentManager.protocolFeeMultiplier.times(gasPrice),
+                    value: DeploymentManager.protocolFeeMultiplier.times(GAS_PRICE),
                 },
             );
 
@@ -112,7 +108,7 @@ blockchainTests('Exchange & Staking', env => {
                     takerAssetFilledAmount: order.takerAssetAmount,
                     makerFeePaid: constants.ZERO_AMOUNT,
                     takerFeePaid: constants.ZERO_AMOUNT,
-                    protocolFeePaid: DeploymentManager.protocolFeeMultiplier.times(gasPrice),
+                    protocolFeePaid: DeploymentManager.protocolFeeMultiplier.times(GAS_PRICE),
                 },
             ]);
 

contracts/integrations/test/utils/address_manager.ts

@@ -17,10 +17,10 @@ interface ConfigurationArgs {
 
 export class AddressManager {
     // A set of addresses that have been configured for market making.
-    public makerAddresses: MarketMaker[];
+    public makers: MarketMaker[];
 
     // A set of addresses that have been configured to take orders.
-    public takerAddresses: string[];
+    public takers: string[];
 
     /**
      * Sets up an address to take orders.
@@ -31,19 +31,7 @@ export class AddressManager {
         await this._configureTokenForAddressAsync(deploymentManager, configArgs.address, configArgs.feeToken);
 
         // Add the taker to the list of configured taker addresses.
-        this.takerAddresses.push(configArgs.address);
+        this.takers.push(configArgs.address);
-    }
-
-    /**
-     * Sets up a list of addresses to take orders.
-     */
-    public async batchAddTakerAsync(
-        deploymentManager: DeploymentManager,
-        configArgs: ConfigurationArgs[],
-    ): Promise<void> {
-        for (const args of configArgs) {
-            await this.addTakerAsync(deploymentManager, args);
-        }
     }
 
     /**
@@ -79,35 +67,18 @@ export class AddressManager {
         await this._configureTokenForAddressAsync(deploymentManager, configArgs.address, configArgs.feeToken);
 
         // Add the maker to the list of configured maker addresses.
-        this.makerAddresses.push({
+        this.makers.push({
             address: configArgs.address,
             orderFactory,
         });
     }
 
-    /**
-     * Sets up several market makers.
-     */
-    public async batchAddMakerAsync(
-        deploymentManager: DeploymentManager,
-        configArgs: ConfigurationArgs[],
-        environment: BlockchainTestsEnvironment,
-        takerToken: DummyERC20TokenContract,
-        feeRecipientAddress: string,
-        chainId: number,
-    ): Promise<void> {
-        for (const args of configArgs) {
-            await this.addMakerAsync(deploymentManager, args, environment, takerToken, feeRecipientAddress, chainId);
-        }
-    }
-
     /**
      * Sets up initial account balances for a token and approves the ERC20 asset proxy
      * to transfer the token.
      */
     protected async _configureTokenForAddressAsync(
-        deploymentManager: DeploymentManager,
+        deploymentManager: DeploymentManager, address: string,
-        address: string,
         token: DummyERC20TokenContract,
     ): Promise<void> {
         await token.setBalance.awaitTransactionSuccessAsync(address, constants.INITIAL_ERC20_BALANCE);
@@ -119,7 +90,7 @@ export class AddressManager {
     }
 
     constructor() {
-        this.makerAddresses = [];
+        this.makers = [];
-        this.takerAddresses = [];
+        this.takers = [];
     }
 }

contracts/integrations/test/utils/cache.ts

@@ -46,12 +46,14 @@ export class GetterCache {
     }
 }
 
+export interface GetterCacheSet {
+    [getter: string]: GetterCache;
+}
+
 export class GetterCacheCollection {
     // A dictionary of getter cache's that allow the user of the collection to reference
     // the getter functions by name.
-    public getters: {
+    public getters: GetterCacheSet;
-        [getter: string]: GetterCache;
-    };
 
     /**
      * Constructs a getter collection with pre-seeded getter names and getters.

contracts/integrations/test/utils/function_assertions.ts

@@ -1,8 +1,5 @@
 import { PromiseWithTransactionHash } from '@0x/base-contract';
-import { BlockParam, CallData, TransactionReceiptWithDecodedLogs, TxData } from 'ethereum-types';
+import { TransactionReceiptWithDecodedLogs } from 'ethereum-types';
-import * as _ from 'lodash';
-
-import { DeploymentManager } from './';
 
 export interface ContractGetterFunction {
     callAsync: (...args: any[]) => Promise<any>;