Skip to content

Commit

Permalink
feat: functional polymorphism support (builds on #40) (#42)
Browse files Browse the repository at this point in the history
* fix: use method signature instead of just the name to allow using functional polymorphism
* fix: comprehensive fix for #40

---------

Co-authored-by: Bruno Mateus <[email protected]>
  • Loading branch information
hiddentao and bmateus committed Aug 12, 2024
1 parent 5a78baa commit e38c03e
Show file tree
Hide file tree
Showing 10 changed files with 3,215 additions and 2,522 deletions.
5,552 changes: 3,102 additions & 2,450 deletions pnpm-lock.yaml

Large diffs are not rendered by default.

18 changes: 16 additions & 2 deletions src/commands/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { getContext } from '../shared/context.js'
import { $, FacetDefinition, ensureGeneratedFolderExists, fileExists, getUserFacetsAndFunctions, saveJson, writeFile, writeTemplate } from '../shared/fs.js'
import path from 'node:path'
import { createCommand, logSuccess } from './common.js'
import { error, info, trace } from '../shared/log.js'
import { error, info, trace, warn } from '../shared/log.js'
import { generateUnifiedAbi } from '../shared/chain.js'

export const command = () =>
Expand Down Expand Up @@ -68,6 +68,8 @@ export const command = () =>
const importPaths: Record<string, string> = {}
let facetSelectorsStr = ''

let hasCustomStructs: string[] = []

facets.forEach((f, facetNum) => {
numMethods += f.functions.length

Expand All @@ -85,13 +87,25 @@ export const command = () =>

facetSelectorsStr += `
${arrayDeclaration}
${f.functions.map((f, i) => `${varName}[${i}] = IDiamondProxy.${f.name}.selector;`).join('\n')}
${f.functions.map((fn, i) => {
if (fn.userDefinedTypesInParams.length) {
hasCustomStructs.push(fn.name)
return `${varName}[${i}] = IDiamondProxy.${fn.name}.selector;`
} else {
return `${varName}[${i}] = bytes4(keccak256(bytes('${fn.signaturePacked}')));`
}
}).join('\n')}
fs[${facetNum}] = FacetSelectors({
addr: address(new ${f.contractName}()),
sels: ${varName}
});
`
})

if (hasCustomStructs.length) {
warn(`Custom structs found in facet method params. Polymorphism won't be supported for these methods: ${hasCustomStructs.join(', ')}`)
warn(`See - https://github.com/gemstation/gemforge/pull/40#issuecomment-2284272273`)
}

writeTemplate('LibDiamondHelper.sol', `${ctx.generatedSolidityPath}/LibDiamondHelper.sol`, {
__SOLC_SPDX__: ctx.config.solc.license,
Expand Down
8 changes: 3 additions & 5 deletions src/shared/diamond.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,12 @@ interface FunctionSelector {
const getSelectors = (artifact: ContractArtifact): FunctionSelector[] => {
const selectors: FunctionSelector[] = []

const names = artifact.abi.filter(f => f.type === 'function').map(f => (f as FunctionFragment).name)

const iface = new Interface(artifact.abi)

names.forEach(fName => {
iface.forEachFunction((f: FunctionFragment) => {
selectors.push({
name: fName,
selector: iface.getFunction(fName)!.selector,
name: f.format("sighash"),
selector: f.selector,
})
})

Expand Down
52 changes: 36 additions & 16 deletions src/shared/fs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,18 @@ export interface FacetDefinition {
name: string,
signature: string,
signaturePacked: string,
userDefinedTypesInParams: string[],
}[],
}

interface ParserMeta {
userDefinedTypes: string[],
userDefinedTypesInReturnValues: string[],
userDefinedTypesInParams: string[],
}

interface FunctionParsingContext {
userDefinedTypes: Set<string>
}


export const getUserFacetsAndFunctions = (ctx: Context): FacetDefinition[] => {
Expand All @@ -130,7 +135,10 @@ export const getUserFacetsAndFunctions = (ctx: Context): FacetDefinition[] => {
const ret: FacetDefinition[] = []
const contractNames: Record<string, boolean> = {}
const functionSigs: Record<string, boolean> = {}
const parserMeta: ParserMeta = { userDefinedTypes: [] }
const parserMeta: ParserMeta = {
userDefinedTypesInParams: [],
userDefinedTypesInReturnValues: [],
}

// load user facets
const facetFiles = glob.sync(ctx.config.paths.src.facets, { cwd: ctx.folder })
Expand Down Expand Up @@ -161,23 +169,32 @@ export const getUserFacetsAndFunctions = (ctx: Context): FacetDefinition[] => {
node => node.visibility === 'external' || (ctx.config.diamond.publicMethods && node.visibility === 'public')
)

// export declare type TypeName = ElementaryTypeName | UserDefinedTypeName | ArrayTypeName;

const functions = functionDefinitions.map(node => {
let signature = `function ${node.name}(${getParamString(node.parameters, parserMeta)}) ${node.visibility}${
const fnParamParsingContext: FunctionParsingContext = {
userDefinedTypes: new Set<string>()
}
const fnReturnParsingContext: FunctionParsingContext = {
userDefinedTypes: new Set<string>()
}

let signature = `function ${node.name}(${getParamString(node.parameters, fnParamParsingContext)}) ${node.visibility}${
node.stateMutability ? ` ${node.stateMutability}` : ''
}`

if (node.returnParameters?.length) {
signature += ` returns (${getParamString(node.returnParameters, parserMeta)})`
signature += ` returns (${getParamString(node.returnParameters, fnReturnParsingContext)})`
}

let signaturePacked = `${node.name}(${getPackedParamString(node.parameters, parserMeta)})`
let signaturePacked = `${node.name}(${getPackedParamString(node.parameters, fnParamParsingContext)})`

parserMeta.userDefinedTypesInParams.push(...fnParamParsingContext.userDefinedTypes)
parserMeta.userDefinedTypesInReturnValues.push(...fnReturnParsingContext.userDefinedTypes)

const r = {
name: node.name!,
signature,
signaturePacked,
userDefinedTypesInParams: Array.from(fnParamParsingContext.userDefinedTypes)
}

if (functionSigs[r.signaturePacked]) {
Expand All @@ -197,8 +214,11 @@ export const getUserFacetsAndFunctions = (ctx: Context): FacetDefinition[] => {
})
})

if (parserMeta.userDefinedTypes.length) {
info(`Custom structs found in facet method signatures: ${parserMeta.userDefinedTypes.join(', ')}`)
if (parserMeta.userDefinedTypesInParams.length) {
info(`Facet method params have custom structs: ${parserMeta.userDefinedTypesInParams.join(', ')}`)
}
if (parserMeta.userDefinedTypesInReturnValues.length) {
info(`Facet method return values have custom structs: ${parserMeta.userDefinedTypesInReturnValues.join(', ')}`)
}

// sort alphabetically
Expand All @@ -209,46 +229,46 @@ export const getUserFacetsAndFunctions = (ctx: Context): FacetDefinition[] => {



const getParamString = (params: VariableDeclaration[], meta: ParserMeta): string => {
const getParamString = (params: VariableDeclaration[], ctx: FunctionParsingContext): string => {
const p: string[] = []

params.map(param => {
const name = param.name ? ` ${param.name}` : ''
const storage = param.storageLocation ? ` ${param.storageLocation}`: ''
const typeNameString = _getTypeNameString(param.typeName!, meta)
const typeNameString = _getTypeNameString(param.typeName!, ctx)
p.push(`${typeNameString}${storage}${name}`)
})

return p.join(', ')
}


const getPackedParamString = (params: VariableDeclaration[], meta: ParserMeta): string => {
const getPackedParamString = (params: VariableDeclaration[], ctx: FunctionParsingContext): string => {
const p: string[] = []

params.map(param => {
const typeNameString = _getTypeNameString(param.typeName!, meta)
const typeNameString = _getTypeNameString(param.typeName!, ctx)
p.push(`${typeNameString}`)
})

return p.join(',')
}


const _getTypeNameString = (typeName: TypeName, meta: ParserMeta): string => {
const _getTypeNameString = (typeName: TypeName, ctx: FunctionParsingContext): string => {
switch (typeName.type) {
case 'ElementaryTypeName': {
const t = typeName as ElementaryTypeName
return t.name
}
case 'UserDefinedTypeName': {
const t = typeName as UserDefinedTypeName
meta.userDefinedTypes.push(t.namePath)
ctx.userDefinedTypes.add(t.namePath)
return t.namePath
}
case 'ArrayTypeName': {
const t = typeName as ArrayTypeName
const innerType = _getTypeNameString(t.baseTypeName as TypeName, meta)
const innerType = _getTypeNameString(t.baseTypeName as TypeName, ctx)
const lenStr = t.length ? `[${(t.length as NumberLiteral).number}]` : '[]'
return `${innerType}${lenStr}`
}
Expand Down
10 changes: 6 additions & 4 deletions test/build.foundry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,18 @@ const setupFolder = () => {
describe("Command: build() - Foundry", () => {
it('builds the project', async () => {
const cwd = setupFolder()
const ret = cli('build', { cwd })
const ret = cli('build', { cwd, verbose: true })
expect(ret.success).to.be.true

const filePath = path.join(cwd, 'out/ExampleFacet.sol/ExampleFacet.json')
const json = loadJsonFile(filePath)
expect(json).to.have.property('abi')
})

addBuildTestSteps({
framework: 'foundry',
setupFolderCallback: setupFolder
describe('steps' , () => {
addBuildTestSteps({
framework: 'foundry',
setupFolderCallback: setupFolder
})
})
})
8 changes: 5 additions & 3 deletions test/build.hardhat.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@ describe("Command: build() - Hardhat", () => {
expect(json).to.have.property('abi')
})

addBuildTestSteps({
framework: 'hardhat',
setupFolderCallback: setupFolder
describe('steps' , () => {
addBuildTestSteps({
framework: 'hardhat',
setupFolderCallback: setupFolder
})
})
})
25 changes: 15 additions & 10 deletions test/common-build-steps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,14 @@ export const addBuildTestSteps = ({
{
"name": "getInt1",
"signature": "function getInt1() external view returns (uint)",
"signaturePacked": "getInt1()"
"signaturePacked": "getInt1()",
"userDefinedTypesInParams": []
},
{
"name": "setInt1",
"signature": "function setInt1(uint i) external",
"signaturePacked": "setInt1(uint)"
"signaturePacked": "setInt1(uint)",
"userDefinedTypesInParams": []
}
]
}
Expand Down Expand Up @@ -95,8 +97,8 @@ function setInt1(uint i) external;`,
__NUM_FACETS__: '1',
__FACET_SELECTORS__: `
bytes4[] memory f = new bytes4[](2);
f[0] = IDiamondProxy.getInt1.selector;
f[1] = IDiamondProxy.setInt1.selector;
f[0] = bytes4(keccak256(bytes('getInt1()')));
f[1] = bytes4(keccak256(bytes('setInt1(uint)')));
fs[0] = FacetSelectors({
addr: address(new ExampleFacet()),
sels: f
Expand Down Expand Up @@ -160,12 +162,14 @@ fs[0] = FacetSelectors({
{
"name": "getInts",
"signature": "function getInts() external view returns (uint a, uint b)",
"signaturePacked": "getInts()"
"signaturePacked": "getInts()",
"userDefinedTypesInParams": []
},
{
"name": "getData",
"signature": "function getData() external view returns (Data memory)",
"signaturePacked": "getData()"
"signaturePacked": "getData()",
"userDefinedTypesInParams": []
}
]
},
Expand All @@ -176,7 +180,8 @@ fs[0] = FacetSelectors({
{
"name": "setData",
"signature": "function setData(Data calldata d) external",
"signaturePacked": "setData(Data)"
"signaturePacked": "setData(Data)",
"userDefinedTypesInParams": ["Data"]
}
]
}
Expand Down Expand Up @@ -219,8 +224,8 @@ fs[0] = FacetSelectors({
});
f = new bytes4[](2);
f[0] = IDiamondProxy.getInts.selector;
f[1] = IDiamondProxy.getData.selector;
f[0] = bytes4(keccak256(bytes('getInts()')));
f[1] = bytes4(keccak256(bytes('getData()')));
fs[1] = FacetSelectors({
addr: address(new ExampleFacet()),
sels: f
Expand Down Expand Up @@ -273,7 +278,7 @@ fs[1] = FacetSelectors({
expectedFacetImports += `${i ? "\n" : ''}import { ${name} } from "../facets/${name}.sol";`
expectedFacetSelectors += `
${prefix}f = new bytes4[](1);
f[0] = IDiamondProxy.getInts${i}.selector;
f[0] = bytes4(keccak256(bytes('getInts${i}()')));
fs[${i}] = FacetSelectors({
addr: address(new ${name}()),
sels: f
Expand Down
Loading

0 comments on commit e38c03e

Please sign in to comment.