From 599d34f1c08c3b41817c8cfe3f4e1b847cda072b Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Thu, 26 Apr 2018 09:29:05 -0700 Subject: Make all lib functions internal, add contracts for testing --- .../current/protocol/Exchange/Exchange.sol | 2 +- .../current/protocol/Exchange/ISigner.sol | 5 +- .../current/protocol/Exchange/LibMath.sol | 71 +++++++++++++++++++ .../current/protocol/Exchange/LibOrder.sol | 5 +- .../current/protocol/Exchange/LibPartialAmount.sol | 38 ---------- .../Exchange/MixinAssetProxyDispatcher.sol | 3 +- .../protocol/Exchange/MixinExchangeCore.sol | 28 +------- .../current/protocol/Exchange/MixinSettlement.sol | 9 +-- .../protocol/Exchange/MixinSignatureValidator.sol | 8 ++- .../protocol/Exchange/MixinWrapperFunctions.sol | 6 +- .../Exchange/mixins/MAssetProxyDispatcher.sol | 3 +- .../Exchange/mixins/MSignatureValidator.sol | 3 +- .../contracts/current/test/TestLibs/TestLibs.sol | 80 ++++++++++++++++++++++ .../TestSignatureValidator.sol | 41 +++++++++++ packages/contracts/src/utils/exchange_wrapper.ts | 31 --------- packages/contracts/src/utils/types.ts | 2 + 16 files changed, 221 insertions(+), 114 deletions(-) create mode 100644 packages/contracts/src/contracts/current/protocol/Exchange/LibMath.sol delete mode 100644 packages/contracts/src/contracts/current/protocol/Exchange/LibPartialAmount.sol create mode 100644 packages/contracts/src/contracts/current/test/TestLibs/TestLibs.sol create mode 100644 packages/contracts/src/contracts/current/test/TestSignatureValidator/TestSignatureValidator.sol (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/Exchange.sol b/packages/contracts/src/contracts/current/protocol/Exchange/Exchange.sol index 47f9e7a4d..e78446b99 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/Exchange.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/Exchange.sol @@ -29,8 +29,8 @@ import "./MixinTransactions.sol"; contract Exchange is MixinWrapperFunctions, MixinExchangeCore, - MixinSignatureValidator, MixinSettlement, + MixinSignatureValidator, MixinTransactions, MixinAssetProxyDispatcher { diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/ISigner.sol b/packages/contracts/src/contracts/current/protocol/Exchange/ISigner.sol index de3989a96..d5641a09f 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/ISigner.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/ISigner.sol @@ -23,7 +23,8 @@ contract ISigner { function isValidSignature( bytes32 hash, - bytes memory signature) - public view + bytes signature) + external + view returns (bool isValid); } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/LibMath.sol b/packages/contracts/src/contracts/current/protocol/Exchange/LibMath.sol new file mode 100644 index 000000000..3b553ebbc --- /dev/null +++ b/packages/contracts/src/contracts/current/protocol/Exchange/LibMath.sol @@ -0,0 +1,71 @@ +/* + + Copyright 2018 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.4.21; +pragma experimental ABIEncoderV2; + +import "../../utils/SafeMath/SafeMath.sol"; + +contract LibMath is SafeMath { + + /// @dev Calculates partial value given a numerator and denominator. + /// @param numerator Numerator. + /// @param denominator Denominator. + /// @param target Value to calculate partial of. + /// @return Partial value of target. + function getPartialAmount( + uint256 numerator, + uint256 denominator, + uint256 target) + internal + pure + returns (uint256 partialAmount) + { + partialAmount = safeDiv( + safeMul(numerator, target), + denominator + ); + return partialAmount; + } + + /// @dev Checks if rounding error > 0.1%. + /// @param numerator Numerator. + /// @param denominator Denominator. + /// @param target Value to multiply with numerator/denominator. + /// @return Rounding error is present. + function isRoundingError( + uint256 numerator, + uint256 denominator, + uint256 target) + internal + pure + returns (bool isError) + { + uint256 remainder = mulmod(target, numerator, denominator); + if (remainder == 0) { + return false; // No rounding error. + } + + uint256 errPercentageTimes1000000 = safeDiv( + safeMul(remainder, 1000000), + safeMul(numerator, target) + ); + isError = errPercentageTimes1000000 > 1000; + return isError; + } +} diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/LibOrder.sol b/packages/contracts/src/contracts/current/protocol/Exchange/LibOrder.sol index a98d7cbeb..df974e177 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/LibOrder.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/LibOrder.sol @@ -55,8 +55,9 @@ contract LibOrder { /// @dev Calculates Keccak-256 hash of the order. /// @param order The order structure. /// @return Keccak-256 EIP712 hash of the order. - function getOrderHash(Order order) - public view + function getOrderHash(Order memory order) + internal + view returns (bytes32 orderHash) { // TODO: EIP712 is not finalized yet diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/LibPartialAmount.sol b/packages/contracts/src/contracts/current/protocol/Exchange/LibPartialAmount.sol deleted file mode 100644 index 2ff244e98..000000000 --- a/packages/contracts/src/contracts/current/protocol/Exchange/LibPartialAmount.sol +++ /dev/null @@ -1,38 +0,0 @@ -/* - - Copyright 2018 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.4.21; -pragma experimental ABIEncoderV2; - -import "../../utils/SafeMath/SafeMath.sol"; - -contract LibPartialAmount is SafeMath { - - /// @dev Calculates partial value given a numerator and denominator. - /// @param numerator Numerator. - /// @param denominator Denominator. - /// @param target Value to calculate partial of. - /// @return Partial value of target. - function getPartialAmount(uint256 numerator, uint256 denominator, uint256 target) - public pure - returns (uint256 partialAmount) - { - partialAmount = safeDiv(safeMul(numerator, target), denominator); - return partialAmount; - } -} diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol index b60a1cb08..b03bf464a 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol @@ -78,7 +78,8 @@ contract MixinAssetProxyDispatcher is /// @param assetProxyId Id of the asset proxy. /// @return The asset proxy registered to assetProxyId. Returns 0x0 if no proxy is registered. function getAssetProxy(uint8 assetProxyId) - external view + external + view returns (address) { IAssetProxy assetProxy = assetProxies[assetProxyId]; diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol index af114930b..4ca271b2a 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol @@ -19,11 +19,10 @@ pragma solidity ^0.4.21; pragma experimental ABIEncoderV2; -import "../../utils/SafeMath/SafeMath.sol"; import "./LibFillResults.sol"; import "./LibOrder.sol"; import "./LibErrors.sol"; -import "./LibPartialAmount.sol"; +import "./LibMath.sol"; import "./mixins/MExchangeCore.sol"; import "./mixins/MSettlement.sol"; import "./mixins/MSignatureValidator.sol"; @@ -33,11 +32,10 @@ import "./mixins/MTransactions.sol"; /// @dev Consumes MSettlement /// @dev Consumes MSignatureValidator contract MixinExchangeCore is - SafeMath, LibOrder, LibFillResults, LibErrors, - LibPartialAmount, + LibMath, MExchangeCore, MSettlement, MSignatureValidator, @@ -219,28 +217,6 @@ contract MixinExchangeCore is emit CancelUpTo(msg.sender, newMakerEpoch); } - /// @dev Checks if rounding error > 0.1%. - /// @param numerator Numerator. - /// @param denominator Denominator. - /// @param target Value to multiply with numerator/denominator. - /// @return Rounding error is present. - function isRoundingError(uint256 numerator, uint256 denominator, uint256 target) - public pure - returns (bool isError) - { - uint256 remainder = mulmod(target, numerator, denominator); - if (remainder == 0) { - return false; // No rounding error. - } - - uint256 errPercentageTimes1000000 = safeDiv( - safeMul(remainder, 1000000), - safeMul(numerator, target) - ); - isError = errPercentageTimes1000000 > 1000; - return isError; - } - /// @dev Logs a Fill event with the given arguments. /// The sole purpose of this function is to get around the stack variable limit. function emitFillEvent( diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol index df86a9dfa..cab2ccfb6 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol @@ -22,19 +22,20 @@ pragma experimental ABIEncoderV2; import "./mixins/MSettlement.sol"; import "./mixins/MAssetProxyDispatcher.sol"; import "./LibOrder.sol"; -import "./LibPartialAmount.sol"; +import "./LibMath.sol"; import "../AssetProxy/IAssetProxy.sol"; /// @dev Provides MixinSettlement contract MixinSettlement is - LibPartialAmount, + LibMath, MSettlement, MAssetProxyDispatcher { - bytes ZRX_PROXY_DATA; + bytes internal ZRX_PROXY_DATA; function zrxProxyData() - external view + external + view returns (bytes memory) { return ZRX_PROXY_DATA; diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol index 46bf01e78..690a70820 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol @@ -24,6 +24,7 @@ import "./ISigner.sol"; /// @dev Provides MSignatureValidator contract MixinSignatureValidator is MSignatureValidator { + enum SignatureType { Illegal, // Default value Invalid, @@ -47,7 +48,8 @@ contract MixinSignatureValidator is MSignatureValidator { bytes32 hash, address signer, bytes memory signature) - public view + internal + view returns (bool isValid) { // TODO: Domain separation: make hash depend on role. (Taker sig should not be valid as maker sig, etc.) @@ -164,8 +166,8 @@ contract MixinSignatureValidator is MSignatureValidator { function preSign( bytes32 hash, address signer, - bytes memory signature) - public + bytes signature) + external { require(isValidSignature(hash, signer, signature)); preSigned[hash][signer] = true; diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol index fe58e3d18..105c0f0a0 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol @@ -19,19 +19,17 @@ pragma solidity ^0.4.21; pragma experimental ABIEncoderV2; -import "../../utils/SafeMath/SafeMath.sol"; import "../../utils/LibBytes/LibBytes.sol"; import "./mixins/MExchangeCore.sol"; -import "./LibPartialAmount.sol"; +import "./LibMath.sol"; import "./LibOrder.sol"; import "./LibFillResults.sol"; /// @dev Consumes MExchangeCore contract MixinWrapperFunctions is - SafeMath, LibOrder, LibFillResults, - LibPartialAmount, + LibMath, LibBytes, MExchangeCore { diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol index a46be9d0f..6d0a3cd38 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol @@ -55,6 +55,7 @@ contract MAssetProxyDispatcher { /// @param assetProxyId Id of the asset proxy. /// @return The asset proxy registered to assetProxyId. Returns 0x0 if no proxy is registered. function getAssetProxy(uint8 assetProxyId) - external view + external + view returns (address); } 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 3f020e0a4..043a7da9c 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol @@ -30,6 +30,7 @@ contract MSignatureValidator { bytes32 hash, address signer, bytes memory signature) - public view + internal + view returns (bool isValid); } diff --git a/packages/contracts/src/contracts/current/test/TestLibs/TestLibs.sol b/packages/contracts/src/contracts/current/test/TestLibs/TestLibs.sol new file mode 100644 index 000000000..a5b327a90 --- /dev/null +++ b/packages/contracts/src/contracts/current/test/TestLibs/TestLibs.sol @@ -0,0 +1,80 @@ +/* + + Copyright 2018 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.4.21; +pragma experimental ABIEncoderV2; + +import "../../protocol/Exchange/LibMath.sol"; +import "../../protocol/Exchange/LibOrder.sol"; +import "../../protocol/Exchange/LibFillResults.sol"; + +contract TestLibs is + LibMath, + LibOrder, + LibFillResults +{ + function publicGetPartialAmount( + uint256 numerator, + uint256 denominator, + uint256 target) + public + pure + returns (uint256 partialAmount) + { + partialAmount = getPartialAmount( + numerator, + denominator, + target + ); + return partialAmount; + } + + function publicIsRoundingError( + uint256 numerator, + uint256 denominator, + uint256 target) + public + pure + returns (bool isError) + { + isError = isRoundingError( + numerator, + denominator, + target + ); + return isError; + } + + function publicGetOrderHash(Order memory order) + public + view + returns (bytes32 orderHash) + { + orderHash = getOrderHash(order); + return orderHash; + } + + function publicAddFillResults(FillResults memory totalFillResults, FillResults memory singleFillResults) + public + pure + returns (FillResults memory) + { + addFillResults(totalFillResults, singleFillResults); + return totalFillResults; + } +} \ No newline at end of file diff --git a/packages/contracts/src/contracts/current/test/TestSignatureValidator/TestSignatureValidator.sol b/packages/contracts/src/contracts/current/test/TestSignatureValidator/TestSignatureValidator.sol new file mode 100644 index 000000000..4ba04b64e --- /dev/null +++ b/packages/contracts/src/contracts/current/test/TestSignatureValidator/TestSignatureValidator.sol @@ -0,0 +1,41 @@ +/* + + Copyright 2018 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.4.21; +pragma experimental ABIEncoderV2; + +import "../../protocol/Exchange/MixinSignatureValidator.sol"; + +contract TestSignatureValidator is MixinSignatureValidator { + + function publicIsValidSignature( + bytes32 hash, + address signer, + bytes memory signature) + public + view + returns (bool isValid) + { + isValid = isValidSignature( + hash, + signer, + signature + ); + return isValid; + } +} \ No newline at end of file diff --git a/packages/contracts/src/utils/exchange_wrapper.ts b/packages/contracts/src/utils/exchange_wrapper.ts index 000905854..27fdd698f 100644 --- a/packages/contracts/src/utils/exchange_wrapper.ts +++ b/packages/contracts/src/utils/exchange_wrapper.ts @@ -221,37 +221,6 @@ export class ExchangeWrapper { const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash); return tx; } - public async getOrderHashAsync(signedOrder: SignedOrder): Promise { - const order = orderUtils.getOrderStruct(signedOrder); - const orderHash = await this._exchange.getOrderHash.callAsync(order); - return orderHash; - } - public async isValidSignatureAsync(signedOrder: SignedOrder): Promise { - const isValidSignature = await this._exchange.isValidSignature.callAsync( - orderUtils.getOrderHashHex(signedOrder), - signedOrder.makerAddress, - signedOrder.signature, - ); - return isValidSignature; - } - public async isRoundingErrorAsync( - numerator: BigNumber, - denominator: BigNumber, - target: BigNumber, - ): Promise { - const isRoundingError = await this._exchange.isRoundingError.callAsync(numerator, denominator, target); - return isRoundingError; - } - public async getPartialAmountAsync( - numerator: BigNumber, - denominator: BigNumber, - target: BigNumber, - ): Promise { - const partialAmount = new BigNumber( - await this._exchange.getPartialAmount.callAsync(numerator, denominator, target), - ); - return partialAmount; - } public async getTakerAssetFilledAmountAsync(orderHashHex: string): Promise { const filledAmount = new BigNumber(await this._exchange.filled.callAsync(orderHashHex)); return filledAmount; diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts index 934dc52fa..6f9aeda94 100644 --- a/packages/contracts/src/utils/types.ts +++ b/packages/contracts/src/utils/types.ts @@ -94,6 +94,8 @@ export enum ContractName { EtherDelta = 'EtherDelta', Arbitrage = 'Arbitrage', TestAssetProxyDispatcher = 'TestAssetProxyDispatcher', + TestLibs = 'TestLibs', + TestSignatureValidator = 'TestSignatureValidator', ERC20Proxy = 'ERC20Proxy', ERC721Proxy = 'ERC721Proxy', DummyERC721Token = 'DummyERC721Token', -- cgit