From e4257fb6c7ba14725afa69373cc328ec6de2276f Mon Sep 17 00:00:00 2001 From: Alex Towle Date: Tue, 13 Aug 2019 11:05:37 -0700 Subject: [PATCH] Addressed review comments and prettified code --- .../test/lib_eip712_exchange_domain.ts | 3 - contracts/exchange-libs/test/lib_order.ts | 38 ++++---- .../test/lib_zero_ex_transaction.ts | 42 ++++---- contracts/utils/test/lib_eip712.ts | 97 +++++++++---------- 4 files changed, 85 insertions(+), 95 deletions(-) diff --git a/contracts/exchange-libs/test/lib_eip712_exchange_domain.ts b/contracts/exchange-libs/test/lib_eip712_exchange_domain.ts index df511da977..245c3ef3fd 100644 --- a/contracts/exchange-libs/test/lib_eip712_exchange_domain.ts +++ b/contracts/exchange-libs/test/lib_eip712_exchange_domain.ts @@ -1,6 +1,4 @@ import { blockchainTests, constants, describe, expect, hexRandom } from '@0x/contracts-test-utils'; -import { eip712Utils, orderHashUtils } from '@0x/order-utils'; -import { Order } from '@0x/types'; import { BigNumber, signTypedDataUtils } from '@0x/utils'; import * as ethUtil from 'ethereumjs-util'; import * as _ from 'lodash'; @@ -13,7 +11,6 @@ blockchainTests('LibEIP712ExchangeDomain', env => { const CHAIN_ID = 1337; // Random generator functions - const randomAddress = () => hexRandom(constants.ADDRESS_LENGTH); const randomHash = () => hexRandom(constants.WORD_LENGTH); /** diff --git a/contracts/exchange-libs/test/lib_order.ts b/contracts/exchange-libs/test/lib_order.ts index 517f996dc5..61a42b4b4f 100644 --- a/contracts/exchange-libs/test/lib_order.ts +++ b/contracts/exchange-libs/test/lib_order.ts @@ -36,17 +36,14 @@ blockchainTests('LibOrder', env => { expirationTimeSeconds: constants.ZERO_AMOUNT, }; - /** - * Tests the `_hashOrder()` function against a reference hash. - */ - async function testHashOrderAsync(order: Order): Promise { - const typedData = eip712Utils.createOrderTypedData(order); - const expectedHash = '0x'.concat( - signTypedDataUtils.generateTypedDataHashWithoutDomain(typedData).toString('hex'), + before(async () => { + libsContract = await TestLibsContract.deployFrom0xArtifactAsync( + artifacts.TestLibs, + env.provider, + env.txDefaults, + new BigNumber(CHAIN_ID), ); - const actualHash = await libsContract.hashOrder.callAsync(order); - expect(actualHash).to.be.eq(expectedHash); - } + }); /** * Tests the `getOrderHash()` function against a reference hash. @@ -57,15 +54,6 @@ blockchainTests('LibOrder', env => { expect(actualHash).to.be.eq(expectedHash); } - before(async () => { - libsContract = await TestLibsContract.deployFrom0xArtifactAsync( - artifacts.TestLibs, - env.provider, - env.txDefaults, - new BigNumber(CHAIN_ID), - ); - }); - describe('getOrderHash', () => { it('should correctly hash an empty order', async () => { await testGetOrderHashAsync({ @@ -101,6 +89,18 @@ blockchainTests('LibOrder', env => { }); }); + /** + * Tests the `_hashOrder()` function against a reference hash. + */ + async function testHashOrderAsync(order: Order): Promise { + const typedData = eip712Utils.createOrderTypedData(order); + const expectedHash = '0x'.concat( + signTypedDataUtils.generateTypedDataHashWithoutDomain(typedData).toString('hex'), + ); + const actualHash = await libsContract.hashOrder.callAsync(order); + expect(actualHash).to.be.eq(expectedHash); + } + describe('hashOrder', () => { it('should correctly hash an empty order', async () => { await testHashOrderAsync(EMPTY_ORDER); diff --git a/contracts/exchange-libs/test/lib_zero_ex_transaction.ts b/contracts/exchange-libs/test/lib_zero_ex_transaction.ts index 42a00cbeb0..f6685e4dc3 100644 --- a/contracts/exchange-libs/test/lib_zero_ex_transaction.ts +++ b/contracts/exchange-libs/test/lib_zero_ex_transaction.ts @@ -26,17 +26,14 @@ blockchainTests('LibZeroExTransaction', env => { }, }; - /** - * Tests the `_hashZeroExTransaction()` function against a reference hash. - */ - async function testHashZeroExTransactionAsync(transaction: ZeroExTransaction): Promise { - const typedData = eip712Utils.createZeroExTransactionTypedData(transaction); - const expectedHash = '0x'.concat( - signTypedDataUtils.generateTypedDataHashWithoutDomain(typedData).toString('hex'), + before(async () => { + libsContract = await TestLibsContract.deployFrom0xArtifactAsync( + artifacts.TestLibs, + env.provider, + env.txDefaults, + new BigNumber(CHAIN_ID), ); - const actualHash = await libsContract.hashZeroExTransaction.callAsync(transaction); - expect(actualHash).to.be.eq(expectedHash); - } + }); /** * Tests the `getTransactionHash()` function against a reference hash. @@ -48,15 +45,6 @@ blockchainTests('LibZeroExTransaction', env => { expect(actualHash).to.be.eq(expectedHash); } - before(async () => { - libsContract = await TestLibsContract.deployFrom0xArtifactAsync( - artifacts.TestLibs, - env.provider, - env.txDefaults, - new BigNumber(CHAIN_ID), - ); - }); - describe('getTransactionHash', () => { it('should correctly hash an empty transaction', async () => { await testGetTransactionHashAsync({ @@ -82,12 +70,24 @@ blockchainTests('LibZeroExTransaction', env => { }); }); + /** + * Tests the `_hashZeroExTransaction()` function against a reference hash. + */ + async function testHashZeroExTransactionAsync(transaction: ZeroExTransaction): Promise { + const typedData = eip712Utils.createZeroExTransactionTypedData(transaction); + const expectedHash = '0x'.concat( + signTypedDataUtils.generateTypedDataHashWithoutDomain(typedData).toString('hex'), + ); + const actualHash = await libsContract.hashZeroExTransaction.callAsync(transaction); + expect(actualHash).to.be.eq(expectedHash); + } + describe('hashOrder', () => { - it('should correctly hash an empty order', async () => { + it('should correctly hash an empty transaction', async () => { await testHashZeroExTransactionAsync(EMPTY_TRANSACTION); }); - it('should correctly hash a non-empty order', async () => { + it('should correctly hash a non-empty transaction', async () => { await testHashZeroExTransactionAsync({ salt: randomUint256(), expirationTimeSeconds: randomUint256(), diff --git a/contracts/utils/test/lib_eip712.ts b/contracts/utils/test/lib_eip712.ts index 0a458e1061..812d3e35e2 100644 --- a/contracts/utils/test/lib_eip712.ts +++ b/contracts/utils/test/lib_eip712.ts @@ -11,36 +11,6 @@ chaiSetup.configure(); const expect = chai.expect; const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); -/** - * Tests a specific instance of EIP712 domain hashing. - * @param lib The LibEIP712 contract to call. - * @param name The name of the domain. - * @param version The version of the domain. - * @param chainId The chain id of the domain. - * @param verifyingContractAddress The verifying contract address of the domain. - */ -async function testHashEIP712DomainAsync( - lib: TestLibEIP712Contract, - name: string, - version: string, - chainId: number, - verifyingContractAddress: string, -): Promise { - const expectedHash = signTypedDataUtils.generateDomainHash({ - name, - version, - chainId, - verifyingContractAddress, - }); - const actualHash = await lib.externalHashEIP712DomainSeperator.callAsync( - name, - version, - new BigNumber(chainId), - verifyingContractAddress, - ); - expect(actualHash).to.be.eq(hexConcat(expectedHash)); -} - describe('LibEIP712', () => { let lib: TestLibEIP712Contract; @@ -54,17 +24,56 @@ describe('LibEIP712', () => { await blockchainLifecycle.revertAsync(); }); + /** + * Tests a specific instance of EIP712 domain hashing. + * @param lib The LibEIP712 contract to call. + * @param name The name of the domain. + * @param version The version of the domain. + * @param chainId The chain id of the domain. + * @param verifyingContractAddress The verifying contract address of the domain. + */ + async function testHashEIP712DomainAsync( + name: string, + version: string, + chainId: number, + verifyingContractAddress: string, + ): Promise { + const expectedHash = signTypedDataUtils.generateDomainHash({ + name, + version, + chainId, + verifyingContractAddress, + }); + const actualHash = await lib.externalHashEIP712DomainSeperator.callAsync( + name, + version, + new BigNumber(chainId), + verifyingContractAddress, + ); + expect(actualHash).to.be.eq(hexConcat(expectedHash)); + } + + describe('_hashEIP712Domain', async () => { + it('should correctly hash empty input', async () => { + await testHashEIP712DomainAsync('', '', 0, constants.NULL_ADDRESS); + }); + + it('should correctly hash non-empty input', async () => { + await testHashEIP712DomainAsync('_hashEIP712Domain', '1.0', 62, lib.address); + }); + + it('should correctly hash non-empty input', async () => { + await testHashEIP712DomainAsync('_hashEIP712Domain', '2.0', 0, lib.address); + }); + }); + /** * Tests a specific instance of EIP712 message hashing. * @param lib The LibEIP712 contract to call. * @param domainHash The hash of the EIP712 domain of this instance. * @param hashStruct The hash of the struct of this instance. */ - async function testHashEIP712MessageAsync( - lib: TestLibEIP712Contract, - domainHash: string, - hashStruct: string, - ): Promise { + async function testHashEIP712MessageAsync(domainHash: string, hashStruct: string): Promise { // Remove the hex prefix from the domain hash and the hash struct const unprefixedDomainHash = domainHash.slice(2, domainHash.length); const unprefixedHashStruct = hashStruct.slice(2, hashStruct.length); @@ -80,28 +89,13 @@ describe('LibEIP712', () => { expect(actualHash).to.be.eq(expectedHash); } - describe('_hashEIP712Domain', async () => { - it('should correctly hash empty input', async () => { - await testHashEIP712DomainAsync(lib, '', '', 0, constants.NULL_ADDRESS); - }); - - it('should correctly hash non-empty input', async () => { - await testHashEIP712DomainAsync(lib, '_hashEIP712Domain', '1.0', 62, lib.address); - }); - - it('should correctly hash non-empty input', async () => { - await testHashEIP712DomainAsync(lib, '_hashEIP712Domain', '2.0', 0, lib.address); - }); - }); - describe('_hashEIP712Message', () => { it('should correctly hash empty input', async () => { - await testHashEIP712MessageAsync(lib, constants.NULL_BYTES32, constants.NULL_BYTES32); + await testHashEIP712MessageAsync(constants.NULL_BYTES32, constants.NULL_BYTES32); }); it('should correctly hash non-empty input', async () => { await testHashEIP712MessageAsync( - lib, '0xb10e2d527612073b26eecdfd717e6a320cf44b4afac2b0732d9fcbe2b7fa0cf6', // keccak256(abi.encode(1)) '0x405787fa12a823e0f2b7631cc41b3ba8828b3321ca811111fa75cd3aa3bb5ace', // keccak256(abi.encode(2)) ); @@ -109,7 +103,6 @@ describe('LibEIP712', () => { it('should correctly hash non-empty input', async () => { await testHashEIP712MessageAsync( - lib, '0x405787fa12a823e0f2b7631cc41b3ba8828b3321ca811111fa75cd3aa3bb5ace', // keccak256(abi.encode(2)) '0xc2575a0e9e593c00f959f8c92f12db2869c3395a3b0502d05e2516446f71f85b', // keccak256(abi.encode(3)) );