EP: Swallow reverts in batchGetLimit/RfqRelevantStates() (#117)

* `@0x/contracts-zero-ex`: Swallow reverts in `batchGetLimit/RfqRelevantStates()`.

* `@0x/contracts-zero-ex`: Fix typos

* `@0x/contracts-zero-ex`: Fix misleading RFQ typehash comment in `LibNativeOrder.sol`

Co-authored-by: Lawrence Forman <me@merklejerk.com>

contracts/zero-ex/CHANGELOG.json

@@ -1,4 +1,13 @@
 [
+    {
+        "version": "0.18.1",
+        "changes": [
+            {
+                "note": "Swallow reverts in `batchGetLimitOrderRelevantStates()` and `batchGetRfqOrderRelevantStates()`",
+                "pr": 117
+            }
+        ]
+    },
     {
         "version": "0.18.0",
         "changes": [

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

@@ -387,7 +387,9 @@ interface INativeOrdersFeature {
             bool isSignatureValid
         );
 
-    /// @dev Batch version of `getLimitOrderRelevantState()`.
+    /// @dev Batch version of `getLimitOrderRelevantState()`, without reverting.
+    ///      Orders that would normally cause `getLimitOrderRelevantState()`
+    ///      to revert will have empty results.
     /// @param orders The limit orders.
     /// @param signatures The order signatures.
     /// @return orderInfos Info about the orders.
@@ -406,7 +408,9 @@ interface INativeOrdersFeature {
             bool[] memory isSignatureValids
         );
 
-    /// @dev Batch version of `getRfqOrderRelevantState()`.
+    /// @dev Batch version of `getRfqOrderRelevantState()`, without reverting.
+    ///      Orders that would normally cause `getRfqOrderRelevantState()`
+    ///      to revert will have empty results.
     /// @param orders The RFQ orders.
     /// @param signatures The order signatures.
     /// @return orderInfos Info about the orders.

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

@@ -111,7 +111,7 @@ contract NativeOrdersFeature is
     /// @dev Name of this feature.
     string public constant override FEATURE_NAME = "LimitOrders";
     /// @dev Version of this feature.
-    uint256 public immutable override FEATURE_VERSION = _encodeVersion(1, 0, 0);
+    uint256 public immutable override FEATURE_VERSION = _encodeVersion(1, 0, 1);
     /// @dev Highest bit of a uint256, used to flag cancelled orders.
     uint256 private constant HIGH_BIT = 1 << 255;
 
@@ -772,7 +772,9 @@ contract NativeOrdersFeature is
             LibSignature.getSignerOfHash(orderInfo.orderHash, signature);
     }
 
-    /// @dev Batch version of `getLimitOrderRelevantState()`.
+    /// @dev Batch version of `getLimitOrderRelevantState()`, without reverting.
+    ///      Orders that would normally cause `getLimitOrderRelevantState()`
+    ///      to revert will have empty results.
     /// @param orders The limit orders.
     /// @param signatures The order signatures.
     /// @return orderInfos Info about the orders.
@@ -800,15 +802,25 @@ contract NativeOrdersFeature is
         actualFillableTakerTokenAmounts = new uint128[](orders.length);
         isSignatureValids = new bool[](orders.length);
         for (uint256 i = 0; i < orders.length; ++i) {
-            (
+            try
-                orderInfos[i],
+                this.getLimitOrderRelevantState(orders[i], signatures[i])
-                actualFillableTakerTokenAmounts[i],
+                    returns (
-                isSignatureValids[i]
+                        LibNativeOrder.OrderInfo memory orderInfo,
-            ) = getLimitOrderRelevantState(orders[i], signatures[i]);
+                        uint128 actualFillableTakerTokenAmount,
+                        bool isSignatureValid
+                    )
+            {
+                orderInfos[i] = orderInfo;
+                actualFillableTakerTokenAmounts[i] = actualFillableTakerTokenAmount;
+                isSignatureValids[i] = isSignatureValid;
+            }
+            catch {}
         }
     }
 
-    /// @dev Batch version of `getRfqOrderRelevantState()`.
+    /// @dev Batch version of `getRfqOrderRelevantState()`, without reverting.
+    ///      Orders that would normally cause `getRfqOrderRelevantState()`
+    ///      to revert will have empty results.
     /// @param orders The RFQ orders.
     /// @param signatures The order signatures.
     /// @return orderInfos Info about the orders.
@@ -836,11 +848,19 @@ contract NativeOrdersFeature is
         actualFillableTakerTokenAmounts = new uint128[](orders.length);
         isSignatureValids = new bool[](orders.length);
         for (uint256 i = 0; i < orders.length; ++i) {
-            (
+            try
-                orderInfos[i],
+                this.getRfqOrderRelevantState(orders[i], signatures[i])
-                actualFillableTakerTokenAmounts[i],
+                    returns (
-                isSignatureValids[i]
+                        LibNativeOrder.OrderInfo memory orderInfo,
-            ) = getRfqOrderRelevantState(orders[i], signatures[i]);
+                        uint128 actualFillableTakerTokenAmount,
+                        bool isSignatureValid
+                    )
+            {
+                orderInfos[i] = orderInfo;
+                actualFillableTakerTokenAmounts[i] = actualFillableTakerTokenAmount;
+                isSignatureValids[i] = isSignatureValid;
+            }
+            catch {}
         }
     }
 

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

@@ -103,8 +103,8 @@ library LibNativeOrder {
     //       "uint128 makerAmount,",
     //       "uint128 takerAmount,",
     //       "address maker,",
-    //       "address txOrigin,",
     //       "address taker,",
+    //       "address txOrigin,",
     //       "bytes32 pool,",
     //       "uint64 expiry,",
     //       "uint256 salt"

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

@@ -1,4 +1,11 @@
-import { blockchainTests, constants, describe, expect, verifyEventsFromLogs } from '@0x/contracts-test-utils';
+import {
+    blockchainTests,
+    constants,
+    describe,
+    expect,
+    randomAddress,
+    verifyEventsFromLogs,
+} from '@0x/contracts-test-utils';
 import {
     LimitOrder,
     LimitOrderFields,
@@ -18,7 +25,7 @@ import { getRandomLimitOrder, getRandomRfqOrder } from '../utils/orders';
 import { TestMintableERC20TokenContract, TestRfqOriginRegistrationContract } from '../wrappers';
 
 blockchainTests.resets('NativeOrdersFeature', env => {
-    const { NULL_ADDRESS, MAX_UINT256, ZERO_AMOUNT } = constants;
+    const { NULL_ADDRESS, MAX_UINT256, NULL_BYTES32, ZERO_AMOUNT } = constants;
     const GAS_PRICE = new BigNumber('123e9');
     const PROTOCOL_FEE_MULTIPLIER = 1337e3;
     const SINGLE_PROTOCOL_FEE = GAS_PRICE.times(PROTOCOL_FEE_MULTIPLIER);
@@ -1668,6 +1675,30 @@ blockchainTests.resets('NativeOrdersFeature', env => {
                 expect(isSignatureValids[i]).to.eq(true);
             }
         });
+        it('swallows reverts', async () => {
+            const orders = new Array(3).fill(0).map(() => getTestLimitOrder());
+            // The second order will revert because its maker token is not valid.
+            orders[1].makerToken = randomAddress();
+            await batchFundOrderMakerAsync(orders);
+            const [orderInfos, fillableTakerAmounts, isSignatureValids] = await zeroEx
+                .batchGetLimitOrderRelevantStates(
+                    orders,
+                    await Promise.all(orders.map(async o => o.getSignatureWithProviderAsync(env.provider))),
+                )
+                .callAsync();
+            expect(orderInfos).to.be.length(orders.length);
+            expect(fillableTakerAmounts).to.be.length(orders.length);
+            expect(isSignatureValids).to.be.length(orders.length);
+            for (let i = 0; i < orders.length; ++i) {
+                expect(orderInfos[i]).to.deep.eq({
+                    orderHash: i === 1 ? NULL_BYTES32 : orders[i].getHash(),
+                    status: i === 1 ? OrderStatus.Invalid : OrderStatus.Fillable,
+                    takerTokenFilledAmount: ZERO_AMOUNT,
+                });
+                expect(fillableTakerAmounts[i]).to.bignumber.eq(i === 1 ? ZERO_AMOUNT : orders[i].takerAmount);
+                expect(isSignatureValids[i]).to.eq(i !== 1);
+            }
+        });
     });
 
     describe('batchGetRfqOrderRelevantStates()', () => {