@0x/contracts-exchange: Remove _assertValidFill().

`@0x/contracts-exchange`: Add `_settleOrder()` unit tests.
`@0x/contracts-exchange`: Add explicit tests for
`_calculateFillResults()`.
`@0x/contracts-exchange`: Add overflow tests to `isolated_fill_order`
tests.
`@0x/contracts-exchange`: Add explicit `takerAssetFillAmount = 0` test
to `isolated_fill_order` tests.

contracts/exchange/CHANGELOG.json

@@ -133,6 +133,10 @@
             {
                 "note": "Remove functions from `TestExchangeInternals.sol` that are no longer tested in this package.",
                 "pr": "TODO"
+            },
+            {
+                "note": "Remove `_assertValidFill()`",
+                "pr": "TODO"
             }
         ]
     },

contracts/exchange/contracts/src/MixinExchangeCore.sol

@@ -210,15 +210,6 @@ contract MixinExchangeCore is
         uint256 remainingTakerAssetAmount = _safeSub(order.takerAssetAmount, orderInfo.orderTakerAssetFilledAmount);
         uint256 takerAssetFilledAmount = _min256(takerAssetFillAmount, remainingTakerAssetAmount);
 
-        // Validate context
-        _assertValidFill(
-            order,
-            orderInfo,
-            takerAssetFillAmount,
-            takerAssetFilledAmount,
-            fillResults.makerAssetFilledAmount
-        );
-
         // Compute proportional fill amounts
         fillResults = _calculateFillResults(order, takerAssetFilledAmount);
 
@@ -389,78 +380,6 @@ contract MixinExchangeCore is
         }
     }
 
-    /// @dev Validates context for fillOrder. Succeeds or throws.
-    /// @param order to be filled.
-    /// @param orderInfo OrderStatus, orderHash, and amount already filled of order.
-    /// @param takerAssetFillAmount Desired amount of order to fill by taker.
-    /// @param takerAssetFilledAmount Amount of takerAsset that will be filled.
-    /// @param makerAssetFilledAmount Amount of makerAsset that will be transfered.
-    function _assertValidFill(
-        Order memory order,
-        OrderInfo memory orderInfo,
-        uint256 takerAssetFillAmount,  // TODO: use FillResults
-        uint256 takerAssetFilledAmount,
-        uint256 makerAssetFilledAmount
-    )
-        internal
-        pure
-    {
-        // Revert if fill amount is invalid
-        // TODO: reconsider necessity for v2.1
-        if (takerAssetFillAmount == 0) {
-            LibRichErrors._rrevert(LibExchangeRichErrors.FillError(
-                FillErrorCodes.INVALID_TAKER_AMOUNT,
-                orderInfo.orderHash
-            ));
-        }
-
-        // Make sure taker does not pay more than desired amount
-        // NOTE: This assertion should never fail, it is here
-        //       as an extra defence against potential bugs.
-        if (takerAssetFilledAmount > takerAssetFillAmount) {
-            LibRichErrors._rrevert(LibExchangeRichErrors.FillError(
-                FillErrorCodes.TAKER_OVERPAY,
-                orderInfo.orderHash
-            ));
-        }
-
-        // Make sure order is not overfilled
-        // NOTE: This assertion should never fail, it is here
-        //       as an extra defence against potential bugs.
-        if (_safeAdd(orderInfo.orderTakerAssetFilledAmount, takerAssetFilledAmount)
-            > order.takerAssetAmount) {
-            LibRichErrors._rrevert(LibExchangeRichErrors.FillError(
-                FillErrorCodes.OVERFILL,
-                orderInfo.orderHash
-            ));
-        }
-
-        // Make sure order is filled at acceptable price.
-        // The order has an implied price from the makers perspective:
-        //    order price = order.makerAssetAmount / order.takerAssetAmount
-        // i.e. the number of makerAsset maker is paying per takerAsset. The
-        // maker is guaranteed to get this price or a better (lower) one. The
-        // actual price maker is getting in this fill is:
-        //    fill price = makerAssetFilledAmount / takerAssetFilledAmount
-        // We need `fill price <= order price` for the fill to be fair to maker.
-        // This amounts to:
-        //     makerAssetFilledAmount        order.makerAssetAmount
-        //    ------------------------  <=  -----------------------
-        //     takerAssetFilledAmount        order.takerAssetAmount
-        // or, equivalently:
-        //     makerAssetFilledAmount * order.takerAssetAmount <=
-        //     order.makerAssetAmount * takerAssetFilledAmount
-        // NOTE: This assertion should never fail, it is here
-        //       as an extra defence against potential bugs.
-        if (_safeMul(makerAssetFilledAmount, order.takerAssetAmount)
-            > _safeMul(order.makerAssetAmount, takerAssetFilledAmount)) {
-            LibRichErrors._rrevert(LibExchangeRichErrors.FillError(
-                FillErrorCodes.INVALID_FILL_PRICE,
-                orderInfo.orderHash
-            ));
-        }
-    }
-
     /// @dev Validates context for cancelOrder. Succeeds or throws.
     /// @param order to be cancelled.
     /// @param orderInfo OrderStatus, orderHash, and amount already filled of order.

contracts/exchange/contracts/src/MixinMatchOrders.sol

@@ -680,22 +680,6 @@ contract MixinMatchOrders is
             shouldMaximallyFillOrders
         );
 
-        // Validate fill contexts
-        _assertValidFill(
-            leftOrder,
-            leftOrderInfo,
-            matchedFillResults.left.takerAssetFilledAmount,
-            matchedFillResults.left.takerAssetFilledAmount,
-            matchedFillResults.left.makerAssetFilledAmount
-        );
-        _assertValidFill(
-            rightOrder,
-            rightOrderInfo,
-            matchedFillResults.right.takerAssetFilledAmount,
-            matchedFillResults.right.takerAssetFilledAmount,
-            matchedFillResults.right.makerAssetFilledAmount
-        );
-
         // Update exchange state
         _updateFilledState(
             leftOrder,

contracts/exchange/contracts/test/TestExchangeInternals.sol

@@ -26,6 +26,14 @@ import "../src/Exchange.sol";
 contract TestExchangeInternals is
     Exchange
 {
+    event DispatchTransferFromCalled(
+        bytes32 orderHash,
+        bytes assetData,
+        address from,
+        address to,
+        uint256 amount
+    );
+
     constructor (uint256 chainId)
         public
         Exchange(chainId)
@@ -62,4 +70,34 @@ contract TestExchangeInternals is
             fillResults
         );
     }
+
+    function settleOrder(
+        bytes32 orderHash,
+        LibOrder.Order memory order,
+        address takerAddress,
+        LibFillResults.FillResults memory fillResults
+    )
+        public
+    {
+        _settleOrder(orderHash, order, takerAddress, fillResults);
+    }
+
+    /// @dev Overidden to only log arguments so we can test `_settleOrder()`.
+    function _dispatchTransferFrom(
+        bytes32 orderHash,
+        bytes memory assetData,
+        address from,
+        address to,
+        uint256 amount
+    )
+        internal
+    {
+        emit DispatchTransferFromCalled(
+            orderHash,
+            assetData,
+            from,
+            to,
+            amount
+        );
+    }
 }

contracts/exchange/test/fill_order.ts

@@ -189,15 +189,7 @@ blockchainTests.resets('FillOrder Tests', ({ web3Wrapper, txDefaults }) => {
             await fillOrderCombinatorialUtils.testFillOrderScenarioFailureAsync(fillScenario);
         });
 
-        it('should revert if takerAssetFillAmount is 0', async () => {
+        it('should throw if an order is expired', async () => {
-            const fillScenario = {
-                ...defaultFillScenario,
-                takerAssetFillAmountScenario: TakerAssetFillAmountScenario.Zero,
-            };
-            await fillOrderCombinatorialUtils.testFillOrderScenarioFailureAsync(fillScenario);
-        });
-
-        it('should revert if an order is expired', async () => {
             const fillScenario = {
                 ...defaultFillScenario,
                 orderScenario: {

contracts/exchange/test/internal.ts

@@ -1,3 +1,4 @@
+import { ReferenceFunctions as LibReferenceFunctions } from '@0x/contracts-exchange-libs';
 import {
     blockchainTests,
     constants,
@@ -8,6 +9,7 @@ import {
     testCombinatoriallyWithReferenceFunc,
     uint256Values,
 } from '@0x/contracts-test-utils';
+import { LibMathRevertErrors } from '@0x/order-utils';
 import { FillResults, OrderWithoutDomain as Order } from '@0x/types';
 import { BigNumber, SafeMathRevertErrors } from '@0x/utils';
 import { LogWithDecodedArgs } from 'ethereum-types';
@@ -18,11 +20,12 @@ import {
     ReferenceFunctions,
     TestExchangeInternalsContract,
     TestExchangeInternalsFillEventArgs,
+    TestExchangeInternalsDispatchTransferFromCalledEventArgs,
 } from '../src';
 
-// TODO(dorothy-zbornak): Add _settleOrder
+blockchainTests('Exchange core internal functions', env => {
-blockchainTests.only('Exchange core internal functions', env => {
     const CHAIN_ID = 1337;
+    const ONE_ETHER = constants.ONE_ETHER;
     const EMPTY_ORDER: Order = {
         senderAddress: constants.NULL_ADDRESS,
         makerAddress: constants.NULL_ADDRESS,
@@ -39,6 +42,9 @@ blockchainTests.only('Exchange core internal functions', env => {
         feeRecipientAddress: constants.NULL_ADDRESS,
         expirationTimeSeconds: constants.ZERO_AMOUNT,
     };
+    const randomAddress = () => hexRandom(constants.ADDRESS_LENGTH);
+    const randomHash = () => hexRandom(constants.WORD_LENGTH);
+    const randomAssetData = () => hexRandom(36);
     let testExchange: TestExchangeInternalsContract;
     let logDecoder: LogDecoder;
     let senderAddress: string;
@@ -54,49 +60,49 @@ blockchainTests.only('Exchange core internal functions', env => {
         logDecoder = new LogDecoder(env.web3Wrapper, artifacts);
     });
 
-    blockchainTests('calculateFillResults', async () => {
+    blockchainTests('calculateFillResults', () => {
-        function makeOrder(
-            makerAssetAmount: BigNumber,
-            takerAssetAmount: BigNumber,
-            makerFee: BigNumber,
-            takerFee: BigNumber,
-        ): Order {
-            return {
-                ...EMPTY_ORDER,
-                makerAssetAmount,
-                takerAssetAmount,
-                makerFee,
-                takerFee,
-            };
-        }
-
-        async function referenceCalculateFillResultsAsync(
-            orderTakerAssetAmount: BigNumber,
-            takerAssetFilledAmount: BigNumber,
-            otherAmount: BigNumber,
-        ): Promise<FillResults> {
-            // Note(albrow): Here we are re-using the same value (otherAmount)
-            // for order.makerAssetAmount, order.makerFee, and order.takerFee.
-            // This should be safe because they are never used with each other
-            // in any mathematical operation in either the reference TypeScript
-            // implementation or the Solidity implementation of
-            // calculateFillResults.
-            return ReferenceFunctions.calculateFillResults(
-                makeOrder(otherAmount, orderTakerAssetAmount, otherAmount, otherAmount),
-                takerAssetFilledAmount,
-            );
-        }
-
-        async function testCalculateFillResultsAsync(
-            orderTakerAssetAmount: BigNumber,
-            takerAssetFilledAmount: BigNumber,
-            otherAmount: BigNumber,
-        ): Promise<FillResults> {
-            const order = makeOrder(otherAmount, orderTakerAssetAmount, otherAmount, otherAmount);
-            return testExchange.calculateFillResults.callAsync(order, takerAssetFilledAmount);
-        }
-
         describe.optional('combinatorial tests', () => {
+            function makeOrder(
+                makerAssetAmount: BigNumber,
+                takerAssetAmount: BigNumber,
+                makerFee: BigNumber,
+                takerFee: BigNumber,
+            ): Order {
+                return {
+                    ...EMPTY_ORDER,
+                    makerAssetAmount,
+                    takerAssetAmount,
+                    makerFee,
+                    takerFee,
+                };
+            }
+
+            async function referenceCalculateFillResultsAsync(
+                orderTakerAssetAmount: BigNumber,
+                takerAssetFilledAmount: BigNumber,
+                otherAmount: BigNumber,
+            ): Promise<FillResults> {
+                // Note(albrow): Here we are re-using the same value (otherAmount)
+                // for order.makerAssetAmount, order.makerFee, and order.takerFee.
+                // This should be safe because they are never used with each other
+                // in any mathematical operation in either the reference TypeScript
+                // implementation or the Solidity implementation of
+                // calculateFillResults.
+                return ReferenceFunctions.calculateFillResults(
+                    makeOrder(otherAmount, orderTakerAssetAmount, otherAmount, otherAmount),
+                    takerAssetFilledAmount,
+                );
+            }
+
+            async function testCalculateFillResultsAsync(
+                orderTakerAssetAmount: BigNumber,
+                takerAssetFilledAmount: BigNumber,
+                otherAmount: BigNumber,
+            ): Promise<FillResults> {
+                const order = makeOrder(otherAmount, orderTakerAssetAmount, otherAmount, otherAmount);
+                return testExchange.calculateFillResults.callAsync(order, takerAssetFilledAmount);
+            }
+
             testCombinatoriallyWithReferenceFunc(
                 'calculateFillResults',
                 referenceCalculateFillResultsAsync,
@@ -104,13 +110,167 @@ blockchainTests.only('Exchange core internal functions', env => {
                 [uint256Values, uint256Values, uint256Values],
             );
         });
+
+        describe('explicit tests', () => {
+            const MAX_UINT256_ROOT = constants.MAX_UINT256_ROOT;
+            function makeOrder(details?: Partial<Order>): Order {
+                return _.assign(
+                    {},
+                    EMPTY_ORDER,
+                    details,
+                );
+            }
+
+            it('matches the output of the reference function', async () => {
+                const order = makeOrder({
+                    makerAssetAmount: ONE_ETHER,
+                    takerAssetAmount: ONE_ETHER.times(2),
+                    makerFee: ONE_ETHER.times(0.0023),
+                    takerFee: ONE_ETHER.times(0.0025),
+                });
+                const takerAssetFilledAmount = ONE_ETHER.dividedToIntegerBy(3);
+                const expected = ReferenceFunctions.calculateFillResults(order, takerAssetFilledAmount);
+                const actual = await testExchange.calculateFillResults.callAsync(order, takerAssetFilledAmount);
+                expect(actual).to.deep.eq(expected);
+            });
+
+            it('reverts if computing `fillResults.makerAssetFilledAmount` overflows', async () => {
+                // All values need to be large to ensure we don't trigger a RoundingError.
+                const order = makeOrder({
+                    makerAssetAmount: MAX_UINT256_ROOT.times(2),
+                    takerAssetAmount: MAX_UINT256_ROOT,
+                });
+                const takerAssetFilledAmount = MAX_UINT256_ROOT;
+                const expectedError = new SafeMathRevertErrors.SafeMathError(
+                    SafeMathRevertErrors.SafeMathErrorCodes.Uint256MultiplicationOverflow,
+                    takerAssetFilledAmount,
+                    order.makerAssetAmount,
+                );
+                return expect(testExchange.calculateFillResults.callAsync(order, takerAssetFilledAmount))
+                    .to.revertWith(expectedError);
+            });
+
+            it('reverts if computing `fillResults.makerFeePaid` overflows', async () => {
+                // All values need to be large to ensure we don't trigger a RoundingError.
+                const order = makeOrder({
+                    makerAssetAmount: MAX_UINT256_ROOT,
+                    takerAssetAmount: MAX_UINT256_ROOT,
+                    makerFee: MAX_UINT256_ROOT.times(11),
+                });
+                const takerAssetFilledAmount = MAX_UINT256_ROOT.dividedToIntegerBy(10);
+                const makerAssetFilledAmount = LibReferenceFunctions.getPartialAmountFloor(
+                    takerAssetFilledAmount,
+                    order.takerAssetAmount,
+                    order.makerAssetAmount,
+                );
+                const expectedError = new SafeMathRevertErrors.SafeMathError(
+                    SafeMathRevertErrors.SafeMathErrorCodes.Uint256MultiplicationOverflow,
+                    makerAssetFilledAmount,
+                    order.makerFee,
+                );
+                return expect(testExchange.calculateFillResults.callAsync(order, takerAssetFilledAmount))
+                    .to.revertWith(expectedError);
+            });
+
+            it('reverts if computing `fillResults.takerFeePaid` overflows', async () => {
+                // All values need to be large to ensure we don't trigger a RoundingError.
+                const order = makeOrder({
+                    makerAssetAmount: MAX_UINT256_ROOT,
+                    takerAssetAmount: MAX_UINT256_ROOT,
+                    takerFee: MAX_UINT256_ROOT.times(11),
+                });
+                const takerAssetFilledAmount = MAX_UINT256_ROOT.dividedToIntegerBy(10);
+                const expectedError = new SafeMathRevertErrors.SafeMathError(
+                    SafeMathRevertErrors.SafeMathErrorCodes.Uint256MultiplicationOverflow,
+                    takerAssetFilledAmount,
+                    order.takerFee,
+                );
+                return expect(testExchange.calculateFillResults.callAsync(order, takerAssetFilledAmount))
+                    .to.revertWith(expectedError);
+            });
+
+            it('reverts if `order.makerAssetAmount` is 0', async () => {
+                const order = makeOrder({
+                    makerAssetAmount: constants.ZERO_AMOUNT,
+                    takerAssetAmount: ONE_ETHER,
+                });
+                const takerAssetFilledAmount = ONE_ETHER;
+                const expectedError = new LibMathRevertErrors.DivisionByZeroError();
+                return expect(testExchange.calculateFillResults.callAsync(order, takerAssetFilledAmount))
+                    .to.revertWith(expectedError);
+            });
+
+            it('reverts if `order.takerAssetAmount` is 0', async () => {
+                const order = makeOrder({
+                    makerAssetAmount: ONE_ETHER,
+                    takerAssetAmount: constants.ZERO_AMOUNT,
+                });
+                const takerAssetFilledAmount = ONE_ETHER;
+                const expectedError = new LibMathRevertErrors.DivisionByZeroError();
+                return expect(testExchange.calculateFillResults.callAsync(order, takerAssetFilledAmount))
+                    .to.revertWith(expectedError);
+            });
+
+            it('reverts if there is a rounding error computing `makerAsssetFilledAmount`', async () => {
+                const order = makeOrder({
+                    makerAssetAmount: new BigNumber(100),
+                    takerAssetAmount: ONE_ETHER,
+                });
+                const takerAssetFilledAmount = order.takerAssetAmount.dividedToIntegerBy(3);
+                const expectedError = new LibMathRevertErrors.RoundingError(
+                    takerAssetFilledAmount,
+                    order.takerAssetAmount,
+                    order.makerAssetAmount,
+                );
+                return expect(testExchange.calculateFillResults.callAsync(order, takerAssetFilledAmount))
+                    .to.revertWith(expectedError);
+            });
+
+            it('reverts if there is a rounding error computing `makerFeePaid`', async () => {
+                const order = makeOrder({
+                    makerAssetAmount: ONE_ETHER,
+                    takerAssetAmount: ONE_ETHER,
+                    makerFee: new BigNumber(100),
+                });
+                const takerAssetFilledAmount = order.takerAssetAmount.dividedToIntegerBy(3);
+                const makerAssetFilledAmount = LibReferenceFunctions.getPartialAmountFloor(
+                    takerAssetFilledAmount,
+                    order.takerAssetAmount,
+                    order.makerAssetAmount,
+                );
+                const expectedError = new LibMathRevertErrors.RoundingError(
+                    makerAssetFilledAmount,
+                    order.makerAssetAmount,
+                    order.makerFee,
+                );
+                return expect(testExchange.calculateFillResults.callAsync(order, takerAssetFilledAmount))
+                    .to.revertWith(expectedError);
+            });
+
+            it('reverts if there is a rounding error computing `takerFeePaid`', async () => {
+                const order = makeOrder({
+                    makerAssetAmount: ONE_ETHER,
+                    takerAssetAmount: ONE_ETHER,
+                    takerFee: new BigNumber(100),
+                });
+                const takerAssetFilledAmount = order.takerAssetAmount.dividedToIntegerBy(3);
+                const makerAssetFilledAmount = LibReferenceFunctions.getPartialAmountFloor(
+                    takerAssetFilledAmount,
+                    order.takerAssetAmount,
+                    order.makerAssetAmount,
+                );
+                const expectedError = new LibMathRevertErrors.RoundingError(
+                    makerAssetFilledAmount,
+                    order.makerAssetAmount,
+                    order.takerFee,
+                );
+                return expect(testExchange.calculateFillResults.callAsync(order, takerAssetFilledAmount))
+                    .to.revertWith(expectedError);
+            });
+        });
     });
 
     blockchainTests.resets('updateFilledState', async () => {
-        const ONE_ETHER = new BigNumber(1e18);
-        const randomAddress = () => hexRandom(constants.ADDRESS_LENGTH);
-        const randomHash = () => hexRandom(constants.WORD_LENGTH);
-        const randomAssetData = () => hexRandom(36);
         const ORDER_DEFAULTS = {
             senderAddress: randomAddress(),
             makerAddress: randomAddress(),
@@ -129,11 +289,7 @@ blockchainTests.only('Exchange core internal functions', env => {
         };
 
         function makeOrder(details?: Partial<Order>): Order {
-            return _.assign(
+            return _.assign({}, ORDER_DEFAULTS, details);
-                {},
-                ORDER_DEFAULTS,
-                details,
-            );
         }
 
         async function testUpdateFilledStateAsync(
@@ -192,7 +348,7 @@ blockchainTests.only('Exchange core internal functions', env => {
             );
         });
 
-        it('throws if `leftOrderTakerAssetFilledAmount + fillResults.takerAssetFilledAmount` overflows', async () => {
+        it('reverts if `leftOrderTakerAssetFilledAmount + fillResults.takerAssetFilledAmount` overflows', async () => {
             const order = makeOrder();
             const orderTakerAssetFilledAmount = constants.MAX_UINT256.dividedToIntegerBy(2);
             const takerAssetFillAmount = constants.MAX_UINT256.dividedToIntegerBy(2).plus(2);
@@ -216,5 +372,71 @@ blockchainTests.only('Exchange core internal functions', env => {
             )).to.revertWith(expectedError);
         });
     });
+
+    blockchainTests('settleOrder', () => {
+        const DEFAULT_ORDER = {
+            senderAddress: randomAddress(),
+            makerAddress: randomAddress(),
+            takerAddress: randomAddress(),
+            makerFee: ONE_ETHER.times(0.001),
+            takerFee: ONE_ETHER.times(0.003),
+            makerAssetAmount: ONE_ETHER,
+            takerAssetAmount: ONE_ETHER.times(0.5),
+            makerAssetData: randomAssetData(),
+            takerAssetData: randomAssetData(),
+            makerFeeAssetData: randomAssetData(),
+            takerFeeAssetData: randomAssetData(),
+            salt: new BigNumber(_.random(0, 1e8)),
+            feeRecipientAddress: randomAddress(),
+            expirationTimeSeconds: new BigNumber(_.random(0, 1e8)),
+        };
+
+        it('calls `_dispatchTransferFrom()` in the right order with the correct arguments', async () => {
+            const order = DEFAULT_ORDER;
+            const orderHash = randomHash();
+            const takerAddress = randomAddress();
+            const fillResults = {
+                makerAssetFilledAmount: ONE_ETHER.times(2),
+                takerAssetFilledAmount: ONE_ETHER.times(10),
+                makerFeePaid: ONE_ETHER.times(0.01),
+                takerFeePaid: ONE_ETHER.times(0.025),
+            };
+            const receipt = await logDecoder.getTxWithDecodedLogsAsync(
+                await testExchange.settleOrder.sendTransactionAsync(
+                    orderHash,
+                    order,
+                    takerAddress,
+                    fillResults,
+                ),
+            );
+            const logs = receipt.logs as Array<LogWithDecodedArgs<TestExchangeInternalsDispatchTransferFromCalledEventArgs>>;
+            expect(logs.length === 4);
+            expect(_.every(logs, log => log.event === 'DispatchTransferFromCalled')).to.be.true();
+            // taker -> maker
+            expect(logs[0].args.orderHash).to.eq(orderHash);
+            expect(logs[0].args.assetData).to.eq(order.takerAssetData);
+            expect(logs[0].args.from).to.eq(takerAddress);
+            expect(logs[0].args.to).to.eq(order.makerAddress);
+            expect(logs[0].args.amount).to.bignumber.eq(fillResults.takerAssetFilledAmount);
+            // maker -> taker
+            expect(logs[1].args.orderHash).to.eq(orderHash);
+            expect(logs[1].args.assetData).to.eq(order.makerAssetData);
+            expect(logs[1].args.from).to.eq(order.makerAddress);
+            expect(logs[1].args.to).to.eq(takerAddress);
+            expect(logs[1].args.amount).to.bignumber.eq(fillResults.makerAssetFilledAmount);
+            // taker fee -> feeRecipient
+            expect(logs[2].args.orderHash).to.eq(orderHash);
+            expect(logs[2].args.assetData).to.eq(order.takerFeeAssetData);
+            expect(logs[2].args.from).to.eq(takerAddress);
+            expect(logs[2].args.to).to.eq(order.feeRecipientAddress);
+            expect(logs[2].args.amount).to.bignumber.eq(fillResults.takerFeePaid);
+            // maker fee -> feeRecipient
+            expect(logs[3].args.orderHash).to.eq(orderHash);
+            expect(logs[3].args.assetData).to.eq(order.makerFeeAssetData);
+            expect(logs[3].args.from).to.eq(order.makerAddress);
+            expect(logs[3].args.to).to.eq(order.feeRecipientAddress);
+            expect(logs[3].args.amount).to.bignumber.eq(fillResults.makerFeePaid);
+        });
+    });
 });
 // tslint:disable-line:max-file-line-count

contracts/exchange/test/isolated_fill_order.ts

@@ -1,3 +1,4 @@
+import { ReferenceFunctions as LibReferenceFunctions } from '@0x/contracts-exchange-libs';
 import {
     blockchainTests,
     constants,
@@ -6,7 +7,7 @@ import {
 } from '@0x/contracts-test-utils';
 import { ExchangeRevertErrors, LibMathRevertErrors } from '@0x/order-utils';
 import { FillResults, OrderInfo, OrderStatus, SignatureType } from '@0x/types';
-import { BigNumber } from '@0x/utils';
+import { BigNumber, SafeMathRevertErrors } from '@0x/utils';
 import * as _ from 'lodash';
 
 import { calculateFillResults } from '../src/reference_functions';
@@ -24,8 +25,7 @@ import {
 blockchainTests('Isolated fillOrder() tests', env => {
     const randomAddress = () => hexRandom(constants.ADDRESS_LENGTH);
     const getCurrentTime = () => Math.floor(_.now() / 1000);
-    const { ZERO_AMOUNT } = constants;
+    const { ZERO_AMOUNT, ONE_ETHER, MAX_UINT256_ROOT } = constants;
-    const ONE_ETHER = new BigNumber(10).pow(18);
     const ONE_DAY = 60 * 60 * 24;
     const TOMORROW = getCurrentTime() + ONE_DAY;
     const DEFAULT_ORDER: Order = {
@@ -464,6 +464,61 @@ blockchainTests('Isolated fillOrder() tests', env => {
                 .to.revertWith(expectedError);
         });
 
+        it('can\'t fill an order that results in a `makerAssetFilledAmount` overflow.', async () => {
+            // All values need to be large to ensure we don't trigger a Rounding.
+            const order = createOrder({
+                makerAssetAmount: MAX_UINT256_ROOT.times(2),
+                takerAssetAmount: MAX_UINT256_ROOT,
+            });
+            const takerAssetFillAmount = MAX_UINT256_ROOT;
+            const expectedError = new SafeMathRevertErrors.SafeMathError(
+                SafeMathRevertErrors.SafeMathErrorCodes.Uint256MultiplicationOverflow,
+                takerAssetFillAmount,
+                order.makerAssetAmount,
+            );
+            return expect(exchange.fillOrderAsync(order, takerAssetFillAmount))
+                .to.revertWith(expectedError);
+        });
+
+        it('can\'t fill an order that results in a `makerFeePaid` overflow.', async () => {
+            // All values need to be large to ensure we don't trigger a Rounding.
+            const order = createOrder({
+                makerAssetAmount: MAX_UINT256_ROOT,
+                takerAssetAmount: MAX_UINT256_ROOT,
+                makerFee: MAX_UINT256_ROOT.times(11),
+            });
+            const takerAssetFillAmount = MAX_UINT256_ROOT.dividedToIntegerBy(10);
+            const makerAssetFilledAmount = LibReferenceFunctions.getPartialAmountFloor(
+                takerAssetFillAmount,
+                order.takerAssetAmount,
+                order.makerAssetAmount,
+            );
+            const expectedError = new SafeMathRevertErrors.SafeMathError(
+                SafeMathRevertErrors.SafeMathErrorCodes.Uint256MultiplicationOverflow,
+                makerAssetFilledAmount,
+                order.makerFee,
+            );
+            return expect(exchange.fillOrderAsync(order, takerAssetFillAmount))
+                .to.revertWith(expectedError);
+        });
+
+        it('can\'t fill an order that results in a `takerFeePaid` overflow.', async () => {
+            // All values need to be large to ensure we don't trigger a Rounding.
+            const order = createOrder({
+                makerAssetAmount: MAX_UINT256_ROOT,
+                takerAssetAmount: MAX_UINT256_ROOT,
+                takerFee: MAX_UINT256_ROOT.times(11),
+            });
+            const takerAssetFillAmount = MAX_UINT256_ROOT.dividedToIntegerBy(10);
+            const expectedError = new SafeMathRevertErrors.SafeMathError(
+                SafeMathRevertErrors.SafeMathErrorCodes.Uint256MultiplicationOverflow,
+                takerAssetFillAmount,
+                order.takerFee,
+            );
+            return expect(exchange.fillOrderAsync(order, takerAssetFillAmount))
+                .to.revertWith(expectedError);
+        });
+
         it('can\'t fill an order with a bad signature', async () => {
             const order = createOrder();
             const signature = createBadSignature();
@@ -529,6 +584,11 @@ blockchainTests('Isolated fillOrder() tests', env => {
     });
 
     describe('permitted fills', () => {
+        it('should allow takerAssetFillAmount to be zero', async () => {
+            const order = createOrder();
+            return fillOrderAndAssertResultsAsync(order, constants.ZERO_AMOUNT);
+        });
+
         it('can fill an order if taker is `takerAddress`', async () => {
             const order = createOrder({
                 takerAddress,

contracts/exchange/test/reference_functions.ts

@@ -0,0 +1,178 @@
+import { ReferenceFunctions as LibReferenceFunctions } from '@0x/contracts-exchange-libs';
+import {
+    constants,
+    describe,
+    expect,
+} from '@0x/contracts-test-utils';
+import { LibMathRevertErrors } from '@0x/order-utils';
+import { OrderWithoutDomain as Order } from '@0x/types';
+import { BigNumber, SafeMathRevertErrors } from '@0x/utils';
+import * as _ from 'lodash';
+
+import { calculateFillResults } from '../src/reference_functions';
+
+describe('Reference functions', () => {
+    const ONE_ETHER = constants.ONE_ETHER;
+    const EMPTY_ORDER: Order = {
+        senderAddress: constants.NULL_ADDRESS,
+        makerAddress: constants.NULL_ADDRESS,
+        takerAddress: constants.NULL_ADDRESS,
+        makerFee: constants.ZERO_AMOUNT,
+        takerFee: constants.ZERO_AMOUNT,
+        makerAssetAmount: constants.ZERO_AMOUNT,
+        takerAssetAmount: constants.ZERO_AMOUNT,
+        makerAssetData: constants.NULL_BYTES,
+        takerAssetData: constants.NULL_BYTES,
+        makerFeeAssetData: constants.NULL_BYTES,
+        takerFeeAssetData: constants.NULL_BYTES,
+        salt: constants.ZERO_AMOUNT,
+        feeRecipientAddress: constants.NULL_ADDRESS,
+        expirationTimeSeconds: constants.ZERO_AMOUNT,
+    };
+
+    describe('calculateFillResults', () => {
+        const MAX_UINT256_ROOT = constants.MAX_UINT256_ROOT;
+        function makeOrder(details?: Partial<Order>): Order {
+            return _.assign(
+                {},
+                EMPTY_ORDER,
+                details,
+            );
+        }
+
+        it('reverts if computing `fillResults.makerAssetFilledAmount` overflows', () => {
+            // All values need to be large to ensure we don't trigger a RondingError.
+            const order = makeOrder({
+                makerAssetAmount: MAX_UINT256_ROOT.times(2),
+                takerAssetAmount: MAX_UINT256_ROOT,
+            });
+            const takerAssetFilledAmount = MAX_UINT256_ROOT;
+            const expectedError = new SafeMathRevertErrors.SafeMathError(
+                SafeMathRevertErrors.SafeMathErrorCodes.Uint256MultiplicationOverflow,
+                takerAssetFilledAmount,
+                order.makerAssetAmount,
+            );
+            return expect(() => calculateFillResults(order, takerAssetFilledAmount))
+                .to.throw(expectedError.message);
+        });
+
+        it('reverts if computing `fillResults.makerFeePaid` overflows', () => {
+            // All values need to be large to ensure we don't trigger a RondingError.
+            const order = makeOrder({
+                makerAssetAmount: MAX_UINT256_ROOT,
+                takerAssetAmount: MAX_UINT256_ROOT,
+                makerFee: MAX_UINT256_ROOT.times(11),
+            });
+            const takerAssetFilledAmount = MAX_UINT256_ROOT.dividedToIntegerBy(10);
+            const makerAssetFilledAmount = LibReferenceFunctions.getPartialAmountFloor(
+                takerAssetFilledAmount,
+                order.takerAssetAmount,
+                order.makerAssetAmount,
+            );
+            const expectedError = new SafeMathRevertErrors.SafeMathError(
+                SafeMathRevertErrors.SafeMathErrorCodes.Uint256MultiplicationOverflow,
+                makerAssetFilledAmount,
+                order.makerFee,
+            );
+            return expect(() => calculateFillResults(order, takerAssetFilledAmount))
+                .to.throw(expectedError.message);
+        });
+
+        it('reverts if computing `fillResults.takerFeePaid` overflows', () => {
+            // All values need to be large to ensure we don't trigger a RondingError.
+            const order = makeOrder({
+                makerAssetAmount: MAX_UINT256_ROOT,
+                takerAssetAmount: MAX_UINT256_ROOT,
+                takerFee: MAX_UINT256_ROOT.times(11),
+            });
+            const takerAssetFilledAmount = MAX_UINT256_ROOT.dividedToIntegerBy(10);
+            const expectedError = new SafeMathRevertErrors.SafeMathError(
+                SafeMathRevertErrors.SafeMathErrorCodes.Uint256MultiplicationOverflow,
+                takerAssetFilledAmount,
+                order.takerFee,
+            );
+            return expect(() => calculateFillResults(order, takerAssetFilledAmount))
+                .to.throw(expectedError.message);
+        });
+
+        it('reverts if `order.makerAssetAmount` is 0', () => {
+            const order = makeOrder({
+                makerAssetAmount: constants.ZERO_AMOUNT,
+                takerAssetAmount: ONE_ETHER,
+            });
+            const takerAssetFilledAmount = ONE_ETHER;
+            const expectedError = new LibMathRevertErrors.DivisionByZeroError();
+            return expect(() => calculateFillResults(order, takerAssetFilledAmount))
+                .to.throw(expectedError.message);
+        });
+
+        it('reverts if `order.takerAssetAmount` is 0', () => {
+            const order = makeOrder({
+                makerAssetAmount: ONE_ETHER,
+                takerAssetAmount: constants.ZERO_AMOUNT,
+            });
+            const takerAssetFilledAmount = ONE_ETHER;
+            const expectedError = new LibMathRevertErrors.DivisionByZeroError();
+            return expect(() => calculateFillResults(order, takerAssetFilledAmount))
+                .to.throw(expectedError.message);
+        });
+
+        it('reverts if there is a rounding error computing `makerAsssetFilledAmount`', () => {
+            const order = makeOrder({
+                makerAssetAmount: new BigNumber(100),
+                takerAssetAmount: ONE_ETHER,
+            });
+            const takerAssetFilledAmount = order.takerAssetAmount.dividedToIntegerBy(3);
+            const expectedError = new LibMathRevertErrors.RoundingError(
+                takerAssetFilledAmount,
+                order.takerAssetAmount,
+                order.makerAssetAmount,
+            );
+            return expect(() => calculateFillResults(order, takerAssetFilledAmount))
+                .to.throw(expectedError.message);
+        });
+
+        it('reverts if there is a rounding error computing `makerFeePaid`', () => {
+            const order = makeOrder({
+                makerAssetAmount: ONE_ETHER,
+                takerAssetAmount: ONE_ETHER,
+                makerFee: new BigNumber(100),
+            });
+            const takerAssetFilledAmount = order.takerAssetAmount.dividedToIntegerBy(3);
+            const makerAssetFilledAmount = LibReferenceFunctions.getPartialAmountFloor(
+                takerAssetFilledAmount,
+                order.takerAssetAmount,
+                order.makerAssetAmount,
+            );
+            const expectedError = new LibMathRevertErrors.RoundingError(
+                makerAssetFilledAmount,
+                order.makerAssetAmount,
+                order.makerFee,
+            );
+            return expect(() => calculateFillResults(order, takerAssetFilledAmount))
+                .to.throw(expectedError.message);
+        });
+
+        it('reverts if there is a rounding error computing `takerFeePaid`', () => {
+            const order = makeOrder({
+                makerAssetAmount: ONE_ETHER,
+                takerAssetAmount: ONE_ETHER,
+                takerFee: new BigNumber(100),
+            });
+            const takerAssetFilledAmount = order.takerAssetAmount.dividedToIntegerBy(3);
+            const makerAssetFilledAmount = LibReferenceFunctions.getPartialAmountFloor(
+                takerAssetFilledAmount,
+                order.takerAssetAmount,
+                order.makerAssetAmount,
+            );
+            const expectedError = new LibMathRevertErrors.RoundingError(
+                makerAssetFilledAmount,
+                order.makerAssetAmount,
+                order.takerFee,
+            );
+            return expect(() => calculateFillResults(order, takerAssetFilledAmount))
+                .to.throw(expectedError.message);
+        });
+    });
+});
+// tslint:disable-line:max-file-line-count