diff options
Diffstat (limited to 'packages/contracts/src')
30 files changed, 482 insertions, 136 deletions
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol index ee0c66fdc..79824fbbb 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol @@ -47,21 +47,25 @@ contract ERC20Proxy is bytes memory assetMetadata, address from, address to, - uint256 amount) + uint256 amount + ) internal { // Data must be intended for this proxy. + uint256 length = assetMetadata.length; + require( - uint8(assetMetadata[0]) == PROXY_ID, - PROXY_ID_MISMATCH + length == 21, + INVALID_METADATA_LENGTH ); - // Decode metadata. require( - assetMetadata.length == 21, - INVALID_METADATA_LENGTH + uint8(assetMetadata[length - 1]) == PROXY_ID, + PROXY_ID_MISMATCH ); - address token = readAddress(assetMetadata, 1); + + // Decode metadata. + address token = readAddress(assetMetadata, 0); // Transfer tokens. bool success = IERC20Token(token).transferFrom(from, to, amount); diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol index 94aab9139..ace3570bc 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol @@ -47,12 +47,20 @@ contract ERC721Proxy is bytes memory assetMetadata, address from, address to, - uint256 amount) + uint256 amount + ) internal { // Data must be intended for this proxy. + uint256 length = assetMetadata.length; + + require( + length == 53, + INVALID_METADATA_LENGTH + ); + require( - uint8(assetMetadata[0]) == PROXY_ID, + uint8(assetMetadata[length - 1]) == PROXY_ID, PROXY_ID_MISMATCH ); @@ -63,12 +71,8 @@ contract ERC721Proxy is ); // Decode metadata - require( - assetMetadata.length == 53, - INVALID_METADATA_LENGTH - ); - address token = readAddress(assetMetadata, 1); - uint256 tokenId = readUint256(assetMetadata, 21); + address token = readAddress(assetMetadata, 0); + uint256 tokenId = readUint256(assetMetadata, 20); // Transfer token. // Either succeeds or throws. diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAssetProxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAssetProxy.sol index 4ec31304f..4bd533148 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAssetProxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAssetProxy.sol @@ -36,7 +36,8 @@ contract MixinAssetProxy is bytes assetMetadata, address from, address to, - uint256 amount) + uint256 amount + ) external onlyAuthorized { @@ -57,7 +58,8 @@ contract MixinAssetProxy is bytes[] memory assetMetadata, address[] memory from, address[] memory to, - uint256[] memory amounts) + uint256[] memory amounts + ) public onlyAuthorized { diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol index 0bbd3b318..4aaeb66d1 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol @@ -87,7 +87,10 @@ contract MixinAuthorizable is /// @dev Removes authorizion of an address. /// @param target Address to remove authorization from. /// @param index Index of target in authorities array. - function removeAuthorizedAddressAtIndex(address target, uint256 index) + function removeAuthorizedAddressAtIndex( + address target, + uint256 index + ) external { require( diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol index 8b30dfabb..7e1848889 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol @@ -34,7 +34,8 @@ contract IAssetProxy is bytes assetMetadata, address from, address to, - uint256 amount) + uint256 amount + ) external; /// @dev Makes multiple transfers of assets. Either succeeds or throws. @@ -46,7 +47,8 @@ contract IAssetProxy is bytes[] memory assetMetadata, address[] memory from, address[] memory to, - uint256[] memory amounts) + uint256[] memory amounts + ) public; /// @dev Gets the proxy id associated with the proxy address. diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAuthorizable.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAuthorizable.sol index d6fe03898..cedd1744c 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAuthorizable.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAuthorizable.sol @@ -45,6 +45,9 @@ contract IAuthorizable is /// @dev Removes authorizion of an address. /// @param target Address to remove authorization from. /// @param index Index of target in authorities array. - function removeAuthorizedAddressAtIndex(address target, uint256 index) + function removeAuthorizedAddressAtIndex( + address target, + uint256 index + ) external; } diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAssetProxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAssetProxy.sol index 3800bf04c..de9d65a53 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAssetProxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAssetProxy.sol @@ -34,6 +34,7 @@ contract MAssetProxy is bytes memory assetMetadata, address from, address to, - uint256 amount) + uint256 amount + ) internal; } diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAuthorizable.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAuthorizable.sol index cdf60bdee..6f35bd7ec 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAuthorizable.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAuthorizable.sol @@ -38,5 +38,5 @@ contract MAuthorizable is ); /// @dev Only authorized addresses can invoke functions with this modifier. - modifier onlyAuthorized { _; } + modifier onlyAuthorized { revert(); _; } } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol index 3b38d1f37..d996f933c 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol @@ -39,7 +39,8 @@ contract MixinAssetProxyDispatcher is function registerAssetProxy( uint8 assetProxyId, address newAssetProxy, - address oldAssetProxy) + address oldAssetProxy + ) external onlyOwner { @@ -86,17 +87,20 @@ contract MixinAssetProxyDispatcher is bytes memory assetMetadata, address from, address to, - uint256 amount) + uint256 amount + ) internal { // Do nothing if no amount should be transferred. if (amount > 0) { + // Lookup asset proxy + uint256 length = assetMetadata.length; require( - assetMetadata.length >= 1, + length > 0, GT_ZERO_LENGTH_REQUIRED ); - uint8 assetProxyId = uint8(assetMetadata[0]); + uint8 assetProxyId = uint8(assetMetadata[length - 1]); IAssetProxy assetProxy = assetProxies[assetProxyId]; // transferFrom will either succeed or throw. diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol index f7fcd36b6..d40974e5f 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol @@ -19,18 +19,24 @@ pragma solidity ^0.4.24; import "./mixins/MSignatureValidator.sol"; -import "./interfaces/ISigner.sol"; +import "./mixins/MTransactions.sol"; +import "./interfaces/IWallet.sol"; +import "./interfaces/IValidator.sol"; import "./libs/LibExchangeErrors.sol"; import "../../utils/LibBytes/LibBytes.sol"; contract MixinSignatureValidator is LibBytes, LibExchangeErrors, - MSignatureValidator + MSignatureValidator, + MTransactions { // Mapping of hash => signer => signed - mapping(bytes32 => mapping(address => bool)) preSigned; + mapping (bytes32 => mapping (address => bool)) public preSigned; + + // Mapping of signer => validator => approved + mapping (address => mapping (address => bool)) public allowedValidators; /// @dev Approves a hash on-chain using any valid signature type. /// After presigning a hash, the preSign signature type will become valid for that hash and signer. @@ -39,7 +45,8 @@ contract MixinSignatureValidator is function preSign( bytes32 hash, address signer, - bytes signature) + bytes signature + ) external { require( @@ -49,6 +56,19 @@ contract MixinSignatureValidator is preSigned[hash][signer] = true; } + /// @dev Approves/unnapproves a Validator contract to verify signatures on signer's behalf. + /// @param validator Address of Validator contract. + /// @param approval Approval or disapproval of Validator contract. + function setSignatureValidatorApproval( + address validator, + bool approval + ) + external + { + address signer = getCurrentContextAddress(); + allowedValidators[signer][validator] = approval; + } + /// @dev Verifies that a hash has been signed by the given signer. /// @param hash Any 32 byte hash. /// @param signer Address that should have signed the given hash. @@ -57,71 +77,69 @@ contract MixinSignatureValidator is function isValidSignature( bytes32 hash, address signer, - bytes memory signature) + bytes memory signature + ) internal view returns (bool isValid) { // TODO: Domain separation: make hash depend on role. (Taker sig should not be valid as maker sig, etc.) - require( signature.length >= 1, INVALID_SIGNATURE_LENGTH ); - SignatureType signatureType = SignatureType(uint8(signature[0])); - // Variables are not scoped in Solidity + // Pop last byte off of signature byte array. + SignatureType signatureType = SignatureType(uint8(popByte(signature))); + + // Variables are not scoped in Solidity. uint8 v; bytes32 r; bytes32 s; address recovered; - - // Always illegal signature + + // Always illegal signature. // This is always an implicit option since a signer can create a // signature array with invalid type or length. We may as well make // it an explicit option. This aids testing and analysis. It is // also the initialization value for the enum type. if (signatureType == SignatureType.Illegal) { - // NOTE: Reason cannot be assigned to a variable because of https://github.com/ethereum/solidity/issues/4051 - revert("Illegal signature type."); + revert(ILLEGAL_SIGNATURE_TYPE); - // Always invalid signature + // Always invalid signature. // Like Illegal, this is always implicitly available and therefore // offered explicitly. It can be implicitly created by providing // a correctly formatted but incorrect signature. } else if (signatureType == SignatureType.Invalid) { require( - signature.length == 1, + signature.length == 0, INVALID_SIGNATURE_LENGTH ); isValid = false; return isValid; - // Implicitly signed by caller - // The signer has initiated the call. In the case of non-contract - // accounts it means the transaction itself was signed. - // Example: let's say for a particular operation three signatures - // A, B and C are required. To submit the transaction, A and B can - // give a signature to C, who can then submit the transaction using - // `Caller` for his own signature. Or A and C can sign and B can - // submit using `Caller`. Having `Caller` allows this flexibility. - } else if (signatureType == SignatureType.Caller) { + // Signature using EIP712 + } else if (signatureType == SignatureType.EIP712) { require( - signature.length == 1, + signature.length == 65, INVALID_SIGNATURE_LENGTH ); - isValid = signer == msg.sender; + v = uint8(signature[0]); + r = readBytes32(signature, 1); + s = readBytes32(signature, 33); + recovered = ecrecover(hash, v, r, s); + isValid = signer == recovered; return isValid; // Signed using web3.eth_sign - } else if (signatureType == SignatureType.Ecrecover) { + } else if (signatureType == SignatureType.EthSign) { require( - signature.length == 66, + signature.length == 65, INVALID_SIGNATURE_LENGTH ); - v = uint8(signature[1]); - r = readBytes32(signature, 2); - s = readBytes32(signature, 34); + v = uint8(signature[0]); + r = readBytes32(signature, 1); + s = readBytes32(signature, 33); recovered = ecrecover( keccak256("\x19Ethereum Signed Message:\n32", hash), v, @@ -131,20 +149,55 @@ contract MixinSignatureValidator is isValid = signer == recovered; return isValid; - // Signature using EIP712 - } else if (signatureType == SignatureType.EIP712) { + // Implicitly signed by caller. + // The signer has initiated the call. In the case of non-contract + // accounts it means the transaction itself was signed. + // Example: let's say for a particular operation three signatures + // A, B and C are required. To submit the transaction, A and B can + // give a signature to C, who can then submit the transaction using + // `Caller` for his own signature. Or A and C can sign and B can + // submit using `Caller`. Having `Caller` allows this flexibility. + } else if (signatureType == SignatureType.Caller) { require( - signature.length == 66, + signature.length == 0, INVALID_SIGNATURE_LENGTH ); - v = uint8(signature[1]); - r = readBytes32(signature, 2); - s = readBytes32(signature, 34); - recovered = ecrecover(hash, v, r, s); - isValid = signer == recovered; + isValid = signer == msg.sender; return isValid; - // Signature from Trezor hardware wallet + // Signature verified by wallet contract. + // If used with an order, the maker of the order is the wallet contract. + } else if (signatureType == SignatureType.Wallet) { + isValid = IWallet(signer).isValidSignature(hash, signature); + return isValid; + + // Signature verified by validator contract. + // If used with an order, the maker of the order can still be an EOA. + // A signature using this type should be encoded as: + // | Offset | Length | Contents | + // | 0x00 | x | Signature to validate | + // | 0x00 + x | 20 | Address of validator contract | + // | 0x14 + x | 1 | Signature type is always "\x06" | + } else if (signatureType == SignatureType.Validator) { + // Pop last 20 bytes off of signature byte array. + address validator = popAddress(signature); + // Ensure signer has approved validator. + if (!allowedValidators[signer][validator]) { + return false; + } + isValid = IValidator(validator).isValidSignature( + hash, + signer, + signature + ); + return isValid; + + // Signer signed hash previously using the preSign function. + } else if (signatureType == SignatureType.PreSigned) { + isValid = preSigned[hash][signer]; + return isValid; + + // Signature from Trezor hardware wallet. // It differs from web3.eth_sign in the encoding of message length // (Bitcoin varint encoding vs ascii-decimal, the latter is not // self-terminating which leads to ambiguities). @@ -154,12 +207,12 @@ contract MixinSignatureValidator is // https://github.com/trezor/trezor-mcu/blob/master/firmware/crypto.c#L36 } else if (signatureType == SignatureType.Trezor) { require( - signature.length == 66, + signature.length == 65, INVALID_SIGNATURE_LENGTH ); - v = uint8(signature[1]); - r = readBytes32(signature, 2); - s = readBytes32(signature, 34); + v = uint8(signature[0]); + r = readBytes32(signature, 1); + s = readBytes32(signature, 33); recovered = ecrecover( keccak256("\x19Ethereum Signed Message:\n\x41", hash), v, @@ -169,11 +222,6 @@ contract MixinSignatureValidator is isValid = signer == recovered; return isValid; - // Signature verified by signer contract - } else if (signatureType == SignatureType.Contract) { - isValid = ISigner(signer).isValidSignature(hash, signature); - return isValid; - // Signer signed hash previously using the preSign function } else if (signatureType == SignatureType.PreSigned) { isValid = preSigned[hash][signer]; @@ -185,7 +233,6 @@ contract MixinSignatureValidator is // that we currently support. In this case returning false // may lead the caller to incorrectly believe that the // signature was invalid.) - // NOTE: Reason cannot be assigned to a variable because of https://github.com/ethereum/solidity/issues/4051 - revert("Unsupported signature type."); + revert(UNSUPPORTED_SIGNATURE_TYPE); } } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol index f93a80705..d153dfa5c 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol @@ -43,15 +43,20 @@ contract MixinTransactions is uint256 salt, address signer, bytes data, - bytes signature) + bytes signature + ) external { // Prevent reentrancy - require(currentContextAddress == address(0)); + require( + currentContextAddress == address(0), + REENTRANCY_NOT_ALLOWED + ); // Calculate transaction hash bytes32 transactionHash = keccak256( address(this), + signer, salt, data ); diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IExchange.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IExchange.sol index fc428e9c0..9f21c18d7 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IExchange.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IExchange.sol @@ -20,17 +20,15 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; import "./IExchangeCore.sol"; -import "./IMatchOrders"; -import "./ISettlement"; -import "./ISignatureValidator"; -import "./ITransactions"; -import "./IAssetProxyDispatcher"; -import "./IWrapperFunctions"; +import "./IMatchOrders.sol"; +import "./ISignatureValidator.sol"; +import "./ITransactions.sol"; +import "./IAssetProxyDispatcher.sol"; +import "./IWrapperFunctions.sol"; contract IExchange is IExchangeCore, IMatchOrders, - ISettlement, ISignatureValidator, ITransactions, IAssetProxyDispatcher, diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISignatureValidator.sol index 65ff45f7b..26e360c91 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISignatureValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISignatureValidator.sol @@ -27,6 +27,16 @@ contract ISignatureValidator { function preSign( bytes32 hash, address signer, - bytes signature) + bytes signature + ) + external; + + /// @dev Approves/unnapproves a Validator contract to verify signatures on signer's behalf. + /// @param validator Address of Validator contract. + /// @param approval Approval or disapproval of Validator contract. + function setSignatureValidatorApproval( + address validator, + bool approval + ) external; } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IValidator.sol new file mode 100644 index 000000000..3e5ccc190 --- /dev/null +++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IValidator.sol @@ -0,0 +1,36 @@ +/* + + 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.23; + +contract IValidator { + + /// @dev Verifies that a signature is valid. + /// @param hash Message hash that is signed. + /// @param signer Address that should have signed the given hash. + /// @param signature Proof of signing. + /// @return Validity of order signature. + function isValidSignature( + bytes32 hash, + address signer, + bytes signature + ) + external + view + returns (bool isValid); +} diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISigner.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IWallet.sol index 53c41d331..c86a2c057 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISigner.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IWallet.sol @@ -18,7 +18,7 @@ pragma solidity ^0.4.24; -contract ISigner { +contract IWallet { /// @dev Verifies that a signature is valid. /// @param hash Message hash that is signed. @@ -26,7 +26,8 @@ contract ISigner { /// @return Validity of order signature. function isValidSignature( bytes32 hash, - bytes signature) + bytes signature + ) external view returns (bool isValid); diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IWrapperFunctions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IWrapperFunctions.sol index 1eb1233ed..8682b394a 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IWrapperFunctions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IWrapperFunctions.sol @@ -19,27 +19,23 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; -import "./libs/LibOrder.sol"; -import "./libs/LibFillResults.sol"; +import "../libs/LibOrder.sol"; +import "../libs/LibFillResults.sol"; contract IWrapperFunctions is - LibBytes, - LibMath, LibOrder, - LibFillResults, - LibExchangeErrors, - MExchangeCore + LibFillResults { /// @dev Fills the input order. Reverts if exact takerAssetFillAmount not filled. /// @param order LibOrder.Order struct containing order specifications. /// @param takerAssetFillAmount Desired amount of takerAsset to sell. /// @param signature Proof that order has been created by maker. function fillOrKillOrder( - LibOrder.LibOrder.Order memory order, + LibOrder.Order memory order, uint256 takerAssetFillAmount, bytes memory signature) public - returns (LibFillResults.LibFillResults.FillResults memory fillResults); + returns (LibFillResults.FillResults memory fillResults); /// @dev Fills an order with specified parameters and ECDSA signature. /// Returns false if the transaction would otherwise revert. diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol index 4712ee36c..7d67e5080 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol @@ -32,6 +32,7 @@ contract LibExchangeErrors { string constant INVALID_ORDER_MAKER_ASSET_AMOUNT = "Invalid order maker asset amount: expected a non-zero value."; // Transaction revert reasons + string constant REENTRANCY_NOT_ALLOWED = "`executeTransaction` is not allowed to call itself recursively."; string constant DUPLICATE_TRANSACTION_HASH = "Transaction has already been executed."; string constant TRANSACTION_EXECUTION_FAILED = "Transaction execution failed."; 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 ccc960d6e..87c5f6361 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol @@ -41,6 +41,7 @@ contract MAssetProxyDispatcher is bytes memory assetMetadata, address from, address to, - uint256 amount) + uint256 amount + ) internal; } 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 3658e7c6f..7eed453ff 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol @@ -25,14 +25,15 @@ contract MSignatureValidator is { // Allowed signature types. enum SignatureType { - Illegal, // Default value - Invalid, - Caller, - Ecrecover, - EIP712, - Trezor, - Contract, - PreSigned + Illegal, // 0x00, default value + Invalid, // 0x01 + EIP712, // 0x02 + EthSign, // 0x03 + Caller, // 0x04 + Wallet, // 0x05 + Validator, // 0x06 + PreSigned, // 0x07 + Trezor // 0x08 } /// @dev Verifies that a signature is valid. @@ -43,7 +44,8 @@ contract MSignatureValidator is function isValidSignature( bytes32 hash, address signer, - bytes memory signature) + bytes memory signature + ) internal view returns (bool isValid); diff --git a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol index 5a6801262..69554605d 100644 --- a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol +++ b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol @@ -25,6 +25,30 @@ contract TestLibBytes is LibBytes { + /// @dev Pops the last byte off of a byte array by modifying its length. + /// @param b Byte array that will be modified. + /// @return The byte that was popped off. + function publicPopByte(bytes memory b) + public + pure + returns (bytes memory, bytes1 result) + { + result = popByte(b); + return (b, result); + } + + /// @dev Pops the last 20 bytes off of a byte array by modifying its length. + /// @param b Byte array that will be modified. + /// @return The 20 byte address that was popped off. + function publicPopAddress(bytes memory b) + public + pure + returns (bytes memory, address result) + { + result = popAddress(b); + return (b, result); + } + /// @dev Tests equality of two byte arrays. /// @param lhs First byte array to compare. /// @param rhs Second byte array to compare. diff --git a/packages/contracts/src/contracts/current/test/TestSignatureValidator/TestSignatureValidator.sol b/packages/contracts/src/contracts/current/test/TestSignatureValidator/TestSignatureValidator.sol index 15d9ca189..0f84678cf 100644 --- a/packages/contracts/src/contracts/current/test/TestSignatureValidator/TestSignatureValidator.sol +++ b/packages/contracts/src/contracts/current/test/TestSignatureValidator/TestSignatureValidator.sol @@ -20,13 +20,18 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; import "../../protocol/Exchange/MixinSignatureValidator.sol"; +import "../../protocol/Exchange/MixinTransactions.sol"; -contract TestSignatureValidator is MixinSignatureValidator { +contract TestSignatureValidator is + MixinSignatureValidator, + MixinTransactions +{ function publicIsValidSignature( bytes32 hash, address signer, - bytes memory signature) + bytes memory signature + ) public view returns (bool isValid) diff --git a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol new file mode 100644 index 000000000..0594e2767 --- /dev/null +++ b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol @@ -0,0 +1,133 @@ +/* + + 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.23; +pragma experimental ABIEncoderV2; + +import "../../protocol/Exchange/interfaces/IExchange.sol"; +import "../../protocol/Exchange/libs/LibOrder.sol"; +import "../../utils/Ownable/Ownable.sol"; + +contract Whitelist is + Ownable +{ + // Revert reasons + string constant MAKER_NOT_WHITELISTED = "Maker address not whitelisted."; + string constant TAKER_NOT_WHITELISTED = "Taker address not whitelisted."; + string constant INVALID_SENDER = "Sender must equal transaction origin."; + + // Mapping of address => whitelist status. + mapping (address => bool) public isWhitelisted; + + // Exchange contract. + IExchange EXCHANGE; + + byte constant VALIDATOR_SIGNATURE_BYTE = "\x06"; + bytes TX_ORIGIN_SIGNATURE; + + constructor (address _exchange) + public + { + EXCHANGE = IExchange(_exchange); + TX_ORIGIN_SIGNATURE = abi.encodePacked(address(this), VALIDATOR_SIGNATURE_BYTE); + } + + /// @dev Adds or removes an address from the whitelist. + /// @param target Address to add or remove from whitelist. + /// @param isApproved Whitelist status to assign to address. + function updateWhitelistStatus( + address target, + bool isApproved + ) + external + onlyOwner + { + isWhitelisted[target] = isApproved; + } + + /// @dev Fills an order using `msg.sender` as the taker. + /// The transaction will revert if both the maker and taker are not whitelisted. + /// Orders should specify this contract as the `senderAddress` in order to gaurantee + /// that both maker and taker have been whitelisted. + /// @param order Order struct containing order specifications. + /// @param takerAssetFillAmount Desired amount of takerAsset to sell. + /// @param salt Arbitrary value to gaurantee uniqueness of 0x transaction hash. + /// @param orderSignature Proof that order has been created by maker. + function fillOrderIfWhitelisted( + LibOrder.Order memory order, + uint256 takerAssetFillAmount, + uint256 salt, + bytes memory orderSignature + ) + public + { + address takerAddress = msg.sender; + + // This contract must be the entry point for the transaction. + require( + takerAddress == tx.origin, + INVALID_SENDER + ); + + // Check if maker is on the whitelist. + require( + isWhitelisted[order.makerAddress], + MAKER_NOT_WHITELISTED + ); + + // Check if taker is on the whitelist. + require( + isWhitelisted[takerAddress], + TAKER_NOT_WHITELISTED + ); + + // Encode arguments into byte array. + bytes memory data = abi.encodeWithSelector( + EXCHANGE.fillOrder.selector, + order, + takerAssetFillAmount, + orderSignature + ); + + // Call `fillOrder` via `executeTransaction`. + EXCHANGE.executeTransaction( + salt, + takerAddress, + data, + TX_ORIGIN_SIGNATURE + ); + } + + /// @dev Verifies signer is same as signer of current Ethereum transaction. + /// NOTE: This function can currently be used to validate signatures coming from outside of this contract. + /// Extra safety checks can be added for a production contract. + /// @param signer Address that should have signed the given hash. + /// @param signature Proof of signing. + /// @return Validity of order signature. + function isValidSignature( + bytes32 hash, + address signer, + bytes signature + ) + external + view + returns (bool isValid) + { + return signer == tx.origin; + } +} diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index 975565773..df2221c93 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -21,15 +21,68 @@ pragma solidity ^0.4.24; contract LibBytes { // Revert reasons + string constant GT_ZERO_LENGTH_REQUIRED = "Length must be greater than 0."; string constant GTE_4_LENGTH_REQUIRED = "Length must be greater than or equal to 4."; string constant GTE_20_LENGTH_REQUIRED = "Length must be greater than or equal to 20."; string constant GTE_32_LENGTH_REQUIRED = "Length must be greater than or equal to 32."; + string constant INDEX_OUT_OF_BOUNDS = "Specified array index is out of bounds."; + + /// @dev Pops the last byte off of a byte array by modifying its length. + /// @param b Byte array that will be modified. + /// @return The byte that was popped off. + function popByte(bytes memory b) + internal + pure + returns (bytes1 result) + { + require( + b.length > 0, + GT_ZERO_LENGTH_REQUIRED + ); + + // Store last byte. + result = b[b.length - 1]; + + assembly { + // Decrement length of byte array. + let newLen := sub(mload(b), 1) + mstore(b, newLen) + } + return result; + } + + /// @dev Pops the last 20 bytes off of a byte array by modifying its length. + /// @param b Byte array that will be modified. + /// @return The 20 byte address that was popped off. + function popAddress(bytes memory b) + internal + pure + returns (address result) + { + require( + b.length >= 20, + GTE_20_LENGTH_REQUIRED + ); + + // Store last 20 bytes. + result = readAddress(b, b.length - 20); + + assembly { + // Subtract 20 from byte array length. + let newLen := sub(mload(b), 20) + mstore(b, newLen) + } + return result; + } /// @dev Tests equality of two byte arrays. /// @param lhs First byte array to compare. /// @param rhs Second byte array to compare. /// @return True if arrays are the same. False otherwise. - function areBytesEqual(bytes memory lhs, bytes memory rhs) + function areBytesEqual( + bytes memory lhs, + bytes memory rhs + ) internal pure returns (bool equal) @@ -63,7 +116,8 @@ contract LibBytes { /// @return address from byte array. function readAddress( bytes memory b, - uint256 index) + uint256 index + ) internal pure returns (address result) @@ -95,7 +149,8 @@ contract LibBytes { function writeAddress( bytes memory b, uint256 index, - address input) + address input + ) internal pure { @@ -132,7 +187,8 @@ contract LibBytes { /// @return bytes32 value from byte array. function readBytes32( bytes memory b, - uint256 index) + uint256 index + ) internal pure returns (bytes32 result) @@ -159,7 +215,8 @@ contract LibBytes { function writeBytes32( bytes memory b, uint256 index, - bytes32 input) + bytes32 input + ) internal pure { @@ -183,7 +240,8 @@ contract LibBytes { /// @return uint256 value from byte array. function readUint256( bytes memory b, - uint256 index) + uint256 index + ) internal pure returns (uint256 result) @@ -198,7 +256,8 @@ contract LibBytes { function writeUint256( bytes memory b, uint256 index, - uint256 input) + uint256 input + ) internal pure { diff --git a/packages/contracts/src/utils/artifacts.ts b/packages/contracts/src/utils/artifacts.ts index fe74ea072..357c66a0a 100644 --- a/packages/contracts/src/utils/artifacts.ts +++ b/packages/contracts/src/utils/artifacts.ts @@ -15,6 +15,7 @@ import * as TestLibs from '../artifacts/TestLibs.json'; import * as TestSignatureValidator from '../artifacts/TestSignatureValidator.json'; import * as TokenRegistry from '../artifacts/TokenRegistry.json'; import * as EtherToken from '../artifacts/WETH9.json'; +import * as Whitelist from '../artifacts/Whitelist.json'; import * as ZRX from '../artifacts/ZRXToken.json'; export const artifacts = { @@ -33,5 +34,6 @@ export const artifacts = { TestLibs: (TestLibs as any) as ContractArtifact, TestSignatureValidator: (TestSignatureValidator as any) as ContractArtifact, TokenRegistry: (TokenRegistry as any) as ContractArtifact, + Whitelist: (Whitelist as any) as ContractArtifact, ZRX: (ZRX as any) as ContractArtifact, }; diff --git a/packages/contracts/src/utils/asset_proxy_utils.ts b/packages/contracts/src/utils/asset_proxy_utils.ts index c042da5d0..a17d4cdfa 100644 --- a/packages/contracts/src/utils/asset_proxy_utils.ts +++ b/packages/contracts/src/utils/asset_proxy_utils.ts @@ -39,7 +39,7 @@ export const assetProxyUtils = { encodeERC20ProxyData(tokenAddress: string): string { const encodedAssetProxyId = assetProxyUtils.encodeAssetProxyId(AssetProxyId.ERC20); const encodedAddress = assetProxyUtils.encodeAddress(tokenAddress); - const encodedMetadata = Buffer.concat([encodedAssetProxyId, encodedAddress]); + const encodedMetadata = Buffer.concat([encodedAddress, encodedAssetProxyId]); const encodedMetadataHex = ethUtil.bufferToHex(encodedMetadata); return encodedMetadataHex; }, @@ -52,7 +52,7 @@ export const assetProxyUtils = { }`, ); } - const encodedAssetProxyId = encodedProxyMetadata.slice(0, 1); + const encodedAssetProxyId = encodedProxyMetadata.slice(-1); const assetProxyId = assetProxyUtils.decodeAssetProxyId(encodedAssetProxyId); if (assetProxyId !== AssetProxyId.ERC20) { throw new Error( @@ -61,7 +61,7 @@ export const assetProxyUtils = { }), but got ${assetProxyId}`, ); } - const encodedTokenAddress = encodedProxyMetadata.slice(1, 21); + const encodedTokenAddress = encodedProxyMetadata.slice(0, 20); const tokenAddress = assetProxyUtils.decodeAddress(encodedTokenAddress); const erc20ProxyData = { assetProxyId, @@ -73,7 +73,7 @@ export const assetProxyUtils = { const encodedAssetProxyId = assetProxyUtils.encodeAssetProxyId(AssetProxyId.ERC721); const encodedAddress = assetProxyUtils.encodeAddress(tokenAddress); const encodedTokenId = assetProxyUtils.encodeUint256(tokenId); - const encodedMetadata = Buffer.concat([encodedAssetProxyId, encodedAddress, encodedTokenId]); + const encodedMetadata = Buffer.concat([encodedAddress, encodedTokenId, encodedAssetProxyId]); const encodedMetadataHex = ethUtil.bufferToHex(encodedMetadata); return encodedMetadataHex; }, @@ -86,7 +86,7 @@ export const assetProxyUtils = { }`, ); } - const encodedAssetProxyId = encodedProxyMetadata.slice(0, 1); + const encodedAssetProxyId = encodedProxyMetadata.slice(-1); const assetProxyId = assetProxyUtils.decodeAssetProxyId(encodedAssetProxyId); if (assetProxyId !== AssetProxyId.ERC721) { throw new Error( @@ -95,9 +95,9 @@ export const assetProxyUtils = { }), but got ${assetProxyId}`, ); } - const encodedTokenAddress = encodedProxyMetadata.slice(1, 21); + const encodedTokenAddress = encodedProxyMetadata.slice(0, 20); const tokenAddress = assetProxyUtils.decodeAddress(encodedTokenAddress); - const encodedTokenId = encodedProxyMetadata.slice(21, 53); + const encodedTokenId = encodedProxyMetadata.slice(20, 52); const tokenId = assetProxyUtils.decodeUint256(encodedTokenId); const erc721ProxyData = { assetProxyId, @@ -115,7 +115,7 @@ export const assetProxyUtils = { }`, ); } - const encodedAssetProxyId = encodedProxyMetadata.slice(0, 1); + const encodedAssetProxyId = encodedProxyMetadata.slice(-1); const assetProxyId = assetProxyUtils.decodeAssetProxyId(encodedAssetProxyId); return assetProxyId; }, diff --git a/packages/contracts/src/utils/constants.ts b/packages/contracts/src/utils/constants.ts index 7a0e26a48..9b0b92545 100644 --- a/packages/contracts/src/utils/constants.ts +++ b/packages/contracts/src/utils/constants.ts @@ -28,6 +28,7 @@ export const constants = { DUMMY_TOKEN_SYMBOL: '', DUMMY_TOKEN_DECIMALS: new BigNumber(18), DUMMY_TOKEN_TOTAL_SUPPLY: new BigNumber(0), + NULL_BYTES: '0x', NUM_DUMMY_ERC20_TO_DEPLOY: 3, NUM_DUMMY_ERC721_TO_DEPLOY: 1, NUM_ERC721_TOKENS_TO_MINT: 2, diff --git a/packages/contracts/src/utils/signing_utils.ts b/packages/contracts/src/utils/signing_utils.ts index 61ab1f138..4c36c8310 100644 --- a/packages/contracts/src/utils/signing_utils.ts +++ b/packages/contracts/src/utils/signing_utils.ts @@ -8,19 +8,19 @@ export const signingUtils = { const prefixedMessage = ethUtil.hashPersonalMessage(message); const ecSignature = ethUtil.ecsign(prefixedMessage, privateKey); const signature = Buffer.concat([ - ethUtil.toBuffer(signatureType), ethUtil.toBuffer(ecSignature.v), ecSignature.r, ecSignature.s, + ethUtil.toBuffer(signatureType), ]); return signature; } else if (signatureType === SignatureType.EIP712) { const ecSignature = ethUtil.ecsign(message, privateKey); const signature = Buffer.concat([ - ethUtil.toBuffer(signatureType), ethUtil.toBuffer(ecSignature.v), ecSignature.r, ecSignature.s, + ethUtil.toBuffer(signatureType), ]); return signature; } else { diff --git a/packages/contracts/src/utils/transaction_factory.ts b/packages/contracts/src/utils/transaction_factory.ts index 941bff96d..65cdb3f89 100644 --- a/packages/contracts/src/utils/transaction_factory.ts +++ b/packages/contracts/src/utils/transaction_factory.ts @@ -7,26 +7,25 @@ import { signingUtils } from './signing_utils'; import { SignatureType, SignedTransaction } from './types'; export class TransactionFactory { - private _signer: string; + private _signerBuff: Buffer; private _exchangeAddress: string; private _privateKey: Buffer; constructor(privateKey: Buffer, exchangeAddress: string) { this._privateKey = privateKey; this._exchangeAddress = exchangeAddress; - const signerBuff = ethUtil.privateToAddress(this._privateKey); - this._signer = `0x${signerBuff.toString('hex')}`; + this._signerBuff = ethUtil.privateToAddress(this._privateKey); } public newSignedTransaction( data: string, signatureType: SignatureType = SignatureType.Ecrecover, ): SignedTransaction { const salt = generatePseudoRandomSalt(); - const txHash = crypto.solSHA3([this._exchangeAddress, salt, ethUtil.toBuffer(data)]); + const txHash = crypto.solSHA3([this._exchangeAddress, this._signerBuff, salt, ethUtil.toBuffer(data)]); const signature = signingUtils.signMessage(txHash, this._privateKey, signatureType); const signedTx = { exchangeAddress: this._exchangeAddress, salt, - signer: this._signer, + signer: `0x${this._signerBuff.toString('hex')}`, data, signature: `0x${signature.toString('hex')}`, }; diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts index 90f90ec27..1eeffc70e 100644 --- a/packages/contracts/src/utils/types.ts +++ b/packages/contracts/src/utils/types.ts @@ -111,16 +111,19 @@ export enum ContractName { DummyERC721Token = 'DummyERC721Token', TestLibBytes = 'TestLibBytes', Authorizable = 'Authorizable', + Whitelist = 'Whitelist', } export enum SignatureType { Illegal, Invalid, - Caller, - Ecrecover, EIP712, - Trezor, + Ecrecover, + TxOrigin, + Caller, Contract, + PreSigned, + Trezor, } export interface SignedTransaction { diff --git a/packages/contracts/src/utils/web3_wrapper.ts b/packages/contracts/src/utils/web3_wrapper.ts index 02595506b..4b8512222 100644 --- a/packages/contracts/src/utils/web3_wrapper.ts +++ b/packages/contracts/src/utils/web3_wrapper.ts @@ -7,7 +7,7 @@ import { coverage } from './coverage'; export const txDefaults = { from: devConstants.TESTRPC_FIRST_ADDRESS, - gas: devConstants.GAS_ESTIMATE, + gas: devConstants.GAS_LIMIT, }; const providerConfigs = { shouldUseInProcessGanache: true }; export const provider = web3Factory.getRpcProvider(providerConfigs); |