@0x:contracts-exchange Refactored the protocol fee tests and added tests for batchFillOrders

contracts/exchange/8:p

@@ -1,29 +0,0 @@
-import { blockchainTests, constants } from '@0x/contracts-test-utils';
-
-import { artifacts, ExchangeContract, TestProtocolFeesContract, TestProtocolFeesReceiverContract } from '../src';
-
-blockchainTests('Protocol Fee Payments', env => {
-    let testProtocolFees: TestProtocolFeesContract;
-    let testProtocolFeesReceiver: TestProtocolFeesReceiverContract;
-
-    before(async () => {
-        testProtocolFees = await TestProtocolFeesContract.deployFrom0xArtifactAsync(
-            artifacts.TestProtocolFees,
-            env.provider,
-            env.txDefaults,
-            {},
-        );
-        testProtocolFeesReceiver = await TestProtocolFeesReceiverContract.deployFrom0xArtifactAsync(
-            artifacts.TestProtocolFeesReceiver,
-            env.provider,
-            env.txDefaults,
-            {},
-        );
-    });
-
-    blockchainTests.resets('fillOrder Protocol Fees', () => {
-        it('should pay protocol fee in ETH when the correct value is sent', async () => {
-            await testProtocolFeesReceiver.testFillOrderProtocolFees.awaitTransactionSuccessAsync(;
-        });
-    });
-});

contracts/exchange/contracts/src/MixinMatchOrders.sol

@@ -505,10 +505,18 @@ contract MixinMatchOrders is
             uint256 valuePaid = 0;
 
             // Pay the left order's protocol fee.
-            if (address(this).balance >= 2 * protocolFee) {
+            if (address(this).balance >= protocolFee) {
-                valuePaid = 2 * protocolFee;
+                valuePaid = protocolFee;
             }
             IStaking(feeCollector).payProtocolFee.value(valuePaid)(leftOrder.makerAddress, takerAddress, protocolFee);
+
+            // Clear value paid for the next call to `payProtocolFee()`.
+            valuePaid = 0;
+
+            // Pay the right order's protocol fee.
+            if (address(this).balance >= protocolFee) {
+                valuePaid = protocolFee;
+            }
             IStaking(feeCollector).payProtocolFee.value(valuePaid)(rightOrder.makerAddress, takerAddress, protocolFee);
         }
 

contracts/exchange/contracts/test/TestProtocolFeesReceiver.sol

@@ -42,6 +42,7 @@ contract TestProtocolFeesReceiver {
 
     /* Testing State */
 
+    // A struct that provides a schema for test data that should be logged.
     struct TestLog {
         address loggedMaker;
         address loggedPayer;
@@ -49,10 +50,83 @@ contract TestProtocolFeesReceiver {
         uint256 loggedValue;
     }
 
+    // The array of testLogs that will be added to by `payProtocolFee` and processed by the tests.
     TestLog[] testLogs;
 
     /* Testing Functions */
 
+    /// @dev Tests the `batchFillOrders` function's payment of protocol fees.
+    /// @param testProtocolFees The TestProtocolFees that should be tested against.
+    /// @param protocolFeeMultiplier The protocol fee multiplier that should be registered
+    ///        in the test suite before executing `batchFillOrders`.
+    /// @param numberOfOrders The number of orders that should be created and executed for this test.
+    /// @param shouldSetProtocolFeeCollector A boolean value indicating whether or not this contract
+    ///        should be registered as the `protocolFeeCollector`.
+    function testBatchFillOrdersProtocolFees(
+        TestProtocolFees testProtocolFees,
+        uint256 protocolFeeMultiplier,
+        uint256 numberOfOrders,
+        bool shouldSetProtocolFeeCollector
+    )
+        external
+        payable
+        handleState(testProtocolFees, protocolFeeMultiplier, shouldSetProtocolFeeCollector)
+    {
+        // Create empty arrays for taker asset filled amounts and signatures, which will suffice for this test.
+        uint256[] memory takerAssetFilledAmounts = new uint256[](numberOfOrders);
+        bytes[] memory signatures = new bytes[](numberOfOrders);
+
+        // Construct an array of orders in which every even-indexed order has a makerAddress of makerAddress1 and
+        // every odd-indexed order has a makerAddress of makerAddress2. This is done to make sure that the correct
+        // makers are being logged.
+        LibOrder.Order[] memory orders = new LibOrder.Order[](numberOfOrders);
+        for (uint256 i = 0; i < numberOfOrders; i++) {
+            orders[i] = createOrder(i % 2 == 0 ? makerAddress1 : makerAddress2);
+        }
+
+        // Forward all of the value sent to the contract to `batchFillOrders()`.
+        testProtocolFees.batchFillOrders.value(msg.value)(orders, takerAssetFilledAmounts, signatures);
+
+        // If the `protocolFeeCollector` was set, ensure that the protocol fees were paid correctly.
+        // Otherwise, the protocol fees should not have been paid.
+        if (shouldSetProtocolFeeCollector) {
+            // Ensure that the correct number of test logs were recorded.
+            require(testLogs.length == numberOfOrders, "Incorrect number of test logs in batchFillOrders test");
+
+            // Calculate the expected protocol fee.
+            uint256 expectedProtocolFeePaid = tx.gasprice.safeMul(protocolFeeMultiplier);
+
+            // Set the expected available balance for the first log.
+            uint256 expectedAvailableBalance = msg.value;
+
+            // Verify all of the test logs.
+            for (uint256 i = 0; i < testLogs.length; i++) {
+                // Verify the logged data.
+                verifyTestLog(
+                    testLogs[i],
+                    expectedAvailableBalance,                    // expectedAvailableBalance
+                    i % 2 == 0 ? makerAddress1 : makerAddress2,  // expectedMakerAddress
+                    address(this),                               // expectedPayerAddress
+                    expectedProtocolFeePaid                      // expectedProtocolFeePaid
+                );
+
+                // Set the expected available balance for the next log.
+                expectedAvailableBalance = expectedAvailableBalance >= expectedProtocolFeePaid ?
+                    expectedAvailableBalance - expectedProtocolFeePaid :
+                    expectedAvailableBalance;
+            }
+        } else {
+            // Ensure that zero test logs were created.
+            require(testLogs.length == 0, "Incorrect number of test logs in batchFillOrders test");
+        }
+    }
+
+    /// @dev Tests the `fillOrder` function's payment of protocol fees.
+    /// @param testProtocolFees The TestProtocolFees that should be tested against.
+    /// @param protocolFeeMultiplier The protocol fee multiplier that should be registered
+    ///        in the test suite before executing `fillOrder`.
+    /// @param shouldSetProtocolFeeCollector A boolean value indicating whether or not this contract
+    ///        should be registered as the `protocolFeeCollector`.
     function testFillOrderProtocolFees(
         TestProtocolFees testProtocolFees,
         uint256 protocolFeeMultiplier,
@@ -60,30 +134,48 @@ contract TestProtocolFeesReceiver {
     )
         external
         payable
+        handleState(testProtocolFees, protocolFeeMultiplier, shouldSetProtocolFeeCollector)
     {
-        // Set up the exchange state for the test.
+        // Create empty values for the takerAssetFilledAmount and the signature since these values don't
-        setExchangeState(
+        // matter for this test.
-            testProtocolFees,
+        uint256 takerAssetFilledAmount = 0;
-            protocolFeeMultiplier,
+        bytes memory signature = new bytes(0);
-            shouldSetProtocolFeeCollector
-        );
 
         // Construct an of order with distinguishing information.
-        LibOrder.Order memory order;
+        LibOrder.Order memory order = createOrder(makerAddress1);
-        order.makerAddress = makerAddress1;
-        order.makerAssetAmount = 1;
-        order.takerAssetAmount = 1;
 
         // Forward all of the value sent to the contract to `fillOrder()`.
-        testProtocolFees.fillOrder.value(msg.value)(order, 0, new bytes(0));
+        testProtocolFees.fillOrder.value(msg.value)(order, takerAssetFilledAmount, signature);
 
-        // Ensure that the results of the test were correct.
+        // If the `protocolFeeCollector` was set, ensure that the protocol fee was paid correctly.
-        verifyFillOrderTestResults(protocolFeeMultiplier, shouldSetProtocolFeeCollector);
+        // Otherwise, the protocol fee should not have been paid.
+        if (shouldSetProtocolFeeCollector) {
+            // Ensure that only one test log was created by the call to `fillOrder()`.
+            require(testLogs.length == 1, "Incorrect number of test logs in fillOrder test");
 
-        // Clear all state that was set to ensure that future tests are unaffected by this one.
+            // Calculate the expected protocol fee.
-        clearState(testProtocolFees);
+            uint256 expectedProtocolFeePaid = tx.gasprice.safeMul(protocolFeeMultiplier);
+
+            // Verify that the test log that was created is correct.
+            verifyTestLog(
+                testLogs[0],
+                msg.value,                // expectedAvailableBalance
+                makerAddress1,            // expectedMakerAddress
+                address(this),            // expectedPayerAddress
+                expectedProtocolFeePaid   // expectedProtocolFeePaid
+            );
+        } else {
+            // Ensure that zero test logs were created.
+            require(testLogs.length == 0, "Incorrect number of test logs in fillOrder test");
+        }
     }
 
+    /// @dev Tests the `matchOrders` function's payment of protocol fees.
+    /// @param testProtocolFees The TestProtocolFees that should be tested against.
+    /// @param protocolFeeMultiplier The protocol fee multiplier that should be registered
+    ///        in the test suite before executing `matchOrders`.
+    /// @param shouldSetProtocolFeeCollector A boolean value indicating whether or not this contract
+    ///        should be registered as the `protocolFeeCollector`.
     function testMatchOrdersProtocolFees(
         TestProtocolFees testProtocolFees,
         uint256 protocolFeeMultiplier,
@@ -91,144 +183,124 @@ contract TestProtocolFeesReceiver {
     )
         external
         payable
+        handleState(testProtocolFees, protocolFeeMultiplier, shouldSetProtocolFeeCollector)
     {
-        // Set up the exchange state for the test.
+        // Create two empty signatures since signatures are not used in this test.
-        setExchangeState(
+        bytes memory leftSignature = new bytes(0);
-            testProtocolFees,
+        bytes memory rightSignature = new bytes(0);
-            protocolFeeMultiplier,
-            shouldSetProtocolFeeCollector
-        );
 
-        // Construct a pair of orders with distinguishing information.
+        // Construct a distinguished left order.
-        LibOrder.Order memory leftOrder;
+        LibOrder.Order memory leftOrder = createOrder(makerAddress1);
-        leftOrder.makerAddress = makerAddress1;
+
-        leftOrder.makerAssetAmount = 1;
+        // Construct a distinguished right order.
-        leftOrder.takerAssetAmount = 1;
+        LibOrder.Order memory rightOrder = createOrder(makerAddress2);
-        LibOrder.Order memory rightOrder;
-        rightOrder.makerAddress = makerAddress2;
-        rightOrder.makerAssetAmount = 1;
-        rightOrder.takerAssetAmount = 1;
 
        // Forward all of the value sent to the contract to `matchOrders()`.
-       testProtocolFees.matchOrders.value(msg.value)(leftOrder, rightOrder, new bytes(0), new bytes(0));
+       testProtocolFees.matchOrders.value(msg.value)(leftOrder, rightOrder, leftSignature, rightSignature);
 
-       // Ensure that the results of the test were correct.
+        // If the `protocolFeeCollector` was set, ensure that the protocol fee was paid correctly.
-       verifyMatchOrdersTestResults(protocolFeeMultiplier, shouldSetProtocolFeeCollector);
+        // Otherwise, the protocol fee should not have been paid.
+        if (shouldSetProtocolFeeCollector) {
+            // Ensure that only one test log was created by the call to `fillOrder()`.
+            require(testLogs.length == 2, "Incorrect number of test logs in matchOrders test");
 
-       // Clear all state that was set to ensure that future tests are unaffected by this one.
+            // Calculate the expected protocol fee.
-       clearState(testProtocolFees);
+            uint256 expectedProtocolFeePaid = tx.gasprice.safeMul(protocolFeeMultiplier);
+
+            // Set the expected available balance for the first log.
+            uint256 expectedAvailableBalance = msg.value;
+
+            // Verify that the first test log that was created is correct.
+            verifyTestLog(
+                testLogs[0],
+                expectedAvailableBalance,  // expectedAvailableBalance
+                makerAddress1,             // expectedMakerAddress
+                address(this),             // expectedPayerAddress
+                expectedProtocolFeePaid    // expectedProtocolFeePaid
+            );
+
+            // Set the expected available balance for the second log.
+            expectedAvailableBalance = expectedAvailableBalance >= expectedProtocolFeePaid ?
+                expectedAvailableBalance - expectedProtocolFeePaid :
+                expectedAvailableBalance;
+
+            // Verify that the second test log that was created is correct.
+            verifyTestLog(
+                testLogs[1],
+                expectedAvailableBalance, // expectedAvailableBalance
+                makerAddress2,             // expectedMakerAddress
+                address(this),             // expectedPayerAddress
+                expectedProtocolFeePaid    // expectedProtocolFeePaid
+            );
+        } else {
+            // Ensure that zero test logs were created.
+            require(testLogs.length == 0, "Incorrect number of test logs in matchOrders test");
+        }
     }
 
     /* Verification Functions */
 
-    function verifyFillOrderTestResults(
+    /// @dev Verifies a test log against expected values.
-        uint256 protocolFeeMultiplier,
+    /// @param expectedAvailableBalance The balance that should be available when this call is made.
-        bool shouldSetProtocolFeeCollector
+    ///                         This is important especially for tests on wrapper functions.
+    /// @param expectedMakerAddress The expected maker address to be recorded in `payProtocolFee`.
+    /// @param expectedPayerAddress The expected payer address to be recorded in `payProtocolFee`.
+    /// @param expectedProtocolFeePaid The expected protocol fee paidto be recorded in `payProtocolFee`.
+    function verifyTestLog(
+        TestLog memory log,
+        uint256 expectedAvailableBalance,
+        address expectedMakerAddress,
+        address expectedPayerAddress,
+        uint256 expectedProtocolFeePaid
     )
         internal
     {
-        // If the `protocolFeeCollector` was set, then this contract should have been called.
+        // If the expected available balance was sufficient to pay the protocol fee, the protocol fee
-        if (shouldSetProtocolFeeCollector) {
+        // should have been paid in ether. Otherwise, no ether should be sent to pay the protocol fee.
-            // Calculate the protocol fee that should be paid.
+        if (expectedAvailableBalance >= expectedProtocolFeePaid) {
-            uint256 protocolFee = tx.gasprice.safeMul(protocolFeeMultiplier);
+            // Ensure that the protocol fee was paid in ether.
-
+            require(
-            // Ensure that one TestLog was recorded.
+                log.loggedValue == expectedProtocolFeePaid,
-            require(testLogs.length == 1, "Incorrect TestLog length for fillOrder test");
+                "Incorrect eth was received during fillOrder test when adequate ETH was sent"
-
+            );
-            // Get an alias to the test log.
-            TestLog memory log = testLogs[0];
-
-            // If the forwarded value was greater than the protocol fee, the protocol fee should
-            // have been sent back to this contract.
-            if (msg.value >= protocolFee) {
-                // Ensure that the protocolFee was forwarded to this contract.
-                require(
-                    log.loggedValue == protocolFee,
-                    "Incorrect eth was received during fillOrder test when adequate ETH was sent"
-                );
-            } else {
-                // Ensure that no ether was forwarded to this contract.
-                require(
-                    log.loggedValue == 0,
-                    "Incorrect eth was received during fillOrder test when inadequate ETH was sent"
-                );
-            }
-
-            // Ensure that the correct data was logged during this test.
-            require(log.loggedMaker == makerAddress1, "Incorrect maker address was logged for fillOrder test");
-            require(log.loggedPayer == address(this), "Incorrect taker address was logged for fillOrder test");
-            require(log.loggedProtocolFeePaid == protocolFee, "Incorrect protocol fee was logged for fillOrder test");
         } else {
-            // Ensure that `protocolFeePaid()` was not called.
+            // Ensure that the protocol fee was not paid in ether.
-            require(testLogs.length == 0, "protocolFeePaid was called");
+            require(
+                log.loggedValue == 0,
+                "Incorrect eth was received during fillOrder test when inadequate ETH was sent"
+            );
         }
+
+        // Ensure that the correct data was logged.
+        require(log.loggedMaker == expectedMakerAddress, "Incorrect maker address was logged");
+        require(log.loggedPayer == expectedPayerAddress, "Incorrect taker address was logged");
+        require(log.loggedProtocolFeePaid == expectedProtocolFeePaid, "Incorrect protocol fee was logged");
     }
 
-    function verifyMatchOrdersTestResults(
+    /* Testing Convenience Functions */
-        uint256 protocolFeeMultiplier,
-        bool shouldSetProtocolFeeCollector
-    )
-        internal
-    {
-        // If the `protocolFeeCollector` was set, then this contract should have been called.
-        if (shouldSetProtocolFeeCollector) {
-            // Calculate the protocol fee that should be paid.
-            uint256 protocolFee = tx.gasprice.safeMul(protocolFeeMultiplier);
 
-            // Ensure that one TestLog was recorded.
+    /// @dev Sets up state that is necessary for tests and then cleans up the state that was written
-            require(testLogs.length == 2, "Incorrect TestLog length for matchOrders test");
+    ///      to so that test cases can be thought of as atomic.
-
+    /// @param testProtocolFees The TestProtocolFees contract that is being used during testing.
-            // Get an alias to the test logs.
+    /// @param protocolFeeMultiplier The protocolFeeMultiplier of this test case.
-            TestLog memory log1 = testLogs[0];
+    /// @param shouldSetProtocolFeeCollector A boolean value that indicates whether or not this address
-            TestLog memory log2 = testLogs[1];
+    ///        should be made the protocol fee collector.
-
+    modifier handleState(
-            // If the forwarded value was greater than the protocol fee, the protocol fee should
-            // have been sent back to this contract.
-            if (msg.value >= 2 * protocolFee) {
-                // Ensure that the protocolFee was forwarded to this contract.
-                require(
-                    log1.loggedValue == protocolFee && log2.loggedValue == protocolFee,
-                    "Incorrect eth was received during matchOrders test when adequate ETH was sent"
-                );
-            } else {
-                // Ensure that no ether was forwarded to this contract.
-                require(
-                    log1.loggedValue == 0 && log2.loggedValue == 0,
-                    "Incorrect eth was received during fillOrder test when inadequate ETH was sent"
-                );
-            }
-
-            // Ensure that the correct data was logged during this test.
-            require(log1.loggedMaker == makerAddress1, "Incorrect maker address was logged for matchOrders test");
-            require(log1.loggedPayer == address(this), "Incorrect taker address was logged for matchOrders test");
-            require(log1.loggedProtocolFeePaid == protocolFee, "Incorrect protocol fee was logged for matchOrders test");
-            require(log2.loggedMaker == makerAddress2, "Incorrect maker address was logged for matchOrders test");
-            require(log2.loggedPayer == address(this), "Incorrect taker address was logged for matchOrders test");
-            require(log2.loggedProtocolFeePaid == protocolFee, "Incorrect protocol fee was logged for matchOrders test");
-        } else {
-            // Ensure that `protocolFeePaid()` was not called.
-            require(testLogs.length == 0, "protocolFeePaid was called");
-        }
-    }
-
-    /* State setup and teardown functions */
-
-    function setExchangeState(
         TestProtocolFees testProtocolFees,
         uint256 protocolFeeMultiplier,
         bool shouldSetProtocolFeeCollector
     )
-        internal
     {
+        // If necessary, set the protocol fee collector field in the exchange.
         if (shouldSetProtocolFeeCollector) {
             testProtocolFees.setProtocolFeeCollector(address(this));
         }
+        // Set the protocol fee multiplier in the exchange.
         testProtocolFees.setProtocolFeeMultiplier(protocolFeeMultiplier);
-    }
 
-    function clearState(TestProtocolFees testProtocolFees)
+        // Execute the test.
-        internal
+        _;
-    {
+
         // Clear exchange state
         testProtocolFees.setProtocolFeeCollector(address(0));
         testProtocolFees.setProtocolFeeMultiplier(0);
@@ -238,6 +310,25 @@ contract TestProtocolFeesReceiver {
         delete testLogs;
     }
 
+    /// @dev Constructs an order with a specified maker address.
+    /// @param makerAddress The maker address of the order.
+    function createOrder(address makerAddress)
+        internal
+        pure
+        returns (LibOrder.Order memory order)
+    {
+        order.makerAddress = makerAddress;
+        order.makerAssetAmount = 1; // This is 1 so that it doesn't trigger a `DivionByZero()` error.
+        order.takerAssetAmount = 1; // This is 1 so that it doesn't trigger a `DivionByZero()` error.
+    }
+
+    /* Protocol Fee Receiver and Fallback */
+
+    /// @dev Receives payments of protocol fees from a TestProtocolFees contract
+    ///      and records the data provided and the message value sent.
+    /// @param makerAddress The maker address that should be recorded.
+    /// @param payerAddress The payer address that should be recorded.
+    /// @param protocolFeePaid The protocol fee that should be recorded.
     function payProtocolFee(
         address makerAddress,
         address payerAddress,
@@ -246,6 +337,7 @@ contract TestProtocolFeesReceiver {
         external
         payable
     {
+        // Push the collected data into `testLogs`.
         testLogs.push(TestLog({
             loggedMaker: makerAddress,
             loggedPayer: payerAddress,
@@ -254,6 +346,8 @@ contract TestProtocolFeesReceiver {
         }));
     }
 
+    /// @dev A payable fallback function that makes this contract "payable". This is necessary to allow
+    ///      this contract to gracefully handle refunds from TestProtocolFees contracts.
     function ()
         external
         payable

contracts/exchange/test/assertion_wrappers/fill_order_wrapper.ts

@@ -33,8 +33,6 @@ export class FillOrderWrapper {
     private readonly _blockchainBalanceStore: BlockchainBalanceStore;
     private readonly _web3Wrapper: Web3Wrapper;
 
-    // FIXME - Punting on these tests for now since no staking contract will be registered. This
-    //         should be revisited when the protocol fee testing has been unit tested well.
     /**
      * Simulates matching two orders by transferring amounts defined in
      * `transferAmounts` and returns the results.
@@ -90,20 +88,6 @@ export class FillOrderWrapper {
             fillResults.makerFeePaid,
             signedOrder.makerFeeAssetData,
         );
-        // FIXME - Punting on these tests for now since no staking contract will be registered. This
-        //         should be revisited when the protocol fee testing has been unit tested well.
-        // if (stakingOpts.messageValue.isGreaterThanOrEqualTo(fillResults.protocolFeePaid)) {
-        //     // Pay the protocol fee in ETH.
-        //     balanceStore.transferAsset(takerAddress, stakingOpts.stakingAddress, fillResults.protocolFeePaid, '');
-        // } else {
-        //     // Pay the protocol fee in WETH.
-        //     balanceStore.transferAsset(
-        //         takerAddress,
-        //         stakingOpts.stakingAddress,
-        //         fillResults.protocolFeePaid,
-        //         AssetProxyId.ERC20,
-        //     );
-        // }
         return [fillResults, fillEvent, balanceStore];
     }
 
@@ -167,8 +151,6 @@ export class FillOrderWrapper {
         return this._blockchainBalanceStore;
     }
 
-    // FIXME - Punting on these tests for now since no staking contract will be registered. This
-    //         should be revisited when the protocol fee testing has been unit tested well.
     /**
      * Fills an order and asserts the effects. This includes
      * 1. The order info (via `getOrderInfo`)

contracts/exchange/test/internal.ts

@@ -164,7 +164,6 @@ blockchainTests('Exchange core internal functions', env => {
             return _.assign({}, ORDER_DEFAULTS, details);
         }
 
-        // FIXME - This test definitely needs to be updated
         async function testUpdateFilledStateAsync(
             order: OrderWithoutDomain,
             orderTakerAssetFilledAmount: BigNumber,
@@ -190,7 +189,7 @@ blockchainTests('Exchange core internal functions', env => {
                     orderHash,
                     orderTakerAssetFilledAmount,
                     fillResults,
-                    // opts, // FIXME
+                    // opts,
                 ),
             );
             // Grab the new `filled` state for this order.
@@ -242,7 +241,6 @@ blockchainTests('Exchange core internal functions', env => {
             const order = makeOrder();
             const orderTakerAssetFilledAmount = constants.MAX_UINT256.dividedToIntegerBy(2);
             const takerAssetFillAmount = constants.MAX_UINT256.dividedToIntegerBy(2).plus(2);
-            // FIXME
             const fillResults = {
                 makerAssetFilledAmount: constants.ZERO_AMOUNT,
                 takerAssetFilledAmount: takerAssetFillAmount,
@@ -289,7 +287,6 @@ blockchainTests('Exchange core internal functions', env => {
             const order = DEFAULT_ORDER;
             const orderHash = randomHash();
             const takerAddress = randomAddress();
-            // FIXME
             const fillResults = {
                 makerAssetFilledAmount: ONE_ETHER.times(2),
                 takerAssetFilledAmount: ONE_ETHER.times(10),
@@ -365,14 +362,14 @@ blockchainTests('Exchange core internal functions', env => {
                     takerAssetFilledAmount: ONE_ETHER.times(10),
                     makerFeePaid: ONE_ETHER.times(0.01),
                     takerFeePaid: constants.MAX_UINT256,
-                    protocolFeePaid: constants.ZERO_AMOUNT, // FIXME
+                    protocolFeePaid: constants.ZERO_AMOUNT,
                 },
                 right: {
                     takerAssetFilledAmount: ONE_ETHER.times(20),
                     makerAssetFilledAmount: ONE_ETHER.times(4),
                     makerFeePaid: ONE_ETHER.times(0.02),
                     takerFeePaid: constants.MAX_UINT256_ROOT,
-                    protocolFeePaid: constants.ZERO_AMOUNT, // FIXME
+                    protocolFeePaid: constants.ZERO_AMOUNT,
                 },
                 profitInLeftMakerAsset: ONE_ETHER,
                 profitInRightMakerAsset: ONE_ETHER.times(2),
@@ -414,14 +411,14 @@ blockchainTests('Exchange core internal functions', env => {
                     takerAssetFilledAmount: ONE_ETHER.times(10),
                     makerFeePaid: ONE_ETHER.times(0.01),
                     takerFeePaid: constants.MAX_UINT256,
-                    protocolFeePaid: constants.ZERO_AMOUNT, // FIXME
+                    protocolFeePaid: constants.ZERO_AMOUNT,
                 },
                 right: {
                     takerAssetFilledAmount: ONE_ETHER.times(20),
                     makerAssetFilledAmount: ONE_ETHER.times(4),
                     makerFeePaid: ONE_ETHER.times(0.02),
                     takerFeePaid: constants.MAX_UINT256_ROOT,
-                    protocolFeePaid: constants.ZERO_AMOUNT, // FIXME
+                    protocolFeePaid: constants.ZERO_AMOUNT,
                 },
                 profitInLeftMakerAsset: ONE_ETHER,
                 profitInRightMakerAsset: ONE_ETHER.times(2),
@@ -452,14 +449,14 @@ blockchainTests('Exchange core internal functions', env => {
                     takerAssetFilledAmount: ONE_ETHER.times(10),
                     makerFeePaid: ONE_ETHER.times(0.01),
                     takerFeePaid: ONE_ETHER.times(0.025),
-                    protocolFeePaid: constants.ZERO_AMOUNT, // FIXME
+                    protocolFeePaid: constants.ZERO_AMOUNT,
                 },
                 right: {
                     takerAssetFilledAmount: ONE_ETHER.times(20),
                     makerAssetFilledAmount: ONE_ETHER.times(4),
                     makerFeePaid: ONE_ETHER.times(0.02),
                     takerFeePaid: ONE_ETHER.times(0.05),
-                    protocolFeePaid: constants.ZERO_AMOUNT, // FIXME
+                    protocolFeePaid: constants.ZERO_AMOUNT,
                 },
                 profitInLeftMakerAsset: ONE_ETHER,
                 profitInRightMakerAsset: ONE_ETHER.times(2),
@@ -550,14 +547,14 @@ blockchainTests('Exchange core internal functions', env => {
                     takerAssetFilledAmount: ONE_ETHER.times(10),
                     makerFeePaid: ONE_ETHER.times(0.01),
                     takerFeePaid: ONE_ETHER.times(0.025),
-                    protocolFeePaid: constants.ZERO_AMOUNT, // FIXME
+                    protocolFeePaid: constants.ZERO_AMOUNT,
                 },
                 right: {
                     takerAssetFilledAmount: ONE_ETHER.times(20),
                     makerAssetFilledAmount: ONE_ETHER.times(4),
                     makerFeePaid: ONE_ETHER.times(0.02),
                     takerFeePaid: ONE_ETHER.times(0.05),
-                    protocolFeePaid: constants.ZERO_AMOUNT, // FIXME
+                    protocolFeePaid: constants.ZERO_AMOUNT,
                 },
                 profitInLeftMakerAsset: ONE_ETHER,
                 profitInRightMakerAsset: ONE_ETHER.times(2),

contracts/exchange/test/protocol_fees.ts

@@ -3,7 +3,10 @@ import { BigNumber } from '@0x/utils';
 
 import { artifacts, TestProtocolFeesContract, TestProtocolFeesReceiverContract } from '../src';
 
-blockchainTests.only('Protocol Fee Payments', env => {
+// The contents of this test suite does not inform the reader about the assertions made in these
+// tests. For more information and a more accurate view of the tests, check out
+// "contracts/test/TestProtocolFeesReceiver.sol".
+blockchainTests('Protocol Fee Payments', env => {
     let testProtocolFees: TestProtocolFeesContract;
     let testProtocolFeesReceiver: TestProtocolFeesReceiverContract;
 
@@ -46,7 +49,7 @@ blockchainTests.only('Protocol Fee Payments', env => {
                 true,
                 {
                     gasPrice: DEFAULT_GAS_PRICE,
-                    value: DEFAULT_PROTOCOL_FEE.minus(new BigNumber(10)),
+                    value: DEFAULT_PROTOCOL_FEE.minus(10),
                 },
             );
         });
@@ -70,7 +73,7 @@ blockchainTests.only('Protocol Fee Payments', env => {
                 true,
                 {
                     gasPrice: DEFAULT_GAS_PRICE,
-                    value: DEFAULT_PROTOCOL_FEE.plus(new BigNumber(10)),
+                    value: DEFAULT_PROTOCOL_FEE.plus(10),
                 },
             );
         });
@@ -89,19 +92,19 @@ blockchainTests.only('Protocol Fee Payments', env => {
             );
         });
 
-        it('should pay protocol fee in WETH when too little value is sent', async () => {
+        it('should pay protocol fee in WETH twice when too little value is sent', async () => {
             await testProtocolFeesReceiver.testMatchOrdersProtocolFees.awaitTransactionSuccessAsync(
                 testProtocolFees.address,
                 DEFAULT_PROTOCOL_FEE_MULTIPLIER,
                 true,
                 {
                     gasPrice: DEFAULT_GAS_PRICE,
-                    value: DEFAULT_PROTOCOL_FEE.minus(new BigNumber(10)),
+                    value: DEFAULT_PROTOCOL_FEE.minus(10),
                 },
             );
         });
 
-        it('should pay protocol fee in ETH when the correct value is sent', async () => {
+        it('should pay protocol fee in ETH and then WETH when the correct value is sent', async () => {
             await testProtocolFeesReceiver.testMatchOrdersProtocolFees.awaitTransactionSuccessAsync(
                 testProtocolFees.address,
                 DEFAULT_PROTOCOL_FEE_MULTIPLIER,
@@ -113,14 +116,157 @@ blockchainTests.only('Protocol Fee Payments', env => {
             );
         });
 
-        it('should pay protocol fee in ETH when extra value is sent', async () => {
+        it('should pay protocol fee in ETH when extra value is sent and then pay in WETH', async () => {
             await testProtocolFeesReceiver.testMatchOrdersProtocolFees.awaitTransactionSuccessAsync(
                 testProtocolFees.address,
                 DEFAULT_PROTOCOL_FEE_MULTIPLIER,
                 true,
                 {
                     gasPrice: DEFAULT_GAS_PRICE,
-                    value: DEFAULT_PROTOCOL_FEE.plus(new BigNumber(10)),
+                    value: DEFAULT_PROTOCOL_FEE.plus(10),
+                },
+            );
+        });
+
+        it('should pay protocol fee in ETH when exactly double the protocol fee is sent', async () => {
+            await testProtocolFeesReceiver.testMatchOrdersProtocolFees.awaitTransactionSuccessAsync(
+                testProtocolFees.address,
+                DEFAULT_PROTOCOL_FEE_MULTIPLIER,
+                true,
+                {
+                    gasPrice: DEFAULT_GAS_PRICE,
+                    value: DEFAULT_PROTOCOL_FEE.times(2),
+                },
+            );
+        });
+
+        it('should pay protocol fee in ETH when more than double the protocol fee is sent', async () => {
+            await testProtocolFeesReceiver.testMatchOrdersProtocolFees.awaitTransactionSuccessAsync(
+                testProtocolFees.address,
+                DEFAULT_PROTOCOL_FEE_MULTIPLIER,
+                true,
+                {
+                    gasPrice: DEFAULT_GAS_PRICE,
+                    value: DEFAULT_PROTOCOL_FEE.times(2).plus(10),
+                },
+            );
+        });
+    });
+
+    blockchainTests.resets('batchFillOrder ProtocolFees', () => {
+        it('should not pay protocol fees when there is not a protocolFeeCollector registered', async () => {
+            await testProtocolFeesReceiver.testBatchFillOrdersProtocolFees.awaitTransactionSuccessAsync(
+                testProtocolFees.address,
+                DEFAULT_PROTOCOL_FEE_MULTIPLIER,
+                new BigNumber(2), // If successful, create a `batchFillOrders` with 2 orders.
+                false,
+                {
+                    gasPrice: DEFAULT_GAS_PRICE,
+                    value: DEFAULT_PROTOCOL_FEE,
+                },
+            );
+        });
+
+        it('should pay one protocol fee in WETH when too little ETH is sent and only one order is in the batch', async () => {
+            await testProtocolFeesReceiver.testBatchFillOrdersProtocolFees.awaitTransactionSuccessAsync(
+                testProtocolFees.address,
+                DEFAULT_PROTOCOL_FEE_MULTIPLIER,
+                new BigNumber(1),
+                true,
+                {
+                    gasPrice: DEFAULT_GAS_PRICE,
+                    value: DEFAULT_PROTOCOL_FEE.minus(10),
+                },
+            );
+        });
+
+        it('should pay one protocol fee in ETH when the exact protocol fee is sent and only one order is in the batch', async () => {
+            await testProtocolFeesReceiver.testBatchFillOrdersProtocolFees.awaitTransactionSuccessAsync(
+                testProtocolFees.address,
+                DEFAULT_PROTOCOL_FEE_MULTIPLIER,
+                new BigNumber(1),
+                true,
+                {
+                    gasPrice: DEFAULT_GAS_PRICE,
+                    value: DEFAULT_PROTOCOL_FEE,
+                },
+            );
+        });
+
+        it('should pay one protocol fee in ETH when more than the exact protocol fee is sent and only one order is in the batch', async () => {
+            await testProtocolFeesReceiver.testBatchFillOrdersProtocolFees.awaitTransactionSuccessAsync(
+                testProtocolFees.address,
+                DEFAULT_PROTOCOL_FEE_MULTIPLIER,
+                new BigNumber(1),
+                true,
+                {
+                    gasPrice: DEFAULT_GAS_PRICE,
+                    value: DEFAULT_PROTOCOL_FEE.plus(10),
+                },
+            );
+        });
+
+        it('should pay both protocol fees in WETH when an insuffiecent amount of ETH for one protocol fee is sent', async () => {
+            await testProtocolFeesReceiver.testBatchFillOrdersProtocolFees.awaitTransactionSuccessAsync(
+                testProtocolFees.address,
+                DEFAULT_PROTOCOL_FEE_MULTIPLIER,
+                new BigNumber(2),
+                true,
+                {
+                    gasPrice: DEFAULT_GAS_PRICE,
+                    value: DEFAULT_PROTOCOL_FEE.minus(10),
+                },
+            );
+        });
+
+        it('should pay a protocol in ETH and then a fee in WETH when exactly one protocol fee in ETH is sent', async () => {
+            await testProtocolFeesReceiver.testBatchFillOrdersProtocolFees.awaitTransactionSuccessAsync(
+                testProtocolFees.address,
+                DEFAULT_PROTOCOL_FEE_MULTIPLIER,
+                new BigNumber(2),
+                true,
+                {
+                    gasPrice: DEFAULT_GAS_PRICE,
+                    value: DEFAULT_PROTOCOL_FEE,
+                },
+            );
+        });
+
+        it('should pay both protocol fees in ETH when exactly two protocol fees in ETH is sent', async () => {
+            await testProtocolFeesReceiver.testBatchFillOrdersProtocolFees.awaitTransactionSuccessAsync(
+                testProtocolFees.address,
+                DEFAULT_PROTOCOL_FEE_MULTIPLIER,
+                new BigNumber(2),
+                true,
+                {
+                    gasPrice: DEFAULT_GAS_PRICE,
+                    value: DEFAULT_PROTOCOL_FEE.times(2),
+                },
+            );
+        });
+
+        it('should pay two protocol fees in ETH and one in WETH when exactly two protocol fees in ETH is sent', async () => {
+            await testProtocolFeesReceiver.testBatchFillOrdersProtocolFees.awaitTransactionSuccessAsync(
+                testProtocolFees.address,
+                DEFAULT_PROTOCOL_FEE_MULTIPLIER,
+                new BigNumber(3),
+                true,
+                {
+                    gasPrice: DEFAULT_GAS_PRICE,
+                    value: DEFAULT_PROTOCOL_FEE.times(2),
+                },
+            );
+        });
+
+        it('should pay three protocol fees in ETH  when more than three protocol fees in ETH is sent', async () => {
+            await testProtocolFeesReceiver.testBatchFillOrdersProtocolFees.awaitTransactionSuccessAsync(
+                testProtocolFees.address,
+                DEFAULT_PROTOCOL_FEE_MULTIPLIER,
+                new BigNumber(3),
+                true,
+                {
+                    gasPrice: DEFAULT_GAS_PRICE,
+                    value: DEFAULT_PROTOCOL_FEE.times(3).plus(10),
                 },
             );
         });

contracts/exchange/test/reference_functions.ts

@@ -65,7 +65,9 @@ describe('Reference functions', () => {
                 makerAssetFilledAmount,
                 order.makerFee,
             );
-            return expect(() => LibReferenceFunctions.calculateFillResults(order, takerAssetFilledAmount)).to.throw(expectedError.message);
+            return expect(() => LibReferenceFunctions.calculateFillResults(order, takerAssetFilledAmount)).to.throw(
+                expectedError.message,
+            );
         });
 
         it('reverts if computing `fillResults.takerFeePaid` overflows', () => {
@@ -81,7 +83,9 @@ describe('Reference functions', () => {
                 takerAssetFilledAmount,
                 order.takerFee,
             );
-            return expect(() => LibReferenceFunctions.calculateFillResults(order, takerAssetFilledAmount)).to.throw(expectedError.message);
+            return expect(() => LibReferenceFunctions.calculateFillResults(order, takerAssetFilledAmount)).to.throw(
+                expectedError.message,
+            );
         });
 
         it('reverts if `order.makerAssetAmount` is 0', () => {
@@ -91,7 +95,9 @@ describe('Reference functions', () => {
             });
             const takerAssetFilledAmount = ONE_ETHER;
             const expectedError = new LibMathRevertErrors.DivisionByZeroError();
-            return expect(() => LibReferenceFunctions.calculateFillResults(order, takerAssetFilledAmount)).to.throw(expectedError.message);
+            return expect(() => LibReferenceFunctions.calculateFillResults(order, takerAssetFilledAmount)).to.throw(
+                expectedError.message,
+            );
         });
 
         it('reverts if `order.takerAssetAmount` is 0', () => {
@@ -101,7 +107,9 @@ describe('Reference functions', () => {
             });
             const takerAssetFilledAmount = ONE_ETHER;
             const expectedError = new LibMathRevertErrors.DivisionByZeroError();
-            return expect(() => LibReferenceFunctions.calculateFillResults(order, takerAssetFilledAmount)).to.throw(expectedError.message);
+            return expect(() => LibReferenceFunctions.calculateFillResults(order, takerAssetFilledAmount)).to.throw(
+                expectedError.message,
+            );
         });
 
         it('reverts if there is a rounding error computing `makerAsssetFilledAmount`', () => {
@@ -115,7 +123,9 @@ describe('Reference functions', () => {
                 order.takerAssetAmount,
                 order.makerAssetAmount,
             );
-            return expect(() => LibReferenceFunctions.calculateFillResults(order, takerAssetFilledAmount)).to.throw(expectedError.message);
+            return expect(() => LibReferenceFunctions.calculateFillResults(order, takerAssetFilledAmount)).to.throw(
+                expectedError.message,
+            );
         });
 
         it('reverts if there is a rounding error computing `makerFeePaid`', () => {
@@ -135,7 +145,9 @@ describe('Reference functions', () => {
                 order.makerAssetAmount,
                 order.makerFee,
             );
-            return expect(() => LibReferenceFunctions.calculateFillResults(order, takerAssetFilledAmount)).to.throw(expectedError.message);
+            return expect(() => LibReferenceFunctions.calculateFillResults(order, takerAssetFilledAmount)).to.throw(
+                expectedError.message,
+            );
         });
 
         it('reverts if there is a rounding error computing `takerFeePaid`', () => {
@@ -155,7 +167,9 @@ describe('Reference functions', () => {
                 order.makerAssetAmount,
                 order.takerFee,
             );
-            return expect(() => LibReferenceFunctions.calculateFillResults(order, takerAssetFilledAmount)).to.throw(expectedError.message);
+            return expect(() => LibReferenceFunctions.calculateFillResults(order, takerAssetFilledAmount)).to.throw(
+                expectedError.message,
+            );
         });
     });
 });

contracts/exchange/test/utils/fill_order_combinatorial_utils.ts

@@ -49,7 +49,6 @@ enum TestOutlook {
     Failure,
 }
 
-// FIXME - Really punting on this for now. It's possible that this won't need to be changed.
 /**
  * Instantiates a new instance of FillOrderCombinatorialUtils. Since this method has some
  * required async setup, a factory method is required.

contracts/exchange/test/utils/fill_order_simulator.ts

@@ -20,7 +20,6 @@ export enum FillOrderError {
     TransferFailed = 'TRANSFER_FAILED',
 }
 
-// FIXME - Punting on protocol fees for now
 /**
  * Simplified fill order simulator.
  */
@@ -122,7 +121,7 @@ export class FillOrderSimulator {
             makerAssetFilledAmount: makerAssetFillAmount,
             makerFeePaid,
             takerFeePaid,
-            protocolFeePaid: constants.ZERO_AMOUNT, // FIXME
+            protocolFeePaid: constants.ZERO_AMOUNT,
         };
     }
 }

contracts/exchange/test/utils/match_order_tester.ts

@@ -235,7 +235,6 @@ export class MatchOrderTester {
         return expectedBatchMatchResults;
     }
 
-    // FIXME - Punting on protocol fees until later
     /**
      * Matches two complementary orders and asserts results.
      * @param orders The matched orders and filled states.
@@ -267,7 +266,7 @@ export class MatchOrderTester {
                 orders.leftOrder,
                 orders.rightOrder,
                 takerAddress,
-                {}, // FIXME
+                {},
             );
             transactionReceipt = await this._executeMatchOrdersWithMaximalFillAsync(
                 orders.leftOrder,
@@ -1200,14 +1199,14 @@ function convertToMatchResults(result: MatchResults): MatchedFillResults {
             takerAssetFilledAmount: result.fills[0].takerAssetFilledAmount,
             makerFeePaid: result.fills[0].makerFeePaid,
             takerFeePaid: result.fills[0].takerFeePaid,
-            protocolFeePaid: constants.ZERO_AMOUNT, // FIXME
+            protocolFeePaid: constants.ZERO_AMOUNT,
         },
         right: {
             makerAssetFilledAmount: result.fills[1].makerAssetFilledAmount,
             takerAssetFilledAmount: result.fills[1].takerAssetFilledAmount,
             makerFeePaid: result.fills[1].makerFeePaid,
             takerFeePaid: result.fills[1].takerFeePaid,
-            protocolFeePaid: constants.ZERO_AMOUNT, // FIXME
+            protocolFeePaid: constants.ZERO_AMOUNT,
         },
         profitInLeftMakerAsset,
         profitInRightMakerAsset,
@@ -1226,7 +1225,7 @@ function convertToFillResults(result: FillEventArgs): FillResults {
         takerAssetFilledAmount: result.takerAssetFilledAmount,
         makerFeePaid: result.makerFeePaid,
         takerFeePaid: result.takerFeePaid,
-        protocolFeePaid: constants.ZERO_AMOUNT, // FIXME
+        protocolFeePaid: constants.ZERO_AMOUNT,
     };
     return fillResults;
 }

contracts/exchange/test/wrapper.ts

@@ -501,8 +501,6 @@ blockchainTests.resets('Exchange wrappers', env => {
                         .dividedToIntegerBy(signedOrder.makerAssetAmount);
 
                     takerAssetFillAmounts.push(takerAssetFillAmount);
-                    // FIXME - Punting on these tests for now since no staking contract will be registered. This
-                    //         should be revisited when the protocol fee testing has been unit tested well.
                     expectedFillResults.push({
                         takerAssetFilledAmount: takerAssetFillAmount,
                         makerAssetFilledAmount,
@@ -534,8 +532,6 @@ blockchainTests.resets('Exchange wrappers', env => {
                     ].plus(makerFee.plus(takerFee));
                 });
 
-                // FIXME - Punting on these tests for now since no staking contract will be registered. This
-                //         should be revisited when the protocol fee testing has been unit tested well.
                 const fillResults = await exchange.batchFillOrders.callAsync(
                     signedOrders,
                     takerAssetFillAmounts,
@@ -573,8 +569,6 @@ blockchainTests.resets('Exchange wrappers', env => {
                         .dividedToIntegerBy(signedOrder.makerAssetAmount);
 
                     takerAssetFillAmounts.push(takerAssetFillAmount);
-                    // FIXME - Punting on these tests for now since no staking contract will be registered. This
-                    //         should be revisited when the protocol fee testing has been unit tested well.
                     expectedFillResults.push({
                         takerAssetFilledAmount: takerAssetFillAmount,
                         makerAssetFilledAmount,
@@ -659,8 +653,6 @@ blockchainTests.resets('Exchange wrappers', env => {
                         .dividedToIntegerBy(signedOrder.makerAssetAmount);
 
                     takerAssetFillAmounts.push(takerAssetFillAmount);
-                    // FIXME - Punting on these tests for now since no staking contract will be registered. This
-                    //         should be revisited when the protocol fee testing has been unit tested well.
                     expectedFillResults.push({
                         takerAssetFilledAmount: takerAssetFillAmount,
                         makerAssetFilledAmount,
@@ -732,8 +724,6 @@ blockchainTests.resets('Exchange wrappers', env => {
                         .dividedToIntegerBy(signedOrder.makerAssetAmount);
 
                     takerAssetFillAmounts.push(takerAssetFillAmount);
-                    // FIXME - Punting on these tests for now since no staking contract will be registered. This
-                    //         should be revisited when the protocol fee testing has been unit tested well.
                     expectedFillResults.push({
                         takerAssetFilledAmount: takerAssetFillAmount,
                         makerAssetFilledAmount,
@@ -876,7 +866,7 @@ blockchainTests.resets('Exchange wrappers', env => {
                         takerAssetFilledAmount: signedOrder.takerAssetAmount,
                         makerFeePaid: signedOrder.makerFee,
                         takerFeePaid: signedOrder.takerFee,
-                        protocolFeePaid: constants.ZERO_AMOUNT, // FIXME - This is what is being used now.
+                        protocolFeePaid: constants.ZERO_AMOUNT,
                     }))
                     .reduce(
                         (totalFillResults, currentFillResults) => ({
@@ -948,7 +938,7 @@ blockchainTests.resets('Exchange wrappers', env => {
                         takerAssetFilledAmount: signedOrder.takerAssetAmount,
                         makerFeePaid: signedOrder.makerFee,
                         takerFeePaid: signedOrder.takerFee,
-                        protocolFeePaid: constants.ZERO_AMOUNT, // FIXME
+                        protocolFeePaid: constants.ZERO_AMOUNT,
                     }))
                     .reduce(
                         (totalFillResults, currentFillResults) => ({
@@ -1064,7 +1054,7 @@ blockchainTests.resets('Exchange wrappers', env => {
                         takerAssetFilledAmount: signedOrder.takerAssetAmount,
                         makerFeePaid: signedOrder.makerFee,
                         takerFeePaid: signedOrder.takerFee,
-                        protocolFeePaid: constants.ZERO_AMOUNT, // FIXME
+                        protocolFeePaid: constants.ZERO_AMOUNT,
                     }))
                     .reduce(
                         (totalFillResults, currentFillResults) => ({
@@ -1137,7 +1127,7 @@ blockchainTests.resets('Exchange wrappers', env => {
                         takerAssetFilledAmount: signedOrder.takerAssetAmount,
                         makerFeePaid: signedOrder.makerFee,
                         takerFeePaid: signedOrder.takerFee,
-                        protocolFeePaid: constants.ZERO_AMOUNT, // FIXME
+                        protocolFeePaid: constants.ZERO_AMOUNT,
                     }))
                     .reduce(
                         (totalFillResults, currentFillResults) => ({