@0x/contracts-exchange: Switch to consolidated IncompleteFillError rich error.

`@0x/contracts-exchange`: Allow `marketSell/BuyOrdersNoThrow` to be destructive to orders again.

contracts/exchange/contracts/src/MixinWrapperFunctions.sol

@@ -189,7 +189,6 @@ contract MixinWrapperFunctions is
             // The `takerAssetData` must be the same for each order.
             // Rather than checking equality, we point the `takerAssetData` of each order to the same memory location.
             // This is less expensive than checking equality.
-            bytes memory originalTakerAssetData = orders[i].takerAssetData;
             orders[i].takerAssetData = takerAssetData;
 
             // Attempt to sell the remaining amount of takerAsset
@@ -199,9 +198,6 @@ contract MixinWrapperFunctions is
                 signatures[i]
             );
 
-            // Restore the original `takerAssetData` so we're non-destructive.
-            orders[i].takerAssetData = originalTakerAssetData;
-
             // Update amounts filled and fees paid by maker and taker
             fillResults = LibFillResults.addFillResults(fillResults, singleFillResults);
 
@@ -245,7 +241,6 @@ contract MixinWrapperFunctions is
             // The `makerAssetData` must be the same for each order.
             // Rather than checking equality, we point the `makerAssetData` of each order to the same memory location.
             // This is less expensive than checking equality.
-            bytes memory originalMakerAssetData = orders[i].makerAssetData;
             orders[i].makerAssetData = makerAssetData;
 
             // Attempt to sell the remaining amount of takerAsset
@@ -255,9 +250,6 @@ contract MixinWrapperFunctions is
                 signatures[i]
             );
 
-            // Restore the original `makerAssetData` so we're non-destructive.
-            orders[i].makerAssetData = originalMakerAssetData;
-
             // Update amounts filled and fees paid by maker and taker
             fillResults = LibFillResults.addFillResults(fillResults, singleFillResults);
 
@@ -284,9 +276,10 @@ contract MixinWrapperFunctions is
     {
         fillResults = marketSellOrdersNoThrow(orders, takerAssetFillAmount, signatures);
         if (fillResults.takerAssetFilledAmount < takerAssetFillAmount) {
-            LibRichErrors.rrevert(LibExchangeRichErrors.IncompleteMarketSellError(
+            LibRichErrors.rrevert(LibExchangeRichErrors.IncompleteFillError(
+                LibExchangeRichErrors.IncompleteFillErrorCode.INCOMPLETE_MARKET_SELL_ORDERS,
                 takerAssetFillAmount,
-                _getOrderHashes(orders)
+                fillResults.takerAssetFilledAmount
             ));
         }
     }
@@ -306,9 +299,10 @@ contract MixinWrapperFunctions is
     {
         fillResults = marketBuyOrdersNoThrow(orders, makerAssetFillAmount, signatures);
         if (fillResults.makerAssetFilledAmount < makerAssetFillAmount) {
-            LibRichErrors.rrevert(LibExchangeRichErrors.IncompleteMarketBuyError(
+            LibRichErrors.rrevert(LibExchangeRichErrors.IncompleteFillError(
+                LibExchangeRichErrors.IncompleteFillErrorCode.INCOMPLETE_MARKET_BUY_ORDERS,
                 makerAssetFillAmount,
-                _getOrderHashes(orders)
+                fillResults.makerAssetFilledAmount
             ));
         }
     }
@@ -360,8 +354,9 @@ contract MixinWrapperFunctions is
         );
         if (fillResults.takerAssetFilledAmount != takerAssetFillAmount) {
             LibRichErrors.rrevert(LibExchangeRichErrors.IncompleteFillError(
+                LibExchangeRichErrors.IncompleteFillErrorCode.INCOMPLETE_FILL_ORDER,
                 takerAssetFillAmount,
-                getOrderInfo(order).orderHash
+                fillResults.takerAssetFilledAmount
             ));
         }
         return fillResults;

contracts/exchange/contracts/src/interfaces/IExchangeRichErrors.sol

@@ -1,39 +0,0 @@
-pragma solidity ^0.5.9;
-
-
-contract IExchangeRichErrors {
-
-    enum AssetProxyDispatchErrorCodes {
-        INVALID_ASSET_DATA_LENGTH,
-        UNKNOWN_ASSET_PROXY
-    }
-
-    enum BatchMatchOrdersErrorCodes {
-        ZERO_LEFT_ORDERS,
-        ZERO_RIGHT_ORDERS,
-        INVALID_LENGTH_LEFT_SIGNATURES,
-        INVALID_LENGTH_RIGHT_SIGNATURES
-    }
-
-    enum FillErrorCodes {
-        INVALID_TAKER_AMOUNT,
-        TAKER_OVERPAY,
-        OVERFILL,
-        INVALID_FILL_PRICE
-    }
-
-    enum SignatureErrorCodes {
-        BAD_SIGNATURE,
-        INVALID_LENGTH,
-        UNSUPPORTED,
-        ILLEGAL,
-        INAPPROPRIATE_SIGNATURE_TYPE,
-        INVALID_SIGNER
-    }
-
-    enum TransactionErrorCodes {
-        NO_REENTRANCY,
-        ALREADY_EXECUTED,
-        EXPIRED
-    }
-}

contracts/exchange/contracts/src/libs/LibExchangeRichErrorDecoder.sol

@@ -382,53 +382,18 @@ contract LibExchangeRichErrorDecoder {
         public
         pure
         returns (
-            uint256 fillAmount,
+            LibExchangeRichErrors.IncompleteFillErrorCode errorCode,
-            bytes32 orderHash
+            uint256 expectedAssetFillAmount,
+            uint256 actualAssetFillAmount
         )
     {
         _assertSelectorBytes(encoded, LibExchangeRichErrors.IncompleteFillErrorSelector());
-        (fillAmount, orderHash) = abi.decode(
+        uint8 _errorCode;
+        (_errorCode, expectedAssetFillAmount, actualAssetFillAmount) = abi.decode(
             encoded.sliceDestructive(4, encoded.length),
-            (uint256, bytes32)
+            (uint8, uint256, uint256)
-        );
-    }
-
-    /// @dev Decompose an ABI-encoded IncompleteMarketSellError.
-    /// @param encoded ABI-encoded revert error.
-    /// @return orderHash Hash of the order being filled.
-    function decodeIncompleteMarketSellError(bytes memory encoded)
-        public
-        pure
-        returns (
-            uint256 takerAssetFillAmount,
-            bytes32[] memory orderHashes
-        )
-    {
-        _assertSelectorBytes(encoded, LibExchangeRichErrors.IncompleteMarketSellErrorSelector());
-        // solhint-disable-next-line
-        (takerAssetFillAmount, orderHashes) = abi.decode(
-            encoded.sliceDestructive(4, encoded.length),
-            (uint256, bytes32[])
-        );
-    }
-
-    /// @dev Decompose an ABI-encoded IncompleteMarketBuyError.
-    /// @param encoded ABI-encoded revert error.
-    /// @return orderHash Hash of the order being filled.
-    function decodeIncompleteMarketBuyError(bytes memory encoded)
-        public
-        pure
-        returns (
-            uint256 makerAssetFillAmount,
-            bytes32[] memory orderHashes
-        )
-    {
-        _assertSelectorBytes(encoded, LibExchangeRichErrors.IncompleteMarketBuyErrorSelector());
-        // solhint-disable-next-line
-        (makerAssetFillAmount, orderHashes) = abi.decode(
-            encoded.sliceDestructive(4, encoded.length),
-            (uint256, bytes32[])
         );
+        errorCode = LibExchangeRichErrors.IncompleteFillErrorCode(_errorCode);
     }
 
     /// @dev Revert if the leading 4 bytes of `encoded` is not `selector`.

contracts/exchange/package.json

@@ -35,7 +35,7 @@
         "compile:truffle": "truffle compile"
     },
     "config": {
-        "abis": "./generated-artifacts/@(Exchange|ExchangeWrapper|IAssetProxy|IAssetProxyDispatcher|IEIP1271Wallet|IExchange|IExchangeCore|IExchangeRichErrors|IMatchOrders|ISignatureValidator|ITransactions|ITransferSimulator|IWallet|IWrapperFunctions|IsolatedExchange|LibExchangeRichErrorDecoder|MixinAssetProxyDispatcher|MixinExchangeCore|MixinMatchOrders|MixinSignatureValidator|MixinTransactions|MixinTransferSimulator|MixinWrapperFunctions|ReentrancyTester|TestAssetProxyDispatcher|TestExchangeInternals|TestLibExchangeRichErrorDecoder|TestSignatureValidator|TestValidatorWallet|TestWrapperFunctions|Whitelist).json",
+        "abis": "./generated-artifacts/@(Exchange|ExchangeWrapper|IAssetProxy|IAssetProxyDispatcher|IEIP1271Wallet|IExchange|IExchangeCore|IMatchOrders|ISignatureValidator|ITransactions|ITransferSimulator|IWallet|IWrapperFunctions|IsolatedExchange|LibExchangeRichErrorDecoder|MixinAssetProxyDispatcher|MixinExchangeCore|MixinMatchOrders|MixinSignatureValidator|MixinTransactions|MixinTransferSimulator|MixinWrapperFunctions|ReentrancyTester|TestAssetProxyDispatcher|TestExchangeInternals|TestLibExchangeRichErrorDecoder|TestSignatureValidator|TestValidatorWallet|TestWrapperFunctions|Whitelist).json",
         "abis:comment": "This list is auto-generated by contracts-gen. Don't edit manually."
     },
     "repository": {

contracts/exchange/src/artifacts.ts

@@ -12,7 +12,6 @@ import * as IAssetProxyDispatcher from '../generated-artifacts/IAssetProxyDispat
 import * as IEIP1271Wallet from '../generated-artifacts/IEIP1271Wallet.json';
 import * as IExchange from '../generated-artifacts/IExchange.json';
 import * as IExchangeCore from '../generated-artifacts/IExchangeCore.json';
-import * as IExchangeRichErrors from '../generated-artifacts/IExchangeRichErrors.json';
 import * as IMatchOrders from '../generated-artifacts/IMatchOrders.json';
 import * as ISignatureValidator from '../generated-artifacts/ISignatureValidator.json';
 import * as ITransactions from '../generated-artifacts/ITransactions.json';
@@ -52,7 +51,6 @@ export const artifacts = {
     IEIP1271Wallet: IEIP1271Wallet as ContractArtifact,
     IExchange: IExchange as ContractArtifact,
     IExchangeCore: IExchangeCore as ContractArtifact,
-    IExchangeRichErrors: IExchangeRichErrors as ContractArtifact,
     IMatchOrders: IMatchOrders as ContractArtifact,
     ISignatureValidator: ISignatureValidator as ContractArtifact,
     ITransactions: ITransactions as ContractArtifact,

contracts/exchange/src/wrappers.ts

@@ -10,7 +10,6 @@ export * from '../generated-wrappers/i_asset_proxy_dispatcher';
 export * from '../generated-wrappers/i_e_i_p1271_wallet';
 export * from '../generated-wrappers/i_exchange';
 export * from '../generated-wrappers/i_exchange_core';
-export * from '../generated-wrappers/i_exchange_rich_errors';
 export * from '../generated-wrappers/i_match_orders';
 export * from '../generated-wrappers/i_signature_validator';
 export * from '../generated-wrappers/i_transactions';

contracts/exchange/test/lib_exchange_rich_error_decoder.ts

@@ -146,20 +146,9 @@ blockchainTests.resets('LibExchangeRichErrorDecoder', ({ provider, txDefaults })
     })();
 
     (() => {
-        const orderHash = orderUtils.generatePseudoRandomOrderHash();
+        const errorCode = ExchangeRevertErrors.IncompleteFillErrorCode.IncompleteMarketSellOrders;
-        const amount = new BigNumber(hexRandom(WORD_LENGTH));
+        const expectedAmount = new BigNumber(hexRandom(WORD_LENGTH));
-        createDecodeTest(ExchangeRevertErrors.IncompleteFillError, [amount, orderHash]);
+        const actualAmount = new BigNumber(hexRandom(WORD_LENGTH));
-    })();
+        createDecodeTest(ExchangeRevertErrors.IncompleteFillError, [errorCode, expectedAmount, actualAmount]);
-
-    (() => {
-        const orderHashes = _.times(3, () => orderUtils.generatePseudoRandomOrderHash());
-        const amount = new BigNumber(hexRandom(WORD_LENGTH));
-        createDecodeTest(ExchangeRevertErrors.IncompleteMarketSellError, [amount, orderHashes]);
-    })();
-
-    (() => {
-        const orderHashes = _.times(3, () => orderUtils.generatePseudoRandomOrderHash());
-        const amount = new BigNumber(hexRandom(WORD_LENGTH));
-        createDecodeTest(ExchangeRevertErrors.IncompleteMarketBuyError, [amount, orderHashes]);
     })();
 });

contracts/exchange/test/wrapper.ts

@@ -197,8 +197,11 @@ blockchainTests.resets('Exchange wrappers', env => {
                 takerAssetFillAmount: signedOrder.takerAssetAmount.dividedToIntegerBy(2),
             });
 
-            const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
+            const expectedError = new ExchangeRevertErrors.IncompleteFillError(
-            const expectedError = new ExchangeRevertErrors.IncompleteFillError(takerAssetFillAmount, orderHashHex);
+                ExchangeRevertErrors.IncompleteFillErrorCode.IncompleteFillOrder,
+                takerAssetFillAmount,
+                takerAssetFillAmount.dividedToIntegerBy(2),
+            );
             const tx = exchangeWrapper.fillOrKillOrderAsync(signedOrder, takerAddress);
             return expect(tx).to.revertWith(expectedError);
         });

contracts/exchange/test/wrapper_unit_tests.ts

@@ -166,7 +166,11 @@ blockchainTests('Exchange wrapper functions unit tests.', env => {
                 takerAssetAmount: fillAmount.minus(1),
             });
             const signature = createOrderSignature(order);
-            const expectedError = new ExchangeRevertErrors.IncompleteFillError(fillAmount, getExpectedOrderHash(order));
+            const expectedError = new ExchangeRevertErrors.IncompleteFillError(
+                ExchangeRevertErrors.IncompleteFillErrorCode.IncompleteFillOrder,
+                fillAmount,
+                fillAmount.minus(1),
+            );
             const tx = testContract.fillOrKillOrder.awaitTransactionSuccessAsync(order, fillAmount, signature);
             return expect(tx).to.revertWith(expectedError);
         });
@@ -179,7 +183,11 @@ blockchainTests('Exchange wrapper functions unit tests.', env => {
                 takerAssetAmount: fillAmount.plus(1),
             });
             const signature = createOrderSignature(order);
-            const expectedError = new ExchangeRevertErrors.IncompleteFillError(fillAmount, getExpectedOrderHash(order));
+            const expectedError = new ExchangeRevertErrors.IncompleteFillError(
+                ExchangeRevertErrors.IncompleteFillErrorCode.IncompleteFillOrder,
+                fillAmount,
+                fillAmount.plus(1),
+            );
             const tx = testContract.fillOrKillOrder.awaitTransactionSuccessAsync(order, fillAmount, signature);
             return expect(tx).to.revertWith(expectedError);
         });
@@ -394,8 +402,9 @@ blockchainTests('Exchange wrapper functions unit tests.', env => {
             // the `takerAssetFilledAmount`.
             failingOrder.takerAssetAmount = failingOrder.takerAssetAmount.minus(1);
             const expectedError = new ExchangeRevertErrors.IncompleteFillError(
+                ExchangeRevertErrors.IncompleteFillErrorCode.IncompleteFillOrder,
                 failingAmount,
-                getExpectedOrderHash(failingOrder),
+                failingAmount.minus(1),
             );
             const tx = txHelper.getResultAndReceiptAsync(
                 testContract.batchFillOrKillOrders,
@@ -418,8 +427,9 @@ blockchainTests('Exchange wrapper functions unit tests.', env => {
             // the `takerAssetFilledAmount`.
             failingOrder.takerAssetAmount = failingOrder.takerAssetAmount.plus(1);
             const expectedError = new ExchangeRevertErrors.IncompleteFillError(
+                ExchangeRevertErrors.IncompleteFillErrorCode.IncompleteFillOrder,
                 failingAmount,
-                getExpectedOrderHash(failingOrder),
+                failingAmount.plus(1),
             );
             const tx = txHelper.getResultAndReceiptAsync(
                 testContract.batchFillOrKillOrders,
@@ -817,9 +827,10 @@ blockchainTests('Exchange wrapper functions unit tests.', env => {
                     (total, o) => o.takerAssetAmount.plus(total),
                     constants.ZERO_AMOUNT,
                 ).plus(1);
-                const expectedError = new ExchangeRevertErrors.IncompleteMarketSellError(
+                const expectedError = new ExchangeRevertErrors.IncompleteFillError(
+                    ExchangeRevertErrors.IncompleteFillErrorCode.IncompleteMarketSellOrders,
                     takerAssetFillAmount,
-                    orders.map(o => getExpectedOrderHash(o)),
+                    takerAssetFillAmount.minus(1),
                 );
                 const tx = txHelper.getResultAndReceiptAsync(
                     testContract.marketSellOrdersFillOrKill,
@@ -869,16 +880,16 @@ blockchainTests('Exchange wrapper functions unit tests.', env => {
                     order.salt = ALWAYS_FAILING_SALT;
                 }
                 const signatures = _.times(COUNT, i => createOrderSignature(orders[i]));
+                const badOrdersAmount = _.reduce(badOrders, (total, o) => o.takerAssetAmount.plus(total), constants.ZERO_AMOUNT);
                 const takerAssetFillAmount = _.reduce(
                     orders,
                     (total, o) => o.takerAssetAmount.plus(total),
                     constants.ZERO_AMOUNT,
-                )
+                ).minus(badOrdersAmount).plus(1);
-                    .minus(_.reduce(badOrders, (total, o) => o.takerAssetAmount.plus(total), constants.ZERO_AMOUNT))
+                const expectedError = new ExchangeRevertErrors.IncompleteFillError(
-                    .plus(1);
+                    ExchangeRevertErrors.IncompleteFillErrorCode.IncompleteMarketSellOrders,
-                const expectedError = new ExchangeRevertErrors.IncompleteMarketSellError(
                     takerAssetFillAmount,
-                    orders.map(o => getExpectedOrderHash(o)),
+                    takerAssetFillAmount.minus(1),
                 );
                 const tx = txHelper.getResultAndReceiptAsync(
                     testContract.marketSellOrdersFillOrKill,
@@ -1166,9 +1177,10 @@ blockchainTests('Exchange wrapper functions unit tests.', env => {
                     (total, o) => o.makerAssetAmount.plus(total),
                     constants.ZERO_AMOUNT,
                 ).plus(1);
-                const expectedError = new ExchangeRevertErrors.IncompleteMarketBuyError(
+                const expectedError = new ExchangeRevertErrors.IncompleteFillError(
+                    ExchangeRevertErrors.IncompleteFillErrorCode.IncompleteMarketBuyOrders,
                     makerAssetFillAmount,
-                    orders.map(o => getExpectedOrderHash(o)),
+                    makerAssetFillAmount.minus(1),
                 );
                 const tx = txHelper.getResultAndReceiptAsync(
                     testContract.marketBuyOrdersFillOrKill,
@@ -1218,16 +1230,16 @@ blockchainTests('Exchange wrapper functions unit tests.', env => {
                     order.salt = ALWAYS_FAILING_SALT;
                 }
                 const signatures = _.times(COUNT, i => createOrderSignature(orders[i]));
+                const badOrdersAmount = _.reduce(badOrders, (total, o) => o.makerAssetAmount.plus(total), constants.ZERO_AMOUNT);
                 const makerAssetFillAmount = _.reduce(
                     orders,
                     (total, o) => o.makerAssetAmount.plus(total),
                     constants.ZERO_AMOUNT,
-                )
+                ).minus(badOrdersAmount).plus(1);
-                    .minus(_.reduce(badOrders, (total, o) => o.makerAssetAmount.plus(total), constants.ZERO_AMOUNT))
+                const expectedError = new ExchangeRevertErrors.IncompleteFillError(
-                    .plus(1);
+                    ExchangeRevertErrors.IncompleteFillErrorCode.IncompleteMarketBuyOrders,
-                const expectedError = new ExchangeRevertErrors.IncompleteMarketBuyError(
                     makerAssetFillAmount,
-                    orders.map(o => getExpectedOrderHash(o)),
+                    makerAssetFillAmount.minus(1),
                 );
                 const tx = txHelper.getResultAndReceiptAsync(
                     testContract.marketBuyOrdersFillOrKill,

contracts/exchange/tsconfig.json

@@ -10,7 +10,6 @@
         "generated-artifacts/IEIP1271Wallet.json",
         "generated-artifacts/IExchange.json",
         "generated-artifacts/IExchangeCore.json",
-        "generated-artifacts/IExchangeRichErrors.json",
         "generated-artifacts/IMatchOrders.json",
         "generated-artifacts/ISignatureValidator.json",
         "generated-artifacts/ITransactions.json",