From b6a133cc641617bd1099ec00c62a749f548316b7 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 27 Feb 2018 13:51:12 -0800 Subject: Improve an error message when an inorrect number of constructor params is passed --- packages/contracts/test/exchange/core.ts | 8 ++++---- packages/contracts/test/exchange/helpers.ts | 6 +++--- packages/contracts/test/exchange/wrapper.ts | 6 +++--- .../contracts/test/token_transfer_proxy/transfer_from.ts | 2 +- packages/contracts/test/unlimited_allowance_token.ts | 2 +- packages/contracts/util/constants.ts | 6 ++++++ packages/deployer/CHANGELOG.md | 3 ++- packages/deployer/src/deployer.ts | 14 +++++++++++++- 8 files changed, 33 insertions(+), 14 deletions(-) diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts index 303d745aa..ad5b3a083 100644 --- a/packages/contracts/test/exchange/core.ts +++ b/packages/contracts/test/exchange/core.ts @@ -56,9 +56,9 @@ describe('Exchange', () => { maker = accounts[0]; [tokenOwner, taker, feeRecipient] = accounts; const [repInstance, dgdInstance, zrxInstance] = await Promise.all([ - deployer.deployAsync(ContractName.DummyToken), - deployer.deployAsync(ContractName.DummyToken), - deployer.deployAsync(ContractName.DummyToken), + deployer.deployAsync(ContractName.DummyToken, []), + deployer.deployAsync(ContractName.DummyToken, constants.DUMMY_TOKEN_ARGS), + deployer.deployAsync(ContractName.DummyToken, constants.DUMMY_TOKEN_ARGS), ]); rep = new DummyTokenContract(web3Wrapper, repInstance.abi, repInstance.address); dgd = new DummyTokenContract(web3Wrapper, dgdInstance.abi, dgdInstance.address); @@ -128,7 +128,7 @@ describe('Exchange', () => { await blockchainLifecycle.revertAsync(); }); describe('internal functions', () => { - it('should include transferViaTokenTransferProxy', () => { + it.only('should include transferViaTokenTransferProxy', () => { expect((exchange as any).transferViaTokenTransferProxy).to.be.undefined(); }); diff --git a/packages/contracts/test/exchange/helpers.ts b/packages/contracts/test/exchange/helpers.ts index 9869c2155..625234729 100644 --- a/packages/contracts/test/exchange/helpers.ts +++ b/packages/contracts/test/exchange/helpers.ts @@ -39,9 +39,9 @@ describe('Exchange', () => { const tokenRegistry = await deployer.deployAsync(ContractName.TokenRegistry); const tokenTransferProxy = await deployer.deployAsync(ContractName.TokenTransferProxy); const [rep, dgd, zrx] = await Promise.all([ - deployer.deployAsync(ContractName.DummyToken), - deployer.deployAsync(ContractName.DummyToken), - deployer.deployAsync(ContractName.DummyToken), + deployer.deployAsync(ContractName.DummyToken, constants.DUMMY_TOKEN_ARGS), + deployer.deployAsync(ContractName.DummyToken, constants.DUMMY_TOKEN_ARGS), + deployer.deployAsync(ContractName.DummyToken, constants.DUMMY_TOKEN_ARGS), ]); const exchangeInstance = await deployer.deployAsync(ContractName.Exchange, [ zrx.address, diff --git a/packages/contracts/test/exchange/wrapper.ts b/packages/contracts/test/exchange/wrapper.ts index 4ea40cb59..239f13a4f 100644 --- a/packages/contracts/test/exchange/wrapper.ts +++ b/packages/contracts/test/exchange/wrapper.ts @@ -56,9 +56,9 @@ describe('Exchange', () => { tokenOwner = accounts[0]; [maker, taker, feeRecipient] = accounts; const [repInstance, dgdInstance, zrxInstance] = await Promise.all([ - deployer.deployAsync(ContractName.DummyToken), - deployer.deployAsync(ContractName.DummyToken), - deployer.deployAsync(ContractName.DummyToken), + deployer.deployAsync(ContractName.DummyToken, constants.DUMMY_TOKEN_ARGS), + deployer.deployAsync(ContractName.DummyToken, constants.DUMMY_TOKEN_ARGS), + deployer.deployAsync(ContractName.DummyToken, constants.DUMMY_TOKEN_ARGS), ]); rep = new DummyTokenContract(web3Wrapper, repInstance.abi, repInstance.address); dgd = new DummyTokenContract(web3Wrapper, dgdInstance.abi, dgdInstance.address); diff --git a/packages/contracts/test/token_transfer_proxy/transfer_from.ts b/packages/contracts/test/token_transfer_proxy/transfer_from.ts index 6b86a0e97..a77590288 100644 --- a/packages/contracts/test/token_transfer_proxy/transfer_from.ts +++ b/packages/contracts/test/token_transfer_proxy/transfer_from.ts @@ -38,7 +38,7 @@ describe('TokenTransferProxy', () => { tokenTransferProxyInstance.abi, tokenTransferProxyInstance.address, ); - const repInstance = await deployer.deployAsync(ContractName.DummyToken); + const repInstance = await deployer.deployAsync(ContractName.DummyToken, constants.DUMMY_TOKEN_ARGS); rep = new DummyTokenContract(web3Wrapper, repInstance.abi, repInstance.address); dmyBalances = new Balances([rep], [accounts[0], accounts[1]]); diff --git a/packages/contracts/test/unlimited_allowance_token.ts b/packages/contracts/test/unlimited_allowance_token.ts index 03eb581ad..553178d80 100644 --- a/packages/contracts/test/unlimited_allowance_token.ts +++ b/packages/contracts/test/unlimited_allowance_token.ts @@ -34,7 +34,7 @@ describe('UnlimitedAllowanceToken', () => { const accounts = await web3Wrapper.getAvailableAddressesAsync(); owner = accounts[0]; spender = accounts[1]; - const tokenInstance = await deployer.deployAsync(ContractName.DummyToken); + const tokenInstance = await deployer.deployAsync(ContractName.DummyToken, constants.DUMMY_TOKEN_ARGS); token = new DummyTokenContract(web3Wrapper, tokenInstance.abi, tokenInstance.address); await token.mint.sendTransactionAsync(MAX_MINT_VALUE, { from: owner }); tokenAddress = token.address; diff --git a/packages/contracts/util/constants.ts b/packages/contracts/util/constants.ts index e61b2f802..9bb090a2a 100644 --- a/packages/contracts/util/constants.ts +++ b/packages/contracts/util/constants.ts @@ -1,3 +1,8 @@ +const DUMMY_TOKEN_NAME = ''; +const DUMMY_TOKEN_SYMBOL = ''; +const DUMMY_TOKEN_DECIMALS = 18; +const DUMMY_TOKEN_TOTAL_SUPPLY = 0; + export const constants = { NULL_BYTES: '0x', INVALID_OPCODE: 'invalid opcode', @@ -6,4 +11,5 @@ export const constants = { MAX_ETHERTOKEN_WITHDRAW_GAS: 43000, MAX_TOKEN_TRANSFERFROM_GAS: 80000, MAX_TOKEN_APPROVE_GAS: 60000, + DUMMY_TOKEN_ARGS: [DUMMY_TOKEN_NAME, DUMMY_TOKEN_SYMBOL, DUMMY_TOKEN_DECIMALS, DUMMY_TOKEN_TOTAL_SUPPLY], }; diff --git a/packages/deployer/CHANGELOG.md b/packages/deployer/CHANGELOG.md index a63d9cf3b..d8bbbbf89 100644 --- a/packages/deployer/CHANGELOG.md +++ b/packages/deployer/CHANGELOG.md @@ -2,7 +2,8 @@ ## v0.2.0 - _TBD, 2018_ - * Check dependencies when determining if contracts should be recompiled (#408). + * Check dependencies when determining if contracts should be recompiled (#408) + * Improve an error message for when deployer is supplied with an incorrect number of constructor arguments (#TBD) ## v0.1.0 - _February 16, 2018_ diff --git a/packages/deployer/src/deployer.ts b/packages/deployer/src/deployer.ts index 6710bcc85..e87d2ab0e 100644 --- a/packages/deployer/src/deployer.ts +++ b/packages/deployer/src/deployer.ts @@ -1,4 +1,4 @@ -import { TxData } from '@0xproject/types'; +import { AbiType, TxData } from '@0xproject/types'; import { Web3Wrapper } from '@0xproject/web3-wrapper'; import * as _ from 'lodash'; import * as Web3 from 'web3'; @@ -49,6 +49,18 @@ export class Deployer { gas, }; const abi = contractNetworkDataIfExists.abi; + const constructorAbi = _.find(abi, { type: AbiType.Constructor }) as Web3.ConstructorAbi; + const constructorArgs = _.isUndefined(constructorAbi) ? [] : constructorAbi.inputs; + if (constructorArgs.length !== args.length) { + const constructorSignature = `constructor(${_.map(constructorArgs, arg => `${arg.type} ${arg.name}`).join( + ', ', + )})`; + throw new Error( + `${contractName} expects ${constructorArgs.length} constructor params: ${constructorSignature}. Got ${ + args.length + }`, + ); + } const web3ContractInstance = await this._deployFromAbiAsync(abi, args, txData); utils.consoleLog(`${contractName}.sol successfully deployed at ${web3ContractInstance.address}`); const contractInstance = new Contract(web3ContractInstance, this._defaults); -- cgit From 90236b87de573bf4ed95f13b5a799d71a9f87f8d Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 27 Feb 2018 14:29:55 -0800 Subject: Add PR name --- packages/deployer/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/deployer/CHANGELOG.md b/packages/deployer/CHANGELOG.md index d8bbbbf89..a1929ba25 100644 --- a/packages/deployer/CHANGELOG.md +++ b/packages/deployer/CHANGELOG.md @@ -3,7 +3,7 @@ ## v0.2.0 - _TBD, 2018_ * Check dependencies when determining if contracts should be recompiled (#408) - * Improve an error message for when deployer is supplied with an incorrect number of constructor arguments (#TBD) + * Improve an error message for when deployer is supplied with an incorrect number of constructor arguments (#419) ## v0.1.0 - _February 16, 2018_ -- cgit From 8b6cc95c1b69fe8372f25168714a5fd683dd2e26 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 27 Feb 2018 14:42:39 -0800 Subject: Fix a typo --- packages/contracts/test/exchange/core.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts index ad5b3a083..09f796663 100644 --- a/packages/contracts/test/exchange/core.ts +++ b/packages/contracts/test/exchange/core.ts @@ -56,7 +56,7 @@ describe('Exchange', () => { maker = accounts[0]; [tokenOwner, taker, feeRecipient] = accounts; const [repInstance, dgdInstance, zrxInstance] = await Promise.all([ - deployer.deployAsync(ContractName.DummyToken, []), + deployer.deployAsync(ContractName.DummyToken, constants.DUMMY_TOKEN_ARGS), deployer.deployAsync(ContractName.DummyToken, constants.DUMMY_TOKEN_ARGS), deployer.deployAsync(ContractName.DummyToken, constants.DUMMY_TOKEN_ARGS), ]); -- cgit From bab8c1eeff287a5dd90754c3391fd702d974d9d6 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 27 Feb 2018 14:43:29 -0800 Subject: Remove only --- packages/contracts/test/exchange/core.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts index 09f796663..710d6fe94 100644 --- a/packages/contracts/test/exchange/core.ts +++ b/packages/contracts/test/exchange/core.ts @@ -128,7 +128,7 @@ describe('Exchange', () => { await blockchainLifecycle.revertAsync(); }); describe('internal functions', () => { - it.only('should include transferViaTokenTransferProxy', () => { + it('should include transferViaTokenTransferProxy', () => { expect((exchange as any).transferViaTokenTransferProxy).to.be.undefined(); }); -- cgit