From 3d95817dbe1b0d59fb38e27c2a651e20eb02bbd0 Mon Sep 17 00:00:00 2001 From: James Towle Date: Thu, 11 Jul 2019 15:16:20 -0500 Subject: [PATCH] `@0x:contracts-exchange` Updated MixinAssetProxyDispatcher and MixinExchangeCore to use library RichErrors --- .../contracts/src/LibExchangeRichErrors.sol | 388 ++++++++++++++++++ .../src/MixinAssetProxyDispatcher.sol | 20 +- .../contracts/src/MixinExchangeCore.sol | 41 +- contracts/exchange/test/wrapper.ts | 2 +- 4 files changed, 427 insertions(+), 24 deletions(-) create mode 100644 contracts/exchange/contracts/src/LibExchangeRichErrors.sol diff --git a/contracts/exchange/contracts/src/LibExchangeRichErrors.sol b/contracts/exchange/contracts/src/LibExchangeRichErrors.sol new file mode 100644 index 0000000000..1fdc68384a --- /dev/null +++ b/contracts/exchange/contracts/src/LibExchangeRichErrors.sol @@ -0,0 +1,388 @@ +/* + + Copyright 2019 ZeroEx Intl. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + +*/ + +pragma solidity ^0.5.9; + +import "@0x/contracts-utils/contracts/src/LibRichErrors.sol"; +import "@0x/contracts-exchange-libs/contracts/src/LibOrder.sol"; +import "./interfaces/IExchangeRichErrors.sol"; + + +library LibExchangeRichErrors { + + // bytes4(keccak256("SignatureError(uint8,bytes32,address,bytes)")) + bytes4 internal constant SIGNATURE_ERROR_SELECTOR = + 0x7e5a2318; + + // bytes4(keccak256("SignatureValidatorNotApprovedError(address,address)")) + bytes4 internal constant SIGNATURE_VALIDATOR_NOT_APPROVED_ERROR_SELECTOR = + 0xa15c0d06; + + // bytes4(keccak256("SignatureValidatorError(bytes32,address,address,bytes,bytes)")) + bytes4 internal constant SIGNATURE_VALIDATOR_ERROR_SELECTOR = + 0xa23838b8; + + // bytes4(keccak256("SignatureWalletError(bytes32,address,bytes,bytes)")) + bytes4 internal constant SIGNATURE_WALLET_ERROR_SELECTOR = + 0x1b8388f7; + + // bytes4(keccak256("OrderStatusError(bytes32,uint8)")) + bytes4 internal constant ORDER_STATUS_ERROR_SELECTOR = + 0xfdb6ca8d; + + // bytes4(keccak256("InvalidSenderError(bytes32,address)")) + bytes4 internal constant INVALID_SENDER_ERROR_SELECTOR = + 0x95b59997; + + // bytes4(keccak256("InvalidMakerError(bytes32,address)")) + bytes4 internal constant INVALID_MAKER_ERROR_SELECTOR = + 0x26bf55d9; + + // bytes4(keccak256("FillError(uint8,bytes32)")) + bytes4 internal constant FILL_ERROR_SELECTOR = + 0xe94a7ed0; + + // bytes4(keccak256("InvalidTakerError(bytes32,address)")) + bytes4 internal constant INVALID_TAKER_ERROR_SELECTOR = + 0xfdb328be; + + // bytes4(keccak256("OrderEpochError(address,address,uint256)")) + bytes4 internal constant ORDER_EPOCH_ERROR_SELECTOR = + 0x4ad31275; + + // bytes4(keccak256("AssetProxyExistsError(address)")) + bytes4 internal constant ASSET_PROXY_EXISTS_ERROR_SELECTOR = + 0xcc8b3b53; + + // bytes4(keccak256("AssetProxyDispatchError(uint8,bytes32,bytes)")) + bytes4 internal constant ASSET_PROXY_DISPATCH_ERROR_SELECTOR = + 0x488219a6; + + // bytes4(keccak256("AssetProxyTransferError(bytes32,bytes,bytes)")) + bytes4 internal constant ASSET_PROXY_TRANSFER_ERROR_SELECTOR = + 0x4678472b; + + // bytes4(keccak256("NegativeSpreadError(bytes32,bytes32)")) + bytes4 internal constant NEGATIVE_SPREAD_ERROR_SELECTOR = + 0xb6555d6f; + + // bytes4(keccak256("TransactionError(uint8,bytes32)")) + bytes4 internal constant TRANSACTION_ERROR_SELECTOR = + 0xf5985184; + + // bytes4(keccak256("TransactionSignatureError(bytes32,address,bytes)")) + bytes4 internal constant TRANSACTION_SIGNATURE_ERROR_SELECTOR = + 0xbfd56ef6; + + // bytes4(keccak256("TransactionExecutionError(bytes32,bytes)")) + bytes4 internal constant TRANSACTION_EXECUTION_ERROR_SELECTOR = + 0x20d11f61; + + // bytes4(keccak256("IncompleteFillError(bytes32)")) + bytes4 internal constant INCOMPLETE_FILL_ERROR_SELECTOR = + 0x152aa60e; + + // solhint-disable func-name-mixedcase + function SignatureError( + IExchangeRichErrors.SignatureErrorCodes errorCode, + bytes32 hash, + address signerAddress, + bytes memory signature + ) + internal + pure + returns (bytes memory) + { + return abi.encodeWithSelector( + SIGNATURE_ERROR_SELECTOR, + errorCode, + hash, + signerAddress, + signature + ); + } + + function SignatureValidatorNotApprovedError( + address signerAddress, + address validatorAddress + ) + internal + pure + returns (bytes memory) + { + return abi.encodeWithSelector( + SIGNATURE_VALIDATOR_NOT_APPROVED_ERROR_SELECTOR, + signerAddress, + validatorAddress + ); + } + + function SignatureValidatorError( + bytes32 hash, + address signerAddress, + address validatorAddress, + bytes memory signature, + bytes memory errorData + ) + internal + pure + returns (bytes memory) + { + return abi.encodeWithSelector( + SIGNATURE_VALIDATOR_ERROR_SELECTOR, + hash, + signerAddress, + validatorAddress, + signature, + errorData + ); + } + + function SignatureWalletError( + bytes32 hash, + address signerAddress, + bytes memory signature, + bytes memory errorData + ) + internal + pure + returns (bytes memory) + { + return abi.encodeWithSelector( + SIGNATURE_WALLET_ERROR_SELECTOR, + hash, + signerAddress, + signature, + errorData + ); + } + + function OrderStatusError( + bytes32 orderHash, + LibOrder.OrderStatus orderStatus + ) + internal + pure + returns (bytes memory) + { + return abi.encodeWithSelector( + ORDER_STATUS_ERROR_SELECTOR, + orderHash, + orderStatus + ); + } + + function InvalidSenderError( + bytes32 orderHash, + address senderAddress + ) + internal + pure + returns (bytes memory) + { + return abi.encodeWithSelector( + INVALID_SENDER_ERROR_SELECTOR, + orderHash, + senderAddress + ); + } + + function InvalidMakerError( + bytes32 orderHash, + address makerAddress + ) + internal + pure + returns (bytes memory) + { + return abi.encodeWithSelector( + INVALID_MAKER_ERROR_SELECTOR, + orderHash, + makerAddress + ); + } + + function FillError( + IExchangeRichErrors.FillErrorCodes errorCode, + bytes32 orderHash + ) + internal + pure + returns (bytes memory) + { + return abi.encodeWithSelector( + FILL_ERROR_SELECTOR, + errorCode, + orderHash + ); + } + + function InvalidTakerError( + bytes32 orderHash, + address takerAddress + ) + internal + pure + returns (bytes memory) + { + return abi.encodeWithSelector( + INVALID_TAKER_ERROR_SELECTOR, + orderHash, + takerAddress + ); + } + + function OrderEpochError( + address makerAddress, + address orderSenderAddress, + uint256 currentEpoch + ) + internal + pure + returns (bytes memory) + { + return abi.encodeWithSelector( + ORDER_EPOCH_ERROR_SELECTOR, + makerAddress, + orderSenderAddress, + currentEpoch + ); + } + + function AssetProxyExistsError( + address proxyAddress + ) + internal + pure + returns (bytes memory) + { + return abi.encodeWithSelector( + ASSET_PROXY_EXISTS_ERROR_SELECTOR, + proxyAddress + ); + } + + function AssetProxyDispatchError( + IExchangeRichErrors.AssetProxyDispatchErrorCodes errorCode, + bytes32 orderHash, + bytes memory assetData + ) + internal + pure + returns (bytes memory) + { + return abi.encodeWithSelector( + ASSET_PROXY_DISPATCH_ERROR_SELECTOR, + errorCode, + orderHash, + assetData + ); + } + + function AssetProxyTransferError( + bytes32 orderHash, + bytes memory assetData, + bytes memory errorData + ) + internal + pure + returns (bytes memory) + { + return abi.encodeWithSelector( + ASSET_PROXY_TRANSFER_ERROR_SELECTOR, + orderHash, + assetData, + errorData + ); + } + + function NegativeSpreadError( + bytes32 leftOrderHash, + bytes32 rightOrderHash + ) + internal + pure + returns (bytes memory) + { + return abi.encodeWithSelector( + NEGATIVE_SPREAD_ERROR_SELECTOR, + leftOrderHash, + rightOrderHash + ); + } + + function TransactionError( + IExchangeRichErrors.TransactionErrorCodes errorCode, + bytes32 transactionHash + ) + internal + pure + returns (bytes memory) + { + return abi.encodeWithSelector( + TRANSACTION_ERROR_SELECTOR, + errorCode, + transactionHash + ); + } + + function TransactionSignatureError( + bytes32 transactionHash, + address signerAddress, + bytes memory signature + ) + internal + pure + returns (bytes memory) + { + return abi.encodeWithSelector( + TRANSACTION_SIGNATURE_ERROR_SELECTOR, + transactionHash, + signerAddress, + signature + ); + } + + function TransactionExecutionError( + bytes32 transactionHash, + bytes memory errorData + ) + internal + pure + returns (bytes memory) + { + return abi.encodeWithSelector( + TRANSACTION_EXECUTION_ERROR_SELECTOR, + transactionHash, + errorData + ); + } + + function IncompleteFillError( + bytes32 orderHash + ) + internal + pure + returns (bytes memory) + { + return abi.encodeWithSelector( + INCOMPLETE_FILL_ERROR_SELECTOR, + orderHash + ); + } +} diff --git a/contracts/exchange/contracts/src/MixinAssetProxyDispatcher.sol b/contracts/exchange/contracts/src/MixinAssetProxyDispatcher.sol index 9240ffea92..4a022f7fc0 100644 --- a/contracts/exchange/contracts/src/MixinAssetProxyDispatcher.sol +++ b/contracts/exchange/contracts/src/MixinAssetProxyDispatcher.sol @@ -19,14 +19,14 @@ pragma solidity ^0.5.9; import "@0x/contracts-utils/contracts/src/Ownable.sol"; -import "@0x/contracts-utils/contracts/src/RichErrors.sol"; +import "@0x/contracts-utils/contracts/src/LibRichErrors.sol"; import "./interfaces/IAssetProxy.sol"; import "./interfaces/IAssetProxyDispatcher.sol"; -import "./MixinExchangeRichErrors.sol"; +import "./interfaces/IExchangeRichErrors.sol"; +import "./LibExchangeRichErrors.sol"; contract MixinAssetProxyDispatcher is - MixinExchangeRichErrors, Ownable, IAssetProxyDispatcher { @@ -44,7 +44,7 @@ contract MixinAssetProxyDispatcher is bytes4 assetProxyId = IAssetProxy(assetProxy).getProxyId(); address currentAssetProxy = assetProxies[assetProxyId]; if (currentAssetProxy != address(0)) { - _rrevert(AssetProxyExistsError(currentAssetProxy)); + LibRichErrors._rrevert(LibExchangeRichErrors.AssetProxyExistsError(currentAssetProxy)); } // Add asset proxy and log registration. @@ -85,8 +85,8 @@ contract MixinAssetProxyDispatcher is if (amount > 0 && from != to) { // Ensure assetData length is valid if (assetData.length <= 3) { - _rrevert(AssetProxyDispatchError( - AssetProxyDispatchErrorCodes.INVALID_ASSET_DATA_LENGTH, + LibRichErrors._rrevert(LibExchangeRichErrors.AssetProxyDispatchError( + IExchangeRichErrors.AssetProxyDispatchErrorCodes.INVALID_ASSET_DATA_LENGTH, orderHash, assetData )); @@ -104,8 +104,8 @@ contract MixinAssetProxyDispatcher is // Ensure that assetProxy exists if (assetProxy == address(0)) { - _rrevert(AssetProxyDispatchError( - AssetProxyDispatchErrorCodes.UNKNOWN_ASSET_PROXY, + LibRichErrors._rrevert(LibExchangeRichErrors.AssetProxyDispatchError( + IExchangeRichErrors.AssetProxyDispatchErrorCodes.UNKNOWN_ASSET_PROXY, orderHash, assetData )); @@ -185,7 +185,7 @@ contract MixinAssetProxyDispatcher is // at `cdStart`. // The first 32 bytes are the length of the data. mstore(cdStart, revertDataSize) - // Copy the revert data immediately after the length. + // Copy the revert data immediately after the length. returndatacopy(add(cdStart, 32), 0, revertDataSize) // We need to move the free memory pointer because we // still have solidity logic that executes after this assembly. @@ -195,7 +195,7 @@ contract MixinAssetProxyDispatcher is } if (!didSucceed) { - _rrevert(AssetProxyTransferError( + LibRichErrors._rrevert(LibExchangeRichErrors.AssetProxyTransferError( orderHash, assetData, revertData diff --git a/contracts/exchange/contracts/src/MixinExchangeCore.sol b/contracts/exchange/contracts/src/MixinExchangeCore.sol index 3b68a65dcf..15a9514fa5 100644 --- a/contracts/exchange/contracts/src/MixinExchangeCore.sol +++ b/contracts/exchange/contracts/src/MixinExchangeCore.sol @@ -19,11 +19,14 @@ pragma solidity ^0.5.9; pragma experimental ABIEncoderV2; import "@0x/contracts-utils/contracts/src/LibBytes.sol"; +import "@0x/contracts-utils/contracts/src/LibRichErrors.sol"; import "@0x/contracts-exchange-libs/contracts/src/LibExchangeSelectors.sol"; import "@0x/contracts-exchange-libs/contracts/src/LibFillResults.sol"; import "@0x/contracts-exchange-libs/contracts/src/LibMath.sol"; import "@0x/contracts-exchange-libs/contracts/src/LibOrder.sol"; import "./interfaces/IExchangeCore.sol"; +import "./interfaces/IExchangeRichErrors.sol"; +import "./LibExchangeRichErrors.sol"; import "./MixinAssetProxyDispatcher.sol"; import "./MixinSignatureValidator.sol"; @@ -66,7 +69,7 @@ contract MixinExchangeCore is // Ensure orderEpoch is monotonically increasing if (newOrderEpoch <= oldOrderEpoch) { - _rrevert(OrderEpochError(makerAddress, orderSenderAddress, oldOrderEpoch)); + LibRichErrors._rrevert(LibExchangeRichErrors.OrderEpochError(makerAddress, orderSenderAddress, oldOrderEpoch)); } // Update orderEpoch @@ -328,7 +331,7 @@ contract MixinExchangeCore is { // An order can only be filled if its status is FILLABLE. if (orderInfo.orderStatus != uint8(OrderStatus.FILLABLE)) { - _rrevert(OrderStatusError( + LibRichErrors._rrevert(LibExchangeRichErrors.OrderStatusError( orderInfo.orderHash, OrderStatus(orderInfo.orderStatus) )); @@ -337,14 +340,14 @@ contract MixinExchangeCore is // Validate sender is allowed to fill this order if (order.senderAddress != address(0)) { if (order.senderAddress != msg.sender) { - _rrevert(InvalidSenderError(orderInfo.orderHash, msg.sender)); + LibRichErrors._rrevert(LibExchangeRichErrors.InvalidSenderError(orderInfo.orderHash, msg.sender)); } } // Validate taker is allowed to fill this order if (order.takerAddress != address(0)) { if (order.takerAddress != takerAddress) { - _rrevert(InvalidTakerError(orderInfo.orderHash, takerAddress)); + LibRichErrors._rrevert(LibExchangeRichErrors.InvalidTakerError(orderInfo.orderHash, takerAddress)); } } @@ -362,8 +365,8 @@ contract MixinExchangeCore is orderInfo.orderHash, makerAddress, signature)) { - _rrevert(SignatureError( - SignatureErrorCodes.BAD_SIGNATURE, + LibRichErrors._rrevert(LibExchangeRichErrors.SignatureError( + IExchangeRichErrors.SignatureErrorCodes.BAD_SIGNATURE, orderInfo.orderHash, makerAddress, signature @@ -391,14 +394,20 @@ contract MixinExchangeCore is // Revert if fill amount is invalid // TODO: reconsider necessity for v2.1 if (takerAssetFillAmount == 0) { - _rrevert(FillError(FillErrorCodes.INVALID_TAKER_AMOUNT, orderInfo.orderHash)); + LibRichErrors._rrevert(LibExchangeRichErrors.FillError( + IExchangeRichErrors.FillErrorCodes.INVALID_TAKER_AMOUNT, + orderInfo.orderHash + )); } // Make sure taker does not pay more than desired amount // NOTE: This assertion should never fail, it is here // as an extra defence against potential bugs. if (takerAssetFilledAmount > takerAssetFillAmount) { - _rrevert(FillError(FillErrorCodes.TAKER_OVERPAY, orderInfo.orderHash)); + LibRichErrors._rrevert(LibExchangeRichErrors.FillError( + IExchangeRichErrors.FillErrorCodes.TAKER_OVERPAY, + orderInfo.orderHash + )); } // Make sure order is not overfilled @@ -406,7 +415,10 @@ contract MixinExchangeCore is // as an extra defence against potential bugs. if (_safeAdd(orderInfo.orderTakerAssetFilledAmount, takerAssetFilledAmount) > order.takerAssetAmount) { - _rrevert(FillError(FillErrorCodes.OVERFILL, orderInfo.orderHash)); + LibRichErrors._rrevert(LibExchangeRichErrors.FillError( + IExchangeRichErrors.FillErrorCodes.OVERFILL, + orderInfo.orderHash + )); } // Make sure order is filled at acceptable price. @@ -428,7 +440,10 @@ contract MixinExchangeCore is // as an extra defence against potential bugs. if (_safeMul(makerAssetFilledAmount, order.takerAssetAmount) > _safeMul(order.makerAssetAmount, takerAssetFilledAmount)) { - _rrevert(FillError(FillErrorCodes.INVALID_FILL_PRICE, orderInfo.orderHash)); + LibRichErrors._rrevert(LibExchangeRichErrors.FillError( + IExchangeRichErrors.FillErrorCodes.INVALID_FILL_PRICE, + orderInfo.orderHash + )); } } @@ -445,7 +460,7 @@ contract MixinExchangeCore is // Ensure order is valid // An order can only be cancelled if its status is FILLABLE. if (orderInfo.orderStatus != uint8(OrderStatus.FILLABLE)) { - _rrevert(OrderStatusError( + LibRichErrors._rrevert(LibExchangeRichErrors.OrderStatusError( orderInfo.orderHash, OrderStatus(orderInfo.orderStatus) )); @@ -454,14 +469,14 @@ contract MixinExchangeCore is // Validate sender is allowed to cancel this order if (order.senderAddress != address(0)) { if (order.senderAddress != msg.sender) { - _rrevert(InvalidSenderError(orderInfo.orderHash, msg.sender)); + LibRichErrors._rrevert(LibExchangeRichErrors.InvalidSenderError(orderInfo.orderHash, msg.sender)); } } // Validate transaction signed by maker address makerAddress = _getCurrentContextAddress(); if (order.makerAddress != makerAddress) { - _rrevert(InvalidMakerError(orderInfo.orderHash, makerAddress)); + LibRichErrors._rrevert(LibExchangeRichErrors.InvalidMakerError(orderInfo.orderHash, makerAddress)); } } diff --git a/contracts/exchange/test/wrapper.ts b/contracts/exchange/test/wrapper.ts index 4c9e5f4556..be30975aef 100644 --- a/contracts/exchange/test/wrapper.ts +++ b/contracts/exchange/test/wrapper.ts @@ -36,7 +36,7 @@ const expect = chai.expect; const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); // tslint:disable:no-unnecessary-type-assertion -describe.only('Exchange wrappers', () => { +describe('Exchange wrappers', () => { let chainId: number; let makerAddress: string; let owner: string;