Merge pull request #936 from 0xProject/feature/contract-wrappers/forwader-optimizations

ForwarderWrapper order optimizations

13 files changed, 241 insertions, 44 deletions

diff --git a/packages/contract-wrappers/CHANGELOG.json b/packages/contract-wrappers/CHANGELOG.json
index 8fa31052c..02e1acf91 100644
--- a/packages/contract-wrappers/CHANGELOG.json
+++ b/packages/contract-wrappers/CHANGELOG.json
@@ -1,10 +1,14 @@
 [
     {
-        "version": "1.1.0-rc.2",
+        "version": "1.0.1-rc.3",
         "changes": [
             {
                 "note": "Add ForwarderWrapper",
                 "pr": 934
+            },
+            {
+                "note": "Optimize orders in ForwarderWrapper",
+                "pr": 936
             }
         ]
     },
diff --git a/packages/contract-wrappers/src/contract_wrappers/forwarder_wrapper.ts b/packages/contract-wrappers/src/contract_wrappers/forwarder_wrapper.ts
index 90cfbe2af..13ef0fe01 100644
--- a/packages/contract-wrappers/src/contract_wrappers/forwarder_wrapper.ts
+++ b/packages/contract-wrappers/src/contract_wrappers/forwarder_wrapper.ts
@@ -10,6 +10,7 @@ import { orderTxOptsSchema } from '../schemas/order_tx_opts_schema';
 import { txOptsSchema } from '../schemas/tx_opts_schema';
 import { TransactionOpts } from '../types';
 import { assert } from '../utils/assert';
+import { calldataOptimizationUtils } from '../utils/calldata_optimization_utils';
 import { constants } from '../utils/constants';
 
 import { ContractWrapper } from './contract_wrapper';
@@ -75,14 +76,19 @@ export class ForwarderWrapper extends ContractWrapper {
             this.getZRXTokenAddress(),
             this.getEtherTokenAddress(),
         );
+        // lowercase input addresses
         const normalizedTakerAddress = takerAddress.toLowerCase();
         const normalizedFeeRecipientAddress = feeRecipientAddress.toLowerCase();
+        // optimize orders
+        const optimizedMarketOrders = calldataOptimizationUtils.optimizeForwarderOrders(signedOrders);
+        const optimizedFeeOrders = calldataOptimizationUtils.optimizeForwarderFeeOrders(signedFeeOrders);
+        // send transaction
         const forwarderContractInstance = await this._getForwarderContractAsync();
         const txHash = await forwarderContractInstance.marketSellOrdersWithEth.sendTransactionAsync(
-            signedOrders,
-            _.map(signedOrders, order => order.signature),
-            signedFeeOrders,
-            _.map(signedFeeOrders, order => order.signature),
+            optimizedMarketOrders,
+            _.map(optimizedMarketOrders, order => order.signature),
+            optimizedFeeOrders,
+            _.map(optimizedFeeOrders, order => order.signature),
             feePercentage,
             feeRecipientAddress,
             {
@@ -138,15 +144,20 @@ export class ForwarderWrapper extends ContractWrapper {
             this.getZRXTokenAddress(),
             this.getEtherTokenAddress(),
         );
+        // lowercase input addresses
         const normalizedTakerAddress = takerAddress.toLowerCase();
         const normalizedFeeRecipientAddress = feeRecipientAddress.toLowerCase();
+        // optimize orders
+        const optimizedMarketOrders = calldataOptimizationUtils.optimizeForwarderOrders(signedOrders);
+        const optimizedFeeOrders = calldataOptimizationUtils.optimizeForwarderFeeOrders(signedFeeOrders);
+        // send transaction
         const forwarderContractInstance = await this._getForwarderContractAsync();
         const txHash = await forwarderContractInstance.marketBuyOrdersWithEth.sendTransactionAsync(
-            signedOrders,
+            optimizedMarketOrders,
             makerAssetFillAmount,
-            _.map(signedOrders, order => order.signature),
-            signedFeeOrders,
-            _.map(signedFeeOrders, order => order.signature),
+            _.map(optimizedMarketOrders, order => order.signature),
+            optimizedFeeOrders,
+            _.map(optimizedFeeOrders, order => order.signature),
             feePercentage,
             feeRecipientAddress,
             {
diff --git a/packages/contract-wrappers/src/utils/calldata_optimization_utils.ts b/packages/contract-wrappers/src/utils/calldata_optimization_utils.ts
new file mode 100644
index 000000000..3172cf531
--- /dev/null
+++ b/packages/contract-wrappers/src/utils/calldata_optimization_utils.ts
@@ -0,0 +1,44 @@
+import { SignedOrder } from '@0xproject/types';
+import * as _ from 'lodash';
+
+import { constants } from './constants';
+
+export const calldataOptimizationUtils = {
+    /**
+     * Takes an array of orders and outputs an array of equivalent orders where all takerAssetData are '0x' and
+     * all makerAssetData are '0x' except for that of the first order, which retains its original value
+     * @param   orders         An array of SignedOrder objects
+     * @returns optimized orders
+     */
+    optimizeForwarderOrders(orders: SignedOrder[]): SignedOrder[] {
+        const optimizedOrders = _.map(orders, (order, index) =>
+            transformOrder(order, {
+                makerAssetData: index === 0 ? order.makerAssetData : constants.NULL_BYTES,
+                takerAssetData: constants.NULL_BYTES,
+            }),
+        );
+        return optimizedOrders;
+    },
+    /**
+     * Takes an array of orders and outputs an array of equivalent orders where all takerAssetData are '0x' and
+     * all makerAssetData are '0x'
+     * @param   orders         An array of SignedOrder objects
+     * @returns optimized orders
+     */
+    optimizeForwarderFeeOrders(orders: SignedOrder[]): SignedOrder[] {
+        const optimizedOrders = _.map(orders, (order, index) =>
+            transformOrder(order, {
+                makerAssetData: constants.NULL_BYTES,
+                takerAssetData: constants.NULL_BYTES,
+            }),
+        );
+        return optimizedOrders;
+    },
+};
+
+const transformOrder = (order: SignedOrder, partialOrder: Partial<SignedOrder>) => {
+    return {
+        ...order,
+        ...partialOrder,
+    };
+};
diff --git a/packages/contract-wrappers/src/utils/constants.ts b/packages/contract-wrappers/src/utils/constants.ts
index d436efefc..2df11538c 100644
--- a/packages/contract-wrappers/src/utils/constants.ts
+++ b/packages/contract-wrappers/src/utils/constants.ts
@@ -2,6 +2,7 @@ import { BigNumber } from '@0xproject/utils';
 
 export const constants = {
     NULL_ADDRESS: '0x0000000000000000000000000000000000000000',
+    NULL_BYTES: '0x',
     TESTRPC_NETWORK_ID: 50,
     INVALID_JUMP_PATTERN: 'invalid JUMP at',
     REVERT: 'revert',
diff --git a/packages/contract-wrappers/test/calldata_optimization_utils_test.ts b/packages/contract-wrappers/test/calldata_optimization_utils_test.ts
new file mode 100644
index 000000000..a4cea772f
--- /dev/null
+++ b/packages/contract-wrappers/test/calldata_optimization_utils_test.ts
@@ -0,0 +1,60 @@
+import { orderFactory } from '@0xproject/order-utils';
+import * as chai from 'chai';
+import * as _ from 'lodash';
+import 'mocha';
+
+import { assert } from '../src/utils/assert';
+import { calldataOptimizationUtils } from '../src/utils/calldata_optimization_utils';
+import { constants } from '../src/utils/constants';
+
+import { chaiSetup } from './utils/chai_setup';
+
+chaiSetup.configure();
+const expect = chai.expect;
+
+// utility for generating a set of order objects with mostly NULL values
+// except for a specified makerAssetData and takerAssetData
+const FAKE_ORDERS_COUNT = 5;
+const generateFakeOrders = (makerAssetData: string, takerAssetData: string) =>
+    _.map(_.range(FAKE_ORDERS_COUNT), index => {
+        const order = orderFactory.createOrder(
+            constants.NULL_ADDRESS,
+            constants.ZERO_AMOUNT,
+            makerAssetData,
+            constants.ZERO_AMOUNT,
+            takerAssetData,
+            constants.NULL_ADDRESS,
+        );
+        return {
+            ...order,
+            signature: 'dummy signature',
+        };
+    });
+
+describe('calldataOptimizationUtils', () => {
+    const fakeMakerAssetData = 'fakeMakerAssetData';
+    const fakeTakerAssetData = 'fakeTakerAssetData';
+    const orders = generateFakeOrders(fakeMakerAssetData, fakeTakerAssetData);
+    describe('#optimizeForwarderOrders', () => {
+        it('should make makerAssetData `0x` unless first order', () => {
+            const optimizedOrders = calldataOptimizationUtils.optimizeForwarderOrders(orders);
+            expect(optimizedOrders[0].makerAssetData).to.equal(fakeMakerAssetData);
+            const ordersWithoutHead = _.slice(optimizedOrders, 1);
+            _.forEach(ordersWithoutHead, order => expect(order.makerAssetData).to.equal(constants.NULL_BYTES));
+        });
+        it('should make all takerAssetData `0x`', () => {
+            const optimizedOrders = calldataOptimizationUtils.optimizeForwarderOrders(orders);
+            _.forEach(optimizedOrders, order => expect(order.takerAssetData).to.equal(constants.NULL_BYTES));
+        });
+    });
+    describe('#optimizeForwarderFeeOrders', () => {
+        it('should make all makerAssetData `0x`', () => {
+            const optimizedOrders = calldataOptimizationUtils.optimizeForwarderFeeOrders(orders);
+            _.forEach(optimizedOrders, order => expect(order.makerAssetData).to.equal(constants.NULL_BYTES));
+        });
+        it('should make all takerAssetData `0x`', () => {
+            const optimizedOrders = calldataOptimizationUtils.optimizeForwarderFeeOrders(orders);
+            _.forEach(optimizedOrders, order => expect(order.takerAssetData).to.equal(constants.NULL_BYTES));
+        });
+    });
+});
diff --git a/packages/fill-scenarios/CHANGELOG.json b/packages/fill-scenarios/CHANGELOG.json
index 954803462..04e076203 100644
--- a/packages/fill-scenarios/CHANGELOG.json
+++ b/packages/fill-scenarios/CHANGELOG.json
@@ -1,5 +1,15 @@
 [
     {
+        "version": "1.0.1-rc.3",
+        "changes": [
+            {
+                "note":
+                    "Updated to use latest orderFactory interface, fixed `feeRecipient` spelling error in public interface",
+                "pr": 936
+            }
+        ]
+    },
+    {
         "version": "1.0.1-rc.2",
         "changes": [
             {
diff --git a/packages/fill-scenarios/src/fill_scenarios.ts b/packages/fill-scenarios/src/fill_scenarios.ts
index 8f2766e24..1a1adb326 100644
--- a/packages/fill-scenarios/src/fill_scenarios.ts
+++ b/packages/fill-scenarios/src/fill_scenarios.ts
@@ -61,7 +61,7 @@ export class FillScenarios {
         makerAddress: string,
         takerAddress: string,
         fillableAmount: BigNumber,
-        feeRecepientAddress: string,
+        feeRecipientAddress: string,
         expirationTimeSeconds?: BigNumber,
     ): Promise<SignedOrder> {
         return this._createAsymmetricFillableSignedOrderWithFeesAsync(
@@ -73,7 +73,7 @@ export class FillScenarios {
             takerAddress,
             fillableAmount,
             fillableAmount,
-            feeRecepientAddress,
+            feeRecipientAddress,
             expirationTimeSeconds,
         );
     }
@@ -88,7 +88,7 @@ export class FillScenarios {
     ): Promise<SignedOrder> {
         const makerFee = new BigNumber(0);
         const takerFee = new BigNumber(0);
-        const feeRecepientAddress = constants.NULL_ADDRESS;
+        const feeRecipientAddress = constants.NULL_ADDRESS;
         return this._createAsymmetricFillableSignedOrderWithFeesAsync(
             makerAssetData,
             takerAssetData,
@@ -98,7 +98,7 @@ export class FillScenarios {
             takerAddress,
             makerFillableAmount,
             takerFillableAmount,
-            feeRecepientAddress,
+            feeRecipientAddress,
             expirationTimeSeconds,
         );
     }
@@ -148,7 +148,7 @@ export class FillScenarios {
         takerAddress: string,
         makerFillableAmount: BigNumber,
         takerFillableAmount: BigNumber,
-        feeRecepientAddress: string,
+        feeRecipientAddress: string,
         expirationTimeSeconds?: BigNumber,
     ): Promise<SignedOrder> {
         const decodedMakerAssetData = assetDataUtils.decodeAssetDataOrThrow(makerAssetData);
@@ -194,17 +194,19 @@ export class FillScenarios {
         const signedOrder = await orderFactory.createSignedOrderAsync(
             this._web3Wrapper.getProvider(),
             makerAddress,
-            takerAddress,
-            senderAddress,
-            makerFee,
-            takerFee,
             makerFillableAmount,
             makerAssetData,
             takerFillableAmount,
             takerAssetData,
             this._exchangeAddress,
-            feeRecepientAddress,
-            expirationTimeSeconds,
+            {
+                takerAddress,
+                senderAddress,
+                makerFee,
+                takerFee,
+                feeRecipientAddress,
+                expirationTimeSeconds,
+            },
         );
         return signedOrder;
     }
diff --git a/packages/order-utils/CHANGELOG.json b/packages/order-utils/CHANGELOG.json
index a399f5ea1..70a75854a 100644
--- a/packages/order-utils/CHANGELOG.json
+++ b/packages/order-utils/CHANGELOG.json
@@ -1,5 +1,15 @@
 [
     {
+        "version": "1.0.1-rc.3",
+        "changes": [
+            {
+                "note":
+                    "Added a synchronous `createOrder` method in `orderFactory`, updated public interfaces to support some optional parameters",
+                "pr": 936
+            }
+        ]
+    },
+    {
         "version": "1.0.1-rc.2",
         "changes": [
             {
diff --git a/packages/order-utils/src/constants.ts b/packages/order-utils/src/constants.ts
index bb7482184..ea3f8b932 100644
--- a/packages/order-utils/src/constants.ts
+++ b/packages/order-utils/src/constants.ts
@@ -10,4 +10,6 @@ export const constants = {
     ERC721_ASSET_DATA_MINIMUM_BYTE_LENGTH: 53,
     SELECTOR_LENGTH: 4,
     BASE_16: 16,
+    INFINITE_TIMESTAMP_SEC: new BigNumber(2524604400), // Close to infinite
+    ZERO_AMOUNT: new BigNumber(0),
 };
diff --git a/packages/order-utils/src/index.ts b/packages/order-utils/src/index.ts
index 76be63bb8..129eb0a3d 100644
--- a/packages/order-utils/src/index.ts
+++ b/packages/order-utils/src/index.ts
@@ -13,7 +13,15 @@ export { orderFactory } from './order_factory';
 export { constants } from './constants';
 export { crypto } from './crypto';
 export { generatePseudoRandomSalt } from './salt';
-export { OrderError, MessagePrefixType, MessagePrefixOpts, EIP712Parameter, EIP712Schema, EIP712Types } from './types';
+export {
+    CreateOrderOpts,
+    OrderError,
+    MessagePrefixType,
+    MessagePrefixOpts,
+    EIP712Parameter,
+    EIP712Schema,
+    EIP712Types,
+} from './types';
 export { AbstractBalanceAndProxyAllowanceFetcher } from './abstract/abstract_balance_and_proxy_allowance_fetcher';
 export { AbstractOrderFilledCancelledFetcher } from './abstract/abstract_order_filled_cancelled_fetcher';
 export { BalanceAndProxyAllowanceLazyStore } from './store/balance_and_proxy_allowance_lazy_store';
diff --git a/packages/order-utils/src/order_factory.ts b/packages/order-utils/src/order_factory.ts
index 803cb82b1..14727fd97 100644
--- a/packages/order-utils/src/order_factory.ts
+++ b/packages/order-utils/src/order_factory.ts
@@ -1,49 +1,63 @@
-import { ECSignature, SignedOrder } from '@0xproject/types';
+import { ECSignature, Order, SignedOrder } from '@0xproject/types';
 import { BigNumber } from '@0xproject/utils';
 import { Provider } from 'ethereum-types';
 import * as ethUtil from 'ethereumjs-util';
 import * as _ from 'lodash';
 
+import { constants } from './constants';
 import { orderHashUtils } from './order_hash';
 import { generatePseudoRandomSalt } from './salt';
 import { ecSignOrderHashAsync } from './signature_utils';
-import { MessagePrefixType } from './types';
+import { CreateOrderOpts, MessagePrefixType } from './types';
 
 export const orderFactory = {
-    async createSignedOrderAsync(
-        provider: Provider,
+    createOrder(
         makerAddress: string,
-        takerAddress: string,
-        senderAddress: string,
-        makerFee: BigNumber,
-        takerFee: BigNumber,
         makerAssetAmount: BigNumber,
         makerAssetData: string,
         takerAssetAmount: BigNumber,
         takerAssetData: string,
         exchangeAddress: string,
-        feeRecipientAddress: string,
-        expirationTimeSecondsIfExists?: BigNumber,
-    ): Promise<SignedOrder> {
-        const defaultExpirationUnixTimestampSec = new BigNumber(2524604400); // Close to infinite
-        const expirationTimeSeconds = _.isUndefined(expirationTimeSecondsIfExists)
-            ? defaultExpirationUnixTimestampSec
-            : expirationTimeSecondsIfExists;
+        createOrderOpts: CreateOrderOpts = generateDefaultCreateOrderOpts(),
+    ): Order {
+        const defaultCreateOrderOpts = generateDefaultCreateOrderOpts();
         const order = {
             makerAddress,
-            takerAddress,
-            senderAddress,
-            makerFee,
-            takerFee,
             makerAssetAmount,
             takerAssetAmount,
             makerAssetData,
             takerAssetData,
-            salt: generatePseudoRandomSalt(),
             exchangeAddress,
-            feeRecipientAddress,
-            expirationTimeSeconds,
+            takerAddress: createOrderOpts.takerAddress || defaultCreateOrderOpts.takerAddress,
+            senderAddress: createOrderOpts.senderAddress || defaultCreateOrderOpts.senderAddress,
+            makerFee: createOrderOpts.makerFee || defaultCreateOrderOpts.makerFee,
+            takerFee: createOrderOpts.takerFee || defaultCreateOrderOpts.takerFee,
+            feeRecipientAddress: createOrderOpts.feeRecipientAddress || defaultCreateOrderOpts.feeRecipientAddress,
+            salt: createOrderOpts.salt || defaultCreateOrderOpts.salt,
+            expirationTimeSeconds:
+                createOrderOpts.expirationTimeSeconds || defaultCreateOrderOpts.expirationTimeSeconds,
         };
+        return order;
+    },
+    async createSignedOrderAsync(
+        provider: Provider,
+        makerAddress: string,
+        makerAssetAmount: BigNumber,
+        makerAssetData: string,
+        takerAssetAmount: BigNumber,
+        takerAssetData: string,
+        exchangeAddress: string,
+        createOrderOpts?: CreateOrderOpts,
+    ): Promise<SignedOrder> {
+        const order = orderFactory.createOrder(
+            makerAddress,
+            makerAssetAmount,
+            makerAssetData,
+            takerAssetAmount,
+            takerAssetData,
+            exchangeAddress,
+            createOrderOpts,
+        );
         const orderHash = orderHashUtils.getOrderHashHex(order);
         const messagePrefixOpts = {
             prefixType: MessagePrefixType.EthSign,
@@ -56,6 +70,26 @@ export const orderFactory = {
     },
 };
 
+function generateDefaultCreateOrderOpts(): {
+    takerAddress: string;
+    senderAddress: string;
+    makerFee: BigNumber;
+    takerFee: BigNumber;
+    feeRecipientAddress: string;
+    salt: BigNumber;
+    expirationTimeSeconds: BigNumber;
+} {
+    return {
+        takerAddress: constants.NULL_ADDRESS,
+        senderAddress: constants.NULL_ADDRESS,
+        makerFee: constants.ZERO_AMOUNT,
+        takerFee: constants.ZERO_AMOUNT,
+        feeRecipientAddress: constants.NULL_ADDRESS,
+        salt: generatePseudoRandomSalt(),
+        expirationTimeSeconds: constants.INFINITE_TIMESTAMP_SEC,
+    };
+}
+
 function getVRSHexString(ecSignature: ECSignature): string {
     const ETH_SIGN_SIGNATURE_TYPE = '03';
     const vrs = `${intToHex(ecSignature.v)}${ethUtil.stripHexPrefix(ecSignature.r)}${ethUtil.stripHexPrefix(
diff --git a/packages/order-utils/src/types.ts b/packages/order-utils/src/types.ts
index b08e74e71..f44e94349 100644
--- a/packages/order-utils/src/types.ts
+++ b/packages/order-utils/src/types.ts
@@ -1,3 +1,5 @@
+import { BigNumber } from '@0xproject/utils';
+
 export enum OrderError {
     InvalidSignature = 'INVALID_SIGNATURE',
 }
@@ -51,3 +53,13 @@ export enum EIP712Types {
     String = 'string',
     Uint256 = 'uint256',
 }
+
+export interface CreateOrderOpts {
+    takerAddress?: string;
+    senderAddress?: string;
+    makerFee?: BigNumber;
+    takerFee?: BigNumber;
+    feeRecipientAddress?: string;
+    salt?: BigNumber;
+    expirationTimeSeconds?: BigNumber;
+}
diff --git a/packages/sol-resolver/CHANGELOG.md b/packages/sol-resolver/CHANGELOG.md
index 8ff6ce6ed..5d2ee154a 100644
--- a/packages/sol-resolver/CHANGELOG.md
+++ b/packages/sol-resolver/CHANGELOG.md
@@ -5,7 +5,6 @@ Edit the package's CHANGELOG.json file only.
 
 CHANGELOG
 
-
 ## v1.0.4 - _July 26, 2018_
 
     * Dependencies updated