From 8219046a53c4a7b71e4aeeb4fe0c546c3fd02916 Mon Sep 17 00:00:00 2001 From: Carl Park Date: Tue, 24 Oct 2023 12:18:35 +0900 Subject: [PATCH 1/5] fix: support custom artifacts for default diamond facets --- src/helpers.ts | 95 ++++++++++++++++++++++++++++++++++++++++++-------- types.ts | 24 +++++++++++++ 2 files changed, 104 insertions(+), 15 deletions(-) diff --git a/src/helpers.ts b/src/helpers.ts index 87d7baa8..066b303a 100644 --- a/src/helpers.ts +++ b/src/helpers.ts @@ -525,7 +525,7 @@ export function addHelpers( options ); - let overrides: PayableOverrides = { + const overrides: PayableOverrides = { gasLimit: options.gasLimit, gasPrice: options.gasPrice, maxFeePerGas: options.maxFeePerGas, @@ -1493,8 +1493,8 @@ Note that in this case, the contract deployment will not behave the same if depl upgradeArgsTemplate = ['{implementation}', '{data}']; } - let proxyAddress = proxy.address; - let upgradeArgs = replaceTemplateArgs(upgradeArgsTemplate, { + const proxyAddress = proxy.address; + const upgradeArgs = replaceTemplateArgs(upgradeArgsTemplate, { implementationAddress: implementation.address, proxyAdmin, data, @@ -1829,10 +1829,61 @@ Note that in this case, the contract deployment will not behave the same if depl // console.log({ oldFacets: JSON.stringify(oldFacets, null, " ") }); const facetsSet = [...options.facets]; + const getContractNameAndArtifact = async ( + defaultName: string, + defaultArtifactData: ArtifactData, + args?: string | {name: string; artifact?: string | ArtifactData} + ) => { + if (args === undefined) { + return { + name: defaultName, + artifact: defaultArtifactData, + }; + } + + if (typeof args === 'string') { + try { + return { + name: defaultName, + artifact: await partialExtension.getExtendedArtifact(args), + }; + } catch (e) {} + + if (args === defaultName) { + return { + name: defaultName, + artifact: defaultArtifactData, + }; + } else { + throw new Error(`no contract found for ${args}`); + } + } + + const name = args.name; + let artifact: ArtifactData | undefined; + if (args.artifact === undefined) { + artifact = defaultArtifactData; + } else if (typeof args.artifact === 'string') { + artifact = await partialExtension.getExtendedArtifact(args.artifact); + } else { + artifact = args.artifact; + } + + return { + name: name, + artifact: artifact, + }; + }; + if (options.defaultCutFacet === undefined || options.defaultCutFacet) { + const diamondCutFacetNameAndArtifact = await getContractNameAndArtifact( + '_DefaultDiamondCutFacet', + diamondCutFacet, + options.viaCutFacetContract + ); facetsSet.push({ - name: '_DefaultDiamondCutFacet', - contract: diamondCutFacet, + name: diamondCutFacetNameAndArtifact.name, + contract: diamondCutFacetNameAndArtifact.artifact, args: [], deterministic: true, }); @@ -1841,16 +1892,26 @@ Note that in this case, the contract deployment will not behave the same if depl options.defaultOwnershipFacet === undefined || options.defaultOwnershipFacet ) { + const ownershipFacetNameAndArtifact = await getContractNameAndArtifact( + '_DefaultDiamondOwnershipFacet', + ownershipFacet, + options.viaOwnershipFacetContract + ); facetsSet.push({ - name: '_DefaultDiamondOwnershipFacet', - contract: ownershipFacet, + name: ownershipFacetNameAndArtifact.name, + contract: ownershipFacetNameAndArtifact.artifact, args: [], deterministic: true, }); } + const diamondLoupeFacetNameAndArtifact = await getContractNameAndArtifact( + '_DefaultDiamondLoupeFacet', + diamondLoupeFacet, + options.viaLoopeFacetContract + ); facetsSet.push({ - name: '_DefaultDiamondLoupeFacet', - contract: diamondLoupeFacet, + name: diamondLoupeFacetNameAndArtifact.name, + contract: diamondLoupeFacetNameAndArtifact.artifact, args: [], deterministic: true, }); @@ -2209,12 +2270,18 @@ Note that in this case, the contract deployment will not behave the same if depl } if (initializationsArgIndex >= 0 || erc165InitArgIndex >= 0) { + const diamondERC165InitNameAndArtifact = + await getContractNameAndArtifact( + '_DefaultDiamondERC165Init', + diamondERC165Init, + options.viaERC165InitContract + ); const diamondERC165InitDeployment = await _deployOne( - '_DefaultDiamondERC165Init', + diamondERC165InitNameAndArtifact.name, { from: options.from, deterministicDeployment: true, - contract: diamondERC165Init, + contract: diamondERC165InitNameAndArtifact.artifact, autoMine: options.autoMine, estimateGasExtra: options.estimateGasExtra, estimatedGasLimit: options.estimatedGasLimit, @@ -2784,9 +2851,7 @@ data: ${data} } async function getSigner(address: string): Promise { await init(); - const { - ethersSigner - } = await getFrom(address); + const {ethersSigner} = await getFrom(address); return ethersSigner; } @@ -2802,7 +2867,7 @@ data: ${data} rawTx, read, deterministic, - getSigner + getSigner, }; const utils = { diff --git a/types.ts b/types.ts index 3d4df3ff..5873a271 100644 --- a/types.ts +++ b/types.ts @@ -83,8 +83,32 @@ export interface DiamondOptions extends TxOptions { diamondContractArgs?: any[]; owner?: Address; // defaultLoopeFacet?: boolean; // TODO // always there + viaLoopeFacetContract?: + | string + | { + name: string; + artifact?: string | ArtifactData; + }; defaultOwnershipFacet?: boolean; + viaOwnershipFacetContract?: + | string + | { + name: string; + artifact?: string | ArtifactData; + }; defaultCutFacet?: boolean; + viaCutFacetContract?: + | string + | { + name: string; + artifact?: string | ArtifactData; + }; + viaERC165InitContract?: + | string + | { + name: string; + artifact?: string | ArtifactData; + }; facets: DiamondFacets; log?: boolean; libraries?: Libraries; From a2ca4a486d892406e39853e1513d5093dc87c006 Mon Sep 17 00:00:00 2001 From: Carl Park Date: Wed, 25 Oct 2023 10:46:17 +0900 Subject: [PATCH 2/5] fix: compare only tx.data for zksync deploy transaction --- src/DeploymentFactory.ts | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/DeploymentFactory.ts b/src/DeploymentFactory.ts index 806c6c33..aa5b2ce8 100644 --- a/src/DeploymentFactory.ts +++ b/src/DeploymentFactory.ts @@ -140,15 +140,17 @@ export class DeploymentFactory { ): Promise { const newTransaction = await this.getDeployTransaction(); const newData = newTransaction.data?.toString(); - if (this.isZkSync) { - const deserialize = zk.utils.parseTransaction(transaction.data) as any; - const desFlattened = hexConcat(deserialize.customData.factoryDeps); - const factoryDeps = await this.extractFactoryDeps(this.artifact); - const newFlattened = hexConcat(factoryDeps); + // NOTE: (25 Oct) zksync-era client does not support factoryDeps in transaction.data + // if (this.isZkSync) { + // const deserialize = zk.utils.parseTransaction(transaction.data) as any; + // const desFlattened = hexConcat(deserialize.customData.factoryDeps); + // const factoryDeps = await this.extractFactoryDeps(this.artifact); + // const newFlattened = hexConcat(factoryDeps); - return deserialize.data !== newData || desFlattened != newFlattened; - } else { - return transaction.data !== newData; - } + // return deserialize.data !== newData || desFlattened != newFlattened; + // } else { + // return transaction.data !== newData; + // } + return transaction.data !== newData; } } From 305edf267d3fb9423142b2eb19bf7bd97ebcffb8 Mon Sep 17 00:00:00 2001 From: Carl Park Date: Thu, 26 Oct 2023 10:49:47 +0900 Subject: [PATCH 3/5] fix: remove via* options --- src/helpers.ts | 71 ++++++++++++++++++++++++++++++-------------------- types.ts | 14 +++++----- 2 files changed, 50 insertions(+), 35 deletions(-) diff --git a/src/helpers.ts b/src/helpers.ts index 066b303a..407dc539 100644 --- a/src/helpers.ts +++ b/src/helpers.ts @@ -1832,9 +1832,19 @@ Note that in this case, the contract deployment will not behave the same if depl const getContractNameAndArtifact = async ( defaultName: string, defaultArtifactData: ArtifactData, - args?: string | {name: string; artifact?: string | ArtifactData} - ) => { - if (args === undefined) { + args?: boolean | string | {name: string; artifact?: string | ArtifactData} + ): Promise< + | { + name: string; + artifact: ArtifactData; + } + | undefined + > => { + if (args === false) { + return undefined; + } + + if (args === undefined || args === true) { return { name: defaultName, artifact: defaultArtifactData, @@ -1875,12 +1885,12 @@ Note that in this case, the contract deployment will not behave the same if depl }; }; - if (options.defaultCutFacet === undefined || options.defaultCutFacet) { - const diamondCutFacetNameAndArtifact = await getContractNameAndArtifact( - '_DefaultDiamondCutFacet', - diamondCutFacet, - options.viaCutFacetContract - ); + const diamondCutFacetNameAndArtifact = await getContractNameAndArtifact( + '_DefaultDiamondCutFacet', + diamondCutFacet, + options.defaultCutFacetContract + ); + if (diamondCutFacetNameAndArtifact) { facetsSet.push({ name: diamondCutFacetNameAndArtifact.name, contract: diamondCutFacetNameAndArtifact.artifact, @@ -1888,15 +1898,13 @@ Note that in this case, the contract deployment will not behave the same if depl deterministic: true, }); } - if ( - options.defaultOwnershipFacet === undefined || - options.defaultOwnershipFacet - ) { - const ownershipFacetNameAndArtifact = await getContractNameAndArtifact( - '_DefaultDiamondOwnershipFacet', - ownershipFacet, - options.viaOwnershipFacetContract - ); + + const ownershipFacetNameAndArtifact = await getContractNameAndArtifact( + '_DefaultDiamondOwnershipFacet', + ownershipFacet, + options.defaultOwnershipFacetContract + ); + if (ownershipFacetNameAndArtifact) { facetsSet.push({ name: ownershipFacetNameAndArtifact.name, contract: ownershipFacetNameAndArtifact.artifact, @@ -1904,17 +1912,20 @@ Note that in this case, the contract deployment will not behave the same if depl deterministic: true, }); } + const diamondLoupeFacetNameAndArtifact = await getContractNameAndArtifact( '_DefaultDiamondLoupeFacet', diamondLoupeFacet, - options.viaLoopeFacetContract + options.defaultLoopeFacetContract ); - facetsSet.push({ - name: diamondLoupeFacetNameAndArtifact.name, - contract: diamondLoupeFacetNameAndArtifact.artifact, - args: [], - deterministic: true, - }); + if (diamondLoupeFacetNameAndArtifact) { + facetsSet.push({ + name: diamondLoupeFacetNameAndArtifact.name, + contract: diamondLoupeFacetNameAndArtifact.artifact, + args: [], + deterministic: true, + }); + } let changesDetected = !oldDeployment; let abi: any[] = diamondArtifact.abi.concat([]); @@ -2262,10 +2273,10 @@ Note that in this case, the contract deployment will not behave the same if depl // TODO option to add more to the list // else mechanism to set it up differently ? LoupeFacet without supportsInterface const interfaceList = ['0x48e2b093']; - if (options.defaultCutFacet) { + if (options.defaultCutFacetContract) { interfaceList.push('0x1f931c1c'); } - if (options.defaultOwnershipFacet) { + if (options.defaultOwnershipFacetContract !== false) { interfaceList.push('0x7f5828d0'); } @@ -2274,8 +2285,12 @@ Note that in this case, the contract deployment will not behave the same if depl await getContractNameAndArtifact( '_DefaultDiamondERC165Init', diamondERC165Init, - options.viaERC165InitContract + options.defaultERC165InitContract ); + if (!diamondERC165InitNameAndArtifact) { + throw new Error('no DiamondERC165Init artifact found'); + } + const diamondERC165InitDeployment = await _deployOne( diamondERC165InitNameAndArtifact.name, { diff --git a/types.ts b/types.ts index 5873a271..29857918 100644 --- a/types.ts +++ b/types.ts @@ -82,28 +82,28 @@ export interface DiamondOptions extends TxOptions { diamondContract?: string | ArtifactData; // TODO diamondContractArgs?: any[]; owner?: Address; - // defaultLoopeFacet?: boolean; // TODO // always there - viaLoopeFacetContract?: + defaultLoopeFacetContract?: + | boolean | string | { name: string; artifact?: string | ArtifactData; }; - defaultOwnershipFacet?: boolean; - viaOwnershipFacetContract?: + defaultOwnershipFacetContract?: + | boolean | string | { name: string; artifact?: string | ArtifactData; }; - defaultCutFacet?: boolean; - viaCutFacetContract?: + defaultCutFacetContract?: + | boolean | string | { name: string; artifact?: string | ArtifactData; }; - viaERC165InitContract?: + defaultERC165InitContract?: | string | { name: string; From 38f92baa7e252b68962d35ce78cc2fcce4d975ec Mon Sep 17 00:00:00 2001 From: Carl Park Date: Thu, 26 Oct 2023 11:10:36 +0900 Subject: [PATCH 4/5] fix: useless false check --- src/helpers.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/helpers.ts b/src/helpers.ts index 407dc539..6e790225 100644 --- a/src/helpers.ts +++ b/src/helpers.ts @@ -2276,7 +2276,7 @@ Note that in this case, the contract deployment will not behave the same if depl if (options.defaultCutFacetContract) { interfaceList.push('0x1f931c1c'); } - if (options.defaultOwnershipFacetContract !== false) { + if (options.defaultOwnershipFacetContract) { interfaceList.push('0x7f5828d0'); } From 21c2d22c782811c75c58c024181eb0c6317a8aa0 Mon Sep 17 00:00:00 2001 From: Carl Park Date: Thu, 26 Oct 2023 11:14:05 +0900 Subject: [PATCH 5/5] fix typo --- src/helpers.ts | 2 +- types.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/helpers.ts b/src/helpers.ts index 6e790225..576cba35 100644 --- a/src/helpers.ts +++ b/src/helpers.ts @@ -1916,7 +1916,7 @@ Note that in this case, the contract deployment will not behave the same if depl const diamondLoupeFacetNameAndArtifact = await getContractNameAndArtifact( '_DefaultDiamondLoupeFacet', diamondLoupeFacet, - options.defaultLoopeFacetContract + options.defaultLoupeFacetContract ); if (diamondLoupeFacetNameAndArtifact) { facetsSet.push({ diff --git a/types.ts b/types.ts index 29857918..0fdbb187 100644 --- a/types.ts +++ b/types.ts @@ -82,7 +82,7 @@ export interface DiamondOptions extends TxOptions { diamondContract?: string | ArtifactData; // TODO diamondContractArgs?: any[]; owner?: Address; - defaultLoopeFacetContract?: + defaultLoupeFacetContract?: | boolean | string | {