diff --git a/contracts/coordinator/CHANGELOG.json b/contracts/coordinator/CHANGELOG.json index 1d4ce3877d..5505b07299 100644 --- a/contracts/coordinator/CHANGELOG.json +++ b/contracts/coordinator/CHANGELOG.json @@ -4,6 +4,9 @@ "changes": [ { "note": "Created Coordinator package" + }, + { + "note": "Use mixed EIP712 domains for transactions and approvals." } ] } diff --git a/contracts/coordinator/contracts/src/MixinCoordinatorApprovalVerifier.sol b/contracts/coordinator/contracts/src/MixinCoordinatorApprovalVerifier.sol index b0d72f73fd..408fa69b4e 100644 --- a/contracts/coordinator/contracts/src/MixinCoordinatorApprovalVerifier.sol +++ b/contracts/coordinator/contracts/src/MixinCoordinatorApprovalVerifier.sol @@ -125,14 +125,14 @@ contract MixinCoordinatorApprovalVerifier is // Hash approval message and recover signer address bytes32 approvalHash = getCoordinatorApprovalHash(approval); address approvalSignerAddress = getSignerAddress(approvalHash, approvalSignatures[i]); - + // Add approval signer to list of signers approvalSignerAddresses = approvalSignerAddresses.append(approvalSignerAddress); } - + // Ethereum transaction signer gives implicit signature of approval approvalSignerAddresses = approvalSignerAddresses.append(tx.origin); - + uint256 ordersLength = orders.length; for (uint256 i = 0; i != ordersLength; i++) { // Do not check approval if the order's senderAddress is null diff --git a/contracts/coordinator/contracts/src/libs/LibEIP712Domain.sol b/contracts/coordinator/contracts/src/libs/LibEIP712Domain.sol index e18393db28..7754bbe92b 100644 --- a/contracts/coordinator/contracts/src/libs/LibEIP712Domain.sol +++ b/contracts/coordinator/contracts/src/libs/LibEIP712Domain.sol @@ -20,6 +20,7 @@ pragma solidity ^0.5.5; import "./LibConstants.sol"; + contract LibEIP712Domain is LibConstants { @@ -39,7 +40,6 @@ contract LibEIP712Domain is // EIP712 Domain Version value for the Exchange string constant internal EIP712_EXCHANGE_DOMAIN_VERSION = "2"; - // Hash of the EIP712 Domain Separator Schema bytes32 constant internal EIP712_DOMAIN_SEPARATOR_SCHEMA_HASH = keccak256(abi.encodePacked( "EIP712Domain(", diff --git a/contracts/coordinator/contracts/test/TestLibs.sol b/contracts/coordinator/contracts/test/TestLibs.sol index 6b4cd169fc..8c615a5d42 100644 --- a/contracts/coordinator/contracts/test/TestLibs.sol +++ b/contracts/coordinator/contracts/test/TestLibs.sol @@ -24,6 +24,7 @@ import "../src/libs/LibCoordinatorApproval.sol"; import "../src/libs/LibZeroExTransaction.sol"; +// solhint-disable no-empty-blocks contract TestLibs is LibConstants, LibCoordinatorApproval, diff --git a/contracts/coordinator/test/libs.ts b/contracts/coordinator/test/libs.ts index 08ea109cc1..85291d3583 100644 --- a/contracts/coordinator/test/libs.ts +++ b/contracts/coordinator/test/libs.ts @@ -1,11 +1,4 @@ -import { - addressUtils, - chaiSetup, - constants, - provider, - txDefaults, - web3Wrapper -} from '@0x/contracts-test-utils'; +import { addressUtils, chaiSetup, constants, provider, txDefaults, web3Wrapper } from '@0x/contracts-test-utils'; import { BlockchainLifecycle } from '@0x/dev-utils'; import { transactionHashUtils } from '@0x/order-utils'; import { BigNumber } from '@0x/utils'; diff --git a/contracts/coordinator/test/mixins.ts b/contracts/coordinator/test/mixins.ts index 48171bb7e7..a862791b0f 100644 --- a/contracts/coordinator/test/mixins.ts +++ b/contracts/coordinator/test/mixins.ts @@ -16,13 +16,7 @@ import { BigNumber } from '@0x/utils'; import * as chai from 'chai'; import * as ethUtil from 'ethereumjs-util'; -import { - ApprovalFactory, - artifacts, - constants, - exchangeDataEncoder, - TestMixinsContract, -} from '../src'; +import { ApprovalFactory, artifacts, constants, exchangeDataEncoder, TestMixinsContract } from '../src'; chaiSetup.configure(); const expect = chai.expect; @@ -88,20 +82,14 @@ describe('Mixins tests', () => { describe('getSignerAddress', () => { it('should return the correct address using the EthSign signature type', async () => { const data = devConstants.NULL_BYTES; - const transaction = transactionFactory.newSignedTransaction( - data, - SignatureType.EthSign, - ); + const transaction = transactionFactory.newSignedTransaction(data, SignatureType.EthSign); const transactionHash = transactionHashUtils.getTransactionHashHex(transaction); const signerAddress = await mixins.getSignerAddress.callAsync(transactionHash, transaction.signature); expect(transaction.signerAddress).to.eq(signerAddress); }); it('should return the correct address using the EIP712 signature type', async () => { const data = devConstants.NULL_BYTES; - const transaction = transactionFactory.newSignedTransaction( - data, - SignatureType.EIP712, - ); + const transaction = transactionFactory.newSignedTransaction(data, SignatureType.EIP712); const transactionHash = transactionHashUtils.getTransactionHashHex(transaction); const signerAddress = await mixins.getSignerAddress.callAsync(transactionHash, transaction.signature); expect(transaction.signerAddress).to.eq(signerAddress); diff --git a/contracts/coordinator/test/utils/approval_factory.ts b/contracts/coordinator/test/utils/approval_factory.ts index 62388e7eb1..f0c5ea145d 100644 --- a/contracts/coordinator/test/utils/approval_factory.ts +++ b/contracts/coordinator/test/utils/approval_factory.ts @@ -9,10 +9,7 @@ export class ApprovalFactory { private readonly _privateKey: Buffer; private readonly _verifyingContractAddress: string; - constructor( - privateKey: Buffer, - verifyingContractAddress: string) { - + constructor(privateKey: Buffer, verifyingContractAddress: string) { this._privateKey = privateKey; this._verifyingContractAddress = verifyingContractAddress; } @@ -23,21 +20,16 @@ export class ApprovalFactory { approvalExpirationTimeSeconds: BigNumber, signatureType: SignatureType = SignatureType.EthSign, ): SignedCoordinatorApproval { - const approvalHashBuff = hashUtils.getApprovalHashBuffer( transaction, this._verifyingContractAddress, txOrigin, approvalExpirationTimeSeconds, ); - const signatureBuff = signingUtils.signMessage( - approvalHashBuff, - this._privateKey, - signatureType, - ); + const signatureBuff = signingUtils.signMessage(approvalHashBuff, this._privateKey, signatureType); const signedApproval = { txOrigin, - transaction: transaction, + transaction, approvalExpirationTimeSeconds, signature: ethUtil.addHexPrefix(signatureBuff.toString('hex')), }; diff --git a/contracts/coordinator/test/utils/hash_utils.ts b/contracts/coordinator/test/utils/hash_utils.ts index e6a2dded45..faca38f2c2 100644 --- a/contracts/coordinator/test/utils/hash_utils.ts +++ b/contracts/coordinator/test/utils/hash_utils.ts @@ -1,5 +1,5 @@ import { constants, eip712Utils, transactionHashUtils } from '@0x/order-utils'; -import { SignedZeroExTransaction, ZeroExTransaction } from '@0x/types'; +import { SignedZeroExTransaction } from '@0x/types'; import { BigNumber, signTypedDataUtils } from '@0x/utils'; import * as _ from 'lodash'; @@ -13,7 +13,7 @@ export const hashUtils = { const domain = { name: constants.COORDINATOR_DOMAIN_NAME, version: constants.COORDINATOR_DOMAIN_VERSION, - verifyingContractAddress: verifyingContractAddress, + verifyingContractAddress, }; const transactionHash = transactionHashUtils.getTransactionHashHex(transaction); const approval = { @@ -40,12 +40,7 @@ export const hashUtils = { approvalExpirationTimeSeconds: BigNumber, ): string { const hashHex = `0x${hashUtils - .getApprovalHashBuffer( - transaction, - verifyingContractAddress, - txOrigin, - approvalExpirationTimeSeconds, - ) + .getApprovalHashBuffer(transaction, verifyingContractAddress, txOrigin, approvalExpirationTimeSeconds) .toString('hex')}`; return hashHex; }, diff --git a/contracts/test-utils/CHANGELOG.json b/contracts/test-utils/CHANGELOG.json index 5a01a0d2b5..a1c776dab7 100644 --- a/contracts/test-utils/CHANGELOG.json +++ b/contracts/test-utils/CHANGELOG.json @@ -5,6 +5,9 @@ { "note": "Set evmVersion to byzantium", "pr": 1678 + }, + { + "note": "Remove Coordinator EIP712 constants. They're now in the `order-utils` package." } ] }, diff --git a/packages/order-utils/CHANGELOG.json b/packages/order-utils/CHANGELOG.json index 5d20b98fa1..9d3859b009 100644 --- a/packages/order-utils/CHANGELOG.json +++ b/packages/order-utils/CHANGELOG.json @@ -1,4 +1,15 @@ [ + { + "version" : "7.1.0", + "changes": [ + { + "note": "Add Coordinator EIP712 constants" + }, + { + "note": "Export constants" + } + ] + }, { "timestamp": 1551479279, "version": "7.0.2", diff --git a/packages/types/CHANGELOG.json b/packages/types/CHANGELOG.json index 36c97647b0..d0f5097780 100644 --- a/packages/types/CHANGELOG.json +++ b/packages/types/CHANGELOG.json @@ -5,6 +5,9 @@ { "note": "Added ERC1155 revert reasons", "pr": 1657 + }, + { + "note": "Add `RevertReason.SignatureInvalid` thrown by Coordinator" } ] }, diff --git a/yarn.lock b/yarn.lock index 4601cd467b..725e4592d8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -13507,15 +13507,6 @@ react-dom@^16.3.2: object-assign "^4.1.1" prop-types "^15.6.0" -react-dom@^16.4.2: - version "16.8.4" - resolved "https://registry.npmjs.org/react-dom/-/react-dom-16.8.4.tgz#1061a8e01a2b3b0c8160037441c3bf00a0e3bc48" - dependencies: - loose-envify "^1.1.0" - object-assign "^4.1.1" - prop-types "^15.6.2" - scheduler "^0.13.4" - react-dom@^16.5.2: version "16.5.2" resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-16.5.2.tgz#b69ee47aa20bab5327b2b9d7c1fe2a30f2cfa9d7" @@ -13817,15 +13808,6 @@ react@^16.3.2: object-assign "^4.1.1" prop-types "^15.6.0" -react@^16.4.2: - version "16.8.4" - resolved "https://registry.npmjs.org/react/-/react-16.8.4.tgz#fdf7bd9ae53f03a9c4cd1a371432c206be1c4768" - dependencies: - loose-envify "^1.1.0" - object-assign "^4.1.1" - prop-types "^15.6.2" - scheduler "^0.13.4" - react@^16.5.2: version "16.5.2" resolved "https://registry.yarnpkg.com/react/-/react-16.5.2.tgz#19f6b444ed139baa45609eee6dc3d318b3895d42" @@ -14697,13 +14679,6 @@ schedule@^0.5.0: dependencies: object-assign "^4.1.1" -scheduler@^0.13.4: - version "0.13.4" - resolved "https://registry.npmjs.org/scheduler/-/scheduler-0.13.4.tgz#8fef05e7a3580c76c0364d2df5e550e4c9140298" - dependencies: - loose-envify "^1.1.0" - object-assign "^4.1.1" - schema-utils@^0.4.4: version "0.4.7" resolved "https://registry.npmjs.org/schema-utils/-/schema-utils-0.4.7.tgz#ba74f597d2be2ea880131746ee17d0a093c68187"