From 8b5b3718213fe329231e6a94dcb77a1fd0e74fd5 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Thu, 29 Mar 2018 14:02:37 -0700 Subject: Return all amounts traded and fees paid --- .../current/protocol/Exchange/ISigner.sol | 2 +- .../protocol/Exchange/MixinExchangeCore.sol | 45 +++---- .../protocol/Exchange/MixinSignatureValidator.sol | 4 +- .../protocol/Exchange/MixinWrapperFunctions.sol | 137 +++++++++++---------- .../protocol/Exchange/mixins/MExchangeCore.sol | 19 ++- .../protocol/Exchange/mixins/MSettlement.sol | 12 +- .../Exchange/mixins/MSignatureValidator.sol | 2 +- 7 files changed, 115 insertions(+), 106 deletions(-) diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/ISigner.sol b/packages/contracts/src/contracts/current/protocol/Exchange/ISigner.sol index d173199b3..de3989a96 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/ISigner.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/ISigner.sol @@ -23,7 +23,7 @@ contract ISigner { function isValidSignature( bytes32 hash, - bytes signature) + bytes memory signature) public view returns (bool isValid); } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol index 375848ed1..aefffe465 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol @@ -89,15 +89,21 @@ contract MixinExchangeCore is uint256 takerSellAmount, bytes memory signature) public - returns (uint256 takerAmountSold) + returns (FillResults memory fillResults) { // Compute the order hash bytes32 orderHash = getOrderHash(order); - // Check if order has been cancelled + // Check if order has been cancelled by salt value + if (order.salt < makerEpoch[order.makerAddress]) { + emit ExchangeError(uint8(Errors.ORDER_CANCELLED), orderHash); + return fillResults; + } + + // Check if order has been cancelled by orderHash if (cancelled[orderHash]) { LogError(uint8(Errors.ORDER_CANCELLED), orderHash); - return 0; + return fillResults; } // Validate order and maker only if first time seen @@ -114,35 +120,30 @@ contract MixinExchangeCore is // Validate order expiration if (block.timestamp >= order.expirationTimeSeconds) { LogError(uint8(Errors.ORDER_EXPIRED), orderHash); - return 0; + return fillResults; } // Validate order availability uint256 remainingMakerBuyAmount = safeSub(order.makerBuyAmount, filled[orderHash]); if (remainingMakerBuyAmount == 0) { LogError(uint8(Errors.ORDER_FULLY_FILLED), orderHash); - return 0; + return fillResults; } // Validate fill order rounding - takerAmountSold = min256(takerSellAmount, remainingMakerBuyAmount); - if (isRoundingError(takerAmountSold, order.makerBuyAmount, order.makerSellAmount)) { + fillResults.takerAmountSold = min256(takerSellAmount, remainingMakerBuyAmount); + if (isRoundingError(fillResults.takerAmountSold, order.makerBuyAmount, order.makerSellAmount)) { LogError(uint8(Errors.ROUNDING_ERROR_TOO_LARGE), orderHash); - return 0; - } - - // Validate order is not cancelled - if (order.salt < makerEpoch[order.makerAddress]) { - LogError(uint8(Errors.ORDER_CANCELLED), orderHash); - return 0; + fillResults.takerAmountSold = 0; + return fillResults; } // Update state - filled[orderHash] = safeAdd(filled[orderHash], takerAmountSold); + filled[orderHash] = safeAdd(filled[orderHash], fillResults.takerAmountSold); // Settle order - var (makerAmountSold, makerFeePaid, takerFeePaid) = - settleOrder(order, msg.sender, takerAmountSold); + (fillResults.makerAmountSold, fillResults.makerFeePaid, fillResults.takerFeePaid) = + settleOrder(order, msg.sender, fillResults.takerAmountSold); // Log order LogFill( @@ -151,13 +152,13 @@ contract MixinExchangeCore is order.feeRecipientAddress, order.makerTokenAddress, order.takerTokenAddress, - makerAmountSold, - takerAmountSold, - makerFeePaid, - takerFeePaid, + fillResults.makerAmountSold, + fillResults.takerAmountSold, + fillResults.makerFeePaid, + fillResults.takerFeePaid, orderHash ); - return takerAmountSold; + return fillResults; } /// @dev After calling, the order can not be filled anymore. diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol index 49b6949e2..d8bb25fca 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol @@ -39,7 +39,7 @@ contract MixinSignatureValidator is function isValidSignature( bytes32 hash, address signer, - bytes signature) + bytes memory signature) public view returns (bool isValid) { @@ -145,7 +145,7 @@ contract MixinSignatureValidator is revert(); } - function get32(bytes b, uint256 index) + function get32(bytes memory b, uint256 index) private pure returns (bytes32 result) { diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol index 030743348..34bd77092 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol @@ -38,14 +38,15 @@ contract MixinWrapperFunctions is uint256 takerSellAmount, bytes memory signature) public + returns (FillResults memory fillResults) { - require( - fillOrder( - order, - takerSellAmount, - signature - ) == takerSellAmount + fillResults = fillOrder( + order, + takerSellAmount, + signature ); + require(fillResults.takerAmountSold == takerSellAmount); + return fillResults; } /// @dev Fills an order with specified parameters and ECDSA signature. @@ -53,13 +54,13 @@ contract MixinWrapperFunctions is /// @param order Order struct containing order specifications. /// @param takerSellAmount Desired amount of takerToken to fill. /// @param signature Maker's signature of the order. - /// @return Total amount of takerToken filled in trade. + /// @return Amounts filled and fees paid by maker and taker. function fillOrderNoThrow( Order memory order, uint256 takerSellAmount, bytes memory signature) public - returns (uint256 takerAmountSold) + returns (FillResults memory fillResults) { // We need to call MExchangeCore.fillOrder using a delegatecall in // assembly so that we can intercept a call that throws. For this, we @@ -133,18 +134,24 @@ contract MixinWrapperFunctions is start, // pointer to start of input add(452, sigLenWithPadding), // input length is 420 + signature length + padding length start, // write output over input - 32 // output size is 32 bytes + 128 // output size is 128 bytes ) switch success case 0 { - takerAmountSold := 0 + mstore(fillResults, 0) + mstore(add(fillResults, 32), 0) + mstore(add(fillResults, 64), 0) + mstore(add(fillResults, 96), 0) } case 1 { - takerAmountSold := mload(start) + mstore(fillResults, mload(start)) + mstore(add(fillResults, 32), mload(add(start, 32))) + mstore(add(fillResults, 64), mload(add(start, 64))) + mstore(add(fillResults, 96), mload(add(start, 96))) } } - return takerAmountSold; + return fillResults; } /// @dev Synchronously executes multiple calls of fillOrder in a single transaction. @@ -214,24 +221,25 @@ contract MixinWrapperFunctions is uint256 takerSellAmount, bytes[] memory signatures) public - returns (uint256 takerAmountSold) + returns (FillResults memory fillResults) { for (uint256 i = 0; i < orders.length; i++) { require(orders[i].takerTokenAddress == orders[0].takerTokenAddress); - uint256 remainingTakerSellAmount = safeSub(takerSellAmount, takerAmountSold); - takerAmountSold = safeAdd( - takerAmountSold, - fillOrder( - orders[i], - remainingTakerSellAmount, - signatures[i] - ) + uint256 remainingTakerSellAmount = safeSub(takerSellAmount, fillResults.takerAmountSold); + FillResults memory currentFillResults = fillOrder( + orders[i], + remainingTakerSellAmount, + signatures[i] ); - if (takerAmountSold == takerSellAmount) { + fillResults.makerAmountSold = safeAdd(fillResults.makerAmountSold, currentFillResults.makerAmountSold); + fillResults.takerAmountSold = safeAdd(fillResults.takerAmountSold, currentFillResults.takerAmountSold); + fillResults.makerFeePaid = safeAdd(fillResults.makerFeePaid, currentFillResults.makerFeePaid); + fillResults.takerFeePaid = safeAdd(fillResults.takerFeePaid, currentFillResults.takerFeePaid); + if (fillResults.takerAmountSold == takerSellAmount) { break; } } - return takerAmountSold; + return fillResults; } /// @dev Synchronously executes multiple calls of fillOrderNoThrow in a single transaction until total amount is sold by taker. @@ -245,24 +253,25 @@ contract MixinWrapperFunctions is uint256 takerSellAmount, bytes[] memory signatures) public - returns (uint256 takerAmountSold) + returns (FillResults memory fillResults) { for (uint256 i = 0; i < orders.length; i++) { require(orders[i].takerTokenAddress == orders[0].takerTokenAddress); - uint256 remainingTakerSellAmount = safeSub(takerSellAmount, takerAmountSold); - takerAmountSold = safeAdd( - takerAmountSold, - fillOrderNoThrow( - orders[i], - remainingTakerSellAmount, - signatures[i] - ) + uint256 remainingTakerSellAmount = safeSub(takerSellAmount, fillResults.takerAmountSold); + FillResults memory currentFillResults = fillOrderNoThrow( + orders[i], + remainingTakerSellAmount, + signatures[i] ); - if (takerAmountSold == takerSellAmount) { + fillResults.makerAmountSold = safeAdd(fillResults.makerAmountSold, currentFillResults.makerAmountSold); + fillResults.takerAmountSold = safeAdd(fillResults.takerAmountSold, currentFillResults.takerAmountSold); + fillResults.makerFeePaid = safeAdd(fillResults.makerFeePaid, currentFillResults.makerFeePaid); + fillResults.takerFeePaid = safeAdd(fillResults.takerFeePaid, currentFillResults.takerFeePaid); + if (fillResults.takerAmountSold == takerSellAmount) { break; } } - return takerAmountSold; + return fillResults; } /// @dev Synchronously executes multiple fill orders in a single transaction until total amount is bought by taker. @@ -275,34 +284,30 @@ contract MixinWrapperFunctions is uint256 takerBuyAmount, bytes[] memory signatures) public - returns (uint256 takerAmountBought) + returns (FillResults memory fillResults) { for (uint256 i = 0; i < orders.length; i++) { require(orders[i].makerTokenAddress == orders[0].makerTokenAddress); - uint256 remainingTakerBuyAmount = safeSub(takerBuyAmount, takerAmountBought); - uint256 takerSellAmount = getPartialAmount( + uint256 remainingTakerBuyAmount = safeSub(takerBuyAmount, fillResults.makerAmountSold); + uint256 remainingTakerSellAmount = getPartialAmount( orders[i].makerBuyAmount, orders[i].makerSellAmount, remainingTakerBuyAmount ); - uint256 takerAmountSold = fillOrder( - orders[i], - takerSellAmount, - signatures[i] - ); - takerAmountBought = safeAdd( - takerAmountBought, - getPartialAmount( - orders[i].makerSellAmount, - orders[i].makerBuyAmount, - takerAmountSold - ) + FillResults memory currentFillResults = fillOrder( + orders[i], + remainingTakerSellAmount, + signatures[i] ); - if (takerAmountBought == takerBuyAmount) { + fillResults.makerAmountSold = safeAdd(fillResults.makerAmountSold, currentFillResults.makerAmountSold); + fillResults.takerAmountSold = safeAdd(fillResults.takerAmountSold, currentFillResults.takerAmountSold); + fillResults.makerFeePaid = safeAdd(fillResults.makerFeePaid, currentFillResults.makerFeePaid); + fillResults.takerFeePaid = safeAdd(fillResults.takerFeePaid, currentFillResults.takerFeePaid); + if (fillResults.makerAmountSold == takerBuyAmount) { break; } } - return takerAmountBought; + return fillResults; } /// @dev Synchronously executes multiple fill orders in a single transaction until total amount is bought by taker. @@ -316,34 +321,30 @@ contract MixinWrapperFunctions is uint256 takerBuyAmount, bytes[] memory signatures) public - returns (uint256 takerAmountBought) + returns (FillResults memory fillResults) { for (uint256 i = 0; i < orders.length; i++) { require(orders[i].makerTokenAddress == orders[0].makerTokenAddress); - uint256 remainingTakerBuyAmount = safeSub(takerBuyAmount, takerAmountBought); - uint256 takerSellAmount = getPartialAmount( + uint256 remainingTakerBuyAmount = safeSub(takerBuyAmount, fillResults.makerAmountSold); + uint256 remainingTakerSellAmount = getPartialAmount( orders[i].makerBuyAmount, orders[i].makerSellAmount, remainingTakerBuyAmount ); - uint256 takerAmountSold = fillOrderNoThrow( - orders[i], - takerSellAmount, - signatures[i] - ); - takerAmountBought = safeAdd( - takerAmountBought, - getPartialAmount( - orders[i].makerSellAmount, - orders[i].makerBuyAmount, - takerAmountSold - ) + FillResults memory currentFillResults = fillOrderNoThrow( + orders[i], + remainingTakerSellAmount, + signatures[i] ); - if (takerAmountBought == takerBuyAmount) { + fillResults.makerAmountSold = safeAdd(fillResults.makerAmountSold, currentFillResults.makerAmountSold); + fillResults.takerAmountSold = safeAdd(fillResults.takerAmountSold, currentFillResults.takerAmountSold); + fillResults.makerFeePaid = safeAdd(fillResults.makerFeePaid, currentFillResults.makerFeePaid); + fillResults.takerFeePaid = safeAdd(fillResults.takerFeePaid, currentFillResults.takerFeePaid); + if (fillResults.makerAmountSold == takerBuyAmount) { break; } } - return takerAmountBought; + return fillResults; } /// @dev Synchronously cancels multiple orders in a single transaction. diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol index a495434e6..968fec195 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol @@ -23,17 +23,24 @@ import "../LibOrder.sol"; contract MExchangeCore is LibOrder { + struct FillResults { + uint256 makerAmountSold; + uint256 takerAmountSold; + uint256 makerFeePaid; + uint256 takerFeePaid; + } + function fillOrder( - Order order, - uint256 takerTokenFillAmount, - bytes signature) + Order memory order, + uint256 takerSellAmount, + bytes memory signature) public - returns (uint256 takerTokenFilledAmount); + returns (FillResults memory fillResults); - function cancelOrder(Order order) + function cancelOrder(Order memory order) public returns (bool); - function cancelOrdersUpTo(uint256 salt) + function cancelOrdersUpTo(uint256 salt) external; } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSettlement.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSettlement.sol index f960e4198..c3e332a87 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSettlement.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSettlement.sol @@ -24,14 +24,14 @@ import "../LibOrder.sol"; contract MSettlement is LibOrder { function settleOrder( - Order order, - address taker, - uint256 takerTokenFilledAmount) + Order memory order, + address takerAddress, + uint256 takerAmountSold) internal returns ( - uint256 makerTokenFilledAmount, - uint256 makerFeeAmountPaid, - uint256 takerFeeAmountPaid + uint256 makerAmountSold, + uint256 makerFeePaid, + uint256 takerFeePaid ); } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol index fa75830b4..3f020e0a4 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol @@ -29,7 +29,7 @@ contract MSignatureValidator { function isValidSignature( bytes32 hash, address signer, - bytes signature) + bytes memory signature) public view returns (bool isValid); } -- cgit v1.2.3