From eecf09f51564df4f63139f26e65efa1102a9958d Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Mon, 9 Apr 2018 11:55:23 -0700 Subject: Added a detailed description of `renameOverloadedMethods` (special thanks to @fabioberger). Updated Javascript styles in the Abi-Gen and Utils packages, around support for function overloading. --- packages/abi-gen/src/index.ts | 4 +- packages/base-contract/src/index.ts | 13 ++--- packages/deployer/src/cli.ts | 2 +- packages/deployer/src/compiler.ts | 17 ++++--- packages/deployer/src/utils/types.ts | 2 +- packages/metacoin/test/metacoin_test.ts | 12 ++--- packages/utils/src/abi_utils.ts | 85 ++++++++++++++++----------------- yarn.lock | 8 +--- 8 files changed, 68 insertions(+), 75 deletions(-) diff --git a/packages/abi-gen/src/index.ts b/packages/abi-gen/src/index.ts index 9ceebe02b..ecef33b16 100644 --- a/packages/abi-gen/src/index.ts +++ b/packages/abi-gen/src/index.ts @@ -125,7 +125,7 @@ for (const abiFileName of abiFileNames) { } const methodAbis = ABI.filter((abi: AbiDefinition) => abi.type === ABI_TYPE_METHOD) as MethodAbi[]; - const methodAbisSanitized = abiUtils.renameOverloadedMethods(methodAbis) as MethodAbi[]; + const sanitizedMethodAbis = abiUtils.renameOverloadedMethods(methodAbis) as MethodAbi[]; const methodsData = _.map(methodAbis, (methodAbi, methodAbiIndex: number) => { _.forEach(methodAbi.inputs, (input, inputIndex: number) => { if (_.isEmpty(input.name)) { @@ -138,7 +138,7 @@ for (const abiFileName of abiFileNames) { ...methodAbi, singleReturnValue: methodAbi.outputs.length === 1, hasReturnValue: methodAbi.outputs.length !== 0, - tsName: methodAbisSanitized[methodAbiIndex].name, + tsName: sanitizedMethodAbis[methodAbiIndex].name, functionSignature: abiUtils.getFunctionSignature(methodAbi), }; return methodData; diff --git a/packages/base-contract/src/index.ts b/packages/base-contract/src/index.ts index f6cea53fa..bfa99fac1 100644 --- a/packages/base-contract/src/index.ts +++ b/packages/base-contract/src/index.ts @@ -89,13 +89,10 @@ export class BaseContract { const methodAbis = this.abi.filter( (abiDefinition: AbiDefinition) => abiDefinition.type === AbiType.Function, ) as MethodAbi[]; - this._ethersInterfacesByFunctionSignature = _.transform( - methodAbis, - (result: EthersInterfaceByFunctionSignature, methodAbi) => { - const functionSignature = abiUtils.getFunctionSignature(methodAbi); - result[functionSignature] = new ethersContracts.Interface([methodAbi]); - }, - {}, - ); + this._ethersInterfacesByFunctionSignature = {}; + _.each(methodAbis, methodAbi => { + const functionSignature = abiUtils.getFunctionSignature(methodAbi); + this._ethersInterfacesByFunctionSignature[functionSignature] = new ethersContracts.Interface([methodAbi]); + }); } } diff --git a/packages/deployer/src/cli.ts b/packages/deployer/src/cli.ts index 3d69925a8..62afe0d4c 100644 --- a/packages/deployer/src/cli.ts +++ b/packages/deployer/src/cli.ts @@ -45,7 +45,7 @@ async function onDeployCommandAsync(argv: CliOptions): Promise { const web3Wrapper = new Web3Wrapper(web3Provider); const networkId = await web3Wrapper.getNetworkIdAsync(); const compilerOpts: CompilerOptions = { - contractDirs: getContractDirectoriesFromList(argv.contractsDir), + contractDirs: getContractDirectoriesFromList(argv.contractDirs), networkId, optimizerEnabled: argv.shouldOptimize, artifactsDir: argv.artifactsDir, diff --git a/packages/deployer/src/compiler.ts b/packages/deployer/src/compiler.ts index beaaab141..e3ecc6c72 100644 --- a/packages/deployer/src/compiler.ts +++ b/packages/deployer/src/compiler.ts @@ -26,7 +26,7 @@ import { CompilerOptions, ContractArtifact, ContractDirectory, - ContractIds, + ContractIdToSourceFileId, ContractNetworkData, ContractNetworks, ContractSourceDataByFileId, @@ -75,6 +75,9 @@ export class Compiler { encoding: 'utf8', }; const source = await fsWrapper.readFileAsync(contentPath, opts); + if (!_.startsWith(contentPath, contractBaseDir)) { + throw new Error(`Expected content path '${contentPath}' to begin with '${contractBaseDir}'`); + } const sourceFilePath = contentPath.slice(contractBaseDir.length); sources[sourceFilePath] = source; logUtils.log(`Reading ${sourceFilePath} source...`); @@ -114,7 +117,7 @@ export class Compiler { await createDirIfDoesNotExistAsync(this._artifactsDir); await createDirIfDoesNotExistAsync(SOLC_BIN_DIR); this._contractSources = {}; - const contractIds: ContractIds = {}; + const contractIdToSourceFileId: ContractIdToSourceFileId = {}; const contractDirs = Array.from(this._contractDirs.values()); for (const contractDir of contractDirs) { const sources = await Compiler._getContractSourcesAsync(contractDir.path, contractDir.path); @@ -127,18 +130,20 @@ export class Compiler { this._contractSources[sourceFileId] = source; // Create a mapping between the contract id and its source file id const contractId = constructContractId(contractDir.namespace, sourceFilePath); - if (!_.isUndefined(contractIds[contractId])) { + if (!_.isUndefined(contractIdToSourceFileId[contractId])) { throw new Error(`Found duplicate contract with ID '${contractId}'`); } - contractIds[contractId] = sourceFileId; + contractIdToSourceFileId[contractId] = sourceFileId; }); } _.forIn(this._contractSources, this._setContractSpecificSourceData.bind(this)); const specifiedContractIds = this._specifiedContracts.has(ALL_CONTRACTS_IDENTIFIER) - ? _.keys(contractIds) + ? _.keys(contractIdToSourceFileId) : Array.from(this._specifiedContracts.values()); await Promise.all( - _.map(specifiedContractIds, async contractId => this._compileContractAsync(contractIds[contractId])), + _.map(specifiedContractIds, async contractId => + this._compileContractAsync(contractIdToSourceFileId[contractId]), + ), ); } /** diff --git a/packages/deployer/src/utils/types.ts b/packages/deployer/src/utils/types.ts index 08cab37b2..1a866b873 100644 --- a/packages/deployer/src/utils/types.ts +++ b/packages/deployer/src/utils/types.ts @@ -83,7 +83,7 @@ export interface ContractSources { [key: string]: string; } -export interface ContractIds { +export interface ContractIdToSourceFileId { [key: string]: string; } diff --git a/packages/metacoin/test/metacoin_test.ts b/packages/metacoin/test/metacoin_test.ts index 4a2307444..51830d1ef 100644 --- a/packages/metacoin/test/metacoin_test.ts +++ b/packages/metacoin/test/metacoin_test.ts @@ -36,12 +36,12 @@ describe('Metacoin', () => { }); }); describe('#transfer', () => { - it(`should successfully transfer tokens (via transfer_1)`, async () => { + it(`should successfully transfer tokens (via transfer1)`, async () => { const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'; const amount = INITIAL_BALANCE.div(2); const oldBalance = await metacoin.balances.callAsync(ZERO_ADDRESS); expect(oldBalance).to.be.bignumber.equal(0); - const txHash = await metacoin.transfer_1.sendTransactionAsync( + const txHash = await metacoin.transfer1.sendTransactionAsync( { to: ZERO_ADDRESS, amount, @@ -59,13 +59,13 @@ describe('Metacoin', () => { expect(newBalance).to.be.bignumber.equal(amount); }); - it(`should successfully transfer tokens (via transfer_2)`, async () => { + it(`should successfully transfer tokens (via transfer2)`, async () => { const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'; const amount = INITIAL_BALANCE.div(2); const oldBalance = await metacoin.balances.callAsync(ZERO_ADDRESS); expect(oldBalance).to.be.bignumber.equal(0); const callback = 59; - const txHash = await metacoin.transfer_2.sendTransactionAsync( + const txHash = await metacoin.transfer2.sendTransactionAsync( { to: ZERO_ADDRESS, amount, @@ -84,13 +84,13 @@ describe('Metacoin', () => { expect(newBalance).to.be.bignumber.equal(amount); }); - it(`should successfully transfer tokens (via transfer_3)`, async () => { + it(`should successfully transfer tokens (via transfer3)`, async () => { const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'; const amount = INITIAL_BALANCE.div(2); const oldBalance = await metacoin.balances.callAsync(ZERO_ADDRESS); expect(oldBalance).to.be.bignumber.equal(0); const callback = 59; - const txHash = await metacoin.transfer_3.sendTransactionAsync( + const txHash = await metacoin.transfer3.sendTransactionAsync( { transferData: { to: ZERO_ADDRESS, diff --git a/packages/utils/src/abi_utils.ts b/packages/utils/src/abi_utils.ts index 843b8589b..c4533d42e 100644 --- a/packages/utils/src/abi_utils.ts +++ b/packages/utils/src/abi_utils.ts @@ -12,63 +12,60 @@ export const abiUtils = { } return param.type; }, - getFunctionSignature(abi: MethodAbi): string { - const functionName = abi.name; - const parameterTypeList = abi.inputs.map((param: DataItem) => this.parseFunctionParam(param)); + getFunctionSignature(methodAbi: MethodAbi): string { + const functionName = methodAbi.name; + const parameterTypeList = _.map(methodAbi.inputs, (param: DataItem) => this.parseFunctionParam(param)); const functionSignature = `${functionName}(${parameterTypeList})`; return functionSignature; }, + /** + * Solidity supports function overloading whereas TypeScript does not. + * See: https://solidity.readthedocs.io/en/v0.4.21/contracts.html?highlight=overload#function-overloading + * In order to support overloaded functions, we suffix overloaded function names with an index. + * This index should be deterministic, regardless of function ordering within the smart contract. To do so, + * we assign indexes based on the alphabetical order of function signatures. + * + * E.g + * ['f(uint)', 'f(uint,byte32)'] + * Should always be renamed to: + * ['f1(uint)', 'f2(uint,byte32)'] + * Regardless of the order in which these these overloaded functions are declared within the contract ABI. + */ renameOverloadedMethods(inputContractAbi: ContractAbi): ContractAbi { const contractAbi = _.cloneDeep(inputContractAbi); const methodAbis = contractAbi.filter((abi: AbiDefinition) => abi.type === AbiType.Function) as MethodAbi[]; - const methodAbisByOriginalIndex = _.transform( - methodAbis, - (result: Array<{ index: number; methodAbi: MethodAbi }>, methodAbi, i: number) => { - result.push({ index: i, methodAbi }); - }, - [], - ); // Sort method Abis into alphabetical order, by function signature - const methodAbisByOriginalIndexOrdered = _.sortBy(methodAbisByOriginalIndex, [ - (entry: { index: number; methodAbi: MethodAbi }) => { - const functionSignature = this.getFunctionSignature(entry.methodAbi); + const methodAbisOrdered = _.sortBy(methodAbis, [ + (methodAbi: MethodAbi) => { + const functionSignature = this.getFunctionSignature(methodAbi); return functionSignature; }, ]); // Group method Abis by name (overloaded methods will be grouped together, in alphabetical order) - const methodAbisByName = _.transform( - methodAbisByOriginalIndexOrdered, - (result: { [key: string]: Array<{ index: number; methodAbi: MethodAbi }> }, entry) => { - (result[entry.methodAbi.name] || (result[entry.methodAbi.name] = [])).push(entry); - }, - {}, - ); - // Rename overloaded methods to overloadedMethoName_1, overloadedMethoName_2, ... - const methodAbisRenamed = _.transform( - methodAbisByName, - (result: MethodAbi[], methodAbisWithSameName: Array<{ index: number; methodAbi: MethodAbi }>) => { - _.forEach(methodAbisWithSameName, (entry, i: number) => { - if (methodAbisWithSameName.length > 1) { - const overloadedMethodId = i + 1; - const sanitizedMethodName = `${entry.methodAbi.name}_${overloadedMethodId}`; - const indexOfExistingAbiWithSanitizedMethodNameIfExists = _.findIndex( - methodAbis, - methodAbi => methodAbi.name === sanitizedMethodName, + const methodAbisByName: { [key: string]: MethodAbi[] } = {}; + _.each(methodAbisOrdered, methodAbi => { + (methodAbisByName[methodAbi.name] || (methodAbisByName[methodAbi.name] = [])).push(methodAbi); + }); + // Rename overloaded methods to overloadedMethodName1, overloadedMethodName2, ... + _.each(methodAbisByName, methodAbisWithSameName => { + _.each(methodAbisWithSameName, (methodAbi, i: number) => { + if (methodAbisWithSameName.length > 1) { + const overloadedMethodId = i + 1; + const sanitizedMethodName = `${methodAbi.name}${overloadedMethodId}`; + const indexOfExistingAbiWithSanitizedMethodNameIfExists = _.findIndex( + methodAbis, + currentMethodAbi => currentMethodAbi.name === sanitizedMethodName, + ); + if (indexOfExistingAbiWithSanitizedMethodNameIfExists >= 0) { + const methodName = methodAbi.name; + throw new Error( + `Failed to rename overloaded method '${methodName}' to '${sanitizedMethodName}'. A method with this name already exists.`, ); - if (indexOfExistingAbiWithSanitizedMethodNameIfExists >= 0) { - const methodName = entry.methodAbi.name; - throw new Error( - `Failed to rename overloaded method '${methodName}' to '${sanitizedMethodName}'. A method with this name already exists.`, - ); - } - entry.methodAbi.name = sanitizedMethodName; } - // Add method to list of ABIs in its original position - result.splice(entry.index, 0, entry.methodAbi); - }); - }, - [...Array(methodAbis.length)], - ); + methodAbi.name = sanitizedMethodName; + } + }); + }); return contractAbi; }, }; diff --git a/yarn.lock b/yarn.lock index d0760cd78..3a2cb66ca 100644 --- a/yarn.lock +++ b/yarn.lock @@ -354,13 +354,7 @@ dependencies: redux "*" -"@types/request-promise-native@^1.0.2": - version "1.0.14" - resolved "https://registry.yarnpkg.com/@types/request-promise-native/-/request-promise-native-1.0.14.tgz#20f2ba136e6f29c2ea745c60767738d434793d31" - dependencies: - "@types/request" "*" - -"@types/request@*", "@types/request@2.47.0": +"@types/request@2.47.0": version "2.47.0" resolved "https://registry.yarnpkg.com/@types/request/-/request-2.47.0.tgz#76a666cee4cb85dcffea6cd4645227926d9e114e" dependencies: -- cgit