From ae859fa01e35901caed2139a5d4a6082b22d9684 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Mon, 29 Jul 2019 21:58:24 -0500 Subject: [PATCH] Update Exchange contract to use libraries --- .../contracts/src/MixinExchangeCore.sol | 15 ++++++--- .../contracts/src/MixinMatchOrders.sol | 33 +++++++++---------- .../contracts/src/MixinSignatureValidator.sol | 8 +++-- .../contracts/src/MixinTransactions.sol | 6 +++- .../contracts/src/MixinWrapperFunctions.sol | 14 +++++--- 5 files changed, 46 insertions(+), 30 deletions(-) diff --git a/contracts/exchange/contracts/src/MixinExchangeCore.sol b/contracts/exchange/contracts/src/MixinExchangeCore.sol index 3aa777b79f..df49a75344 100644 --- a/contracts/exchange/contracts/src/MixinExchangeCore.sol +++ b/contracts/exchange/contracts/src/MixinExchangeCore.sol @@ -19,9 +19,11 @@ pragma solidity ^0.5.9; pragma experimental ABIEncoderV2; import "@0x/contracts-utils/contracts/src/LibRichErrors.sol"; +import "@0x/contracts-utils/contracts/src/LibSafeMath.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 "@0x/contracts-exchange-libs/contracts/src/LibEIP712ExchangeDomain.sol"; import "./interfaces/IExchangeCore.sol"; import "./interfaces/IExchangeRichErrors.sol"; import "./LibExchangeRichErrors.sol"; @@ -30,11 +32,14 @@ import "./MixinSignatureValidator.sol"; contract MixinExchangeCore is + LibEIP712ExchangeDomain, IExchangeCore, - LibMath, MixinAssetProxyDispatcher, MixinSignatureValidator { + using LibOrder for LibOrder.Order; + using LibSafeMath for uint256; + // Mapping of orderHash => amount of takerAsset already bought by maker mapping (bytes32 => uint256) public filled; @@ -121,7 +126,7 @@ contract MixinExchangeCore is returns (LibOrder.OrderInfo memory orderInfo) { // Compute the order hash - orderInfo.orderHash = LibOrder.getOrderHash(order); + orderInfo.orderHash = order.getOrderHash(EIP712_EXCHANGE_DOMAIN_HASH); // Fetch filled amount orderInfo.orderTakerAssetFilledAmount = filled[orderInfo.orderHash]; @@ -200,8 +205,8 @@ contract MixinExchangeCore is ); // Get amount of takerAsset to fill - uint256 remainingTakerAssetAmount = _safeSub(order.takerAssetAmount, orderInfo.orderTakerAssetFilledAmount); - uint256 takerAssetFilledAmount = _min256(takerAssetFillAmount, remainingTakerAssetAmount); + uint256 remainingTakerAssetAmount = order.takerAssetAmount.safeSub(orderInfo.orderTakerAssetFilledAmount); + uint256 takerAssetFilledAmount = LibSafeMath.min256(takerAssetFillAmount, remainingTakerAssetAmount); // Compute proportional fill amounts fillResults = LibFillResults.calculateFillResults(order, takerAssetFilledAmount); @@ -263,7 +268,7 @@ contract MixinExchangeCore is internal { // Update state - filled[orderHash] = _safeAdd(orderTakerAssetFilledAmount, fillResults.takerAssetFilledAmount); + filled[orderHash] = orderTakerAssetFilledAmount.safeAdd(fillResults.takerAssetFilledAmount); // Emit a Fill() event THE HARD WAY to avoid a stack overflow. // All this logic is equivalent to: diff --git a/contracts/exchange/contracts/src/MixinMatchOrders.sol b/contracts/exchange/contracts/src/MixinMatchOrders.sol index ec73b79fba..6d7f2ac02e 100644 --- a/contracts/exchange/contracts/src/MixinMatchOrders.sol +++ b/contracts/exchange/contracts/src/MixinMatchOrders.sol @@ -29,6 +29,7 @@ contract MixinMatchOrders is IMatchOrders { using LibBytes for bytes; + using LibSafeMath for uint256; /// @dev Match complementary orders that have a profitable spread. /// Each order is filled at their respective price point, and @@ -163,8 +164,8 @@ contract MixinMatchOrders is // AND // / >= / // These equations can be combined to get the following: - if (_safeMul(leftOrder.makerAssetAmount, rightOrder.makerAssetAmount) < - _safeMul(leftOrder.takerAssetAmount, rightOrder.takerAssetAmount)) { + if (leftOrder.makerAssetAmount.safeMul(rightOrder.makerAssetAmount) < + leftOrder.takerAssetAmount.safeMul(rightOrder.takerAssetAmount)) { LibRichErrors._rrevert(LibExchangeRichErrors.NegativeSpreadError( leftOrderInfo.orderHash, rightOrderInfo.orderHash @@ -245,33 +246,29 @@ contract MixinMatchOrders is ); // Update the orderInfo structs with the updated takerAssetFilledAmount - leftOrderInfo.orderTakerAssetFilledAmount = _safeAdd( - leftOrderInfo.orderTakerAssetFilledAmount, + leftOrderInfo.orderTakerAssetFilledAmount = leftOrderInfo.orderTakerAssetFilledAmount.safeAdd( matchResults.left.takerAssetFilledAmount ); - rightOrderInfo.orderTakerAssetFilledAmount = _safeAdd( - rightOrderInfo.orderTakerAssetFilledAmount, + rightOrderInfo.orderTakerAssetFilledAmount = rightOrderInfo.orderTakerAssetFilledAmount.safeAdd( matchResults.right.takerAssetFilledAmount ); // Aggregate the new fill results with the previous fill results for the current orders. - leftFillResults = LibFillResults._addFillResults( + leftFillResults = LibFillResults.addFillResults( leftFillResults, matchResults.left ); - rightFillResults = LibFillResults._addFillResults( + rightFillResults = LibFillResults.addFillResults( rightFillResults, matchResults.right ); // Update the profit in the left and right maker assets using the profits from // the match. - batchMatchedFillResults.profitInLeftMakerAsset = _safeAdd( - batchMatchedFillResults.profitInLeftMakerAsset, + batchMatchedFillResults.profitInLeftMakerAsset = batchMatchedFillResults.profitInLeftMakerAsset.safeAdd( matchResults.profitInLeftMakerAsset ); - batchMatchedFillResults.profitInRightMakerAsset = _safeAdd( - batchMatchedFillResults.profitInRightMakerAsset, + batchMatchedFillResults.profitInRightMakerAsset = batchMatchedFillResults.profitInRightMakerAsset.safeAdd( matchResults.profitInRightMakerAsset ); @@ -368,7 +365,12 @@ contract MixinMatchOrders is takerAddress, rightSignature ); - _assertValidMatch(leftOrder, rightOrder); + _assertValidMatch( + leftOrder, + rightOrder, + leftOrderInfo, + rightOrderInfo + ); // Compute proportional fill amounts matchedFillResults = LibFillResults.calculateMatchedFillResults( @@ -493,10 +495,7 @@ contract MixinMatchOrders is leftOrder.takerFeeAssetData, takerAddress, leftFeeRecipientAddress, - _safeAdd( - matchedFillResults.left.takerFeePaid, - matchedFillResults.right.takerFeePaid - ) + matchedFillResults.left.takerFeePaid.safeAdd(matchedFillResults.right.takerFeePaid) ); } else { // Right taker fee -> right fee recipient diff --git a/contracts/exchange/contracts/src/MixinSignatureValidator.sol b/contracts/exchange/contracts/src/MixinSignatureValidator.sol index 301793b2d4..ae3ab91086 100644 --- a/contracts/exchange/contracts/src/MixinSignatureValidator.sol +++ b/contracts/exchange/contracts/src/MixinSignatureValidator.sol @@ -25,6 +25,7 @@ import "@0x/contracts-utils/contracts/src/LibRichErrors.sol"; import "@0x/contracts-utils/contracts/src/ReentrancyGuard.sol"; import "@0x/contracts-exchange-libs/contracts/src/LibOrder.sol"; import "@0x/contracts-exchange-libs/contracts/src/LibZeroExTransaction.sol"; +import "@0x/contracts-exchange-libs/contracts/src/LibEIP712ExchangeDomain.sol"; import "./interfaces/IWallet.sol"; import "./interfaces/IEIP1271Wallet.sol"; import "./interfaces/IExchangeRichErrors.sol"; @@ -35,11 +36,14 @@ import "./MixinTransactions.sol"; contract MixinSignatureValidator is ReentrancyGuard, + LibEIP712ExchangeDomain, LibEIP1271, ISignatureValidator, MixinTransactions { using LibBytes for bytes; + using LibOrder for LibOrder.Order; + using LibZeroExTransaction for LibZeroExTransaction.ZeroExTransaction; // Magic bytes to be returned by `Wallet` signature type validators. // bytes4(keccak256("isValidWalletSignature(bytes32,address,bytes)")) @@ -134,7 +138,7 @@ contract MixinSignatureValidator is view returns (bool isValid) { - bytes32 orderHash = LibOrder.getOrderHash(order); + bytes32 orderHash = order.getOrderHash(EIP712_EXCHANGE_DOMAIN_HASH); return _isValidOrderWithHashSignature( order, orderHash, @@ -154,7 +158,7 @@ contract MixinSignatureValidator is view returns (bool isValid) { - bytes32 transactionHash = LibZeroExTransaction.getTransactionHash(transaction); + bytes32 transactionHash = transaction.getTransactionHash(EIP712_EXCHANGE_DOMAIN_HASH); isValid = _isValidTransactionWithHashSignature( transaction, transactionHash, diff --git a/contracts/exchange/contracts/src/MixinTransactions.sol b/contracts/exchange/contracts/src/MixinTransactions.sol index 7dc523bdc2..63a8b30e90 100644 --- a/contracts/exchange/contracts/src/MixinTransactions.sol +++ b/contracts/exchange/contracts/src/MixinTransactions.sol @@ -20,6 +20,7 @@ pragma solidity ^0.5.9; pragma experimental ABIEncoderV2; import "@0x/contracts-exchange-libs/contracts/src/LibZeroExTransaction.sol"; +import "@0x/contracts-exchange-libs/contracts/src/LibEIP712ExchangeDomain.sol"; import "@0x/contracts-utils/contracts/src/LibRichErrors.sol"; import "./interfaces/IExchangeRichErrors.sol"; import "./interfaces/ITransactions.sol"; @@ -28,9 +29,12 @@ import "./LibExchangeRichErrors.sol"; contract MixinTransactions is + LibEIP712ExchangeDomain, ISignatureValidator, ITransactions { + using LibZeroExTransaction for LibZeroExTransaction.ZeroExTransaction; + // Mapping of transaction hash => executed // This prevents transactions from being executed more than once. mapping (bytes32 => bool) public transactionsExecuted; @@ -82,7 +86,7 @@ contract MixinTransactions is internal returns (bytes memory) { - bytes32 transactionHash = LibZeroExTransaction.getTransactionHash(transaction); + bytes32 transactionHash = transaction.getTransactionHash(EIP712_EXCHANGE_DOMAIN_HASH); // Check transaction is not expired // solhint-disable-next-line not-rely-on-time diff --git a/contracts/exchange/contracts/src/MixinWrapperFunctions.sol b/contracts/exchange/contracts/src/MixinWrapperFunctions.sol index cfc0484c3f..14a7ca382d 100644 --- a/contracts/exchange/contracts/src/MixinWrapperFunctions.sol +++ b/contracts/exchange/contracts/src/MixinWrapperFunctions.sol @@ -19,8 +19,10 @@ pragma solidity ^0.5.9; pragma experimental ABIEncoderV2; +import "@0x/contracts-utils/contracts/src/LibSafeMath.sol"; import "@0x/contracts-utils/contracts/src/LibRichErrors.sol"; import "@0x/contracts-exchange-libs/contracts/src/LibOrder.sol"; +import "@0x/contracts-exchange-libs/contracts/src/LibMath.sol"; import "@0x/contracts-exchange-libs/contracts/src/LibFillResults.sol"; import "./interfaces/IExchangeCore.sol"; import "./interfaces/IExchangeRichErrors.sol"; @@ -33,6 +35,8 @@ contract MixinWrapperFunctions is IWrapperFunctions, MixinExchangeCore { + using LibSafeMath for uint256; + /// @dev Fills the input order. Reverts if exact takerAssetFillAmount not filled. /// @param order Order struct containing order specifications. /// @param takerAssetFillAmount Desired amount of takerAsset to sell. @@ -186,7 +190,7 @@ contract MixinWrapperFunctions is orders[i].takerAssetData = takerAssetData; // Calculate the remaining amount of takerAsset to sell - uint256 remainingTakerAssetFillAmount = _safeSub(takerAssetFillAmount, fillResults.takerAssetFilledAmount); + uint256 remainingTakerAssetFillAmount = takerAssetFillAmount.safeSub(fillResults.takerAssetFilledAmount); // Attempt to sell the remaining amount of takerAsset LibFillResults.FillResults memory singleFillResults = fillOrderNoThrow( @@ -196,7 +200,7 @@ contract MixinWrapperFunctions is ); // Update amounts filled and fees paid by maker and taker - fillResults = LibFillResults._addFillResults(fillResults, singleFillResults); + fillResults = LibFillResults.addFillResults(fillResults, singleFillResults); // Stop execution if the entire amount of takerAsset has been sold if (fillResults.takerAssetFilledAmount >= takerAssetFillAmount) { @@ -230,11 +234,11 @@ contract MixinWrapperFunctions is orders[i].makerAssetData = makerAssetData; // Calculate the remaining amount of makerAsset to buy - uint256 remainingMakerAssetFillAmount = _safeSub(makerAssetFillAmount, fillResults.makerAssetFilledAmount); + uint256 remainingMakerAssetFillAmount = makerAssetFillAmount.safeSub(fillResults.makerAssetFilledAmount); // Convert the remaining amount of makerAsset to buy into remaining amount // of takerAsset to sell, assuming entire amount can be sold in the current order - uint256 remainingTakerAssetFillAmount = _getPartialAmountFloor( + uint256 remainingTakerAssetFillAmount = LibMath.getPartialAmountFloor( orders[i].takerAssetAmount, orders[i].makerAssetAmount, remainingMakerAssetFillAmount @@ -248,7 +252,7 @@ contract MixinWrapperFunctions is ); // Update amounts filled and fees paid by maker and taker - fillResults = LibFillResults._addFillResults(fillResults, singleFillResults); + fillResults = LibFillResults.addFillResults(fillResults, singleFillResults); // Stop execution if the entire amount of makerAsset has been bought if (fillResults.makerAssetFilledAmount >= makerAssetFillAmount) {