Skip to content

Commit

Permalink
feat: looser diamond checking (#37) + custom proxy templates (#38)
Browse files Browse the repository at this point in the history
feat: looser diamond checking (#37) + custom proxy templates (#38)
  • Loading branch information
hiddentao committed Aug 13, 2024
1 parent 907ea42 commit 1925673
Show file tree
Hide file tree
Showing 13 changed files with 293 additions and 28 deletions.
5 changes: 2 additions & 3 deletions src/commands/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export const command = () =>
__SOLC_SPDX__: ctx.config.solc.license,
__SOLC_VERSION__: ctx.config.solc.version,
__LIB_DIAMOND_PATH__: ctx.config.paths.lib.diamond,
})
}, ctx.config.generator.proxy?.template)

info('Generating IDiamondProxy.sol...')
let customImportsStr = ''
Expand Down Expand Up @@ -103,8 +103,7 @@ fs[${facetNum}] = FacetSelectors({
})

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`)
warn(`Custom structs found in facet method params. Polymorphism won't be supported for these methods: ${hasCustomStructs.join(', ')}\n\nSee - https://github.com/gemstation/gemforge/pull/40#issuecomment-2284272273`)
}

writeTemplate('LibDiamondHelper.sol', `${ctx.generatedSolidityPath}/LibDiamondHelper.sol`, {
Expand Down
31 changes: 21 additions & 10 deletions src/commands/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { Signer } from 'ethers'
import { ContractArtifact, Target, getContractAt, getContractValue, loadContractArtifact, readDeploymentInfo } from '../shared/chain.js'
import { Context } from '../shared/context.js'
import { FacetDefinition, loadJson } from '../shared/fs.js'
import { error, info } from '../shared/log.js'
import { error, info, trace, warn } from '../shared/log.js'

export interface CreateCommandOptions {
skipConfigOption?: boolean
Expand Down Expand Up @@ -43,21 +43,32 @@ export const loadExistingDeploymentAndLog = async ({ ctx, signer, targetArg, tar
info(`Checking if existing deployment is still valid...`)
const proxyInterface = await getContractAt(ctx, 'IDiamondProxy', signer, existingProxy.onChain.address)

let supportsInterface = false
let facets: any[] = []

try {
const isDiamond = await getContractValue(proxyInterface, 'supportsInterface', ['0x01ffc9a7'], true)
if (!isDiamond) {
throw new Error(`supportsInterface() error`)
supportsInterface = await getContractValue<boolean>(proxyInterface, 'supportsInterface', ['0x01ffc9a7'], true)
if (!supportsInterface) {
warn(` Does not support Diamond interface`)
}
} catch (err: any) {
warn(` Unable to call supportsInterface(): ${err.message}`)
}

const facets = await getContractValue(proxyInterface, 'facets', [], true)
if (!facets) {
throw new Error(`facets() error`)
try {
facets = await getContractValue<any[]>(proxyInterface, 'facets', [], true)
if (!facets.length) {
warn(` No facets found`)
}

return proxyInterface
} catch (err: any) {
error(`Existing deployment is not a diamond: ${err.message}\n\nYou may want to run with --new to force a fresh deployment.`)
warn(` Unable to call facets(): ${err.message}`)
}

if (!supportsInterface && !facets.length) {
error(`Existing deployment is not a diamond.\n\nYou may want to run with --new to force a fresh deployment.`)
}

return proxyInterface
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/commands/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export const command = () =>
ctx,
targetArg,
target,
signer
signer,
})

if (p) {
Expand Down
4 changes: 2 additions & 2 deletions src/shared/chain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -489,12 +489,12 @@ export const deployContract3 = async (
}
}

export const getContractValue = async (contract: OnChainContract, method: string, args: any[], dontExitOnError = false): Promise<any> => {
export const getContractValue = async <T = any>(contract: OnChainContract, method: string, args: any[], dontExitOnError = false): Promise<T> => {
const label = `${method}() on contract ${contract.artifact.name} deployed at ${contract.address} with args (${args.join(', ')})`

try {
trace(`Calling ${label} ...`)
return (await contract.contract[method](...args)) as any
return (await contract.contract[method](...args)) as T
} catch (err: any) {
const errorMessage = `Failed to call ${label}: ${err.message}`
if (dontExitOnError) {
Expand Down
6 changes: 6 additions & 0 deletions src/shared/config/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ export interface GemforgeConfig {
}
},
generator: {
proxy?: {
template: string,
},
proxyInterface: {
imports: string[],
},
Expand Down Expand Up @@ -134,6 +137,9 @@ export const sanitizeConfig = (config: GemforgeConfig) => {

// generator
ensureArray(config, 'generator.proxyInterface.imports')
if (get(config, 'generator.proxy')) {
ensureIsType(config, 'generator.proxy.template', ['string'])
}

// diamond
ensureBool(config, 'diamond.publicMethods')
Expand Down
2 changes: 1 addition & 1 deletion src/shared/diamond.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const getSelectors = (artifact: ContractArtifact): FunctionSelector[] => {
const getLiveFunctions = async (diamondProxy: OnChainContract): Promise<Record<string, string>> => {
// get what's on-chain
trace('Resolving methods on-chain ...')
const facets = await getContractValue(diamondProxy, 'facets', [])
const facets = await getContractValue<any[]>(diamondProxy, 'facets', [])
const liveFunctions: Record<string, string> = {}
facets.forEach((v: any) => {
v[1].forEach((f: string) => {
Expand Down
18 changes: 14 additions & 4 deletions src/shared/fs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,18 @@ export const fileExists = (file: string) => {
return existsSync(file)
}

export const writeTemplate = (file: string, dst: string, replacements: Record<string, string> = {}) => {
let str = readFileSync(new URL(`../../templates/${file}`, import.meta.url), 'utf-8')
export const writeTemplate = (file: string, dst: string, replacements: Record<string, string> = {}, customTemplatePath?: string) => {
let templatePath
if (customTemplatePath) {
templatePath = customTemplatePath
trace(` Using custom template: ${customTemplatePath}`)
} else {
templatePath = new URL(`../../templates/${file}`, import.meta.url)
trace(` Using default template: ${templatePath}`)
}

let str = readFileSync(templatePath!, 'utf-8')

Object.keys(replacements).forEach(key => {
str = str.replaceAll(key, replacements[key])
})
Expand Down Expand Up @@ -215,10 +225,10 @@ export const getUserFacetsAndFunctions = (ctx: Context): FacetDefinition[] => {
})

if (parserMeta.userDefinedTypesInParams.length) {
info(`Facet method params have custom structs: ${parserMeta.userDefinedTypesInParams.join(', ')}`)
trace(`Facet method params have custom structs: ${parserMeta.userDefinedTypesInParams.join(', ')}`)
}
if (parserMeta.userDefinedTypesInReturnValues.length) {
info(`Facet method return values have custom structs: ${parserMeta.userDefinedTypesInReturnValues.join(', ')}`)
trace(`Facet method return values have custom structs: ${parserMeta.userDefinedTypesInReturnValues.join(', ')}`)
}

// sort alphabetically
Expand Down
26 changes: 25 additions & 1 deletion test/common-build-steps.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import 'mocha'
import path, { join } from "node:path"
import { GemforgeConfig, assertFileMatchesTemplate, cli, expect, loadFile, loadJsonFile, removeFile, updateConfigFile, writeFile } from './utils.js'
import { GemforgeConfig, assertFileMatchesCustomTemplate, assertFileMatchesTemplate, cli, expect, getTestDataFolderPath, loadFile, loadJsonFile, removeFile, updateConfigFile, writeFile } from './utils.js'

export const addBuildTestSteps = ({
framework,
Expand Down Expand Up @@ -471,6 +471,30 @@ fs[${i}] = FacetSelectors({
expect(loadFile(join(cwd, '.gemforge/facets.json'))).to.equal('test')
})
})

describe('supports custom proxy templates', async () => {
const customTemplatePath = getTestDataFolderPath(`test-templates/DiamondProxy-custom.sol`)

beforeEach(async () => {
await updateConfigFile(join(cwd, 'gemforge.config.cjs'), (cfg: GemforgeConfig) => {
cfg.generator.proxy = {
template: customTemplatePath
}
return cfg
})
})

it("generates proxy contract", async () => {
expect(cli('build', { cwd }).success).to.be.true

const filePath = path.join(cwd, `${contractSrcBasePath}/generated/DiamondProxy.sol`)
assertFileMatchesCustomTemplate(filePath, customTemplatePath, {
__SOLC_SPDX__: 'MIT',
__SOLC_VERSION__: '0.8.21',
__LIB_DIAMOND_PATH__: 'lib/diamond-2-hardhat',
})
})
})
}


57 changes: 56 additions & 1 deletion test/common-deploy-steps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import get from "lodash.get"
import { ZeroAddress, ethers } from 'ethers'
import path, { join } from "node:path"
import { setTimeout } from "node:timers/promises"
import { GemforgeConfig, cli, expect, fileExists, loadDiamondContract, loadFile, loadJsonFile, loadWallet, removeFile, sendTx, updateConfigFile, writeFile } from './utils.js'
import { GemforgeConfig, cli, expect, fileExists, getTestDataFolderPath, loadDiamondContract, loadFile, loadJsonFile, loadWallet, removeFile, sendTx, updateConfigFile, writeFile } from './utils.js'


export const addDeployTestSteps = ({
Expand Down Expand Up @@ -622,5 +622,60 @@ export const addDeployTestSteps = ({
})
})

describe('can handle if supportsInterface() method is missing in live deployment', () => {
beforeEach(async () => {
cwd = setupFolderCallback()

await updateConfigFile(join(cwd, 'gemforge.config.cjs'), (cfg: GemforgeConfig) => {
cfg.generator.proxy = {
template: getTestDataFolderPath(`test-templates/DiamondProxy-noSupportsInterface.sol`)
}
return cfg
})

expect(cli('build', { cwd, verbose: false }).success).to.be.true
expect(cli('deploy', 'local', '-n', { cwd, verbose: false }).success).to.be.true
})

it('and can handle an upgrade', async () => {
writeFile(join(cwd, `${contractSrcBasePath}/facets/ExampleFacet.sol`), `
pragma solidity >=0.8.21;
import "../libs/LibAppStorage.sol";
contract ExampleFacet {
// keep method same
function getInt1() external view returns (uint) {
AppStorage storage s = LibAppStorage.diamondStorage();
return s.data.i1;
}
// change method behaviour
function setInt1(uint i) external {
AppStorage storage s = LibAppStorage.diamondStorage();
s.data.i1 = i + 1;
}
// add new method
function setInt1New(uint i) external {
AppStorage storage s = LibAppStorage.diamondStorage();
s.data.i1 = i + 2;
}
}
`)

// build and re-deploy
expect(cli('build', { cwd, verbose: false }).success).to.be.true
const ret = cli('deploy', 'local', { cwd, verbose: false })
expect(ret.success).to.be.true
expect(ret.output).to.contain('Unable to call supportsInterface')

const { contract } = await loadDiamondContract(cwd)

await sendTx(contract.setInt1(2))
let n = await contract.getInt1()
expect(n.toString()).to.equal('3') // 2 + 1
await sendTx(contract.setInt1New(2))
n = await contract.getInt1()
expect(n.toString()).to.equal('4') // 2 + 2
})
})
}
29 changes: 28 additions & 1 deletion test/common-query-steps.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import 'mocha'
import { join } from "node:path"
import { cli, expect, loadFile, loadJsonFile, writeFile } from './utils.js'
import { cli, expect, GemforgeConfig, getTestDataFolderPath, loadFile, loadJsonFile, updateConfigFile, writeFile } from './utils.js'

const loadContractAddresses = (cwd: string) => {
const deployedContracts = loadJsonFile(join(cwd, 'gemforge.deployments.json')).local.contracts
Expand Down Expand Up @@ -168,4 +168,31 @@ export const addQueryTestSteps = ({
expect(out.facets[a.ExampleFacet].functions.find((f: any) => f.selector === '0x4d2c097d')).to.have.property('unrecognized', true)
})
})

describe('can handle if supportsInterface() method is missing in live deployment', () => {
beforeEach(async () => {
cwd = setupFolderCallback()

await updateConfigFile(join(cwd, 'gemforge.config.cjs'), (cfg: GemforgeConfig) => {
cfg.generator.proxy = {
template: getTestDataFolderPath(`test-templates/DiamondProxy-noSupportsInterface.sol`)
}
return cfg
})

expect(cli('build', { cwd, verbose: false }).success).to.be.true
expect(cli('deploy', 'local', { cwd, verbose: false }).success).to.be.true
})

it('outputs text', async () => {
const outFilePath = join(cwd, 'query-output')
const ret = cli('query', 'local', '--output', outFilePath, { cwd })
expect(ret.success).to.be.true

const out = loadFile(outFilePath)

expect(out).to.contain(`fn: facets() (0x7a0ed627)`)
expect(out).to.not.contain(`fn: supportsInterface(bytes4) (0x01ffc9a7)`)
})
})
}
65 changes: 65 additions & 0 deletions test/data/test-templates/DiamondProxy-custom.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
// SPDX-License-Identifier: __SOLC_SPDX__
pragma solidity >=__SOLC_VERSION__;

/// ------------------------------------------------------------------------------------------------------------
///
/// NOTE: This file is auto-generated by Gemforge.
///
/// ------------------------------------------------------------------------------------------------------------

import { Diamond } from '__LIB_DIAMOND_PATH__/contracts/Diamond.sol';
import { LibDiamond } from '__LIB_DIAMOND_PATH__/contracts/libraries/LibDiamond.sol';
import { DiamondCutFacet } from '__LIB_DIAMOND_PATH__/contracts/facets/DiamondCutFacet.sol';
import { DiamondLoupeFacet } from '__LIB_DIAMOND_PATH__/contracts/facets/DiamondLoupeFacet.sol';
import { OwnershipFacet } from '__LIB_DIAMOND_PATH__/contracts/facets/OwnershipFacet.sol';
import { IDiamondCut } from '__LIB_DIAMOND_PATH__/contracts/interfaces/IDiamondCut.sol';
import { IDiamondLoupe } from '__LIB_DIAMOND_PATH__/contracts/interfaces/IDiamondLoupe.sol';
import { IERC165 } from '__LIB_DIAMOND_PATH__/contracts/interfaces/IERC165.sol';
import { IERC173 } from '__LIB_DIAMOND_PATH__/contracts/interfaces/IERC173.sol';

contract DiamondProxy is Diamond {
constructor(address _contractOwner) payable Diamond(_contractOwner, address(new DiamondCutFacet())) {
// add core facets
_initCoreFacets();

// setup ERC165 data
LibDiamond.DiamondStorage storage ds = LibDiamond.diamondStorage();
ds.supportedInterfaces[type(IERC165).interfaceId] = true;
ds.supportedInterfaces[type(IDiamondCut).interfaceId] = true;
ds.supportedInterfaces[type(IDiamondLoupe).interfaceId] = true;
ds.supportedInterfaces[type(IERC173).interfaceId] = true;
ds.supportedInterfaces[0xdeadbeef] = true;
}

// Internal

function _initCoreFacets() private {
// add basic facets
IDiamondCut.FacetCut[] memory cut = new IDiamondCut.FacetCut[](2);

address _diamondLoupeFacet = address(new DiamondLoupeFacet());
bytes4[] memory f1 = new bytes4[](5);
f1[0] = IDiamondLoupe.facets.selector;
f1[1] = IDiamondLoupe.facetFunctionSelectors.selector;
f1[2] = IDiamondLoupe.facetAddresses.selector;
f1[3] = IDiamondLoupe.facetAddress.selector;
f1[4] = IERC165.supportsInterface.selector;
cut[0] = IDiamondCut.FacetCut({
facetAddress: _diamondLoupeFacet,
action: IDiamondCut.FacetCutAction.Add,
functionSelectors: f1
});

address _ownershipFacet = address(new OwnershipFacet());
bytes4[] memory f2 = new bytes4[](2);
f2[0] = IERC173.owner.selector;
f2[1] = IERC173.transferOwnership.selector;
cut[1] = IDiamondCut.FacetCut({
facetAddress: _ownershipFacet,
action: IDiamondCut.FacetCutAction.Add,
functionSelectors: f2
});

LibDiamond.diamondCut(cut, address(0), "");
}
}
Loading

0 comments on commit 1925673

Please sign in to comment.