diff --git a/contracts/zero-ex/CHANGELOG.json b/contracts/zero-ex/CHANGELOG.json index 7f9e5050fb..c865df9968 100644 --- a/contracts/zero-ex/CHANGELOG.json +++ b/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": [ diff --git a/contracts/zero-ex/contracts/src/features/INativeOrdersFeature.sol b/contracts/zero-ex/contracts/src/features/INativeOrdersFeature.sol index daafc5d3de..a88a5ae0a5 100644 --- a/contracts/zero-ex/contracts/src/features/INativeOrdersFeature.sol +++ b/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. diff --git a/contracts/zero-ex/contracts/src/features/NativeOrdersFeature.sol b/contracts/zero-ex/contracts/src/features/NativeOrdersFeature.sol index e690752a9f..d4ad86bf47 100644 --- a/contracts/zero-ex/contracts/src/features/NativeOrdersFeature.sol +++ b/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) { - ( - orderInfos[i], - actualFillableTakerTokenAmounts[i], - isSignatureValids[i] - ) = getLimitOrderRelevantState(orders[i], signatures[i]); + try + this.getLimitOrderRelevantState(orders[i], signatures[i]) + returns ( + LibNativeOrder.OrderInfo memory orderInfo, + 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) { - ( - orderInfos[i], - actualFillableTakerTokenAmounts[i], - isSignatureValids[i] - ) = getRfqOrderRelevantState(orders[i], signatures[i]); + try + this.getRfqOrderRelevantState(orders[i], signatures[i]) + returns ( + LibNativeOrder.OrderInfo memory orderInfo, + uint128 actualFillableTakerTokenAmount, + bool isSignatureValid + ) + { + orderInfos[i] = orderInfo; + actualFillableTakerTokenAmounts[i] = actualFillableTakerTokenAmount; + isSignatureValids[i] = isSignatureValid; + } + catch {} } } diff --git a/contracts/zero-ex/contracts/src/features/libs/LibNativeOrder.sol b/contracts/zero-ex/contracts/src/features/libs/LibNativeOrder.sol index dc8f9bbcf9..df7115801c 100644 --- a/contracts/zero-ex/contracts/src/features/libs/LibNativeOrder.sol +++ b/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" diff --git a/contracts/zero-ex/test/features/native_orders_feature_test.ts b/contracts/zero-ex/test/features/native_orders_feature_test.ts index e5f198f3f7..5dbe7405c9 100644 --- a/contracts/zero-ex/test/features/native_orders_feature_test.ts +++ b/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()', () => {