diff --git a/CHANGELOG.md b/CHANGELOG.md index 357749f..1af8594 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## Unreleased + +- Fix function clashes when ABI types are not able to disambiguate multiple functions. + ## 0.3.14 - Fix import handling to fully consider indirect imports. diff --git a/contracts/Test.sol b/contracts/Test.sol index 31d04f4..17020ca 100644 --- a/contracts/Test.sol +++ b/contracts/Test.sol @@ -263,3 +263,34 @@ library LibraryHasStruct { function foo() internal returns (Inner memory) {} } + +library UdvtConflict { + type myFirstType is bytes32; + type mySecondType is bytes32; + + function unwrap(myFirstType t) internal pure returns (bytes32) { + return myFirstType.unwrap(t); + } + + function unwrap(mySecondType t) internal pure returns (bytes32) { + return mySecondType.unwrap(t); + } +} + +library UdvtNoConflict { + type myFirstType is bytes32; + type mySecondType is uint256; + + function unwrap(myFirstType t) internal pure returns (bytes32) { + return myFirstType.unwrap(t); + } + + function unwrap(mySecondType t) internal pure returns (uint256) { + return mySecondType.unwrap(t); + } +} + +contract Conflicts { + function _a(HasEnum) internal {} + function _a(HasReceiveFunction) internal {} +} diff --git a/src/core.test.ts.md b/src/core.test.ts.md index 6784d97..744bb8f 100644 --- a/src/core.test.ts.md +++ b/src/core.test.ts.md @@ -824,6 +824,57 @@ Generated by [AVA](https://avajs.dev). ␊ receive() external payable {}␊ }␊ + ␊ + contract $UdvtConflict {␊ + bytes32 public constant __hh_exposed_bytecode_marker = "hardhat-exposed";␊ + ␊ + constructor() payable {␊ + }␊ + ␊ + function $unwrap_UdvtConflict_myFirstType(UdvtConflict.myFirstType t) external pure returns (bytes32 ret0) {␊ + (ret0) = UdvtConflict.unwrap(t);␊ + }␊ + ␊ + function $unwrap_UdvtConflict_mySecondType(UdvtConflict.mySecondType t) external pure returns (bytes32 ret0) {␊ + (ret0) = UdvtConflict.unwrap(t);␊ + }␊ + ␊ + receive() external payable {}␊ + }␊ + ␊ + contract $UdvtNoConflict {␊ + bytes32 public constant __hh_exposed_bytecode_marker = "hardhat-exposed";␊ + ␊ + constructor() payable {␊ + }␊ + ␊ + function $unwrap(UdvtNoConflict.myFirstType t) external pure returns (bytes32 ret0) {␊ + (ret0) = UdvtNoConflict.unwrap(t);␊ + }␊ + ␊ + function $unwrap(UdvtNoConflict.mySecondType t) external pure returns (uint256 ret0) {␊ + (ret0) = UdvtNoConflict.unwrap(t);␊ + }␊ + ␊ + receive() external payable {}␊ + }␊ + ␊ + contract $Conflicts is Conflicts {␊ + bytes32 public constant __hh_exposed_bytecode_marker = "hardhat-exposed";␊ + ␊ + constructor() payable {␊ + }␊ + ␊ + function $_a_HasEnum(HasEnum arg0) external {␊ + super._a(arg0);␊ + }␊ + ␊ + function $_a_HasReceiveFunction(HasReceiveFunction arg0) external {␊ + super._a(arg0);␊ + }␊ + ␊ + receive() external payable {}␊ + }␊ ` ## snapshot initializers diff --git a/src/core.test.ts.snap b/src/core.test.ts.snap index 62e03a2..642ca28 100644 Binary files a/src/core.test.ts.snap and b/src/core.test.ts.snap differ diff --git a/src/core.ts b/src/core.ts index 8d2c676..2534717 100644 --- a/src/core.ts +++ b/src/core.ts @@ -2,9 +2,9 @@ import path from 'path'; import { findAll, astDereferencer, ASTDereferencer } from 'solidity-ast/utils'; import { formatLines, Lines, spaceBetween } from './utils/format-lines'; -import type { Visibility, SourceUnit, ContractDefinition, FunctionDefinition, VariableDeclaration, StorageLocation, TypeDescriptions, TypeName, InheritanceSpecifier, ModifierInvocation, FunctionCall } from 'solidity-ast'; +import type { Visibility, SourceUnit, ContractDefinition, FunctionDefinition, VariableDeclaration, StorageLocation, TypeName, UserDefinedTypeName } from 'solidity-ast'; import type { FileContent, ProjectPathsConfig, ResolvedFile } from 'hardhat/types'; -import type { ExposedConfig, ExposedUserConfig } from './config'; +import type { ExposedConfig } from './config'; import assert from 'assert'; export interface SolcOutput { @@ -205,7 +205,7 @@ function getExposedContent(ast: SourceUnit, relativizePath: (p: string) => strin ]), // external functions ...externalizableFunctions.map(fn => { - const fnName = clashingFunctions[getFunctionId(fn, c, deref)] === 1 ? fn.name : getFunctionNameQualified(fn, c, deref); + const fnName = clashingFunctions[getFunctionId(fn, c, deref)] === 1 ? fn.name : getFunctionNameQualified(fn, c, deref, true); const fnArgs = getFunctionArguments(fn, c, deref); const fnRets = getFunctionReturnParameters(fn, c, deref); const evName = isNonViewWithReturns(fn) && (clashingEvents[fn.name] === 1 ? fn.name : getFunctionNameQualified(fn, c, deref, false)); @@ -282,13 +282,16 @@ function areFunctionsFullyImplemented(contract: ContractDefinition, deref: ASTDe } function getFunctionId(fn: FunctionDefinition, context: ContractDefinition, deref: ASTDereferencer): string { - const storageArgs = new Set(getStorageArguments(fn, context, deref)); - const nonStorageArgs = getFunctionArguments(fn, context, deref).filter(a => !storageArgs.has(a)); - return fn.name + nonStorageArgs.map(a => a.type).join(''); + const abiTypes = getFunctionArguments(fn, context, deref).map(a => a.abiType); + return fn.name + abiTypes.join(','); } -function getFunctionNameQualified(fn: FunctionDefinition, context: ContractDefinition, deref: ASTDereferencer, onlyStorage: boolean = true): string { - return fn.name + (onlyStorage ? getStorageArguments(fn, context, deref) : getFunctionArguments(fn, context, deref)) +function getFunctionNameQualified(fn: FunctionDefinition, context: ContractDefinition, deref: ASTDereferencer, onlyConflicting: boolean): string { + let args = getFunctionArguments(fn, context, deref); + if (onlyConflicting) { + args = args.filter(a => a.type !== a.abiType || a.storageType !== undefined); + } + return fn.name + args .map(arg => arg.storageType ?? arg.type) .map(type => type.replace(/ .*/,'').replace(/[^0-9a-zA-Z$_]+/g, '_')) // sanitize .join('_') @@ -418,8 +421,8 @@ function isExternalizable(fnDef: FunctionDefinition, deref: ASTDereferencer): bo function isTypeExternalizable(typeName: TypeName | null | undefined, deref: ASTDereferencer): boolean { if (typeName == undefined) { return true; - } if (typeName.nodeType === 'UserDefinedTypeName') { - const typeDef = deref(['StructDefinition', 'EnumDefinition', 'ContractDefinition', 'UserDefinedValueTypeDefinition'], typeName.referencedDeclaration); + } else if (typeName.nodeType === 'UserDefinedTypeName') { + const typeDef = derefUserDefinedTypeName(deref, typeName); if (typeDef.nodeType !== 'StructDefinition') { return true; } else { @@ -437,6 +440,7 @@ function isNonViewWithReturns(fnDef: FunctionDefinition): boolean { interface Argument { type: string; name: string; + abiType: string; storageVar?: string; storageType?: string; } @@ -450,10 +454,12 @@ function getFunctionArguments(fnDef: FunctionDefinition, context: ContractDefini const storageType = getVarType(p, context, deref, null); const storageVar = 'v_' + storageType.replace(/[^0-9a-zA-Z$_]+/g, '_'); // The argument is an index to an array in storage. - return { name, type: 'uint256', storageVar, storageType }; + const type = 'uint256'; + return { name, type, abiType: type, storageVar, storageType }; } else { const type = getVarType(p, context, deref, 'calldata'); - return { name, type }; + const abiType = getVarAbiType(p, context, deref, 'calldata'); + return { name, type, abiType }; } }); } @@ -475,7 +481,8 @@ function getFunctionReturnParameters(fnDef: FunctionDefinition, context: Contrac return fnDef.returnParameters.parameters.map((p, i) => { const name = p.name || `ret${i}`; const type = getVarType(p, context, deref, location); - return { name, type }; + const abiType = getVarAbiType(p, context, deref, location); + return { name, type, abiType }; }); } @@ -508,6 +515,49 @@ function getType(typeName: TypeName, context: ContractDefinition, deref: ASTDere return type; } +function getVarAbiType(varDecl: VariableDeclaration, context: ContractDefinition, deref: ASTDereferencer, location: StorageLocation | null = varDecl.storageLocation): string { + if (!varDecl.typeName) { + throw new Error('Missing type information'); + } + return getAbiType(varDecl.typeName, context, deref, location); +} + +function getAbiType(typeName: TypeName, context: ContractDefinition, deref: ASTDereferencer, location: StorageLocation | null): string { + switch (typeName.nodeType) { + case 'ElementaryTypeName': + case 'ArrayTypeName': + const { typeString } = typeName.typeDescriptions; + assert(typeString != undefined); + return typeString; + + case 'UserDefinedTypeName': + const typeDef = derefUserDefinedTypeName(deref, typeName); + switch (typeDef.nodeType) { + case 'UserDefinedValueTypeDefinition': + const { typeString } = typeDef.underlyingType.typeDescriptions; + assert(typeString != undefined); + return typeString; + + case 'EnumDefinition': + assert(typeDef.members.length < 256); + return 'uint8'; + + case 'ContractDefinition': + return 'address'; + + case 'StructDefinition': + if (location === 'storage') { + throw new Error('Unexpected error'); // is treated separately in getFunctionArguments + } else { + return '(' + typeDef.members.map(v => getVarAbiType(v, context, deref, location)).join(',') + ')'; + } + } + + default: + throw new Error('Unknown ABI type'); + } +} + function getVariables(contract: ContractDefinition, deref: ASTDereferencer, subset?: Visibility[]): VariableDeclaration[] { const parents = contract.linearizedBaseContracts.map(deref('ContractDefinition')); @@ -530,7 +580,11 @@ function getVarGetterArgs(v: VariableDeclaration, context: ContractDefinition, d } const types = []; for (let t = v.typeName; t.nodeType === 'Mapping'; t = t.valueType) { - types.push({ name: `arg${types.length}`, type: getType(t.keyType, context, deref, 'memory') }) + types.push({ + name: `arg${types.length}`, + type: getType(t.keyType, context, deref, 'memory'), + abiType: getAbiType(t.keyType, context, deref, 'memory'), + }) } return types; } @@ -598,3 +652,7 @@ function createNonCryptographicHashBasedIdentifier(input: Buffer): Buffer { const { createHash } = require("crypto") as typeof import('crypto'); return createHash("md5").update(input).digest(); } + +function derefUserDefinedTypeName(deref: ASTDereferencer, typeName: UserDefinedTypeName) { + return deref(['StructDefinition', 'EnumDefinition', 'ContractDefinition', 'UserDefinedValueTypeDefinition'], typeName.referencedDeclaration); +}