From e5fed57b8b229c6eaa4c581662fe8a48d4757ecf Mon Sep 17 00:00:00 2001 From: Lawrence Forman Date: Sat, 30 Mar 2019 14:26:20 -0400 Subject: [PATCH] Make `contracts/exchange-libs/.../LibEIP712.sol` stateless --- .../coordinator/contracts/src/Coordinator.sol | 12 +++++----- .../contracts/src/libs/LibConstants.sol | 6 ++--- .../contracts/src/libs/LibEIP712Domain.sol | 5 ++++- .../exchange-libs/contracts/src/LibEIP712.sol | 22 +++++-------------- .../contracts/src/LibEIP712ExchangeDomain.sol | 4 +++- .../exchange-libs/contracts/test/TestLibs.sol | 2 +- contracts/exchange/contracts/src/Exchange.sol | 11 +++++----- .../contracts/test/TestSignatureValidator.sol | 2 +- 8 files changed, 28 insertions(+), 36 deletions(-) diff --git a/contracts/coordinator/contracts/src/Coordinator.sol b/contracts/coordinator/contracts/src/Coordinator.sol index 8b64e3c598..6a0bdf5a01 100644 --- a/contracts/coordinator/contracts/src/Coordinator.sol +++ b/contracts/coordinator/contracts/src/Coordinator.sol @@ -19,7 +19,6 @@ pragma solidity ^0.5.5; pragma experimental "ABIEncoderV2"; -import "@0x/contracts-exchange-libs/contracts/src/LibEIP712.sol"; import "./libs/LibConstants.sol"; import "./MixinSignatureValidator.sol"; import "./MixinCoordinatorApprovalVerifier.sol"; @@ -29,16 +28,15 @@ import "./MixinCoordinatorCore.sol"; // solhint-disable no-empty-blocks contract Coordinator is LibConstants, - LibEIP712, MixinSignatureValidator, MixinCoordinatorApprovalVerifier, MixinCoordinatorCore { - /// @param _exchange Address of the 0x Exchange contract. - /// @param _chainId Chain ID of the network this contract is deployed on. - constructor (address _exchange, uint256 _chainId) + /// @param exchange Address of the 0x Exchange contract. + /// @param chainId Chain ID of the network this contract is deployed on. + constructor (address exchange, uint256 chainId) public - LibConstants(_exchange) - LibEIP712(_chainId) + LibConstants(exchange) + LibEIP712Domain(chainId) {} } diff --git a/contracts/coordinator/contracts/src/libs/LibConstants.sol b/contracts/coordinator/contracts/src/libs/LibConstants.sol index 1c6a4356ea..f19ea4e3c1 100644 --- a/contracts/coordinator/contracts/src/libs/LibConstants.sol +++ b/contracts/coordinator/contracts/src/libs/LibConstants.sol @@ -27,10 +27,10 @@ contract LibConstants { // The 0x Exchange contract. ITransactions internal EXCHANGE; - /// @param _exchange Address of the 0x Exchange contract. - constructor (address _exchange) + /// @param exchange Address of the 0x Exchange contract. + constructor (address exchange) public { - EXCHANGE = ITransactions(_exchange); + EXCHANGE = ITransactions(exchange); } } diff --git a/contracts/coordinator/contracts/src/libs/LibEIP712Domain.sol b/contracts/coordinator/contracts/src/libs/LibEIP712Domain.sol index aa10023eef..c648901caa 100644 --- a/contracts/coordinator/contracts/src/libs/LibEIP712Domain.sol +++ b/contracts/coordinator/contracts/src/libs/LibEIP712Domain.sol @@ -42,17 +42,20 @@ contract LibEIP712Domain is // Hash of the EIP712 Domain Separator data for the Exchange bytes32 public EIP712_EXCHANGE_DOMAIN_HASH; - constructor () + /// @param chainId Chain ID of the network this contract is deployed on. + constructor (uint256 chainId) public { EIP712_COORDINATOR_DOMAIN_HASH = hashEIP712Domain( EIP712_COORDINATOR_DOMAIN_NAME, EIP712_COORDINATOR_DOMAIN_VERSION, + chainId, address(this) ); EIP712_EXCHANGE_DOMAIN_HASH = hashEIP712Domain( EIP712_EXCHANGE_DOMAIN_NAME, EIP712_EXCHANGE_DOMAIN_VERSION, + chainId, address(EXCHANGE) ); } diff --git a/contracts/exchange-libs/contracts/src/LibEIP712.sol b/contracts/exchange-libs/contracts/src/LibEIP712.sol index 4e4928f10b..792a509409 100644 --- a/contracts/exchange-libs/contracts/src/LibEIP712.sol +++ b/contracts/exchange-libs/contracts/src/LibEIP712.sol @@ -27,30 +27,20 @@ contract LibEIP712 { "string name,", "string version,", "uint256 chainId,", - "address verifyingContract", + "address verifyingContractAddress", ")" )); - // Chain ID of the network this contract is deployed on. - // solhint-disable-next-line var-name-mixedcase - uint256 internal CHAIN_ID; - - /// @param chainId Chain ID of the network this contract is deployed on. - constructor (uint256 chainId) - public - { - CHAIN_ID = chainId; - } - /// @dev Calculates a EIP712 domain separator. /// @param name The EIP712 domain name. /// @param version The EIP712 domain version. - /// @param verifyingContract The EIP712 verifying contract. + /// @param verifyingContractAddress The EIP712 verifying contract. /// @return EIP712 domain separator. function hashEIP712Domain( string memory name, string memory version, - address verifyingContract + uint256 chainId, + address verifyingContractAddress ) internal view @@ -60,8 +50,8 @@ contract LibEIP712 { EIP712_DOMAIN_SEPARATOR_SCHEMA_HASH, keccak256(bytes(name)), keccak256(bytes(version)), - CHAIN_ID, - uint256(verifyingContract) + chainId, + uint256(verifyingContractAddress) )); } diff --git a/contracts/exchange-libs/contracts/src/LibEIP712ExchangeDomain.sol b/contracts/exchange-libs/contracts/src/LibEIP712ExchangeDomain.sol index cf44e178d8..a3c57ad7e1 100644 --- a/contracts/exchange-libs/contracts/src/LibEIP712ExchangeDomain.sol +++ b/contracts/exchange-libs/contracts/src/LibEIP712ExchangeDomain.sol @@ -30,12 +30,14 @@ contract LibEIP712ExchangeDomain is // solhint-disable-next-line var-name-mixedcase bytes32 internal EIP712_EXCHANGE_DOMAIN_HASH; - constructor () + /// @param chainId Chain ID of the network this contract is deployed on. + constructor (uint256 chainId) public { EIP712_EXCHANGE_DOMAIN_HASH = hashEIP712Domain( EIP712_EXCHANGE_DOMAIN_NAME, EIP712_EXCHANGE_DOMAIN_VERSION, + chainId, address(this) ); } diff --git a/contracts/exchange-libs/contracts/test/TestLibs.sol b/contracts/exchange-libs/contracts/test/TestLibs.sol index 161e4169b7..a7afc988e6 100644 --- a/contracts/exchange-libs/contracts/test/TestLibs.sol +++ b/contracts/exchange-libs/contracts/test/TestLibs.sol @@ -36,7 +36,7 @@ contract TestLibs is { constructor (uint256 chainId) public - LibEIP712(chainId) + LibEIP712ExchangeDomain(chainId) {} function publicAbiEncodeFillOrder( diff --git a/contracts/exchange/contracts/src/Exchange.sol b/contracts/exchange/contracts/src/Exchange.sol index 524e5f4e65..ebf39c932f 100644 --- a/contracts/exchange/contracts/src/Exchange.sol +++ b/contracts/exchange/contracts/src/Exchange.sol @@ -20,7 +20,6 @@ pragma solidity ^0.5.5; pragma experimental ABIEncoderV2; import "@0x/contracts-exchange-libs/contracts/src/LibConstants.sol"; -import "@0x/contracts-exchange-libs/contracts/src/LibEIP712.sol"; import "./MixinExchangeCore.sol"; import "./MixinSignatureValidator.sol"; import "./MixinWrapperFunctions.sol"; @@ -41,12 +40,12 @@ contract Exchange is string constant public VERSION = "3.0.0"; /// @dev Mixins are instantiated in the order they are inherited - /// @param _zrxAssetData Asset data for ZRX token. Used for fee transfers. - /// @param _chainId Chain ID of the network this contract is deployed on. - constructor (bytes memory _zrxAssetData, uint256 _chainId) + /// @param zrxAssetData Asset data for ZRX token. Used for fee transfers. + /// @param chainId Chain ID of the network this contract is deployed on. + constructor (bytes memory zrxAssetData, uint256 chainId) public - LibConstants(_zrxAssetData) // @TODO: Remove _zrxAssetData when we deploy. - LibEIP712(_chainId) + LibConstants(zrxAssetData) // @TODO: Remove zrxAssetData when we deploy. + LibEIP712ExchangeDomain(chainId) MixinExchangeCore() MixinMatchOrders() MixinSignatureValidator() diff --git a/contracts/exchange/contracts/test/TestSignatureValidator.sol b/contracts/exchange/contracts/test/TestSignatureValidator.sol index b6158e38e9..4961e15296 100644 --- a/contracts/exchange/contracts/test/TestSignatureValidator.sol +++ b/contracts/exchange/contracts/test/TestSignatureValidator.sol @@ -32,7 +32,7 @@ contract TestSignatureValidator is // solhint-disable no-empty-blocks constructor (uint256 chainId) public - LibEIP712(chainId) + LibEIP712ExchangeDomain(chainId) {} function publicIsValidSignature(