@0x/contracts-zero-ex: Address review feedback.

contracts/zero-ex/test/artifacts.ts

@@ -10,8 +10,8 @@ import * as AllowanceTarget from '../test/generated-artifacts/AllowanceTarget.js
 import * as Bootstrap from '../test/generated-artifacts/Bootstrap.json';
 import * as FillQuoteTransformer from '../test/generated-artifacts/FillQuoteTransformer.json';
 import * as FixinCommon from '../test/generated-artifacts/FixinCommon.json';
-import * as FixinGasToken from '../test/generated-artifacts/FixinGasToken.json';
 import * as FixinEIP712 from '../test/generated-artifacts/FixinEIP712.json';
+import * as FixinGasToken from '../test/generated-artifacts/FixinGasToken.json';
 import * as FlashWallet from '../test/generated-artifacts/FlashWallet.json';
 import * as FullMigration from '../test/generated-artifacts/FullMigration.json';
 import * as IAllowanceTarget from '../test/generated-artifacts/IAllowanceTarget.json';
@@ -114,8 +114,8 @@ export const artifacts = {
     TokenSpender: TokenSpender as ContractArtifact,
     TransformERC20: TransformERC20 as ContractArtifact,
     FixinCommon: FixinCommon as ContractArtifact,
-    FixinGasToken: FixinGasToken as ContractArtifact,
     FixinEIP712: FixinEIP712 as ContractArtifact,
+    FixinGasToken: FixinGasToken as ContractArtifact,
     FullMigration: FullMigration as ContractArtifact,
     InitialMigration: InitialMigration as ContractArtifact,
     LibBootstrap: LibBootstrap as ContractArtifact,

contracts/zero-ex/test/features/meta_transactions_test.ts

@@ -74,7 +74,7 @@ blockchainTests.resets('MetaTransactions feature', env => {
             sender,
             minGasPrice: getRandomInteger('2', '1e9'),
             maxGasPrice: getRandomInteger('1e9', '100e9'),
-            expirationTime: new BigNumber(Math.floor(_.now() / 1000) + 360),
+            expirationTimeSeconds: new BigNumber(Math.floor(_.now() / 1000) + 360),
             salt: new BigNumber(hexUtils.random()),
             callData: hexUtils.random(4),
             value: getRandomInteger(1, '1e18'),
@@ -362,7 +362,7 @@ blockchainTests.resets('MetaTransactions feature', env => {
 
         it('fails if expired', async () => {
             const mtx = getRandomMetaTransaction({
-                expirationTime: new BigNumber(Math.floor(_.now() / 1000 - 60)),
+                expirationTimeSeconds: new BigNumber(Math.floor(_.now() / 1000 - 60)),
             });
             const mtxHash = getExchangeProxyMetaTransactionHash(mtx);
             const signature = await signMetaTransactionAsync(mtx);
@@ -375,7 +375,7 @@ blockchainTests.resets('MetaTransactions feature', env => {
                 new ZeroExRevertErrors.MetaTransactions.MetaTransactionExpiredError(
                     mtxHash,
                     undefined,
-                    mtx.expirationTime,
+                    mtx.expirationTimeSeconds,
                 ),
             );
         });
@@ -425,7 +425,7 @@ blockchainTests.resets('MetaTransactions feature', env => {
         });
     });
 
-    describe('executeMetaTransactions()', () => {
+    describe('batchExecuteMetaTransactions()', () => {
         it('can execute multiple transactions', async () => {
             const mtxs = _.times(2, i => {
                 const args = getRandomTransformERC20Args();
@@ -447,9 +447,39 @@ blockchainTests.resets('MetaTransactions feature', env => {
                 gasPrice: BigNumber.max(...mtxs.map(mtx => mtx.minGasPrice)),
                 value: BigNumber.sum(...mtxs.map(mtx => mtx.value)),
             };
-            const rawResults = await feature.executeMetaTransactions(mtxs, signatures).callAsync(callOpts);
+            const rawResults = await feature.batchExecuteMetaTransactions(mtxs, signatures).callAsync(callOpts);
             expect(rawResults).to.eql(mtxs.map(() => RAW_SUCCESS_RESULT));
         });
+
+        it('cannot execute the same transaction twice', async () => {
+            const mtx = (() => {
+                const args = getRandomTransformERC20Args();
+                return getRandomMetaTransaction({
+                    signer: _.sampleSize(signers, 1)[0],
+                    callData: transformERC20Feature
+                        .transformERC20(
+                            args.inputToken,
+                            args.outputToken,
+                            args.inputTokenAmount,
+                            args.minOutputTokenAmount,
+                            args.transformations,
+                        )
+                        .getABIEncodedTransactionData(),
+                });
+            })();
+            const mtxHash = getExchangeProxyMetaTransactionHash(mtx);
+            const mtxs = _.times(2, () => mtx);
+            const signatures = await Promise.all(mtxs.map(async mtx => signMetaTransactionAsync(mtx)));
+            const callOpts = {
+                gasPrice: BigNumber.max(...mtxs.map(mtx => mtx.minGasPrice)),
+                value: BigNumber.sum(...mtxs.map(mtx => mtx.value)),
+            };
+            const block = await env.web3Wrapper.getBlockNumberAsync();
+            const tx = feature.batchExecuteMetaTransactions(mtxs, signatures).callAsync(callOpts);
+            return expect(tx).to.revertWith(
+                new ZeroExRevertErrors.MetaTransactions.MetaTransactionAlreadyExecutedError(mtxHash, block),
+            );
+        });
     });
 
     describe('getMetaTransactionExecutedBlock()', () => {

contracts/zero-ex/test/features/signature_validator_test.ts

@@ -66,13 +66,12 @@ blockchainTests.resets('SignatureValidator feature', env => {
             const hash = hexUtils.random();
             const signer = _.sampleSize(signers, 1)[0];
             const signature = hexUtils.slice(await signatureUtils.ecSignHashAsync(env.provider, hash, signer), 1);
-            const notSigner = randomAddress();
-            const tx = feature.validateHashSignature(hash, notSigner, signature).callAsync();
+            const tx = feature.validateHashSignature(hash, signer, signature).callAsync();
             return expect(tx).to.revertWith(
                 new ZeroExRevertErrors.SignatureValidator.SignatureValidationError(
                     ZeroExRevertErrors.SignatureValidator.SignatureValidationErrorCodes.InvalidLength,
                     hash,
-                    notSigner,
+                    signer,
                     signature,
                 ),
             );

contracts/zero-ex/test/full_migration_test.ts

@@ -44,7 +44,9 @@ blockchainTests.resets('Full migration', env => {
             await migrator.getBootstrapper().callAsync(),
         );
         features = await deployFullFeaturesAsync(env.provider, env.txDefaults, zeroEx.address);
-        await migrator.deploy(owner, zeroEx.address, features, { transformerDeployer }).awaitTransactionSuccessAsync();
+        await migrator
+            .initializeZeroEx(owner, zeroEx.address, features, { transformerDeployer })
+            .awaitTransactionSuccessAsync();
     });
 
     it('ZeroEx has the correct owner', async () => {
@@ -58,10 +60,10 @@ blockchainTests.resets('Full migration', env => {
         expect(dieRecipient).to.eq(owner);
     });
 
-    it('Non-deployer cannot call deploy()', async () => {
+    it('Non-deployer cannot call initializeZeroEx()', async () => {
         const notDeployer = randomAddress();
         const tx = migrator
-            .deploy(owner, zeroEx.address, features, { transformerDeployer })
+            .initializeZeroEx(owner, zeroEx.address, features, { transformerDeployer })
             .callAsync({ from: notDeployer });
         return expect(tx).to.revertWith('FullMigration/INVALID_SENDER');
     });
@@ -89,7 +91,7 @@ blockchainTests.resets('Full migration', env => {
             contractType: IMetaTransactionsContract,
             fns: [
                 'executeMetaTransaction',
-                'executeMetaTransactions',
+                'batchExecuteMetaTransactions',
                 '_executeMetaTransaction',
                 'getMetaTransactionExecutedBlock',
                 'getMetaTransactionHashExecutedBlock',

contracts/zero-ex/test/initial_migration_test.ts

@@ -42,7 +42,7 @@ blockchainTests.resets('Initial migration', env => {
             artifacts,
             migrator.address,
         );
-        await migrator.deploy(owner, zeroEx.address, features).awaitTransactionSuccessAsync();
+        await migrator.initializeZeroEx(owner, zeroEx.address, features).awaitTransactionSuccessAsync();
     });
 
     it('Self-destructs after deployment', async () => {
@@ -50,9 +50,9 @@ blockchainTests.resets('Initial migration', env => {
         expect(dieRecipient).to.eq(owner);
     });
 
-    it('Non-deployer cannot call deploy()', async () => {
+    it('Non-deployer cannot call initializeZeroEx()', async () => {
         const notDeployer = randomAddress();
-        const tx = migrator.deploy(owner, zeroEx.address, features).callAsync({ from: notDeployer });
+        const tx = migrator.initializeZeroEx(owner, zeroEx.address, features).callAsync({ from: notDeployer });
         return expect(tx).to.revertWith('InitialMigration/INVALID_SENDER');
     });
 

contracts/zero-ex/test/wrappers.ts

@@ -8,8 +8,8 @@ export * from '../test/generated-wrappers/allowance_target';
 export * from '../test/generated-wrappers/bootstrap';
 export * from '../test/generated-wrappers/fill_quote_transformer';
 export * from '../test/generated-wrappers/fixin_common';
-export * from '../test/generated-wrappers/fixin_gas_token';
 export * from '../test/generated-wrappers/fixin_e_i_p712';
+export * from '../test/generated-wrappers/fixin_gas_token';
 export * from '../test/generated-wrappers/flash_wallet';
 export * from '../test/generated-wrappers/full_migration';
 export * from '../test/generated-wrappers/i_allowance_target';