Check storage layout consistency in PRs (#3967)

Co-authored-by: Francisco <frangio.1@gmail.com>

.github/actions/storage-layout/action.yml

@@ -0,0 +1,55 @@
+name: Compare storage layouts
+inputs:
+  token:
+    description: github token
+    required: true
+  buildinfo:
+    description: compilation artifacts
+    required: false
+    default: artifacts/build-info/*.json
+  layout:
+    description: extracted storage layout
+    required: false
+    default: HEAD.layout.json
+  out_layout:
+    description: storage layout to upload
+    required: false
+    default: ${{ github.ref_name }}.layout.json
+  ref_layout:
+    description: storage layout for the reference branch
+    required: false
+    default: ${{ github.base_ref }}.layout.json
+
+runs:
+  using: composite
+  steps:
+    - name: Extract layout
+      run: |
+        node scripts/checks/extract-layout.js ${{ inputs.buildinfo }} > ${{ inputs.layout }}
+      shell: bash
+    - name: Download reference
+      if: github.event_name == 'pull_request'
+      run: |
+        RUN_ID=`gh run list --repo ${{ github.repository }} --branch ${{ github.base_ref }} --workflow ${{ github.workflow }} --limit 100 --json 'conclusion,databaseId,event' --jq 'map(select(.conclusion=="success" and .event!="pull_request"))[0].databaseId'`
+        gh run download ${RUN_ID} --repo ${{ github.repository }} -n layout
+      env:
+        GITHUB_TOKEN: ${{ inputs.token }}
+      shell: bash
+      continue-on-error: true
+      id: reference
+    - name: Compare layouts
+      if: steps.reference.outcome == 'success' && github.event_name == 'pull_request'
+      run: |
+        node scripts/checks/compare-layout.js --head ${{ inputs.layout }} --ref ${{ inputs.ref_layout }} >> $GITHUB_STEP_SUMMARY
+      shell: bash
+    - name: Rename artifacts for upload
+      if: github.event_name != 'pull_request'
+      run: |
+        mv ${{ inputs.layout }} ${{ inputs.out_layout }}
+      shell: bash
+    - name: Save artifacts
+      if: github.event_name != 'pull_request'
+      uses: actions/upload-artifact@v3
+      with:
+        name: layout
+        path: ${{ inputs.out_layout }}

.github/workflows/checks.yml

@@ -42,6 +42,10 @@ jobs:
         uses: ./.github/actions/gas-compare
         with:
           token: ${{ github.token }}
+      - name: Check storage layout
+        uses: ./.github/actions/storage-layout
+        with:
+          token: ${{ github.token }}
 
   foundry-tests:
     if: github.repository != 'OpenZeppelin/openzeppelin-contracts-upgradeable'

package-lock.json

@@ -21,6 +21,7 @@
         "@nomiclabs/hardhat-web3": "^2.0.0",
         "@openzeppelin/docs-utils": "^0.1.3",
         "@openzeppelin/test-helpers": "^0.5.13",
+        "@openzeppelin/upgrades-core": "^1.20.6",
         "chai": "^4.2.0",
         "eslint": "^8.30.0",
         "eslint-config-prettier": "^8.5.0",
@@ -2405,6 +2406,58 @@
         "semver": "bin/semver"
       }
     },
+    "node_modules/@openzeppelin/upgrades-core": {
+      "version": "1.20.6",
+      "resolved": "https://registry.npmjs.org/@openzeppelin/upgrades-core/-/upgrades-core-1.20.6.tgz",
+      "integrity": "sha512-KWdtlahm+iunlAlzLsdpBueanwEx0LLPfAkDL1p0C4SPjMiUqHHFlyGtmmWwdiqDpJ//605vfwkd5RqfnFrHSg==",
+      "dev": true,
+      "dependencies": {
+        "cbor": "^8.0.0",
+        "chalk": "^4.1.0",
+        "compare-versions": "^5.0.0",
+        "debug": "^4.1.1",
+        "ethereumjs-util": "^7.0.3",
+        "proper-lockfile": "^4.1.1",
+        "solidity-ast": "^0.4.15"
+      }
+    },
+    "node_modules/@openzeppelin/upgrades-core/node_modules/cbor": {
+      "version": "8.1.0",
+      "resolved": "https://registry.npmjs.org/cbor/-/cbor-8.1.0.tgz",
+      "integrity": "sha512-DwGjNW9omn6EwP70aXsn7FQJx5kO12tX0bZkaTjzdVFM6/7nhA4t0EENocKGx6D2Bch9PE2KzCUf5SceBdeijg==",
+      "dev": true,
+      "dependencies": {
+        "nofilter": "^3.1.0"
+      },
+      "engines": {
+        "node": ">=12.19"
+      }
+    },
+    "node_modules/@openzeppelin/upgrades-core/node_modules/chalk": {
+      "version": "4.1.2",
+      "resolved": "https://registry.npmjs.org/chalk/-/chalk-4.1.2.tgz",
+      "integrity": "sha512-oKnbhFyRIXpUuez8iBMmyEa4nbj4IOQyuhc/wy9kY7/WVPcwIO9VA668Pu8RkO7+0G76SLROeyw9CpQ061i4mA==",
+      "dev": true,
+      "dependencies": {
+        "ansi-styles": "^4.1.0",
+        "supports-color": "^7.1.0"
+      },
+      "engines": {
+        "node": ">=10"
+      },
+      "funding": {
+        "url": "https://github.com/chalk/chalk?sponsor=1"
+      }
+    },
+    "node_modules/@openzeppelin/upgrades-core/node_modules/nofilter": {
+      "version": "3.1.0",
+      "resolved": "https://registry.npmjs.org/nofilter/-/nofilter-3.1.0.tgz",
+      "integrity": "sha512-l2NNj07e9afPnhAhvgVrCD/oy2Ai1yfLpuo3EpiO1jFTsB4sFz6oIfAfSZyQzVpkZQ9xS8ZS5g1jCBgq4Hwo0g==",
+      "dev": true,
+      "engines": {
+        "node": ">=12.19"
+      }
+    },
     "node_modules/@scure/base": {
       "version": "1.1.1",
       "resolved": "https://registry.npmjs.org/@scure/base/-/base-1.1.1.tgz",
@@ -4571,6 +4624,12 @@
       "integrity": "sha512-6CYPa+JP2ftfRU2qkDK+UTVeQYosOg/2GbcjIcKPHfinyOLPVGXu/ovN86RP49Re5ndJK1N0kuiidFFuepc4ZQ==",
       "dev": true
     },
+    "node_modules/compare-versions": {
+      "version": "5.0.3",
+      "resolved": "https://registry.npmjs.org/compare-versions/-/compare-versions-5.0.3.tgz",
+      "integrity": "sha512-4UZlZP8Z99MGEY+Ovg/uJxJuvoXuN4M6B3hKaiackiHrgzQFEe3diJi1mf1PNHbFujM7FvLrK2bpgIaImbtZ1A==",
+      "dev": true
+    },
     "node_modules/concat-map": {
       "version": "0.0.1",
       "resolved": "https://registry.npmjs.org/concat-map/-/concat-map-0.0.1.tgz",
@@ -11430,6 +11489,17 @@
         "asap": "~2.0.6"
       }
     },
+    "node_modules/proper-lockfile": {
+      "version": "4.1.2",
+      "resolved": "https://registry.npmjs.org/proper-lockfile/-/proper-lockfile-4.1.2.tgz",
+      "integrity": "sha512-TjNPblN4BwAWMXU8s9AEz4JmQxnD1NNL7bNOY/AKUzyamc379FWASUhc/K1pL2noVb+XmZKLL68cjzLsiOAMaA==",
+      "dev": true,
+      "dependencies": {
+        "graceful-fs": "^4.2.4",
+        "retry": "^0.12.0",
+        "signal-exit": "^3.0.2"
+      }
+    },
     "node_modules/proxy-addr": {
       "version": "2.0.7",
       "resolved": "https://registry.npmjs.org/proxy-addr/-/proxy-addr-2.0.7.tgz",
@@ -12019,6 +12089,15 @@
         "node": ">=4"
       }
     },
+    "node_modules/retry": {
+      "version": "0.12.0",
+      "resolved": "https://registry.npmjs.org/retry/-/retry-0.12.0.tgz",
+      "integrity": "sha512-9LkiTwjUh6rT555DtE9rTX+BKByPfrMzEAtnlEtdEwr3Nkffwiihqe2bWADg+OQRjt9gl6ICdmB/ZFDCGAtSow==",
+      "dev": true,
+      "engines": {
+        "node": ">= 4"
+      }
+    },
     "node_modules/reusify": {
       "version": "1.0.4",
       "resolved": "https://registry.npmjs.org/reusify/-/reusify-1.0.4.tgz",
@@ -17993,6 +18072,48 @@
         }
       }
     },
+    "@openzeppelin/upgrades-core": {
+      "version": "1.20.6",
+      "resolved": "https://registry.npmjs.org/@openzeppelin/upgrades-core/-/upgrades-core-1.20.6.tgz",
+      "integrity": "sha512-KWdtlahm+iunlAlzLsdpBueanwEx0LLPfAkDL1p0C4SPjMiUqHHFlyGtmmWwdiqDpJ//605vfwkd5RqfnFrHSg==",
+      "dev": true,
+      "requires": {
+        "cbor": "^8.0.0",
+        "chalk": "^4.1.0",
+        "compare-versions": "^5.0.0",
+        "debug": "^4.1.1",
+        "ethereumjs-util": "^7.0.3",
+        "proper-lockfile": "^4.1.1",
+        "solidity-ast": "^0.4.15"
+      },
+      "dependencies": {
+        "cbor": {
+          "version": "8.1.0",
+          "resolved": "https://registry.npmjs.org/cbor/-/cbor-8.1.0.tgz",
+          "integrity": "sha512-DwGjNW9omn6EwP70aXsn7FQJx5kO12tX0bZkaTjzdVFM6/7nhA4t0EENocKGx6D2Bch9PE2KzCUf5SceBdeijg==",
+          "dev": true,
+          "requires": {
+            "nofilter": "^3.1.0"
+          }
+        },
+        "chalk": {
+          "version": "4.1.2",
+          "resolved": "https://registry.npmjs.org/chalk/-/chalk-4.1.2.tgz",
+          "integrity": "sha512-oKnbhFyRIXpUuez8iBMmyEa4nbj4IOQyuhc/wy9kY7/WVPcwIO9VA668Pu8RkO7+0G76SLROeyw9CpQ061i4mA==",
+          "dev": true,
+          "requires": {
+            "ansi-styles": "^4.1.0",
+            "supports-color": "^7.1.0"
+          }
+        },
+        "nofilter": {
+          "version": "3.1.0",
+          "resolved": "https://registry.npmjs.org/nofilter/-/nofilter-3.1.0.tgz",
+          "integrity": "sha512-l2NNj07e9afPnhAhvgVrCD/oy2Ai1yfLpuo3EpiO1jFTsB4sFz6oIfAfSZyQzVpkZQ9xS8ZS5g1jCBgq4Hwo0g==",
+          "dev": true
+        }
+      }
+    },
     "@scure/base": {
       "version": "1.1.1",
       "resolved": "https://registry.npmjs.org/@scure/base/-/base-1.1.1.tgz",
@@ -19767,6 +19888,12 @@
       "integrity": "sha512-6CYPa+JP2ftfRU2qkDK+UTVeQYosOg/2GbcjIcKPHfinyOLPVGXu/ovN86RP49Re5ndJK1N0kuiidFFuepc4ZQ==",
       "dev": true
     },
+    "compare-versions": {
+      "version": "5.0.3",
+      "resolved": "https://registry.npmjs.org/compare-versions/-/compare-versions-5.0.3.tgz",
+      "integrity": "sha512-4UZlZP8Z99MGEY+Ovg/uJxJuvoXuN4M6B3hKaiackiHrgzQFEe3diJi1mf1PNHbFujM7FvLrK2bpgIaImbtZ1A==",
+      "dev": true
+    },
     "concat-map": {
       "version": "0.0.1",
       "resolved": "https://registry.npmjs.org/concat-map/-/concat-map-0.0.1.tgz",
@@ -25154,6 +25281,17 @@
         "asap": "~2.0.6"
       }
     },
+    "proper-lockfile": {
+      "version": "4.1.2",
+      "resolved": "https://registry.npmjs.org/proper-lockfile/-/proper-lockfile-4.1.2.tgz",
+      "integrity": "sha512-TjNPblN4BwAWMXU8s9AEz4JmQxnD1NNL7bNOY/AKUzyamc379FWASUhc/K1pL2noVb+XmZKLL68cjzLsiOAMaA==",
+      "dev": true,
+      "requires": {
+        "graceful-fs": "^4.2.4",
+        "retry": "^0.12.0",
+        "signal-exit": "^3.0.2"
+      }
+    },
     "proxy-addr": {
       "version": "2.0.7",
       "resolved": "https://registry.npmjs.org/proxy-addr/-/proxy-addr-2.0.7.tgz",
@@ -25598,6 +25736,12 @@
         "signal-exit": "^3.0.2"
       }
     },
+    "retry": {
+      "version": "0.12.0",
+      "resolved": "https://registry.npmjs.org/retry/-/retry-0.12.0.tgz",
+      "integrity": "sha512-9LkiTwjUh6rT555DtE9rTX+BKByPfrMzEAtnlEtdEwr3Nkffwiihqe2bWADg+OQRjt9gl6ICdmB/ZFDCGAtSow==",
+      "dev": true
+    },
     "reusify": {
       "version": "1.0.4",
       "resolved": "https://registry.npmjs.org/reusify/-/reusify-1.0.4.tgz",

package.json

@@ -62,6 +62,7 @@
     "@nomiclabs/hardhat-web3": "^2.0.0",
     "@openzeppelin/docs-utils": "^0.1.3",
     "@openzeppelin/test-helpers": "^0.5.13",
+    "@openzeppelin/upgrades-core": "^1.20.6",
     "chai": "^4.2.0",
     "eslint": "^8.30.0",
     "eslint-config-prettier": "^8.5.0",

scripts/checks/compare-layout.js

@@ -0,0 +1,20 @@
+const fs = require('fs');
+const { getStorageUpgradeReport } = require('@openzeppelin/upgrades-core/dist/storage');
+
+const { ref, head } = require('yargs').argv;
+
+const oldLayout = JSON.parse(fs.readFileSync(ref));
+const newLayout = JSON.parse(fs.readFileSync(head));
+
+for (const name in oldLayout) {
+  if (name in newLayout) {
+    const report = getStorageUpgradeReport(oldLayout[name], newLayout[name], {});
+    if (!report.ok) {
+      console.log(`ERROR: Storage incompatibility in ${name}`);
+      console.log(report.explain());
+      process.exitCode = 1;
+    }
+  } else {
+    console.log(`WARNING: ${name} is missing from the current branch`);
+  }
+}

scripts/checks/extract-layout.js

@@ -0,0 +1,40 @@
+const fs = require('fs');
+const { findAll } = require('solidity-ast/utils');
+const { astDereferencer } = require('@openzeppelin/upgrades-core/dist/ast-dereferencer');
+const { solcInputOutputDecoder } = require('@openzeppelin/upgrades-core/dist/src-decoder');
+const { extractStorageLayout } = require('@openzeppelin/upgrades-core/dist/storage/extract');
+
+const { _ } = require('yargs').argv;
+
+const skipPath = ['contracts/mocks/', 'contracts-exposed/'];
+const skipKind = ['interface', 'library'];
+
+function extractLayouts(path) {
+  const layout = {};
+  const { input, output } = JSON.parse(fs.readFileSync(path));
+
+  const decoder = solcInputOutputDecoder(input, output);
+  const deref = astDereferencer(output);
+
+  for (const src in output.contracts) {
+    if (skipPath.some(prefix => src.startsWith(prefix))) {
+      continue;
+    }
+
+    for (const contractDef of findAll('ContractDefinition', output.sources[src].ast)) {
+      if (skipKind.includes(contractDef.contractKind)) {
+        continue;
+      }
+
+      layout[contractDef.name] = extractStorageLayout(
+        contractDef,
+        decoder,
+        deref,
+        output.contracts[src][contractDef.name].storageLayout,
+      );
+    }
+  }
+  return layout;
+}
+
+console.log(JSON.stringify(Object.assign(..._.map(extractLayouts))));