From f535d7f2f3ceab1933620f2d999684a4cc611336 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 28 Mar 2024 20:31:38 +0100 Subject: [PATCH] Fix function clash for non-storage arguments (#34) Co-authored-by: Francisco Giordano --- CHANGELOG.md | 4 ++ contracts/Test.sol | 31 +++++++++++++++ src/core.test.ts.md | 51 +++++++++++++++++++++++++ src/core.test.ts.snap | Bin 2425 -> 2555 bytes src/core.ts | 86 +++++++++++++++++++++++++++++++++++------- 5 files changed, 158 insertions(+), 14 deletions(-) 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 62e03a23d55114641f1386e8e172d2c346e88598..642ca284b5cb4ba380d1eed3abce506abf6ce01a 100644 GIT binary patch literal 2555 zcmV{>@BoeNMQgDNjIn}5u;nor zY+0?E?t(vbmso&_AQXCpm$@c#UZrU3-b8jqG1?k@O0jXMhu7Qbn% zuS=a*&rcs6>MFE6_~<}`7Shjk=zb?19~~aF_m4XJ&zF}S7hIS?@@zxbbTpK{-`QwA zZEZYTUe-;=c9Hb3)mm3=3%Ni=-unJPHToLVTDZYO3>4HzD?yEjCtLf%x=%bfmr8 zSCQ>DS0x7wLDztc)F%q!LKW(l5Ld$ig02OO5!|QcPs>beOZOvMYb=dPYX_t-)LJHt z90U5D?Lg}aIu0~+i-UJudoWyY8@2_#q&1Fg6d?2s!hfArn{eFotWTZq^S-4b-L|9^ z1(QacDx17oEu9mLBJh8_z60G>q9ulYR*ZIoRF*MjY%vcgN*mRu8&5)~yPYF7^tzn8>DYpx|-r=IQGelAk=4 z2CD(h=Af9#0`NTZgm9(ol2D60Qp*IqHc-njP>b9gUOxsYs;!YETa%OIDv)HWd3AkB z+CAOSh^Mlr+EJNOQh1K}EqK`6MS2XqTB zxXK{rnt{sQ$R07)-mNs&Ues9KOr0{|uGm<6B4ZUn%r(m(aqHZxP28(}E8VM2^`EKA z044_sr|Safc<9>sXV>{piSwV%g!2z7IsZVY-P5SC2AMi#zyqa7z z`UWqhi);c;ZlmypZTlhRe~cxhOyD@WbrG6#=X7;U#nxZ8QN}zsM0rk5))W^?H-UMMLak0z z$F6$pN(Lkjj++^~2%>tFtVhXul$@_o@=)(idw7JGc=gO2BRGF%onU7P+g<{r4_B*< zkq1X__Fo_E9x5lhuf@U1L{I5Gv3goO4uRcX9q-{)|8eCsa$)dAA z$mq(7(;;YXyyqxRzhbu$xL->6B$%MC$#cOgg^YxO$mr23?OvGj|xE z>ygs{_+t)^F0^e+#ma#62wV7NjyRWj&gbF;@|&RpiI32SNsR$1MZr`!Iyx{wh1G_i zl@B%VTYb})wdp@dFmGH9!D041fGL!KQdQ9raFtO9sX8dHTX%0=H7? z%x@iIT8YEA0ew&6c+UWe+pl&b%CGlTYaMcO%v-4;?W8R7?QlKGmEXVD%n z9!uayQXRU#S}<`zKsvgzO>5%WCVY7bUBd<%Cd}ZQM8!oLF_Cf_X+u0S(p9PrGLvKz zD`5+>Zy?<VtZd=O*?hu=YO>Duu0EDK}c=S-!HdLG*~kMBYV=;aN4iDu`c zP4)Q5;a4R`x)P>1)-L@7*g01Cn9NX0rG4w83mk>#N->8obl0Q#SiYjgRYcOdsXKdO zeMfLo2E=*abOt+E7nYXB;izWJES9wSkv(%(Z$|mLN%L}8iy1h{OCdw%E^rDlQ!dj_ zFnKJo&%R9GqzX{j=-UiN;n>0@F^vKIdPIOQHhXQROlYf+$gLy<4e%RtM)#EuK?-=g*)N;KJeM~6=l8X z8v96{Pnn2TB=y^D(J@PW))bkMp^f|fH0Dly{%g+!>V z1s3#xfo@!+CY`{o@Vw*WV=jYyMR*`5kiouPbIG-CY0c RifJ7{`9G6IjrlHT007D*@rnQd literal 2425 zcmV-<35NDTRzVgSH9{jLs0Wzm{Gd9kyEsN1cPGtE-j`&UGL;riN66N76UD zTaCw!ttYFiNViNIOZOX%O~o{@4HWEb9t;(2phC4lD%>YT{vBz}zat?>3a)=zmB^k01)>W`Zo?slwgI#;-TT$|tITREcSBaItxU*jZCV&u zEtN)&5PfS}(71$_1vO-F^p!3oj0q2xQ2NQ+>bM`pxuboB-Bsx36XgzaxpP-Ois?gSUw2_Mtga}fgz_N zSdKtgUL*|5Jcwtf`J4gl_J2bF`_Ju30nF$_8xh+oFb=Wjz$QYrT?hG>6L-%58CZVw zSQ@VTGV8;9F7wH=+!N3h@=MGt^4Kg>K&@|R~1m1QRKiVQ-tYoLy94m+9wOVGc{E+Fx3>}U@=YgFfi4_*_dj4c{#pR zL`EXw6kmTKy)_`cy)7|SL;*KB`KpG%@Vjx^YF^VoJ=>uet z1y>QqTs2U-8`-DM+P{&`+7COcmugc++!H%%U*xPjjJawlB5oYKS`=Qb8wszLXa7`F z`ZO6#I9umn$5U6}pI`8wQ1GA31^9=hz&{jP_ayABVX93T@lfon6Ops>Fy^WaOFQe7 zI_vaCI_oq?Pj#BrQ=Jy-sp8asBq+W1u=IMW`41v5NA9Bn*8+lztx&(*4y{n{e>tHQ z>Pxbe&a(+Py^X@}Ow$c1_iG{{r31@C#(Ch*-EXAMx&RDzXcEpr#!jx~y^!biB zDm#)6b<-Xtgry?@sy?3V*ymd%;$461R(vOlgW-0WPJ~nwGL%EuAJ{M-Ls`hMS&m^- z$nasgCO;G~ycDx*6EK}aJAvJ0+HNcfj?6Z@1zSAhx>vBvXVEC5G)Wz7yCEPH=Fs~L zwz)+595bYnU=mC16ck9AWee<(_yy8QWxAn=2L9Ira3n9RgGOz zHFi~FS3Dp|a9q#WMHyA2WHm}wqvT?Zl82}_>){bzkkvB-hX4GTaZJDx)V%=O0IruA zBilzW4_+MY9d?iRUWkK}H9evCB<5-GJOqAw{%W7B`mZ8n%pnaLgz^f#s4Hj8CQZ)4 zFr_QYFNd<55I))i7DYhlh=ZkaB2kno&;1-N6C=)pEc>psgCG||1Jr&^V;8ga1_N+iDeUs@~{P{{LmVB$f zD?M#!jm^)x*F#eQRB12gTre_Km*) zcB#~vTTd{wkXv#l%eU{uNko@{n0GUODsd;7c{4SaNC;9gYu=jof;a#W=$( zkHoAW=QB$V{U2wU=c|yY4XJ0EntQo@6{%l&Qr-XCDYAD;%FZzCj!=5R5O1?d<_jjC zMSHY(B7q-Dbs%@Oppyb$ba-W(RK+oM`0N7Onh8`Qn9yaiDV{ax zOp;Bkgf;YmhLNQ~X24KmHUdMEU)!ZJIdN-o{w|tHPGFY2Hjfjs_9yBXwzMeR#~Go( z17b}hEGesGRR!26ps(+vnds}oGELt&D3jDIl;BTbnXGP{FilfYFbuOm8;O*t;v1~k zS3fFeD9}Di$MGuLm;MPjFFfk+{AQD`G@*0`Q>f_Tj6M=Y&pz?zt-XTT&+z^LZw?6K zhGt+qu+Yl|SQC3k+&JCC1#GzHwURC5p8ihgEK*xvbf=n323?o?+$OTm76XF$1A zJ)CPJ4RAl%XNIcNqVscFlsjIQ<<_jjYt^~Zn+L?{ohvO5M6WR81ZZ_CwF;e^dMcI6 zHDrZO*2$~t{ObIiUoG!euQ1~wOOuT&o3_ckkX^s(rFlTVAT&AvDXM*H)Hl zx9-(G20gNBA7J<#;J#nD8?5hr%xP_sk+E4vI+h+=D{b2wAouaER 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); +}