diff options
Diffstat (limited to 'packages/contracts')
28 files changed, 889 insertions, 321 deletions
diff --git a/packages/contracts/compiler.json b/packages/contracts/compiler.json index a5cfa8761..fa7ede817 100644 --- a/packages/contracts/compiler.json +++ b/packages/contracts/compiler.json @@ -36,6 +36,8 @@ "TestLibMem", "TestLibs", "TestSignatureValidator", + "TestValidator", + "TestWallet", "TokenRegistry", "Whitelist", "WETH9", diff --git a/packages/contracts/package.json b/packages/contracts/package.json index cf3f6f01d..9e7dad674 100644 --- a/packages/contracts/package.json +++ b/packages/contracts/package.json @@ -34,7 +34,7 @@ }, "config": { "abis": - "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Receiver|DummyERC721Token|ERC20Proxy|ERC721Proxy|Exchange|ExchangeWrapper|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|TestAssetDataDecoders|TestAssetProxyDispatcher|TestLibBytes|TestLibMem|TestLibs|TestSignatureValidator|TokenRegistry|Whitelist|WETH9|ZRXToken).json" + "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Receiver|DummyERC721Token|ERC20Proxy|ERC721Proxy|Exchange|ExchangeWrapper|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|TestAssetDataDecoders|TestAssetProxyDispatcher|TestLibBytes|TestLibMem|TestLibs|TestSignatureValidator|TestValidator|TestWallet|TokenRegistry|Whitelist|WETH9|ZRXToken).json" }, "repository": { "type": "git", diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol index d09aba599..9dc9e6525 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol @@ -53,13 +53,7 @@ contract MixinERC721Transfer is bytes memory receiverData ) = decodeERC721AssetData(assetData); - // Transfer token. Saves gas by calling safeTransferFrom only - // when there is receiverData present. Either succeeds or throws. - if (receiverData.length > 0) { - ERC721Token(token).safeTransferFrom(from, to, tokenId, receiverData); - } else { - ERC721Token(token).transferFrom(from, to, tokenId); - } + ERC721Token(token).safeTransferFrom(from, to, tokenId, receiverData); } /// @dev Decodes ERC721 Asset data. diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/Exchange.sol b/packages/contracts/src/contracts/current/protocol/Exchange/Exchange.sol index 51f99bafa..d36e9633e 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/Exchange.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/Exchange.sol @@ -19,9 +19,9 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; +import "./libs/LibConstants.sol"; import "./MixinExchangeCore.sol"; import "./MixinSignatureValidator.sol"; -import "./MixinSettlement.sol"; import "./MixinWrapperFunctions.sol"; import "./MixinAssetProxyDispatcher.sol"; import "./MixinTransactions.sol"; @@ -30,7 +30,6 @@ import "./MixinMatchOrders.sol"; contract Exchange is MixinExchangeCore, MixinMatchOrders, - MixinSettlement, MixinSignatureValidator, MixinTransactions, MixinAssetProxyDispatcher, @@ -42,9 +41,9 @@ contract Exchange is // Mixins are instantiated in the order they are inherited constructor (bytes memory _zrxAssetData) public + LibConstants(_zrxAssetData) // @TODO: Remove when we deploy. MixinExchangeCore() MixinMatchOrders() - MixinSettlement(_zrxAssetData) MixinSignatureValidator() MixinTransactions() MixinAssetProxyDispatcher() diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol index c406354a7..8539342b7 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol @@ -19,22 +19,26 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; +import "./libs/LibConstants.sol"; +import "../../utils/LibBytes/LibBytes.sol"; import "./libs/LibFillResults.sol"; import "./libs/LibOrder.sol"; import "./libs/LibMath.sol"; import "./libs/LibExchangeErrors.sol"; import "./mixins/MExchangeCore.sol"; -import "./mixins/MSettlement.sol"; import "./mixins/MSignatureValidator.sol"; import "./mixins/MTransactions.sol"; +import "./mixins/MAssetProxyDispatcher.sol"; contract MixinExchangeCore is + LibConstants, + LibBytes, LibMath, LibOrder, LibFillResults, LibExchangeErrors, + MAssetProxyDispatcher, MExchangeCore, - MSettlement, MSignatureValidator, MTransactions { @@ -389,4 +393,48 @@ contract MixinExchangeCore is return fillResults; } + + /// @dev Settles an order by transferring assets between counterparties. + /// @param order Order struct containing order specifications. + /// @param takerAddress Address selling takerAsset and buying makerAsset. + /// @param fillResults Amounts to be filled and fees paid by maker and taker. + function settleOrder( + LibOrder.Order memory order, + address takerAddress, + LibFillResults.FillResults memory fillResults + ) + private + { + uint8 makerAssetProxyId = uint8(popLastByte(order.makerAssetData)); + uint8 takerAssetProxyId = uint8(popLastByte(order.takerAssetData)); + bytes memory zrxAssetData = ZRX_ASSET_DATA; + dispatchTransferFrom( + order.makerAssetData, + makerAssetProxyId, + order.makerAddress, + takerAddress, + fillResults.makerAssetFilledAmount + ); + dispatchTransferFrom( + order.takerAssetData, + takerAssetProxyId, + takerAddress, + order.makerAddress, + fillResults.takerAssetFilledAmount + ); + dispatchTransferFrom( + zrxAssetData, + ZRX_PROXY_ID, + order.makerAddress, + order.feeRecipientAddress, + fillResults.makerFeePaid + ); + dispatchTransferFrom( + zrxAssetData, + ZRX_PROXY_ID, + takerAddress, + order.feeRecipientAddress, + fillResults.takerFeePaid + ); + } } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol index 517b743fe..82325a29f 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol @@ -14,21 +14,25 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; +import "./libs/LibConstants.sol"; +import "../../utils/LibBytes/LibBytes.sol"; import "./libs/LibMath.sol"; import "./libs/LibOrder.sol"; import "./libs/LibFillResults.sol"; import "./libs/LibExchangeErrors.sol"; import "./mixins/MExchangeCore.sol"; import "./mixins/MMatchOrders.sol"; -import "./mixins/MSettlement.sol"; import "./mixins/MTransactions.sol"; +import "./mixins/MAssetProxyDispatcher.sol"; contract MixinMatchOrders is + LibConstants, + LibBytes, LibMath, LibExchangeErrors, + MAssetProxyDispatcher, MExchangeCore, MMatchOrders, - MSettlement, MTransactions { @@ -224,4 +228,89 @@ contract MixinMatchOrders is // Return fill results return matchedFillResults; } + + /// @dev Settles matched order by transferring appropriate funds between order makers, taker, and fee recipient. + /// @param leftOrder First matched order. + /// @param rightOrder Second matched order. + /// @param takerAddress Address that matched the orders. The taker receives the spread between orders as profit. + /// @param matchedFillResults Struct holding amounts to transfer between makers, taker, and fee recipients. + function settleMatchedOrders( + LibOrder.Order memory leftOrder, + LibOrder.Order memory rightOrder, + address takerAddress, + LibFillResults.MatchedFillResults memory matchedFillResults + ) + private + { + uint8 leftMakerAssetProxyId = uint8(popLastByte(leftOrder.makerAssetData)); + uint8 rightMakerAssetProxyId = uint8(popLastByte(rightOrder.makerAssetData)); + bytes memory zrxAssetData = ZRX_ASSET_DATA; + // Order makers and taker + dispatchTransferFrom( + leftOrder.makerAssetData, + leftMakerAssetProxyId, + leftOrder.makerAddress, + rightOrder.makerAddress, + matchedFillResults.right.takerAssetFilledAmount + ); + dispatchTransferFrom( + rightOrder.makerAssetData, + rightMakerAssetProxyId, + rightOrder.makerAddress, + leftOrder.makerAddress, + matchedFillResults.left.takerAssetFilledAmount + ); + dispatchTransferFrom( + leftOrder.makerAssetData, + leftMakerAssetProxyId, + leftOrder.makerAddress, + takerAddress, + matchedFillResults.leftMakerAssetSpreadAmount + ); + + // Maker fees + dispatchTransferFrom( + zrxAssetData, + ZRX_PROXY_ID, + leftOrder.makerAddress, + leftOrder.feeRecipientAddress, + matchedFillResults.left.makerFeePaid + ); + dispatchTransferFrom( + zrxAssetData, + ZRX_PROXY_ID, + rightOrder.makerAddress, + rightOrder.feeRecipientAddress, + matchedFillResults.right.makerFeePaid + ); + + // Taker fees + if (leftOrder.feeRecipientAddress == rightOrder.feeRecipientAddress) { + dispatchTransferFrom( + zrxAssetData, + ZRX_PROXY_ID, + takerAddress, + leftOrder.feeRecipientAddress, + safeAdd( + matchedFillResults.left.takerFeePaid, + matchedFillResults.right.takerFeePaid + ) + ); + } else { + dispatchTransferFrom( + zrxAssetData, + ZRX_PROXY_ID, + takerAddress, + leftOrder.feeRecipientAddress, + matchedFillResults.left.takerFeePaid + ); + dispatchTransferFrom( + zrxAssetData, + ZRX_PROXY_ID, + takerAddress, + rightOrder.feeRecipientAddress, + matchedFillResults.right.takerFeePaid + ); + } + } } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol deleted file mode 100644 index 29a9c87bd..000000000 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol +++ /dev/null @@ -1,182 +0,0 @@ -/* - - Copyright 2018 ZeroEx Intl. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - -*/ - -pragma solidity ^0.4.24; -pragma experimental ABIEncoderV2; - -import "../../utils/LibBytes/LibBytes.sol"; -import "./libs/LibMath.sol"; -import "./libs/LibFillResults.sol"; -import "./libs/LibOrder.sol"; -import "./libs/LibExchangeErrors.sol"; -import "./mixins/MMatchOrders.sol"; -import "./mixins/MSettlement.sol"; -import "./mixins/MAssetProxyDispatcher.sol"; - -contract MixinSettlement is - LibBytes, - LibMath, - LibExchangeErrors, - MMatchOrders, - MSettlement, - MAssetProxyDispatcher -{ - // ZRX address encoded as a byte array. - // This will be constant throughout the life of the Exchange contract, - // since ZRX will always be transferred via the ERC20 AssetProxy. - bytes internal ZRX_ASSET_DATA; - uint8 constant ZRX_PROXY_ID = 1; - - /// TODO: _zrxAssetData should be a constant in production. - /// @dev Constructor sets the metadata that will be used for paying ZRX fees. - /// @param _zrxAssetData Byte array containing ERC20 proxy id concatenated with address of ZRX. - constructor (bytes memory _zrxAssetData) - public - { - ZRX_ASSET_DATA = _zrxAssetData; - } - - /// @dev Settles an order by transferring assets between counterparties. - /// @param order Order struct containing order specifications. - /// @param takerAddress Address selling takerAsset and buying makerAsset. - /// @param fillResults Amounts to be filled and fees paid by maker and taker. - function settleOrder( - LibOrder.Order memory order, - address takerAddress, - LibFillResults.FillResults memory fillResults - ) - internal - { - uint8 makerAssetProxyId = uint8(popLastByte(order.makerAssetData)); - uint8 takerAssetProxyId = uint8(popLastByte(order.takerAssetData)); - bytes memory zrxAssetData = ZRX_ASSET_DATA; - dispatchTransferFrom( - order.makerAssetData, - makerAssetProxyId, - order.makerAddress, - takerAddress, - fillResults.makerAssetFilledAmount - ); - dispatchTransferFrom( - order.takerAssetData, - takerAssetProxyId, - takerAddress, - order.makerAddress, - fillResults.takerAssetFilledAmount - ); - dispatchTransferFrom( - zrxAssetData, - ZRX_PROXY_ID, - order.makerAddress, - order.feeRecipientAddress, - fillResults.makerFeePaid - ); - dispatchTransferFrom( - zrxAssetData, - ZRX_PROXY_ID, - takerAddress, - order.feeRecipientAddress, - fillResults.takerFeePaid - ); - } - - /// @dev Settles matched order by transferring appropriate funds between order makers, taker, and fee recipient. - /// @param leftOrder First matched order. - /// @param rightOrder Second matched order. - /// @param takerAddress Address that matched the orders. The taker receives the spread between orders as profit. - /// @param matchedFillResults Struct holding amounts to transfer between makers, taker, and fee recipients. - function settleMatchedOrders( - LibOrder.Order memory leftOrder, - LibOrder.Order memory rightOrder, - address takerAddress, - LibFillResults.MatchedFillResults memory matchedFillResults - ) - internal - { - uint8 leftMakerAssetProxyId = uint8(popLastByte(leftOrder.makerAssetData)); - uint8 rightMakerAssetProxyId = uint8(popLastByte(rightOrder.makerAssetData)); - bytes memory zrxAssetData = ZRX_ASSET_DATA; - // Order makers and taker - dispatchTransferFrom( - leftOrder.makerAssetData, - leftMakerAssetProxyId, - leftOrder.makerAddress, - rightOrder.makerAddress, - matchedFillResults.right.takerAssetFilledAmount - ); - dispatchTransferFrom( - rightOrder.makerAssetData, - rightMakerAssetProxyId, - rightOrder.makerAddress, - leftOrder.makerAddress, - matchedFillResults.left.takerAssetFilledAmount - ); - dispatchTransferFrom( - leftOrder.makerAssetData, - leftMakerAssetProxyId, - leftOrder.makerAddress, - takerAddress, - matchedFillResults.leftMakerAssetSpreadAmount - ); - - // Maker fees - dispatchTransferFrom( - zrxAssetData, - ZRX_PROXY_ID, - leftOrder.makerAddress, - leftOrder.feeRecipientAddress, - matchedFillResults.left.makerFeePaid - ); - dispatchTransferFrom( - zrxAssetData, - ZRX_PROXY_ID, - rightOrder.makerAddress, - rightOrder.feeRecipientAddress, - matchedFillResults.right.makerFeePaid - ); - - // Taker fees - if (leftOrder.feeRecipientAddress == rightOrder.feeRecipientAddress) { - dispatchTransferFrom( - zrxAssetData, - ZRX_PROXY_ID, - takerAddress, - leftOrder.feeRecipientAddress, - safeAdd( - matchedFillResults.left.takerFeePaid, - matchedFillResults.right.takerFeePaid - ) - ); - } else { - dispatchTransferFrom( - zrxAssetData, - ZRX_PROXY_ID, - takerAddress, - leftOrder.feeRecipientAddress, - matchedFillResults.left.takerFeePaid - ); - dispatchTransferFrom( - zrxAssetData, - ZRX_PROXY_ID, - takerAddress, - rightOrder.feeRecipientAddress, - matchedFillResults.right.takerFeePaid - ); - } - } -} diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol index 8ad15aaff..4a2beff57 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol @@ -33,7 +33,7 @@ contract MixinSignatureValidator is { // Personal message headers string constant ETH_PERSONAL_MESSAGE = "\x19Ethereum Signed Message:\n32"; - string constant TREZOR_PERSONAL_MESSAGE = "\x19Ethereum Signed Message:\n\x41"; + string constant TREZOR_PERSONAL_MESSAGE = "\x19Ethereum Signed Message:\n\x20"; // Mapping of hash => signer => signed mapping (bytes32 => mapping (address => bool)) public preSigned; @@ -92,8 +92,15 @@ contract MixinSignatureValidator is LENGTH_GREATER_THAN_0_REQUIRED ); + // Ensure signature is supported + uint8 signatureTypeRaw = uint8(popLastByte(signature)); + require( + signatureTypeRaw < uint8(SignatureType.NSignatureTypes), + SIGNATURE_UNSUPPORTED + ); + // Pop last byte off of signature byte array. - SignatureType signatureType = SignatureType(uint8(popLastByte(signature))); + SignatureType signatureType = SignatureType(signatureTypeRaw); // Variables are not scoped in Solidity. uint8 v; diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol index 30b0102fd..f1332363c 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol @@ -20,8 +20,11 @@ pragma solidity ^0.4.24; import "./libs/LibExchangeErrors.sol"; import "./mixins/MSignatureValidator.sol"; import "./mixins/MTransactions.sol"; +import "./libs/LibExchangeErrors.sol"; +import "./libs/LibEIP712.sol"; contract MixinTransactions is + LibEIP712, LibExchangeErrors, MSignatureValidator, MTransactions @@ -34,6 +37,33 @@ contract MixinTransactions is // Address of current transaction signer address public currentContextAddress; + // Hash for the EIP712 ZeroEx Transaction Schema + bytes32 constant EIP712_ZEROEX_TRANSACTION_SCHEMA_HASH = keccak256(abi.encodePacked( + "ZeroExTransaction(", + "uint256 salt,", + "address signer,", + "bytes data", + ")" + )); + + /// @dev Calculates EIP712 hash of the Transaction. + /// @param salt Arbitrary number to ensure uniqueness of transaction hash. + /// @param signer Address of transaction signer. + /// @param data AbiV2 encoded calldata. + /// @return EIP712 hash of the Transaction. + function hashZeroExTransaction(uint256 salt, address signer, bytes data) + internal + pure + returns (bytes32) + { + return keccak256(abi.encode( + EIP712_ZEROEX_TRANSACTION_SCHEMA_HASH, + salt, + signer, + keccak256(data) + )); + } + /// @dev Executes an exchange method call in the context of signer. /// @param salt Arbitrary number to ensure uniqueness of transaction hash. /// @param signer Address of transaction signer. @@ -53,13 +83,7 @@ contract MixinTransactions is REENTRANCY_ILLEGAL ); - // Calculate transaction hash - bytes32 transactionHash = keccak256(abi.encodePacked( - address(this), - signer, - salt, - data - )); + bytes32 transactionHash = hashEIP712Message(hashZeroExTransaction(salt, signer, data)); // Validate transaction has not been executed require( diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibConstants.sol b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibConstants.sol new file mode 100644 index 000000000..4a9452448 --- /dev/null +++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibConstants.sol @@ -0,0 +1,37 @@ +/* + + 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.24; + +contract LibConstants { + + // Asset data for ZRX token. Used for fee transfers. + // @TODO: Hardcode constant when we deploy. Currently + // not constant to make testing easier. + bytes public ZRX_ASSET_DATA; + + // Proxy Id for ZRX token. + uint8 constant ZRX_PROXY_ID = 1; + + // @TODO: Remove when we deploy. + constructor (bytes memory zrxAssetData) + public + { + ZRX_ASSET_DATA = zrxAssetData; + } +} diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibEIP712.sol b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibEIP712.sol new file mode 100644 index 000000000..b983347a4 --- /dev/null +++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibEIP712.sol @@ -0,0 +1,64 @@ +/* + + 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.24; + +contract LibEIP712 { + // EIP191 header for EIP712 prefix + string constant EIP191_HEADER = "\x19\x01"; + + // EIP712 Domain Name value + string constant EIP712_DOMAIN_NAME = "0x Protocol"; + + // EIP712 Domain Version value + string constant EIP712_DOMAIN_VERSION = "2"; + + // Hash of the EIP712 Domain Separator Schema + bytes32 public constant EIP712_DOMAIN_SEPARATOR_SCHEMA_HASH = keccak256(abi.encodePacked( + "EIP712Domain(", + "string name,", + "string version,", + "address verifyingContract", + ")" + )); + + // Hash of the EIP712 Domain Separator data + bytes32 public EIP712_DOMAIN_HASH; + + constructor () + public + { + EIP712_DOMAIN_HASH = keccak256(abi.encode( + EIP712_DOMAIN_SEPARATOR_SCHEMA_HASH, + keccak256(bytes(EIP712_DOMAIN_NAME)), + keccak256(bytes(EIP712_DOMAIN_VERSION)), + address(this) + )); + } + + /// @dev Calculates EIP712 encoding for a hash struct in this EIP712 Domain. + /// @param hashStruct The EIP712 hash struct. + /// @return EIP712 hash applied to this EIP712 Domain. + function hashEIP712Message(bytes32 hashStruct) + internal + view + returns (bytes32) + { + return keccak256(abi.encodePacked(EIP191_HEADER, EIP712_DOMAIN_HASH, hashStruct)); + } +} 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 adf27bec3..aab428e74 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol @@ -56,6 +56,6 @@ contract LibExchangeErrors { /// Length validation errors /// string constant LENGTH_GREATER_THAN_0_REQUIRED = "LENGTH_GREATER_THAN_0_REQUIRED"; // Byte array must have a length greater than 0. - string constant LENGTH_0_REQUIRED = "LENGTH_1_REQUIRED"; // Byte array must have a length of 1. - string constant LENGTH_65_REQUIRED = "LENGTH_66_REQUIRED"; // Byte array must have a length of 66. + string constant LENGTH_0_REQUIRED = "LENGTH_0_REQUIRED"; // Byte array must have a length of 0. + string constant LENGTH_65_REQUIRED = "LENGTH_65_REQUIRED"; // Byte array must have a length of 65. } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibOrder.sol b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibOrder.sol index 05ea27ffc..bfc7aaae0 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibOrder.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibOrder.sol @@ -18,13 +18,14 @@ pragma solidity ^0.4.24; -contract LibOrder { +import "./LibEIP712.sol"; - bytes32 constant DOMAIN_SEPARATOR_SCHEMA_HASH = keccak256(abi.encodePacked( - "DomainSeparator(address contract)" - )); +contract LibOrder is + LibEIP712 +{ - bytes32 constant ORDER_SCHEMA_HASH = keccak256(abi.encodePacked( + // Hash for the EIP712 Order Schema + bytes32 constant EIP712_ORDER_SCHEMA_HASH = keccak256(abi.encodePacked( "Order(", "address makerAddress,", "address takerAddress,", @@ -37,7 +38,7 @@ contract LibOrder { "uint256 expirationTimeSeconds,", "uint256 salt,", "bytes makerAssetData,", - "bytes takerAssetData,", + "bytes takerAssetData", ")" )); @@ -85,27 +86,38 @@ contract LibOrder { view returns (bytes32 orderHash) { - // TODO: EIP712 is not finalized yet - // Source: https://github.com/ethereum/EIPs/pull/712 - orderHash = keccak256(abi.encodePacked( - DOMAIN_SEPARATOR_SCHEMA_HASH, - keccak256(abi.encodePacked(address(this))), - ORDER_SCHEMA_HASH, - keccak256(abi.encodePacked( - order.makerAddress, - order.takerAddress, - order.feeRecipientAddress, - order.senderAddress, - order.makerAssetAmount, - order.takerAssetAmount, - order.makerFee, - order.takerFee, - order.expirationTimeSeconds, - order.salt, - keccak256(abi.encodePacked(order.makerAssetData)), - keccak256(abi.encodePacked(order.takerAssetData)) - )) - )); + orderHash = hashEIP712Message(hashOrder(order)); return orderHash; } + + /// @dev Calculates EIP712 hash of the order. + /// @param order The order structure. + /// @return EIP712 hash of the order. + function hashOrder(Order memory order) + internal + pure + returns (bytes32 result) + { + bytes32 schemaHash = EIP712_ORDER_SCHEMA_HASH; + bytes32 makerAssetDataHash = keccak256(order.makerAssetData); + bytes32 takerAssetDataHash = keccak256(order.takerAssetData); + assembly { + // Backup + let temp1 := mload(sub(order, 32)) + let temp2 := mload(add(order, 320)) + let temp3 := mload(add(order, 352)) + + // Hash in place + mstore(sub(order, 32), schemaHash) + mstore(add(order, 320), makerAssetDataHash) + mstore(add(order, 352), takerAssetDataHash) + result := keccak256(sub(order, 32), 416) + + // Restore + mstore(sub(order, 32), temp1) + mstore(add(order, 320), temp2) + mstore(add(order, 352), temp3) + } + return result; + } } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol index fb345294c..1737f5baf 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol @@ -121,4 +121,5 @@ contract MExchangeCore is internal pure returns (LibFillResults.FillResults memory fillResults); + } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MMatchOrders.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MMatchOrders.sol index e52963a26..abe7c3596 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MMatchOrders.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MMatchOrders.sol @@ -54,4 +54,5 @@ contract MMatchOrders is internal pure returns (LibFillResults.MatchedFillResults memory matchedFillResults); + } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSettlement.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSettlement.sol deleted file mode 100644 index 2c403162f..000000000 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSettlement.sol +++ /dev/null @@ -1,49 +0,0 @@ -/* - - Copyright 2018 ZeroEx Intl. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - -*/ - -pragma solidity ^0.4.24; - -import "../libs/LibOrder.sol"; -import "../libs/LibFillResults.sol"; - -contract MSettlement { - - /// @dev Settles an order by transferring assets between counterparties. - /// @param order Order struct containing order specifications. - /// @param takerAddress Address selling takerAsset and buying makerAsset. - /// @param fillResults Amounts to be filled and fees paid by maker and taker. - function settleOrder( - LibOrder.Order memory order, - address takerAddress, - LibFillResults.FillResults memory fillResults - ) - internal; - - /// @dev Settles matched order by transferring appropriate funds between order makers, taker, and fee recipient. - /// @param leftOrder First matched order. - /// @param rightOrder Second matched order. - /// @param takerAddress Address that matched the orders. The taker receives the spread between orders as profit. - /// @param matchedFillResults Struct holding amounts to transfer between makers, taker, and fee recipients. - function settleMatchedOrders( - LibOrder.Order memory leftOrder, - LibOrder.Order memory rightOrder, - address takerAddress, - LibFillResults.MatchedFillResults memory matchedFillResults - ) - 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 5e286e43a..9c6fbe22b 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, // 0x00, default value - Invalid, // 0x01 - EIP712, // 0x02 - EthSign, // 0x03 - Caller, // 0x04 - Wallet, // 0x05 - Validator, // 0x06 - PreSigned, // 0x07 - Trezor // 0x08 + Illegal, // 0x00, default value + Invalid, // 0x01 + EIP712, // 0x02 + EthSign, // 0x03 + Caller, // 0x04 + Wallet, // 0x05 + Validator, // 0x06 + PreSigned, // 0x07 + Trezor, // 0x08 + NSignatureTypes // 0x09, number of signature types. Always leave at end. } } diff --git a/packages/contracts/src/contracts/current/test/TestLibs/TestLibs.sol b/packages/contracts/src/contracts/current/test/TestLibs/TestLibs.sol index 47ce0dcf3..010080703 100644 --- a/packages/contracts/src/contracts/current/test/TestLibs/TestLibs.sol +++ b/packages/contracts/src/contracts/current/test/TestLibs/TestLibs.sol @@ -74,7 +74,7 @@ contract TestLibs is pure returns (bytes32) { - return ORDER_SCHEMA_HASH; + return EIP712_ORDER_SCHEMA_HASH; } function getDomainSeparatorSchemaHash() @@ -82,7 +82,7 @@ contract TestLibs is pure returns (bytes32) { - return DOMAIN_SEPARATOR_SCHEMA_HASH; + return EIP712_DOMAIN_SEPARATOR_SCHEMA_HASH; } function publicAddFillResults(FillResults memory totalFillResults, FillResults memory singleFillResults) diff --git a/packages/contracts/src/contracts/current/test/TestValidator/TestValidator.sol b/packages/contracts/src/contracts/current/test/TestValidator/TestValidator.sol new file mode 100644 index 000000000..13953b482 --- /dev/null +++ b/packages/contracts/src/contracts/current/test/TestValidator/TestValidator.sol @@ -0,0 +1,52 @@ +/* + + 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.24; + +import "../../protocol/Exchange/interfaces/IValidator.sol"; + +contract TestValidator is + IValidator +{ + + // The single valid signer for this wallet. + address VALID_SIGNER; + + /// @dev constructs a new `TestValidator` with a single valid signer. + /// @param validSigner The sole, valid signer. + constructor (address validSigner) public { + VALID_SIGNER = validSigner; + } + + /// @dev Verifies that a signature is valid. `signer` must match `VALID_SIGNER`. + /// @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 signature. + function isValidSignature( + bytes32 hash, + address signer, + bytes signature + ) + external + view + returns (bool isValid) + { + return (signer == VALID_SIGNER); + } +} diff --git a/packages/contracts/src/contracts/current/test/TestWallet/TestWallet.sol b/packages/contracts/src/contracts/current/test/TestWallet/TestWallet.sol new file mode 100644 index 000000000..aca74b409 --- /dev/null +++ b/packages/contracts/src/contracts/current/test/TestWallet/TestWallet.sol @@ -0,0 +1,65 @@ +/* + + 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.24; + +import "../../protocol/Exchange/interfaces/IWallet.sol"; +import "../../utils/LibBytes/LibBytes.sol"; + +contract TestWallet is + IWallet, + LibBytes +{ + + string constant LENGTH_65_REQUIRED = "LENGTH_65_REQUIRED"; + + // The owner of this wallet. + address WALLET_OWNER; + + /// @dev constructs a new `TestWallet` with a single owner. + /// @param walletOwner The owner of this wallet. + constructor (address walletOwner) public { + WALLET_OWNER = walletOwner; + } + + /// @dev Validates an EIP712 signature. + /// The signer must match the owner of this wallet. + /// @param hash Message hash that is signed. + /// @param eip712Signature Proof of signing. + /// @return Validity of signature. + function isValidSignature( + bytes32 hash, + bytes eip712Signature + ) + external + view + returns (bool isValid) + { + require( + eip712Signature.length == 65, + LENGTH_65_REQUIRED + ); + + uint8 v = uint8(eip712Signature[0]); + bytes32 r = readBytes32(eip712Signature, 1); + bytes32 s = readBytes32(eip712Signature, 33); + address recoveredAddress = ecrecover(hash, v, r, s); + isValid = WALLET_OWNER == recoveredAddress; + return isValid; + } +} diff --git a/packages/contracts/src/utils/artifacts.ts b/packages/contracts/src/utils/artifacts.ts index 7ba467708..4375d87c6 100644 --- a/packages/contracts/src/utils/artifacts.ts +++ b/packages/contracts/src/utils/artifacts.ts @@ -17,6 +17,8 @@ import * as TestLibBytes from '../artifacts/TestLibBytes.json'; import * as TestLibMem from '../artifacts/TestLibMem.json'; import * as TestLibs from '../artifacts/TestLibs.json'; import * as TestSignatureValidator from '../artifacts/TestSignatureValidator.json'; +import * as TestValidator from '../artifacts/TestValidator.json'; +import * as TestWallet from '../artifacts/TestWallet.json'; import * as TokenRegistry from '../artifacts/TokenRegistry.json'; import * as EtherToken from '../artifacts/WETH9.json'; import * as Whitelist from '../artifacts/Whitelist.json'; @@ -41,6 +43,8 @@ export const artifacts = { TestLibMem: (TestLibMem as any) as ContractArtifact, TestLibs: (TestLibs as any) as ContractArtifact, TestSignatureValidator: (TestSignatureValidator as any) as ContractArtifact, + TestValidator: (TestValidator as any) as ContractArtifact, + TestWallet: (TestWallet 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 ec3c8fd36..f21b8c7a0 100644 --- a/packages/contracts/src/utils/constants.ts +++ b/packages/contracts/src/utils/constants.ts @@ -27,6 +27,11 @@ export const constants = { LIB_BYTES_GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED', ERC20_INSUFFICIENT_BALANCE: 'Insufficient balance to complete transfer.', ERC20_INSUFFICIENT_ALLOWANCE: 'Insufficient allowance to complete transfer.', + EXCHANGE_LENGTH_GREATER_THAN_0_REQUIRED: 'LENGTH_GREATER_THAN_0_REQUIRED', + EXCHANGE_SIGNATURE_UNSUPPORTED: 'SIGNATURE_UNSUPPORTED', + EXCHANGE_SIGNATURE_ILLEGAL: 'SIGNATURE_ILLEGAL', + EXCHANGE_LENGTH_0_REQUIRED: 'LENGTH_0_REQUIRED', + EXCHANGE_LENGTH_65_REQUIRED: 'LENGTH_65_REQUIRED', TESTRPC_NETWORK_ID: 50, // Note(albrow): In practice V8 and most other engines limit the minimum // interval for setInterval to 10ms. We still set it to 0 here in order to diff --git a/packages/contracts/src/utils/order_factory.ts b/packages/contracts/src/utils/order_factory.ts index 6e4c9a883..009dbc396 100644 --- a/packages/contracts/src/utils/order_factory.ts +++ b/packages/contracts/src/utils/order_factory.ts @@ -26,7 +26,7 @@ export class OrderFactory { ...this._defaultOrderParams, ...customOrderParams, } as any) as Order; - const orderHashBuff = orderHashUtils.getOrderHashBuff(order); + const orderHashBuff = orderHashUtils.getOrderHashBuffer(order); const signature = signingUtils.signMessage(orderHashBuff, this._privateKey, signatureType); const signedOrder = { ...order, diff --git a/packages/contracts/src/utils/transaction_factory.ts b/packages/contracts/src/utils/transaction_factory.ts index a060263b1..19ef4e1bf 100644 --- a/packages/contracts/src/utils/transaction_factory.ts +++ b/packages/contracts/src/utils/transaction_factory.ts @@ -1,10 +1,19 @@ -import { crypto, generatePseudoRandomSalt } from '@0xproject/order-utils'; +import { EIP712Schema, EIP712Types, EIP712Utils, generatePseudoRandomSalt } from '@0xproject/order-utils'; import { SignatureType } from '@0xproject/types'; import * as ethUtil from 'ethereumjs-util'; import { signingUtils } from './signing_utils'; import { SignedTransaction } from './types'; +const EIP712_ZEROEX_TRANSACTION_SCHEMA: EIP712Schema = { + name: 'ZeroExTransaction', + parameters: [ + { name: 'salt', type: EIP712Types.Uint256 }, + { name: 'signer', type: EIP712Types.Address }, + { name: 'data', type: EIP712Types.Bytes }, + ], +}; + export class TransactionFactory { private _signerBuff: Buffer; private _exchangeAddress: string; @@ -16,14 +25,22 @@ export class TransactionFactory { } public newSignedTransaction(data: string, signatureType: SignatureType = SignatureType.EthSign): SignedTransaction { const salt = generatePseudoRandomSalt(); - const txHash = crypto.solSHA3([this._exchangeAddress, this._signerBuff, salt, ethUtil.toBuffer(data)]); + const signer = `0x${this._signerBuff.toString('hex')}`; + const executeTransactionData = { + salt, + signer, + data, + }; + const executeTransactionHashBuff = EIP712Utils.structHash( + EIP712_ZEROEX_TRANSACTION_SCHEMA, + executeTransactionData, + ); + const txHash = EIP712Utils.createEIP712Message(executeTransactionHashBuff, this._exchangeAddress); const signature = signingUtils.signMessage(txHash, this._privateKey, signatureType); const signedTx = { exchangeAddress: this._exchangeAddress, - salt, - signer: `0x${this._signerBuff.toString('hex')}`, - data, signature: `0x${signature.toString('hex')}`, + ...executeTransactionData, }; return signedTx; } diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts index bb8c12088..a6d0c2a88 100644 --- a/packages/contracts/src/utils/types.ts +++ b/packages/contracts/src/utils/types.ts @@ -100,6 +100,7 @@ export enum ContractName { DummyERC721Receiver = 'DummyERC721Receiver', DummyERC721Token = 'DummyERC721Token', TestLibBytes = 'TestLibBytes', + TestWallet = 'TestWallet', Authorizable = 'Authorizable', Whitelist = 'Whitelist', } diff --git a/packages/contracts/test/asset_proxy/proxies.ts b/packages/contracts/test/asset_proxy/proxies.ts index 9760d3b9c..2c27f7382 100644 --- a/packages/contracts/test/asset_proxy/proxies.ts +++ b/packages/contracts/test/asset_proxy/proxies.ts @@ -276,7 +276,7 @@ describe('Asset Transfer Proxies', () => { expect(newOwnerMakerAsset).to.be.bignumber.equal(takerAddress); }); - it('should not call onERC721Received when transferring to a smart contract without receiver data', async () => { + it('should call onERC721Received when transferring to a smart contract without receiver data', async () => { // Construct ERC721 asset data const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId); const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2); @@ -297,7 +297,11 @@ describe('Asset Transfer Proxies', () => { const logDecoder = new LogDecoder(web3Wrapper, erc721Receiver.address); const tx = await logDecoder.getTxWithDecodedLogsAsync(txHash); // Verify that no log was emitted by erc721 receiver - expect(tx.logs.length).to.be.equal(0); + expect(tx.logs.length).to.be.equal(1); + const tokenReceivedLog = tx.logs[0] as LogWithDecodedArgs<TokenReceivedContractEventArgs>; + expect(tokenReceivedLog.args.from).to.be.equal(makerAddress); + expect(tokenReceivedLog.args.tokenId).to.be.bignumber.equal(erc721MakerTokenId); + expect(tokenReceivedLog.args.data).to.be.equal(constants.NULL_BYTES); // Verify transfer was successful const newOwnerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); expect(newOwnerMakerAsset).to.be.bignumber.equal(erc721Receiver.address); diff --git a/packages/contracts/test/exchange/libs.ts b/packages/contracts/test/exchange/libs.ts index eff05981d..c08001198 100644 --- a/packages/contracts/test/exchange/libs.ts +++ b/packages/contracts/test/exchange/libs.ts @@ -1,5 +1,5 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; -import { assetProxyUtils, orderHashUtils } from '@0xproject/order-utils'; +import { assetProxyUtils, EIP712Utils, orderHashUtils } from '@0xproject/order-utils'; import { SignedOrder } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import * as chai from 'chai'; @@ -56,13 +56,17 @@ describe('Exchange libs', () => { describe('getOrderSchema', () => { it('should output the correct order schema hash', async () => { const orderSchema = await libs.getOrderSchemaHash.callAsync(); - expect(orderHashUtils._getOrderSchemaHex()).to.be.equal(orderSchema); + const schemaHashBuffer = orderHashUtils._getOrderSchemaBuffer(); + const schemaHashHex = `0x${schemaHashBuffer.toString('hex')}`; + expect(schemaHashHex).to.be.equal(orderSchema); }); }); describe('getDomainSeparatorSchema', () => { it('should output the correct domain separator schema hash', async () => { const domainSeparatorSchema = await libs.getDomainSeparatorSchemaHash.callAsync(); - expect(orderHashUtils._getDomainSeparatorSchemaHex()).to.be.equal(domainSeparatorSchema); + const domainSchemaBuffer = EIP712Utils._getDomainSeparatorSchemaBuffer(); + const schemaHashHex = `0x${domainSchemaBuffer.toString('hex')}`; + expect(schemaHashHex).to.be.equal(domainSeparatorSchema); }); }); describe('getOrderHash', () => { diff --git a/packages/contracts/test/exchange/signature_validator.ts b/packages/contracts/test/exchange/signature_validator.ts index c39fd6ee4..2a94ab25a 100644 --- a/packages/contracts/test/exchange/signature_validator.ts +++ b/packages/contracts/test/exchange/signature_validator.ts @@ -1,12 +1,15 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; -import { assetProxyUtils, orderHashUtils } from '@0xproject/order-utils'; -import { SignedOrder } from '@0xproject/types'; +import { addSignedMessagePrefix, assetProxyUtils, MessagePrefixType, orderHashUtils } from '@0xproject/order-utils'; +import { SignatureType, SignedOrder } from '@0xproject/types'; import * as chai from 'chai'; import ethUtil = require('ethereumjs-util'); import { TestSignatureValidatorContract } from '../../src/generated_contract_wrappers/test_signature_validator'; +import { TestValidatorContract } from '../../src/generated_contract_wrappers/test_validator'; +import { TestWalletContract } from '../../src/generated_contract_wrappers/test_wallet'; import { addressUtils } from '../../src/utils/address_utils'; import { artifacts } from '../../src/utils/artifacts'; +import { expectRevertOrOtherErrorAsync } from '../../src/utils/assertions'; import { chaiSetup } from '../../src/utils/chai_setup'; import { constants } from '../../src/utils/constants'; import { OrderFactory } from '../../src/utils/order_factory'; @@ -21,6 +24,12 @@ describe('MixinSignatureValidator', () => { let signedOrder: SignedOrder; let orderFactory: OrderFactory; let signatureValidator: TestSignatureValidatorContract; + let testWallet: TestWalletContract; + let testValidator: TestValidatorContract; + let signerAddress: string; + let signerPrivateKey: Buffer; + let notSignerAddress: string; + let notSignerPrivateKey: Buffer; before(async () => { await blockchainLifecycle.startAsync(); @@ -31,11 +40,31 @@ describe('MixinSignatureValidator', () => { before(async () => { const accounts = await web3Wrapper.getAvailableAddressesAsync(); const makerAddress = accounts[0]; + signerAddress = makerAddress; + notSignerAddress = accounts[1]; signatureValidator = await TestSignatureValidatorContract.deployFrom0xArtifactAsync( artifacts.TestSignatureValidator, provider, txDefaults, ); + testWallet = await TestWalletContract.deployFrom0xArtifactAsync( + artifacts.TestWallet, + provider, + txDefaults, + signerAddress, + ); + testValidator = await TestValidatorContract.deployFrom0xArtifactAsync( + artifacts.TestValidator, + provider, + txDefaults, + signerAddress, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await signatureValidator.setSignatureValidatorApproval.sendTransactionAsync(testValidator.address, true, { + from: signerAddress, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); const defaultOrderParams = { ...constants.STATIC_ORDER_PARAMS, @@ -45,8 +74,9 @@ describe('MixinSignatureValidator', () => { makerAssetData: assetProxyUtils.encodeERC20AssetData(addressUtils.generatePseudoRandomAddress()), takerAssetData: assetProxyUtils.encodeERC20AssetData(addressUtils.generatePseudoRandomAddress()), }; - const privateKey = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(makerAddress)]; - orderFactory = new OrderFactory(privateKey, defaultOrderParams); + signerPrivateKey = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(makerAddress)]; + notSignerPrivateKey = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(notSignerAddress)]; + orderFactory = new OrderFactory(signerPrivateKey, defaultOrderParams); }); beforeEach(async () => { @@ -61,29 +91,367 @@ describe('MixinSignatureValidator', () => { signedOrder = orderFactory.newSignedOrder(); }); - it('should return true with a valid signature', async () => { + it('should revert when signature is empty', async () => { + const emptySignature = '0x'; + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + return expectRevertOrOtherErrorAsync( + signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + signedOrder.makerAddress, + emptySignature, + ), + constants.EXCHANGE_LENGTH_GREATER_THAN_0_REQUIRED, + ); + }); + + it('should revert when signature type is unsupported', async () => { + const unsupportedSignatureType = SignatureType.NSignatureTypes; + const unsupportedSignatureHex = `0x${unsupportedSignatureType}`; + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + return expectRevertOrOtherErrorAsync( + signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + signedOrder.makerAddress, + unsupportedSignatureHex, + ), + constants.EXCHANGE_SIGNATURE_UNSUPPORTED, + ); + }); + + it('should revert when SignatureType=Illegal', async () => { + const unsupportedSignatureHex = `0x${SignatureType.Illegal}`; + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + return expectRevertOrOtherErrorAsync( + signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + signedOrder.makerAddress, + unsupportedSignatureHex, + ), + constants.EXCHANGE_SIGNATURE_ILLEGAL, + ); + }); + + it('should return false when SignatureType=Invalid and signature has a length of zero', async () => { + const signatureHex = `0x${SignatureType.Invalid}`; + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + signedOrder.makerAddress, + signatureHex, + ); + expect(isValidSignature).to.be.false(); + }); + + it('should revert when SignatureType=Invalid and signature length is non-zero', async () => { + const fillerData = ethUtil.toBuffer('0xdeadbeef'); + const signatureType = ethUtil.toBuffer(`0x${SignatureType.Invalid}`); + const signatureBuffer = Buffer.concat([fillerData, signatureType]); + const signatureHex = ethUtil.bufferToHex(signatureBuffer); + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + return expectRevertOrOtherErrorAsync( + signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + signedOrder.makerAddress, + signatureHex, + ), + constants.EXCHANGE_LENGTH_0_REQUIRED, + ); + }); + + it('should return true when SignatureType=EIP712 and signature is valid', async () => { + // Create EIP712 signature + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const orderHashBuffer = ethUtil.toBuffer(orderHashHex); + const ecSignature = ethUtil.ecsign(orderHashBuffer, signerPrivateKey); + // Create 0x signature from EIP712 signature + const signature = Buffer.concat([ + ethUtil.toBuffer(ecSignature.v), + ecSignature.r, + ecSignature.s, + ethUtil.toBuffer(`0x${SignatureType.EIP712}`), + ]); + const signatureHex = ethUtil.bufferToHex(signature); + // Validate signature + const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + signerAddress, + signatureHex, + ); + expect(isValidSignature).to.be.true(); + }); + + it('should return false when SignatureType=EIP712 and signature is invalid', async () => { + // Create EIP712 signature + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const orderHashBuffer = ethUtil.toBuffer(orderHashHex); + const ecSignature = ethUtil.ecsign(orderHashBuffer, signerPrivateKey); + // Create 0x signature from EIP712 signature + const signature = Buffer.concat([ + ethUtil.toBuffer(ecSignature.v), + ecSignature.r, + ecSignature.s, + ethUtil.toBuffer(`0x${SignatureType.EIP712}`), + ]); + const signatureHex = ethUtil.bufferToHex(signature); + // Validate signature. + // This will fail because `signerAddress` signed the message, but we're passing in `notSignerAddress` + const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + notSignerAddress, + signatureHex, + ); + expect(isValidSignature).to.be.false(); + }); + + it('should return true when SignatureType=EthSign and signature is valid', async () => { + // Create EthSign signature + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const orderHashWithEthSignPrefixHex = addSignedMessagePrefix(orderHashHex, MessagePrefixType.EthSign); + const orderHashWithEthSignPrefixBuffer = ethUtil.toBuffer(orderHashWithEthSignPrefixHex); + const ecSignature = ethUtil.ecsign(orderHashWithEthSignPrefixBuffer, signerPrivateKey); + // Create 0x signature from EthSign signature + const signature = Buffer.concat([ + ethUtil.toBuffer(ecSignature.v), + ecSignature.r, + ecSignature.s, + ethUtil.toBuffer(`0x${SignatureType.EthSign}`), + ]); + const signatureHex = ethUtil.bufferToHex(signature); + // Validate signature + const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + signerAddress, + signatureHex, + ); + expect(isValidSignature).to.be.true(); + }); + + it('should return false when SignatureType=EthSign and signature is invalid', async () => { + // Create EthSign signature + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const orderHashWithEthSignPrefixHex = addSignedMessagePrefix(orderHashHex, MessagePrefixType.EthSign); + const orderHashWithEthSignPrefixBuffer = ethUtil.toBuffer(orderHashWithEthSignPrefixHex); + const ecSignature = ethUtil.ecsign(orderHashWithEthSignPrefixBuffer, signerPrivateKey); + // Create 0x signature from EthSign signature + const signature = Buffer.concat([ + ethUtil.toBuffer(ecSignature.v), + ecSignature.r, + ecSignature.s, + ethUtil.toBuffer(`0x${SignatureType.EthSign}`), + ]); + const signatureHex = ethUtil.bufferToHex(signature); + // Validate signature. + // This will fail because `signerAddress` signed the message, but we're passing in `notSignerAddress` + const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + notSignerAddress, + signatureHex, + ); + expect(isValidSignature).to.be.false(); + }); + + it('should return true when SignatureType=Caller and signer is caller', async () => { + const signature = ethUtil.toBuffer(`0x${SignatureType.Caller}`); + const signatureHex = ethUtil.bufferToHex(signature); + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + signerAddress, + signatureHex, + { from: signerAddress }, + ); + expect(isValidSignature).to.be.true(); + }); + + it('should return false when SignatureType=Caller and signer is not caller', async () => { + const signature = ethUtil.toBuffer(`0x${SignatureType.Caller}`); + const signatureHex = ethUtil.bufferToHex(signature); + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + signerAddress, + signatureHex, + { from: notSignerAddress }, + ); + expect(isValidSignature).to.be.false(); + }); + + it('should return true when SignatureType=Wallet and signature is valid', async () => { + // Create EIP712 signature + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const orderHashBuffer = ethUtil.toBuffer(orderHashHex); + const ecSignature = ethUtil.ecsign(orderHashBuffer, signerPrivateKey); + // Create 0x signature from EIP712 signature + const signature = Buffer.concat([ + ethUtil.toBuffer(ecSignature.v), + ecSignature.r, + ecSignature.s, + ethUtil.toBuffer(`0x${SignatureType.Wallet}`), + ]); + const signatureHex = ethUtil.bufferToHex(signature); + // Validate signature + const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + testWallet.address, + signatureHex, + ); + expect(isValidSignature).to.be.true(); + }); + + it('should return false when SignatureType=Wallet and signature is invalid', async () => { + // Create EIP712 signature using a private key that does not belong to the wallet owner. + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const orderHashBuffer = ethUtil.toBuffer(orderHashHex); + const notWalletOwnerPrivateKey = notSignerPrivateKey; + const ecSignature = ethUtil.ecsign(orderHashBuffer, notWalletOwnerPrivateKey); + // Create 0x signature from EIP712 signature + const signature = Buffer.concat([ + ethUtil.toBuffer(ecSignature.v), + ecSignature.r, + ecSignature.s, + ethUtil.toBuffer(`0x${SignatureType.Wallet}`), + ]); + const signatureHex = ethUtil.bufferToHex(signature); + // Validate signature + const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + testWallet.address, + signatureHex, + ); + expect(isValidSignature).to.be.false(); + }); + + it('should return true when SignatureType=Validator, signature is valid and validator is approved', async () => { + const validatorAddress = ethUtil.toBuffer(`${testValidator.address}`); + const signatureType = ethUtil.toBuffer(`0x${SignatureType.Validator}`); + const signature = Buffer.concat([validatorAddress, signatureType]); + const signatureHex = ethUtil.bufferToHex(signature); + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + signerAddress, + signatureHex, + ); + expect(isValidSignature).to.be.true(); + }); + + it('should return false when SignatureType=Validator, signature is invalid and validator is approved', async () => { + const validatorAddress = ethUtil.toBuffer(`${testValidator.address}`); + const signatureType = ethUtil.toBuffer(`0x${SignatureType.Validator}`); + const signature = Buffer.concat([validatorAddress, signatureType]); + const signatureHex = ethUtil.bufferToHex(signature); + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + // This will return false because we signed the message with `signerAddress`, but + // are validating against `notSignerAddress` + const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + notSignerAddress, + signatureHex, + ); + expect(isValidSignature).to.be.false(); + }); + + it('should return false when SignatureType=Validator, signature is valid and validator is not approved', async () => { + // Set approval of signature validator to false + await web3Wrapper.awaitTransactionSuccessAsync( + await signatureValidator.setSignatureValidatorApproval.sendTransactionAsync( + testValidator.address, + false, + { from: signerAddress }, + ), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + // Validate signature + const validatorAddress = ethUtil.toBuffer(`${testValidator.address}`); + const signatureType = ethUtil.toBuffer(`0x${SignatureType.Validator}`); + const signature = Buffer.concat([validatorAddress, signatureType]); + const signatureHex = ethUtil.bufferToHex(signature); + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + signerAddress, + signatureHex, + ); + expect(isValidSignature).to.be.false(); + }); + + it('should return true when SignatureType=Trezor and signature is valid', async () => { + // Create Trezor signature const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const orderHashWithTrezorPrefixHex = addSignedMessagePrefix(orderHashHex, MessagePrefixType.Trezor); + const orderHashWithTrezorPrefixBuffer = ethUtil.toBuffer(orderHashWithTrezorPrefixHex); + const ecSignature = ethUtil.ecsign(orderHashWithTrezorPrefixBuffer, signerPrivateKey); + // Create 0x signature from Trezor signature + const signature = Buffer.concat([ + ethUtil.toBuffer(ecSignature.v), + ecSignature.r, + ecSignature.s, + ethUtil.toBuffer(`0x${SignatureType.Trezor}`), + ]); + const signatureHex = ethUtil.bufferToHex(signature); + // Validate signature + const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + signerAddress, + signatureHex, + ); + expect(isValidSignature).to.be.true(); + }); + + it('should return false when SignatureType=Trezor and signature is invalid', async () => { + // Create Trezor signature + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const orderHashWithTrezorPrefixHex = addSignedMessagePrefix(orderHashHex, MessagePrefixType.Trezor); + const orderHashWithTrezorPrefixBuffer = ethUtil.toBuffer(orderHashWithTrezorPrefixHex); + const ecSignature = ethUtil.ecsign(orderHashWithTrezorPrefixBuffer, signerPrivateKey); + // Create 0x signature from Trezor signature + const signature = Buffer.concat([ + ethUtil.toBuffer(ecSignature.v), + ecSignature.r, + ecSignature.s, + ethUtil.toBuffer(`0x${SignatureType.Trezor}`), + ]); + const signatureHex = ethUtil.bufferToHex(signature); + // Validate signature. + // This will fail because `signerAddress` signed the message, but we're passing in `notSignerAddress` + const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + notSignerAddress, + signatureHex, + ); + expect(isValidSignature).to.be.false(); + }); + + it('should return true when SignatureType=Presigned and signer has presigned hash', async () => { + // Presign hash + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + await web3Wrapper.awaitTransactionSuccessAsync( + await signatureValidator.preSign.sendTransactionAsync( + orderHashHex, + signedOrder.makerAddress, + signedOrder.signature, + ), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + // Validate presigned signature + const signature = ethUtil.toBuffer(`0x${SignatureType.PreSigned}`); + const signatureHex = ethUtil.bufferToHex(signature); const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( orderHashHex, signedOrder.makerAddress, - signedOrder.signature, + signatureHex, ); expect(isValidSignature).to.be.true(); }); - 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 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; + it('should return false when SignatureType=Presigned and signer has not presigned hash', async () => { + const signature = ethUtil.toBuffer(`0x${SignatureType.PreSigned}`); + const signatureHex = ethUtil.bufferToHex(signature); const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( orderHashHex, signedOrder.makerAddress, - signedOrder.signature, + signatureHex, ); expect(isValidSignature).to.be.false(); }); |