@0x/contracts-zero-ex: Add taker field to RFQ orders.`

`@0x/contracts-zero-ex`: Introduce protocol fees to all limit orders again.

contracts/zero-ex/CHANGELOG.json

@@ -31,7 +31,7 @@
                 "pr": 44
             },
             {
-                "note": "Remove protocol fees from all RFQ orders and limit orders if taker is set",
+                "note": "Remove protocol fees from all RFQ orders and add `taker` field to RFQ orders",
                 "pr": 45
             }
         ]

contracts/zero-ex/contracts/src/features/NativeOrdersFeature.sol

@@ -785,10 +785,8 @@ contract NativeOrdersFeature is
             }
         }
 
-        // Pay the protocol fee if this is an open-orderbook order (taker is not known).
-        if (params.order.taker == address(0)) {
-            results.ethProtocolFeePaid = _collectProtocolFee(params.order.pool);
-        }
+        // Pay the protocol fee.
+        results.ethProtocolFeePaid = _collectProtocolFee(params.order.pool);
 
         // Settle between the maker and taker.
         (results.takerTokenFilledAmount, results.makerTokenFilledAmount) = _settleOrder(
@@ -869,6 +867,15 @@ contract NativeOrdersFeature is
             ).rrevert();
         }
 
+        // Must be fillable by the taker.
+        if (order.taker != address(0) && order.taker != taker) {
+            LibNativeOrdersRichErrors.OrderNotFillableByTakerError(
+                orderInfo.orderHash,
+                taker,
+                order.taker
+            ).rrevert();
+        }
+
         // Signature must be valid for the order.
         {
             address signer = LibSignature.getSignerOfHash(orderInfo.orderHash, signature);

contracts/zero-ex/contracts/src/features/libs/LibNativeOrder.sol

@@ -56,6 +56,7 @@ library LibNativeOrder {
         uint128 makerAmount;
         uint128 takerAmount;
         address maker;
+        address taker;
         address txOrigin;
         bytes32 pool;
         uint64 expiry;
@@ -102,13 +103,14 @@ library LibNativeOrder {
     //       "uint128 takerAmount,",
     //       "address maker,",
     //       "address txOrigin,",
+    //       "address taker,",
     //       "bytes32 pool,",
     //       "uint64 expiry,",
     //       "uint256 salt"
     //     ")"
     // ))
     uint256 private constant _RFQ_ORDER_TYPEHASH =
-        0xc6b3034376598bc7f28b05e81db7ed88486dcdb6b4a6c7300353fffc5f31f382;
+        0xe593d3fdfa8b60e5e17a1b2204662ecbe15c23f2084b9ad5bae40359540a7da9;
 
     /// @dev Get the struct hash of a limit order.
     /// @param order The limit order.
@@ -181,6 +183,7 @@ library LibNativeOrder {
         //   order.makerAmount,
         //   order.takerAmount,
         //   order.maker,
+        //   order.taker,
         //   order.txOrigin,
         //   order.pool,
         //   order.expiry,
@@ -199,15 +202,17 @@ library LibNativeOrder {
             mstore(add(mem, 0x80), and(UINT_128_MASK, mload(add(order, 0x60))))
             // order.maker;
             mstore(add(mem, 0xA0), and(ADDRESS_MASK, mload(add(order, 0x80))))
-            // order.txOrigin;
+            // order.taker;
             mstore(add(mem, 0xC0), and(ADDRESS_MASK, mload(add(order, 0xA0))))
+            // order.txOrigin;
+            mstore(add(mem, 0xE0), and(ADDRESS_MASK, mload(add(order, 0xC0))))
             // order.pool;
-            mstore(add(mem, 0xE0), mload(add(order, 0xC0)))
+            mstore(add(mem, 0x100), mload(add(order, 0xE0)))
             // order.expiry;
-            mstore(add(mem, 0x100), and(UINT_64_MASK, mload(add(order, 0xE0))))
+            mstore(add(mem, 0x120), and(UINT_64_MASK, mload(add(order, 0x100))))
             // order.salt;
-            mstore(add(mem, 0x120), mload(add(order, 0x100)))
-            structHash := keccak256(mem, 0x140)
+            mstore(add(mem, 0x140), mload(add(order, 0x120)))
+            structHash := keccak256(mem, 0x160)
         }
     }
 }

contracts/zero-ex/contracts/test/TestMetaTransactionsNativeOrdersFeature.sol

@@ -87,7 +87,6 @@ contract TestMetaTransactionsNativeOrdersFeature is
     )
         public
         override
-        payable
         returns (uint128 takerTokenFilledAmount, uint128 makerTokenFilledAmount)
     {
         emit FillRfqOrderCalled(

contracts/zero-ex/src/orders.ts

@@ -20,6 +20,7 @@ const COMMON_ORDER_DEFAULT_VALUES = {
     makerAmount: ZERO,
     takerAmount: ZERO,
     maker: NULL_ADDRESS,
+    taker: NULL_ADDRESS,
     pool: hexUtils.leftPad(0),
     expiry: ZERO,
     salt: ZERO,
@@ -29,7 +30,6 @@ const COMMON_ORDER_DEFAULT_VALUES = {
 const LIMIT_ORDER_DEFAULT_VALUES = {
     ...COMMON_ORDER_DEFAULT_VALUES,
     takerTokenFeeAmount: ZERO,
-    taker: NULL_ADDRESS,
     sender: NULL_ADDRESS,
     feeRecipient: NULL_ADDRESS,
 };
@@ -63,6 +63,7 @@ export abstract class OrderBase {
     public takerAmount: BigNumber;
     public maker: string;
     public pool: string;
+    public taker: string;
     public expiry: BigNumber;
     public salt: BigNumber;
     public chainId: number;
@@ -75,6 +76,7 @@ export abstract class OrderBase {
         this.makerAmount = _fields.makerAmount;
         this.takerAmount = _fields.takerAmount;
         this.maker = _fields.maker;
+        this.taker = _fields.taker;
         this.pool = _fields.pool;
         this.expiry = _fields.expiry;
         this.salt = _fields.salt;
@@ -142,15 +144,13 @@ export class LimitOrder extends OrderBase {
     );
 
     public takerTokenFeeAmount: BigNumber;
-    public taker: string;
     public sender: string;
     public feeRecipient: string;
 
     constructor(fields: Partial<LimitOrderFields> = {}) {
         const _fields = { ...LIMIT_ORDER_DEFAULT_VALUES, ...fields };
         super(_fields);
         this.takerTokenFeeAmount = _fields.takerTokenFeeAmount;
-        this.taker = _fields.taker;
         this.sender = _fields.sender;
         this.feeRecipient = _fields.feeRecipient;
     }
@@ -246,6 +246,7 @@ export class RfqOrder extends OrderBase {
                         'uint128 makerAmount',
                         'uint128 takerAmount',
                         'address maker',
+                        'address taker',
                         'address txOrigin',
                         'bytes32 pool',
                         'uint64 expiry',
@@ -272,6 +273,7 @@ export class RfqOrder extends OrderBase {
             makerAmount: this.makerAmount,
             takerAmount: this.takerAmount,
             maker: this.maker,
+            taker: this.taker,
             txOrigin: this.txOrigin,
             pool: this.pool,
             expiry: this.expiry,
@@ -291,6 +293,7 @@ export class RfqOrder extends OrderBase {
                 hexUtils.leftPad(this.makerAmount),
                 hexUtils.leftPad(this.takerAmount),
                 hexUtils.leftPad(this.maker),
+                hexUtils.leftPad(this.taker),
                 hexUtils.leftPad(this.txOrigin),
                 hexUtils.leftPad(this.pool),
                 hexUtils.leftPad(this.expiry),
@@ -309,6 +312,7 @@ export class RfqOrder extends OrderBase {
                     { type: 'uint128', name: 'makerAmount' },
                     { type: 'uint128', name: 'takerAmount' },
                     { type: 'address', name: 'maker' },
+                    { type: 'address', name: 'taker' },
                     { type: 'address', name: 'txOrigin' },
                     { type: 'bytes32', name: 'pool' },
                     { type: 'uint64', name: 'expiry' },
@@ -323,6 +327,7 @@ export class RfqOrder extends OrderBase {
                 makerAmount: this.makerAmount.toString(10),
                 takerAmount: this.takerAmount.toString(10),
                 maker: this.maker,
+                taker: this.taker,
                 txOrigin: this.txOrigin,
                 pool: this.pool,
                 expiry: this.expiry.toString(10),

contracts/zero-ex/test/features/native_orders_feature_test.ts

@@ -140,7 +140,7 @@ blockchainTests.resets('NativeOrdersFeature', env => {
             ...opts,
         };
         await prepareBalancesForOrderAsync(order, _taker);
-        const _protocolFee = protocolFee || (order.taker !== NULL_ADDRESS ? 0 : SINGLE_PROTOCOL_FEE);
+        const _protocolFee = protocolFee === undefined ? SINGLE_PROTOCOL_FEE : protocolFee;
         return zeroEx
             .fillLimitOrder(order, await order.getSignatureWithProviderAsync(env.provider), new BigNumber(fillAmount))
             .awaitTransactionSuccessAsync({ from: _taker, value: _protocolFee });
@@ -973,17 +973,6 @@ blockchainTests.resets('NativeOrdersFeature', env => {
             return expect(tx).to.revertWith(new AnyRevertError());
         });
 
-        it('does not require a protocol fee if taker is supplied', async () => {
-            const order = getTestLimitOrder({ taker });
-            const receipt = await fillLimitOrderAsync(order);
-            verifyEventsFromLogs(
-                receipt.logs,
-                [createLimitOrderFilledEventArgs(order)],
-                IZeroExEvents.LimitOrderFilled,
-            );
-            await assertExpectedFinalBalancesFromLimitOrderFillAsync(order, { receipt });
-        });
-
         it('refunds excess protocol fee', async () => {
             const order = getTestLimitOrder();
             const receipt = await fillLimitOrderAsync(order, { protocolFee: SINGLE_PROTOCOL_FEE.plus(1) });
@@ -1162,6 +1151,14 @@ blockchainTests.resets('NativeOrdersFeature', env => {
             );
         });
 
+        it('non-taker cannot fill order', async () => {
+            const order = getTestRfqOrder({ taker });
+            const tx = fillRfqOrderAsync(order, order.takerAmount, notTaker);
+            return expect(tx).to.revertWith(
+                new RevertErrors.OrderNotFillableByTakerError(order.getHash(), notTaker, order.taker),
+            );
+        });
+
         it('cannot fill an expired order', async () => {
             const order = getTestRfqOrder({ expiry: createExpiry(-60) });
             const tx = fillRfqOrderAsync(order);

docs/basics/orders.rst

@@ -282,7 +282,7 @@ RFQ orders are a stripped down version of standard limit orders, supporting fewe
 
 Some notable differences from regular limit orders are:
 
-* The only fill restrictions that can be placed on an RFQ order is on the ``tx.origin`` of the transaction.
+* The only fill restrictions that can be placed on an RFQ order is on the ``tx.origin`` and ``taker`` of the transaction.
 * There are no taker token fees.
 
 Structure
@@ -303,6 +303,8 @@ The ``RFQOrder`` struct has the following fields:
 +-----------------+-------------+-----------------------------------------------------------------------------+
 | ``maker``       | ``address`` | The address of the maker, and signer, of this order.                        |
 +-----------------+-------------+-----------------------------------------------------------------------------+
+| ``taker``       | ``address`` | Allowed taker address. Set to zero to allow any taker.                      |
++-----------------+-------------+-----------------------------------------------------------------------------+
 | ``txOrigin``    | ``address`` | The allowed address of the EOA that submitted the Ethereum transaction.     |
 +-----------------+-------------+-----------------------------------------------------------------------------+
 | ``pool``        | ``bytes32`` | The staking pool to attribute the 0x protocol fee from this order.          |
@@ -348,6 +350,7 @@ The hash of the order is used to uniquely identify an order inside the protocol.
                 'uint128 makerAmount,',
                 'uint128 takerAmount,',
                 'address maker,'
+                'address taker,'
                 'address txOrigin,'
                 'bytes32 pool,',
                 'uint64 expiry,',
@@ -359,6 +362,7 @@ The hash of the order is used to uniquely identify an order inside the protocol.
             order.makerAmount,
             order.takerAmount,
             order.maker,
+            order.taker,
             order.txOrigin,
             order.pool,
             order.expiry,