diff options
Diffstat (limited to 'packages/contracts')
42 files changed, 717 insertions, 188 deletions
diff --git a/packages/contracts/compiler.json b/packages/contracts/compiler.json index 7d469d2c3..ed59069fe 100644 --- a/packages/contracts/compiler.json +++ b/packages/contracts/compiler.json @@ -33,6 +33,7 @@ "TestLibs", "TestSignatureValidator", "TokenRegistry", + "Whitelist", "WETH9", "ZRXToken" ] diff --git a/packages/contracts/package.json b/packages/contracts/package.json index 4e9eaba76..8d29366a4 100644 --- a/packages/contracts/package.json +++ b/packages/contracts/package.json @@ -29,7 +29,7 @@ "test:circleci": "yarn test" }, "config": { - "abis": "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Token|ERC20Proxy|ERC721Proxy|Exchange|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock||TestAssetProxyDispatcher|TestLibBytes|TestLibs|TestSignatureValidator|TokenRegistry|WETH9|ZRXToken).json" + "abis": "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Token|ERC20Proxy|ERC721Proxy|Exchange|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|TestAssetProxyDispatcher|TestLibBytes|TestLibs|TestSignatureValidator|TokenRegistry|Whitelist|WETH9|ZRXToken).json" }, "repository": { "type": "git", 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 af1199f66..79dc87af0 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)) public 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/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/exchange_wrapper.ts b/packages/contracts/src/utils/exchange_wrapper.ts index f7bd207a4..8f5915d97 100644 --- a/packages/contracts/src/utils/exchange_wrapper.ts +++ b/packages/contracts/src/utils/exchange_wrapper.ts @@ -7,16 +7,18 @@ import { ExchangeContract } from '../contract_wrappers/generated/exchange'; import { constants } from './constants'; import { formatters } from './formatters'; -import { logDecoder } from './log_decoder'; +import { LogDecoder } from './log_decoder'; import { orderUtils } from './order_utils'; import { OrderInfo, SignedTransaction } from './types'; export class ExchangeWrapper { private _exchange: ExchangeContract; private _web3Wrapper: Web3Wrapper; + private _logDecoder: LogDecoder; constructor(exchangeContract: ExchangeContract, provider: Provider) { this._exchange = exchangeContract; this._web3Wrapper = new Web3Wrapper(provider); + this._logDecoder = new LogDecoder(this._web3Wrapper, this._exchange.address); } public async fillOrderAsync( signedOrder: SignedOrder, @@ -30,13 +32,13 @@ export class ExchangeWrapper { params.signature, { from }, ); - const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash); + const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; } public async cancelOrderAsync(signedOrder: SignedOrder, from: string): Promise<TransactionReceiptWithDecodedLogs> { const params = orderUtils.createCancel(signedOrder); const txHash = await this._exchange.cancelOrder.sendTransactionAsync(params.order, { from }); - const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash); + const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; } public async fillOrKillOrderAsync( @@ -51,7 +53,7 @@ export class ExchangeWrapper { params.signature, { from }, ); - const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash); + const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; } public async fillOrderNoThrowAsync( @@ -66,7 +68,7 @@ export class ExchangeWrapper { params.signature, { from }, ); - const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash); + const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; } public async batchFillOrdersAsync( @@ -81,7 +83,7 @@ export class ExchangeWrapper { params.signatures, { from }, ); - const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash); + const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; } public async batchFillOrKillOrdersAsync( @@ -96,7 +98,7 @@ export class ExchangeWrapper { params.signatures, { from }, ); - const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash); + const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; } public async batchFillOrdersNoThrowAsync( @@ -111,7 +113,7 @@ export class ExchangeWrapper { params.signatures, { from }, ); - const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash); + const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; } public async marketSellOrdersAsync( @@ -126,7 +128,7 @@ export class ExchangeWrapper { params.signatures, { from }, ); - const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash); + const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; } public async marketSellOrdersNoThrowAsync( @@ -141,7 +143,7 @@ export class ExchangeWrapper { params.signatures, { from }, ); - const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash); + const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; } public async marketBuyOrdersAsync( @@ -156,7 +158,7 @@ export class ExchangeWrapper { params.signatures, { from }, ); - const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash); + const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; } public async marketBuyOrdersNoThrowAsync( @@ -171,7 +173,7 @@ export class ExchangeWrapper { params.signatures, { from }, ); - const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash); + const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; } public async batchCancelOrdersAsync( @@ -180,12 +182,12 @@ export class ExchangeWrapper { ): Promise<TransactionReceiptWithDecodedLogs> { const params = formatters.createBatchCancel(orders); const txHash = await this._exchange.batchCancelOrders.sendTransactionAsync(params.orders, { from }); - const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash); + const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; } public async cancelOrdersUpToAsync(salt: BigNumber, from: string): Promise<TransactionReceiptWithDecodedLogs> { const txHash = await this._exchange.cancelOrdersUpTo.sendTransactionAsync(salt, { from }); - const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash); + const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; } public async registerAssetProxyAsync( @@ -203,7 +205,7 @@ export class ExchangeWrapper { oldAssetProxyAddress, { from }, ); - const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash); + const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; } public async executeTransactionAsync( @@ -217,7 +219,7 @@ export class ExchangeWrapper { signedTx.signature, { from }, ); - const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash); + const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; } public async getTakerAssetFilledAmountAsync(orderHashHex: string): Promise<BigNumber> { @@ -241,13 +243,7 @@ export class ExchangeWrapper { params.rightSignature, { from }, ); - const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash); - return tx; - } - private async _getTxWithDecodedExchangeLogsAsync(txHash: string): Promise<TransactionReceiptWithDecodedLogs> { - const tx = await this._web3Wrapper.awaitTransactionSuccessAsync(txHash, constants.AWAIT_TRANSACTION_MINED_MS); - tx.logs = _.filter(tx.logs, log => log.address === this._exchange.address); - tx.logs = _.map(tx.logs, log => logDecoder.decodeLogOrThrow(log)); + const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; } } diff --git a/packages/contracts/src/utils/log_decoder.ts b/packages/contracts/src/utils/log_decoder.ts index d2e65d176..32819b657 100644 --- a/packages/contracts/src/utils/log_decoder.ts +++ b/packages/contracts/src/utils/log_decoder.ts @@ -1,19 +1,23 @@ import { ContractArtifact } from '@0xproject/sol-compiler'; -import { AbiDefinition, LogEntry, LogWithDecodedArgs, RawLog } from '@0xproject/types'; +import { + AbiDefinition, + LogEntry, + LogWithDecodedArgs, + RawLog, + TransactionReceiptWithDecodedLogs, +} from '@0xproject/types'; import { AbiDecoder, BigNumber } from '@0xproject/utils'; +import { Web3Wrapper } from '@0xproject/web3-wrapper'; import * as _ from 'lodash'; import { artifacts } from './artifacts'; +import { constants } from './constants'; -const abiArrays: AbiDefinition[][] = []; -_.forEach(artifacts, (artifact: ContractArtifact) => { - const compilerOutput = artifact.compilerOutput; - abiArrays.push(compilerOutput.abi); -}); -const abiDecoder = new AbiDecoder(abiArrays); - -export const logDecoder = { - wrapLogBigNumbers(log: any): any { +export class LogDecoder { + private _web3Wrapper: Web3Wrapper; + private _contractAddress: string; + private _abiDecoder: AbiDecoder; + public static wrapLogBigNumbers(log: any): any { const argNames = _.keys(log.args); for (const argName of argNames) { const isWeb3BigNumber = _.startsWith(log.args[argName].constructor.toString(), 'function BigNumber('); @@ -21,13 +25,29 @@ export const logDecoder = { log.args[argName] = new BigNumber(log.args[argName]); } } - }, - decodeLogOrThrow<ArgsType>(log: LogEntry): LogWithDecodedArgs<ArgsType> | RawLog { - const logWithDecodedArgsOrLog = abiDecoder.tryToDecodeLogOrNoop(log); + } + constructor(web3Wrapper: Web3Wrapper, contractAddress: string) { + this._web3Wrapper = web3Wrapper; + this._contractAddress = contractAddress; + const abiArrays: AbiDefinition[][] = []; + _.forEach(artifacts, (artifact: ContractArtifact) => { + const compilerOutput = artifact.compilerOutput; + abiArrays.push(compilerOutput.abi); + }); + this._abiDecoder = new AbiDecoder(abiArrays); + } + public decodeLogOrThrow<ArgsType>(log: LogEntry): LogWithDecodedArgs<ArgsType> | RawLog { + const logWithDecodedArgsOrLog = this._abiDecoder.tryToDecodeLogOrNoop(log); if (_.isUndefined((logWithDecodedArgsOrLog as LogWithDecodedArgs<ArgsType>).args)) { throw new Error(`Unable to decode log: ${JSON.stringify(log)}`); } - logDecoder.wrapLogBigNumbers(logWithDecodedArgsOrLog); + LogDecoder.wrapLogBigNumbers(logWithDecodedArgsOrLog); return logWithDecodedArgsOrLog; - }, -}; + } + public async getTxWithDecodedLogsAsync(txHash: string): Promise<TransactionReceiptWithDecodedLogs> { + const tx = await this._web3Wrapper.awaitTransactionSuccessAsync(txHash, constants.AWAIT_TRANSACTION_MINED_MS); + tx.logs = _.filter(tx.logs, log => log.address === this._contractAddress); + tx.logs = _.map(tx.logs, log => this.decodeLogOrThrow(log)); + return tx; + } +} diff --git a/packages/contracts/src/utils/multi_sig_wrapper.ts b/packages/contracts/src/utils/multi_sig_wrapper.ts index 5c73cdf5a..d67692194 100644 --- a/packages/contracts/src/utils/multi_sig_wrapper.ts +++ b/packages/contracts/src/utils/multi_sig_wrapper.ts @@ -7,14 +7,16 @@ import { AssetProxyOwnerContract } from '../contract_wrappers/generated/asset_pr import { MultiSigWalletContract } from '../contract_wrappers/generated/multi_sig_wallet'; import { constants } from './constants'; -import { logDecoder } from './log_decoder'; +import { LogDecoder } from './log_decoder'; export class MultiSigWrapper { private _multiSig: MultiSigWalletContract; private _web3Wrapper: Web3Wrapper; + private _logDecoder: LogDecoder; constructor(multiSigContract: MultiSigWalletContract, provider: Provider) { this._multiSig = multiSigContract; this._web3Wrapper = new Web3Wrapper(provider); + this._logDecoder = new LogDecoder(this._web3Wrapper, this._multiSig.address); } public async submitTransactionAsync( destination: string, @@ -26,17 +28,17 @@ export class MultiSigWrapper { const txHash = await this._multiSig.submitTransaction.sendTransactionAsync(destination, value, data, { from, }); - const tx = await this._getTxWithDecodedMultiSigLogsAsync(txHash); + const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; } public async confirmTransactionAsync(txId: BigNumber, from: string): Promise<TransactionReceiptWithDecodedLogs> { const txHash = await this._multiSig.confirmTransaction.sendTransactionAsync(txId, { from }); - const tx = await this._getTxWithDecodedMultiSigLogsAsync(txHash); + const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; } public async executeTransactionAsync(txId: BigNumber, from: string): Promise<TransactionReceiptWithDecodedLogs> { const txHash = await this._multiSig.executeTransaction.sendTransactionAsync(txId, { from }); - const tx = await this._getTxWithDecodedMultiSigLogsAsync(txHash); + const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; } public async executeRemoveAuthorizedAddressAsync( @@ -45,13 +47,7 @@ export class MultiSigWrapper { ): Promise<TransactionReceiptWithDecodedLogs> { const txHash = await (this ._multiSig as AssetProxyOwnerContract).executeRemoveAuthorizedAddress.sendTransactionAsync(txId, { from }); - const tx = await this._getTxWithDecodedMultiSigLogsAsync(txHash); - return tx; - } - private async _getTxWithDecodedMultiSigLogsAsync(txHash: string): Promise<TransactionReceiptWithDecodedLogs> { - const tx = await this._web3Wrapper.awaitTransactionMinedAsync(txHash, constants.AWAIT_TRANSACTION_MINED_MS); - tx.logs = _.filter(tx.logs, log => log.address === this._multiSig.address); - tx.logs = _.map(tx.logs, log => logDecoder.decodeLogOrThrow(log)); + const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; } } diff --git a/packages/contracts/src/utils/signing_utils.ts b/packages/contracts/src/utils/signing_utils.ts index 41d63832e..d56d2cd44 100644 --- a/packages/contracts/src/utils/signing_utils.ts +++ b/packages/contracts/src/utils/signing_utils.ts @@ -7,19 +7,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 41907132a..f66d55f9b 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 { 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 9a0c95504..a6c90dae0 100644 --- a/packages/contracts/src/utils/types.ts +++ b/packages/contracts/src/utils/types.ts @@ -105,6 +105,7 @@ export enum ContractName { DummyERC721Token = 'DummyERC721Token', TestLibBytes = 'TestLibBytes', Authorizable = 'Authorizable', + Whitelist = 'Whitelist', } 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); diff --git a/packages/contracts/test/asset_proxy_owner.ts b/packages/contracts/test/asset_proxy_owner.ts index db68b5678..43c5da512 100644 --- a/packages/contracts/test/asset_proxy_owner.ts +++ b/packages/contracts/test/asset_proxy_owner.ts @@ -36,6 +36,12 @@ describe('AssetProxyOwner', () => { let multiSigWrapper: MultiSigWrapper; before(async () => { + await blockchainLifecycle.startAsync(); + }); + after(async () => { + await blockchainLifecycle.revertAsync(); + }); + before(async () => { const accounts = await web3Wrapper.getAvailableAddressesAsync(); owners = [accounts[0], accounts[1]]; const initialOwner = (authorized = accounts[0]); diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts index e8397f8a2..adc57c7a4 100644 --- a/packages/contracts/test/exchange/core.ts +++ b/packages/contracts/test/exchange/core.ts @@ -457,10 +457,11 @@ describe('Exchange core', () => { makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), }); + const v = ethUtil.toBuffer(signedOrder.signature.slice(0, 4)); const invalidR = ethUtil.sha3('invalidR'); const invalidS = ethUtil.sha3('invalidS'); - const signatureTypeAndV = signedOrder.signature.slice(0, 6); - const invalidSigBuff = Buffer.concat([ethUtil.toBuffer(signatureTypeAndV), invalidR, invalidS]); + const signatureType = ethUtil.toBuffer(`0x${signedOrder.signature.slice(-2)}`); + const invalidSigBuff = Buffer.concat([v, invalidR, invalidS, signatureType]); const invalidSigHex = `0x${invalidSigBuff.toString('hex')}`; signedOrder.signature = invalidSigHex; return expect(exchangeWrapper.fillOrderAsync(signedOrder, takerAddress)).to.be.rejectedWith( diff --git a/packages/contracts/test/exchange/libs.ts b/packages/contracts/test/exchange/libs.ts index 5f2c24ac0..10cb8b34e 100644 --- a/packages/contracts/test/exchange/libs.ts +++ b/packages/contracts/test/exchange/libs.ts @@ -48,7 +48,6 @@ describe('Exchange libs', () => { beforeEach(async () => { await blockchainLifecycle.startAsync(); - signedOrder = orderFactory.newSignedOrder(); }); afterEach(async () => { await blockchainLifecycle.revertAsync(); @@ -68,7 +67,8 @@ describe('Exchange libs', () => { }); }); describe('getOrderHash', () => { - it('should output the correct order hash', async () => { + it('should output the correct orderHash', async () => { + signedOrder = orderFactory.newSignedOrder(); const orderHashHex = await libs.publicGetOrderHash.callAsync(signedOrder); expect(orderHashUtils.getOrderHashHex(signedOrder)).to.be.equal(orderHashHex); }); diff --git a/packages/contracts/test/exchange/signature_validator.ts b/packages/contracts/test/exchange/signature_validator.ts index a41a19426..194053abc 100644 --- a/packages/contracts/test/exchange/signature_validator.ts +++ b/packages/contracts/test/exchange/signature_validator.ts @@ -53,7 +53,6 @@ describe('MixinSignatureValidator', () => { beforeEach(async () => { await blockchainLifecycle.startAsync(); - signedOrder = orderFactory.newSignedOrder(); }); afterEach(async () => { await blockchainLifecycle.revertAsync(); @@ -75,13 +74,11 @@ describe('MixinSignatureValidator', () => { }); it('should return false with an invalid signature', async () => { + const v = ethUtil.toBuffer(signedOrder.signature.slice(0, 4)); const invalidR = ethUtil.sha3('invalidR'); const invalidS = ethUtil.sha3('invalidS'); - const invalidSigBuff = Buffer.concat([ - ethUtil.toBuffer(signedOrder.signature.slice(0, 6)), - invalidR, - invalidS, - ]); + const signatureType = ethUtil.toBuffer(`0x${signedOrder.signature.slice(-2)}`); + const invalidSigBuff = Buffer.concat([v, invalidR, invalidS, signatureType]); const invalidSigHex = `0x${invalidSigBuff.toString('hex')}`; signedOrder.signature = invalidSigHex; const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); diff --git a/packages/contracts/test/exchange/transactions.ts b/packages/contracts/test/exchange/transactions.ts index 7ee194aa1..ef7595562 100644 --- a/packages/contracts/test/exchange/transactions.ts +++ b/packages/contracts/test/exchange/transactions.ts @@ -1,5 +1,5 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; -import { assetProxyUtils } from '@0xproject/order-utils'; +import { assetProxyUtils, generatePseudoRandomSalt } from '@0xproject/order-utils'; import { AssetProxyId, Order, OrderWithoutExchangeAddress, SignatureType, SignedOrder } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import * as chai from 'chai'; @@ -9,6 +9,7 @@ import * as Web3 from 'web3'; import { DummyERC20TokenContract } from '../../src/contract_wrappers/generated/dummy_e_r_c20_token'; import { ERC20ProxyContract } from '../../src/contract_wrappers/generated/e_r_c20_proxy'; import { ExchangeContract } from '../../src/contract_wrappers/generated/exchange'; +import { WhitelistContract } from '../../src/contract_wrappers/generated/whitelist'; import { artifacts } from '../../src/utils/artifacts'; import { chaiSetup } from '../../src/utils/chai_setup'; import { constants } from '../../src/utils/constants'; @@ -49,6 +50,8 @@ describe('Exchange transactions', () => { let defaultMakerTokenAddress: string; let defaultTakerTokenAddress: string; + let makerPrivateKey: Buffer; + let takerPrivateKey: Buffer; before(async () => { await blockchainLifecycle.startAsync(); @@ -92,8 +95,8 @@ describe('Exchange transactions', () => { makerAssetData: assetProxyUtils.encodeERC20ProxyData(defaultMakerTokenAddress), takerAssetData: assetProxyUtils.encodeERC20ProxyData(defaultTakerTokenAddress), }; - const makerPrivateKey = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(makerAddress)]; - const takerPrivateKey = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(takerAddress)]; + makerPrivateKey = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(makerAddress)]; + takerPrivateKey = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(takerAddress)]; orderFactory = new OrderFactory(makerPrivateKey, defaultOrderParams); makerTransactionFactory = new TransactionFactory(makerPrivateKey, exchange.address); takerTransactionFactory = new TransactionFactory(takerPrivateKey, exchange.address); @@ -197,4 +200,131 @@ describe('Exchange transactions', () => { }); }); }); + + describe('Whitelist', () => { + let whitelist: WhitelistContract; + let whitelistOrderFactory: OrderFactory; + + before(async () => { + whitelist = await WhitelistContract.deployFrom0xArtifactAsync( + artifacts.Whitelist, + provider, + txDefaults, + exchange.address, + ); + const isApproved = true; + await web3Wrapper.awaitTransactionSuccessAsync( + await exchange.approveSignatureValidator.sendTransactionAsync(whitelist.address, isApproved, { + from: takerAddress, + }), + ); + const defaultOrderParams = { + ...constants.STATIC_ORDER_PARAMS, + senderAddress: whitelist.address, + exchangeAddress: exchange.address, + makerAddress, + feeRecipientAddress, + makerAssetData: assetProxyUtils.encodeERC20ProxyData(defaultMakerTokenAddress), + takerAssetData: assetProxyUtils.encodeERC20ProxyData(defaultTakerTokenAddress), + }; + whitelistOrderFactory = new OrderFactory(makerPrivateKey, defaultOrderParams); + }); + + beforeEach(async () => { + signedOrder = whitelistOrderFactory.newSignedOrder(); + erc20Balances = await erc20Wrapper.getBalancesAsync(); + }); + + it('should revert if maker has not been whitelisted', async () => { + const isApproved = true; + await web3Wrapper.awaitTransactionSuccessAsync( + await whitelist.updateWhitelistStatus.sendTransactionAsync(takerAddress, isApproved, { from: owner }), + ); + + orderWithoutExchangeAddress = orderUtils.getOrderWithoutExchangeAddress(signedOrder); + const takerAssetFillAmount = signedOrder.takerAssetAmount; + const salt = generatePseudoRandomSalt(); + return expect( + whitelist.fillOrderIfWhitelisted.sendTransactionAsync( + orderWithoutExchangeAddress, + takerAssetFillAmount, + salt, + signedOrder.signature, + { from: takerAddress }, + ), + ).to.be.rejectedWith(constants.REVERT); + }); + + it('should revert if taker has not been whitelisted', async () => { + const isApproved = true; + await web3Wrapper.awaitTransactionSuccessAsync( + await whitelist.updateWhitelistStatus.sendTransactionAsync(makerAddress, isApproved, { from: owner }), + ); + + orderWithoutExchangeAddress = orderUtils.getOrderWithoutExchangeAddress(signedOrder); + const takerAssetFillAmount = signedOrder.takerAssetAmount; + const salt = generatePseudoRandomSalt(); + return expect( + whitelist.fillOrderIfWhitelisted.sendTransactionAsync( + orderWithoutExchangeAddress, + takerAssetFillAmount, + salt, + signedOrder.signature, + { from: takerAddress }, + ), + ).to.be.rejectedWith(constants.REVERT); + }); + + it('should fill the order if maker and taker have been whitelisted', async () => { + const isApproved = true; + await web3Wrapper.awaitTransactionSuccessAsync( + await whitelist.updateWhitelistStatus.sendTransactionAsync(makerAddress, isApproved, { from: owner }), + ); + + await web3Wrapper.awaitTransactionSuccessAsync( + await whitelist.updateWhitelistStatus.sendTransactionAsync(takerAddress, isApproved, { from: owner }), + ); + + orderWithoutExchangeAddress = orderUtils.getOrderWithoutExchangeAddress(signedOrder); + const takerAssetFillAmount = signedOrder.takerAssetAmount; + const salt = generatePseudoRandomSalt(); + await web3Wrapper.awaitTransactionSuccessAsync( + await whitelist.fillOrderIfWhitelisted.sendTransactionAsync( + orderWithoutExchangeAddress, + takerAssetFillAmount, + salt, + signedOrder.signature, + { from: takerAddress }, + ), + ); + + const newBalances = await erc20Wrapper.getBalancesAsync(); + + const makerAssetFillAmount = signedOrder.makerAssetAmount; + const makerFeePaid = signedOrder.makerFee; + const takerFeePaid = signedOrder.takerFee; + + expect(newBalances[makerAddress][defaultMakerTokenAddress]).to.be.bignumber.equal( + erc20Balances[makerAddress][defaultMakerTokenAddress].minus(makerAssetFillAmount), + ); + expect(newBalances[makerAddress][defaultTakerTokenAddress]).to.be.bignumber.equal( + erc20Balances[makerAddress][defaultTakerTokenAddress].add(takerAssetFillAmount), + ); + expect(newBalances[makerAddress][zrxToken.address]).to.be.bignumber.equal( + erc20Balances[makerAddress][zrxToken.address].minus(makerFeePaid), + ); + expect(newBalances[takerAddress][defaultTakerTokenAddress]).to.be.bignumber.equal( + erc20Balances[takerAddress][defaultTakerTokenAddress].minus(takerAssetFillAmount), + ); + expect(newBalances[takerAddress][defaultMakerTokenAddress]).to.be.bignumber.equal( + erc20Balances[takerAddress][defaultMakerTokenAddress].add(makerAssetFillAmount), + ); + expect(newBalances[takerAddress][zrxToken.address]).to.be.bignumber.equal( + erc20Balances[takerAddress][zrxToken.address].minus(takerFeePaid), + ); + expect(newBalances[feeRecipientAddress][zrxToken.address]).to.be.bignumber.equal( + erc20Balances[feeRecipientAddress][zrxToken.address].add(makerFeePaid.add(takerFeePaid)), + ); + }); + }); }); diff --git a/packages/contracts/test/libraries/lib_bytes.ts b/packages/contracts/test/libraries/lib_bytes.ts index de4011853..32b319e56 100644 --- a/packages/contracts/test/libraries/lib_bytes.ts +++ b/packages/contracts/test/libraries/lib_bytes.ts @@ -21,6 +21,7 @@ describe('LibBytes', () => { let owner: string; let libBytes: TestLibBytesContract; const byteArrayShorterThan32Bytes = '0x012345'; + const byteArrayShorterThan20Bytes = byteArrayShorterThan32Bytes; const byteArrayLongerThan32Bytes = '0x0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef'; const byteArrayLongerThan32BytesFirstBytesSwapped = @@ -59,6 +60,36 @@ describe('LibBytes', () => { await blockchainLifecycle.revertAsync(); }); + describe('popByte', () => { + it('should revert if length is 0', async () => { + return expect(libBytes.publicPopByte.callAsync(constants.NULL_BYTES)).to.be.rejectedWith(constants.REVERT); + }); + + it('should pop the last byte from the input and return it', async () => { + const [newBytes, poppedByte] = await libBytes.publicPopByte.callAsync(byteArrayLongerThan32Bytes); + const expectedNewBytes = byteArrayLongerThan32Bytes.slice(0, -2); + const expectedPoppedByte = `0x${byteArrayLongerThan32Bytes.slice(-2)}`; + expect(newBytes).to.equal(expectedNewBytes); + expect(poppedByte).to.equal(expectedPoppedByte); + }); + }); + + describe('popAddress', () => { + it('should revert if length is less than 20', async () => { + return expect(libBytes.publicPopAddress.callAsync(byteArrayShorterThan20Bytes)).to.be.rejectedWith( + constants.REVERT, + ); + }); + + it('should pop the last 20 bytes from the input and return it', async () => { + const [newBytes, poppedAddress] = await libBytes.publicPopAddress.callAsync(byteArrayLongerThan32Bytes); + const expectedNewBytes = byteArrayLongerThan32Bytes.slice(0, -40); + const expectedPoppedAddress = `0x${byteArrayLongerThan32Bytes.slice(-40)}`; + expect(newBytes).to.equal(expectedNewBytes); + expect(poppedAddress).to.equal(expectedPoppedAddress); + }); + }); + describe('areBytesEqual', () => { it('should return true if byte arrays are equal (both arrays < 32 bytes)', async () => { const areBytesEqual = await libBytes.publicAreBytesEqual.callAsync( diff --git a/packages/contracts/test/multi_sig_with_time_lock.ts b/packages/contracts/test/multi_sig_with_time_lock.ts index ace0f0045..7b7885a00 100644 --- a/packages/contracts/test/multi_sig_with_time_lock.ts +++ b/packages/contracts/test/multi_sig_with_time_lock.ts @@ -27,6 +27,12 @@ describe('MultiSigWalletWithTimeLock', () => { const SECONDS_TIME_LOCKED = new BigNumber(1000000); before(async () => { + await blockchainLifecycle.startAsync(); + }); + after(async () => { + await blockchainLifecycle.revertAsync(); + }); + before(async () => { const accounts = await web3Wrapper.getAvailableAddressesAsync(); owners = [accounts[0], accounts[1]]; }); diff --git a/packages/contracts/test/token_registry.ts b/packages/contracts/test/token_registry.ts index 1cc519c53..7980977c8 100644 --- a/packages/contracts/test/token_registry.ts +++ b/packages/contracts/test/token_registry.ts @@ -23,6 +23,7 @@ describe('TokenRegistry', () => { let notOwner: string; let tokenReg: TokenRegistryContract; let tokenRegWrapper: TokenRegWrapper; + before(async () => { await blockchainLifecycle.startAsync(); }); |