feat: Remove dangling deployment artifacts (#300)

Author: zguesmi

.github/workflows/upgrade-facets.yml

@@ -14,7 +14,7 @@ on:
           - arbitrumSepolia
           - arbitrum
         default: 'arbitrumSepolia'
-      dry_run:
+      dry-run:
         description: 'Dry Run (fork test only, no actual deployment)'
         required: false
         type: boolean
@@ -27,13 +27,12 @@ jobs:
   upgrade:
     needs: pre-upgrade
     runs-on: ubuntu-latest
-    environment: ${{ inputs.network }} # Use the selected environment
+    environment: ${{ inputs.network }}
     permissions:
       contents: write # Required to commit artifacts.
       pull-requests: write # Required to create pull requests.
     env:
       UPGRADE_SCRIPT: 'v6.1.0-bulk-processing.ts' # Update this for each specific upgrade.
-      # For commit action
       COMMIT_MESSAGE: 'chore: Save upgrade artifacts - ${{ inputs.network }} (runId:${{ github.run_id }})'
       GHA_BOT_NAME: 'GitHub Actions Bot'
       GHA_BOT_EMAIL: 'github-actions[bot]@users.noreply.github.com'
@@ -54,7 +53,7 @@ jobs:
         run: npm run build
 
       - name: Run fork test (dry run)
-        if: inputs.dry_run == true
+        if: inputs.dry-run == true
         env:
           # Note: it is required to define both private key env variables when calling Hardhat.
           DEPLOYER_PRIVATE_KEY: ${{ secrets.DEPLOYER_PRIVATE_KEY }}
@@ -66,10 +65,10 @@ jobs:
           elif [ "${{ inputs.network }}" == "arbitrum" ]; then
             export ARBITRUM_FORK=true
           fi
-          npx hardhat run scripts/upgrades/${{ env.UPGRADE_SCRIPT }} --network hardhat
+          UPGRADE_SCRIPT=${{ env.UPGRADE_SCRIPT }} bash ./scripts/upgrades/dry-run.sh
 
       - name: Execute upgrade on live network
-        if: inputs.dry_run == false
+        if: inputs.dry-run == false
         env:
           # Note: it is required to define both private key env variables when calling Hardhat.
           DEPLOYER_PRIVATE_KEY: ${{ secrets.DEPLOYER_PRIVATE_KEY }}
@@ -80,7 +79,7 @@ jobs:
           npx hardhat run scripts/upgrades/${{ env.UPGRADE_SCRIPT }} --network ${{ inputs.network }}
 
       - name: Push artifacts to the current branch
-        if: inputs.dry_run == false && github.ref != 'refs/heads/main'
+        if: inputs.dry-run == false && github.ref != 'refs/heads/main'
         uses: stefanzweifel/git-auto-commit-action@v5
         with:
           file_pattern: |
@@ -92,7 +91,7 @@ jobs:
 
       # Since the `main` branch is protected, create a PR to push artifacts.
       - name: Push artifacts through a pull request
-        if: inputs.dry_run == false && github.ref == 'refs/heads/main'
+        if: inputs.dry-run == false && github.ref == 'refs/heads/main'
         uses: peter-evans/create-pull-request@v7
         with:
           add-paths: |

.gitignore

@@ -15,7 +15,8 @@ cache
 artifacts
 
 contracts/hardhat-dependency-compiler/
-deployments/*hardhat*
+deployments/hardhat
+deployments/external-hardhat
 deployments/dev-*
 
 # Flattened Solidity file used to generate diagrams.

scripts/upgrades/README.md

@@ -1,39 +1,41 @@
 # PoCo Smart Contracts Upgrade Guide
 
-This document explains the recommended steps for creating and applying
-a safe, traceable, and reproducible upgrade to the PoCo Diamond proxy.
+This document explains the recommended steps for creating and applying a safe, traceable,
+and reproducible upgrade to the PoCo Diamond proxy.
 
 ## Upgrade Steps
 
 0. **Ensure all tests pass**:<br>
     Run the full test suite to make sure everything is working before starting an upgrade.
 
 1. **Create a new upgrade script**:<br>
-    Name the script using the version and upgrade name format: `vX.Y.Z-upgrade-name.ts`.
+    Name the script using the version and upgrade name in the form `vX.Y.Z-upgrade-name.ts`
+    and implement the upgrade logic.
 
 2. **Create a corresponding Markdown report**:<br>
-    Copy the template file `v0.0.0-template.md` and rename it to match the script: `vX.Y.Z-upgrade-name.md`.
+    Copy the template file `v0.0.0-template.md` and rename it to match the script's name
+    (`vX.Y.Z-upgrade-name.md`).
 
-3. **Update GitHub Actions**:<br>
+3. **Test dry-runs locally**:<br>
+    Use the script [./dry-run.sh](./dry-run.sh) and check the logs.
+
+4. **Update GitHub Actions**:<br>
     Modify `upgrade-facets.yml` workflow to call the new upgrade script.
 
-4. **Upgrade on Testnet**:
+5. **Upgrade on Testnet**:
    - ⚠️ Always upgrade on the testnet first.
    - Trigger the upgrade workflow on GitHub and choose the testnet network.
    - Start with a **dry run** to simulate the upgrade.
    - Once verified, apply the upgrade on the live testnet.
 
-5. **Upgrade on Mainnet**:
+6. **Upgrade on Mainnet**:
    - Trigger the upgrade workflow on GitHub and choose the mainnet network.
    - Perform a dry run first.
    - Apply the upgrade on the mainnet.
    - Merge the artifacts PR after successful execution.
 
-6. **Update upgrade report**:<br>
+7. **Update upgrade report**:<br>
     Fill in all required information in `vX.Y.Z-upgrade-name.ts` (tx hashes, logs, ...).
 
-7. **Create a release**
+8. **Create a release**
    - Use **Release Please** to tag the upgrade version and create the release on GitHub.
-
-
-Following these steps ensures upgrades are **safe, traceable, and reproducible**.

scripts/upgrades/dry-run.sh

@@ -0,0 +1,31 @@
+#!/bin/bash
+set -e
+
+# Usage:
+#   ARBITRUM_SEPOLIA_FORK=true \
+#   UPGRADE_SCRIPT=<script_name> \
+#       bash ./scripts/upgrades/dry-run.sh
+
+
+DEPLOYMENTS_FOLDER=''
+if [ "${ARBITRUM_SEPOLIA_FORK}" == "true" ]; then
+    DEPLOYMENTS_FOLDER=arbitrumSepolia
+elif [ "${ARBITRUM_FORK}" == "true" ]; then
+    DEPLOYMENTS_FOLDER=arbitrum
+fi
+if [ -z "${DEPLOYMENTS_FOLDER}" ]; then
+    echo "Error: You must set either ARBITRUM_SEPOLIA_FORK=true or ARBITRUM_FORK=true."
+    exit 1
+fi
+# Copy the forked network deployments to the hardhat network folder.
+rm -rf deployments/hardhat
+cp -r deployments/${DEPLOYMENTS_FOLDER} deployments/hardhat
+# Stage the old deployments to have a clean diff after the upgrade script run.
+cp .gitignore .gitignore.bak
+sed -i.bak '/deployments\/hardhat/d' .gitignore
+git add deployments/hardhat
+# Run the upgrade and print the git diff.
+npx hardhat run scripts/upgrades/${UPGRADE_SCRIPT} --network hardhat
+echo "\n=== Upgrade diff ==="
+git --no-pager diff --name-status
+mv .gitignore.bak .gitignore

scripts/upgrades/v6.1.0-bulk-processing.ts

@@ -9,6 +9,7 @@ import {
     getUpgradeContext,
     linkFacetsToDiamond,
     printOnchainProxyFunctions,
+    removeDanglingFacetDeploymentArtifacts,
     removeFacetsFromDiamond,
     removeFunctionsFromDiamond,
 } from '../../utils/proxy-tools';
@@ -103,6 +104,7 @@ async function main() {
     await linkFacetsToDiamond(proxyAddress, proxyOwner, facetsToAdd);
     await printOnchainProxyFunctions(proxyAddress);
     console.log('Upgrade performed successfully!');
+    await removeDanglingFacetDeploymentArtifacts(proxyAddress);
     await tryVerify(facetsToAdd.map((facet) => facet.name));
 }
 

utils/proxy-tools.ts

@@ -3,7 +3,7 @@
 
 import { SignerWithAddress } from '@nomicfoundation/hardhat-ethers/signers';
 import { ContractFactory, FunctionFragment, Interface, ZeroAddress } from 'ethers';
-import { ethers } from 'hardhat';
+import { deployments, ethers } from 'hardhat';
 import { FacetCut, FacetCutAction } from 'hardhat-deploy/dist/types';
 import type { IDiamond } from '../typechain';
 import {
@@ -279,7 +279,7 @@ export async function removeFacetsFromDiamond(
             throw new Error(`Facet ${facet.name} is empty or does not exist on-chain`);
         }
         console.log(
-            `Will remove the whole facet ${facet.name} [address: ${facet.address}, functions:${selectors.length}]`,
+            `Will remove facet [name:${facet.name}, address: ${facet.address}, functions:${selectors.length}]`,
         );
         facetCuts.push({
             facetAddress: ZeroAddress,
@@ -350,3 +350,25 @@ export async function removeFunctionsFromDiamond(
     await tx.wait();
     console.log('Functions removed successfully!');
 }
+
+/**
+ * Removes dangling deployment artifacts for facets that are no longer linked to the diamond proxy.
+ * This is not done automatically in `removeFacetsFromDiamond` because we deploy new facets first,
+ * then remove old facets, which sometimes overwrites the existing ones.
+ * @param proxyAddress address of the diamond proxy
+ */
+export async function removeDanglingFacetDeploymentArtifacts(proxyAddress: string) {
+    console.log('\n=== Removing dangling deployment artifacts ===');
+    const allDeployments = await deployments.all();
+    const diamondLoupe = DiamondLoupeFacet__factory.connect(proxyAddress, ethers.provider);
+    const onchainFacets = await diamondLoupe.facetAddresses();
+    for (const deploymentName of Object.keys(allDeployments)) {
+        const deploymentAddress = allDeployments[deploymentName].address;
+        if (deploymentName.endsWith('Facet') && !onchainFacets.includes(deploymentAddress)) {
+            console.log(
+                `Deleting dangling facet artifact [name:${deploymentName}, address:${deploymentAddress}]`,
+            );
+            await deployments.delete(deploymentName);
+        }
+    }
+}