Fix function clash for non-storage arguments (#34)

Co-authored-by: Francisco Giordano <[email protected]>
frangio · Mar 28, 2024 · f535d7f · f535d7f
1 parent 3e41699
commit f535d7f
Show file tree

Hide file tree

Showing 5 changed files with 158 additions and 14 deletions.
diff --git 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
@@ -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
@@ -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
diff --git 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<Argument>(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);
+}