Readability improvements

contracts/exchange/test/assertion_wrappers/fill_order_wrapper.ts

@@ -1,9 +1,15 @@
-import { artifacts as proxyArtifacts , ERC1155ProxyWrapper, ERC20Wrapper, ERC721Wrapper } from '@0x/contracts-asset-proxy';
+import {
+    artifacts as proxyArtifacts,
+    ERC1155ProxyWrapper,
+    ERC20Wrapper,
+    ERC721Wrapper,
+} from '@0x/contracts-asset-proxy';
 import { artifacts as erc20Artifacts } from '@0x/contracts-erc20';
 import { artifacts as erc721Artifacts } from '@0x/contracts-erc721';
 import {
-    chaiSetup,
+    expect,
     FillEventArgs,
+    filterLogsToArguments,
     LogDecoder,
     OrderStatus,
     orderUtils,
@@ -13,22 +19,20 @@ import { orderHashUtils } from '@0x/order-utils';
 import { FillResults, SignedOrder } from '@0x/types';
 import { BigNumber } from '@0x/utils';
 import { Web3Wrapper } from '@0x/web3-wrapper';
-import * as chai from 'chai';
+import { TransactionReceiptWithDecodedLogs, ZeroExProvider } from 'ethereum-types';
-import { LogWithDecodedArgs, TransactionReceiptWithDecodedLogs, ZeroExProvider } from 'ethereum-types';
 import * as _ from 'lodash';
 
 import { artifacts, ExchangeContract } from '../../src';
+import { calculateFillResults } from '../../src/reference_functions';
 import { BalanceStore } from '../balance_stores/balance_store';
 import { BlockchainBalanceStore } from '../balance_stores/blockchain_balance_store';
 import { LocalBalanceStore } from '../balance_stores/local_balance_store';
 
-chaiSetup.configure();
-const expect = chai.expect;
-
 export class FillOrderWrapper {
     private readonly _exchange: ExchangeContract;
     private readonly _blockchainBalanceStore: BlockchainBalanceStore;
     private readonly _web3Wrapper: Web3Wrapper;
+
     /**
      * Simulates matching two orders by transferring amounts defined in
      * `transferAmounts` and returns the results.
@@ -43,16 +47,11 @@ export class FillOrderWrapper {
         takerAddress: string,
         opts: { takerAssetFillAmount?: BigNumber } = {},
         initBalanceStore: BalanceStore,
-        partialFillResults: Partial<FillResults>,
     ): [FillResults, FillEventArgs, BalanceStore] {
         const balanceStore = LocalBalanceStore.create(initBalanceStore);
         const takerAssetFillAmount =
             opts.takerAssetFillAmount !== undefined ? opts.takerAssetFillAmount : signedOrder.takerAssetAmount;
-        const fillResults = FillOrderWrapper._createFillResultsFromPartial(
+        const fillResults = calculateFillResults(signedOrder, takerAssetFillAmount);
-            partialFillResults,
-            signedOrder,
-            takerAssetFillAmount,
-        );
         const fillEvent = FillOrderWrapper.simulateFillEvent(signedOrder, takerAddress, fillResults);
         // Taker -> Maker
         balanceStore.transferAsset(
@@ -61,13 +60,6 @@ export class FillOrderWrapper {
             fillResults.takerAssetFilledAmount,
             signedOrder.takerAssetData,
         );
-        // Taker -> Fee Recipient
-        balanceStore.transferAsset(
-            takerAddress,
-            signedOrder.feeRecipientAddress,
-            fillResults.takerFeePaid,
-            signedOrder.takerFeeAssetData,
-        );
         // Maker -> Taker
         balanceStore.transferAsset(
             signedOrder.makerAddress,
@@ -75,6 +67,13 @@ export class FillOrderWrapper {
             fillResults.makerAssetFilledAmount,
             signedOrder.makerAssetData,
         );
+        // Taker -> Fee Recipient
+        balanceStore.transferAsset(
+            takerAddress,
+            signedOrder.feeRecipientAddress,
+            fillResults.takerFeePaid,
+            signedOrder.takerFeeAssetData,
+        );
         // Maker -> Fee Recipient
         balanceStore.transferAsset(
             signedOrder.makerAddress,
@@ -84,6 +83,7 @@ export class FillOrderWrapper {
         );
         return [fillResults, fillEvent, balanceStore];
     }
+
     /**
      *  Simulates the event emitted by the exchange contract when an order is filled.
      */
@@ -99,61 +99,24 @@ export class FillOrderWrapper {
             takerFeePaid: fillResults.takerFeePaid,
         };
     }
+
     /**
      * Extract the exchanges `Fill` event from a transaction receipt.
      */
-    public static _extractFillEventsfromReceipt(receipt: TransactionReceiptWithDecodedLogs): FillEventArgs[] {
+    private static _extractFillEventsfromReceipt(receipt: TransactionReceiptWithDecodedLogs): FillEventArgs[] {
-        interface RawFillEventArgs {
+        const events = filterLogsToArguments<FillEventArgs>(receipt.logs, 'Fill');
-            orderHash: string;
+        const fieldsOfInterest = [
-            makerAddress: string;
+            'orderHash',
-            takerAddress: string;
+            'makerAddress',
-            makerAssetFilledAmount: string;
+            'takerAddress',
-            takerAssetFilledAmount: string;
+            'makerAssetFilledAmount',
-            makerFeePaid: string;
+            'takerAssetFilledAmount',
-            takerFeePaid: string;
+            'makerFeePaid',
-        }
+            'takerFeePaid',
-        const actualFills = (_.filter(receipt.logs, ['event', 'Fill']) as any) as Array<
+        ];
-            LogWithDecodedArgs<RawFillEventArgs>
+        return events.map(event => _.pick(event, fieldsOfInterest)) as FillEventArgs[];
-        >;
-        // Convert RawFillEventArgs to FillEventArgs.
-        return actualFills.map(fill => ({
-            orderHash: fill.args.orderHash,
-            makerAddress: fill.args.makerAddress,
-            takerAddress: fill.args.takerAddress,
-            makerAssetFilledAmount: new BigNumber(fill.args.makerAssetFilledAmount),
-            takerAssetFilledAmount: new BigNumber(fill.args.takerAssetFilledAmount),
-            makerFeePaid: new BigNumber(fill.args.makerFeePaid),
-            takerFeePaid: new BigNumber(fill.args.takerFeePaid),
-        }));
-    }
-    /**
-     * Returns a completed `FillResults` given a partial instance. The correct values are inferred by the
-     * values in the `signedOrder` and `takerAssetFillAmount`.
-     */
-    private static _createFillResultsFromPartial(
-        partialFillResults: Partial<FillResults>,
-        signedOrder: SignedOrder,
-        takerAssetFillAmount: BigNumber,
-    ): FillResults {
-        return {
-            makerAssetFilledAmount:
-                partialFillResults.makerAssetFilledAmount !== undefined
-                    ? partialFillResults.makerAssetFilledAmount
-                    : signedOrder.makerAssetAmount.times(takerAssetFillAmount).dividedBy(signedOrder.takerAssetAmount),
-            takerAssetFilledAmount:
-                partialFillResults.takerAssetFilledAmount !== undefined
-                    ? partialFillResults.takerAssetFilledAmount
-                    : signedOrder.takerAssetAmount.times(takerAssetFillAmount).dividedBy(signedOrder.takerAssetAmount),
-            makerFeePaid:
-                partialFillResults.makerFeePaid !== undefined
-                    ? partialFillResults.makerFeePaid
-                    : signedOrder.makerFee.times(takerAssetFillAmount).dividedBy(signedOrder.takerAssetAmount),
-            takerFeePaid:
-                partialFillResults.takerFeePaid !== undefined
-                    ? partialFillResults.takerFeePaid
-                    : signedOrder.takerFee.times(takerAssetFillAmount).dividedBy(signedOrder.takerAssetAmount),
-        };
     }
+
     /**
      * Constructor.
      * @param exchangeContract Insstance of the deployed exchange contract
@@ -173,12 +136,14 @@ export class FillOrderWrapper {
         this._blockchainBalanceStore = new BlockchainBalanceStore(erc20Wrapper, erc721Wrapper, erc1155ProxyWrapper);
         this._web3Wrapper = new Web3Wrapper(provider);
     }
+
     /**
      * Returns the balance store used by this wrapper.
      */
     public getBlockchainBalanceStore(): BlockchainBalanceStore {
         return this._blockchainBalanceStore;
     }
+
     /**
      * Fills an order and asserts the effects. This includes
      * 1. The order info (via `getOrderInfo`)
@@ -190,7 +155,6 @@ export class FillOrderWrapper {
         signedOrder: SignedOrder,
         from: string,
         opts: { takerAssetFillAmount?: BigNumber } = {},
-        expectedFillResults: Partial<FillResults> = {},
     ): Promise<void> {
         // Get init state
         await this._blockchainBalanceStore.updateBalancesAsync();
@@ -204,13 +168,7 @@ export class FillOrderWrapper {
             simulatedFillResults,
             simulatedFillEvent,
             simulatedFinalBalanceStore,
-        ] = FillOrderWrapper.simulateFillOrder(
+        ] = FillOrderWrapper.simulateFillOrder(signedOrder, from, opts, this._blockchainBalanceStore);
-            signedOrder,
-            from,
-            opts,
-            this._blockchainBalanceStore,
-            expectedFillResults,
-        );
         const [fillResults, fillEvent] = await this._fillOrderAsync(signedOrder, from, opts);
         // Assert state transition
         expect(simulatedFillResults, 'Fill Results').to.be.deep.equal(fillResults);
@@ -221,6 +179,7 @@ export class FillOrderWrapper {
         const finalTakerAssetFilledAmount = initTakerAssetFilledAmount.plus(fillResults.takerAssetFilledAmount);
         await this._assertOrderStateAsync(signedOrder, finalTakerAssetFilledAmount);
     }
+
     /**
      * Fills an order on-chain. As an optimization this function auto-updates the blockchain balance store
      * used by this contract.
@@ -255,6 +214,7 @@ export class FillOrderWrapper {
         await this._blockchainBalanceStore.updateBalancesAsync();
         return [fillResults, fillEvent];
     }
+
     /**
      * Asserts that the provided order's fill amount and order status
      * are the expected values.

contracts/exchange/test/balance_stores/balance_store.ts

@@ -2,8 +2,15 @@ import { ERC1155Holdings, ERC1155HoldingsByOwner, TokenBalances } from '@0x/cont
 import { BigNumber } from '@0x/utils';
 import * as _ from 'lodash';
 
+/**
+ * Note - this should live in `@0x/contracts-test-utils` but that would create a circular
+ * dependency in `BlockchainBlanceStore`. We should be able to mvoe this once we can rely
+ * solely on auto-generated wrappers opposed to the existing ERC20Wrapper, ERC721Wrapper,
+ * and ERC1155Wrapper.
+ */
 export class BalanceStore {
     protected _balances: TokenBalances;
+
     /**
      * Returns true iff balance stores do have the same entries.
      * @param lhs First balance store to compare
@@ -12,6 +19,7 @@ export class BalanceStore {
     public static isEqual(lhs: BalanceStore, rhs: BalanceStore): boolean {
         return _.isEqual(lhs.getRawBalances(), rhs.getRawBalances());
     }
+
     /**
      * Throws iff balance stores do not have the same entries.
      * @param lhs First balance store to compare
@@ -22,6 +30,7 @@ export class BalanceStore {
             throw new Error(`Balance stores are not equal:\n\nLeft:\n${lhs}\n\nRight:\n${rhs}`);
         }
     }
+
     /**
      * Restructures `ERC1155HoldingsByOwner` to be compatible with `TokenBalances.erc1155`.
      * @param erc1155HoldingsByOwner Holdings returned by `ERC1155ProxyWrapper.getBalancesAsync()`.
@@ -41,6 +50,7 @@ export class BalanceStore {
         }
         return result;
     }
+
     /**
      * Encodes token balances in a way that they can be compared by lodash.
      */
@@ -57,12 +67,14 @@ export class BalanceStore {
         const keys = _.keys(obj).sort();
         return _.zip(keys, keys.map(k => BalanceStore._encodeTokenBalances(obj[k])));
     }
+
     /**
      * Constructor.
      */
     public constructor() {
         this._balances = { erc20: {}, erc721: {}, erc1155: {} };
     }
+
     /**
      * Copies the balance from an existing balance store.
      * @param balanceStore to copy balances from.
@@ -70,6 +82,7 @@ export class BalanceStore {
     public copyBalancesFrom(balanceStore: BalanceStore): void {
         this._balances = _.cloneDeep(balanceStore._balances);
     }
+
     /**
      * Returns the raw `TokenBalances` that this class encapsulates.
      */

contracts/exchange/test/balance_stores/blockchain_balance_store.ts

@@ -1,5 +1,4 @@
 import { ERC1155ProxyWrapper, ERC20Wrapper, ERC721Wrapper } from '@0x/contracts-asset-proxy';
-import * as _ from 'lodash';
 
 import { BalanceStore } from './balance_store';
 
@@ -7,6 +6,7 @@ export class BlockchainBalanceStore extends BalanceStore {
     private readonly _erc20Wrapper: ERC20Wrapper;
     private readonly _erc721Wrapper: ERC721Wrapper;
     private readonly _erc1155ProxyWrapper: ERC1155ProxyWrapper;
+
     /**
      * Constructor.
      * @param erc20Wrapper The ERC20 Wrapper used to interface with deployed erc20 tokens.
@@ -23,6 +23,7 @@ export class BlockchainBalanceStore extends BalanceStore {
         this._erc721Wrapper = erc721Wrapper;
         this._erc1155ProxyWrapper = erc1155ProxyWrapper;
     }
+
     /**
      * Updates balances by querying on-chain values managed by the erc20, erc721, and erc1155 wrappers.
      */

contracts/exchange/test/balance_stores/local_balance_store.ts

@@ -17,12 +17,14 @@ export class LocalBalanceStore extends BalanceStore {
         }
         return localBalanceStore;
     }
+
     /**
      * Constructor.
      */
     public constructor() {
         super();
     }
+
     /**
      * Transfers assets from `fromAddress` to `toAddress`.
      * @param fromAddress Sender of asset(s)
@@ -84,7 +86,7 @@ export class LocalBalanceStore extends BalanceStore {
                     }
                 }
                 // sort NFT's by name
-                toBalances.nonFungible = _.sortBy(toBalances.nonFungible);
+                toBalances.nonFungible.sort();
                 break;
             }
             case AssetProxyId.MultiAsset: {

contracts/exchange/test/core.ts

@@ -17,9 +17,11 @@ import {
 } from '@0x/contracts-erc20';
 import { DummyERC721TokenContract } from '@0x/contracts-erc721';
 import {
-    chaiSetup,
+    blockchainTests,
     constants,
+    describe,
     ERC20BalancesByOwner,
+    expect,
     getLatestBlockTimestampAsync,
     hexConcat,
     increaseTimeAndMineBlockAsync,
@@ -29,12 +31,10 @@ import {
     txDefaults,
     web3Wrapper,
 } from '@0x/contracts-test-utils';
-import { BlockchainLifecycle } from '@0x/dev-utils';
 import { assetDataUtils, ExchangeRevertErrors, LibMathRevertErrors, orderHashUtils } from '@0x/order-utils';
 import { RevertReason, SignatureType, SignedOrder } from '@0x/types';
 import { BigNumber, providerUtils, ReentrancyGuardRevertErrors, StringRevertError } from '@0x/utils';
 import { Web3Wrapper } from '@0x/web3-wrapper';
-import * as chai from 'chai';
 import { LogWithDecodedArgs } from 'ethereum-types';
 import * as _ from 'lodash';
 
@@ -51,14 +51,10 @@ import {
     ValidatorWalletDataType,
 } from '../src';
 
-chaiSetup.configure();
-const expect = chai.expect;
-const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper);
-
 import { FillOrderWrapper } from './assertion_wrappers/fill_order_wrapper';
 
 // tslint:disable:no-unnecessary-type-assertion
-describe('Exchange core', () => {
+blockchainTests.resets('Exchange core', () => {
     let chainId: number;
     let makerAddress: string;
     let owner: string;
@@ -102,12 +98,6 @@ describe('Exchange core', () => {
 
     let fillOrderWrapper: FillOrderWrapper;
 
-    before(async () => {
-        await blockchainLifecycle.startAsync();
-    });
-    after(async () => {
-        await blockchainLifecycle.revertAsync();
-    });
     before(async () => {
         chainId = await providerUtils.getChainIdAsync(provider);
         const accounts = await web3Wrapper.getAvailableAddressesAsync();
@@ -240,12 +230,6 @@ describe('Exchange core', () => {
         orderFactory = new OrderFactory(privateKey, defaultOrderParams);
         fillOrderWrapper = new FillOrderWrapper(exchange, erc20Wrapper, erc721Wrapper, erc1155ProxyWrapper, provider);
     });
-    beforeEach(async () => {
-        await blockchainLifecycle.startAsync();
-    });
-    afterEach(async () => {
-        await blockchainLifecycle.revertAsync();
-    });
     describe('fillOrder', () => {
         beforeEach(async () => {
             erc20Balances = await erc20Wrapper.getBalancesAsync();