diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..37396de --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,13 @@ +version: 2 +updates: + - package-ecosystem: github-actions + directory: / + schedule: + interval: monthly + - package-ecosystem: npm + directory: / + schedule: + interval: monthly + groups: + dev: + dependency-type: development diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..43ad0d6 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,33 @@ +name: CI + +on: + push: + branches: [main] + pull_request: + +permissions: + contents: read + +jobs: + test: + runs-on: ubuntu-latest + strategy: + matrix: + # The lowest version `engines` allows, the active LTS, and the newest line. + node: ["22.13.0", "24", "26"] + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: pnpm/action-setup@ea17c68df8912ef543352723c149a84f56e3d413 # v6.1.0 + - uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7.1.0 + with: + node-version: ${{ matrix.node }} + cache: pnpm + - run: pnpm install --frozen-lockfile + - run: pnpm lint + - run: pnpm typecheck + - run: pnpm test + - run: pnpm build + # The package must keep loading from CommonJS. + - run: node -e "require('./dist/index.js')" diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml new file mode 100644 index 0000000..ab649fd --- /dev/null +++ b/.github/workflows/publish.yml @@ -0,0 +1,51 @@ +name: Publish + +# Run by hand: `gh workflow run publish.yml -f dist-tag=alpha`. npm trusts +# this workflow through trusted publishing (OIDC), so no npm token is stored, +# and the package gets a provenance attestation linking it to this repository +# and commit. It only stages the release: nothing is public until a maintainer +# runs `npm stage approve ` with 2FA. +on: + workflow_dispatch: + inputs: + dist-tag: + description: npm dist-tag + required: true + type: choice + options: [alpha, latest] + +permissions: + contents: read + +jobs: + publish: + if: github.ref == 'refs/heads/main' + runs-on: ubuntu-latest + environment: npm + permissions: + contents: read + id-token: write + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: pnpm/action-setup@ea17c68df8912ef543352723c149a84f56e3d413 # v6.1.0 + - uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7.1.0 + with: + # The newest Node 24, for its npm: `npm stage` needs npm 11.15.0+ (trusted + # publishing alone needs 11.5.1+). No registry-url: trusted publishing needs no .npmrc. + node-version: "24" + check-latest: true + - run: pnpm install --frozen-lockfile + - name: Check npm supports staged publishing + run: | + npm --version + # Compares versions part by part as numbers, so 11.9 sorts before 11.15. + node -e 'if (process.argv[1].localeCompare("11.15.0", "en", { numeric: true }) < 0) { + console.error(`npm ${process.argv[1]} has no "npm stage"; it needs 11.15.0 or newer.`); + process.exit(1); + }' "$(npm --version)" + # prepublishOnly runs lint and tests; prepack builds. Prints the stage id. + - run: npm stage publish --tag "$DIST_TAG" + env: + DIST_TAG: ${{ inputs.dist-tag }} diff --git a/.github/workflows/scorecard.yml b/.github/workflows/scorecard.yml new file mode 100644 index 0000000..3cbd8d9 --- /dev/null +++ b/.github/workflows/scorecard.yml @@ -0,0 +1,35 @@ +name: Scorecard + +on: + push: + branches: [main] + schedule: + # Mondays 16:00 UTC: 10:00 CST (11:00 during daylight time; cron is UTC only). + - cron: "0 16 * * 1" + workflow_dispatch: + +permissions: read-all + +jobs: + analysis: + runs-on: ubuntu-latest + permissions: + security-events: write + id-token: write + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: ossf/scorecard-action@fa7117db83c52d787ae5a89f350999701505d78e # v2.5.0 + with: + results_file: results.sarif + results_format: sarif + publish_results: true + - uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 + with: + name: scorecard + path: results.sarif + retention-days: 5 + - uses: github/codeql-action/upload-sarif@24c54180a607b1449ed407dd24f251e4e9147c8d # v4.38.3 + with: + sarif_file: results.sarif diff --git a/.prettierrc b/.prettierrc index 0bf6e04..74856ee 100644 --- a/.prettierrc +++ b/.prettierrc @@ -4,5 +4,6 @@ "trailingComma": "all", "printWidth": 100, "tabWidth": 2, - "endOfLine": "lf" + "endOfLine": "lf", + "proseWrap": "never" } diff --git a/CHANGELOG.md b/CHANGELOG.md index e19e89a..e8f32ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,19 +1,26 @@ # Changelog +## 0.2.0-alpha.1 + +- CLI: `slotdiff `, where each side is a contract in your build info or an address (a proxy, read as what it runs now, or an implementation; needs `--rpc` and `ETHERSCAN_API_KEY`). Checks the storage layout, the ERC-7201 slots and OpenZeppelin's safety rules for ``, with the proxy kind taken from ``'s implementation, so a UUPS implementation missing `upgradeToAndCall` fails. Exits 0 when every check passed, 1 when one failed, 2 when something could not be checked. `ETHERSCAN_API_KEY` is sent only to Etherscan, never to `--explorer-url`. +- `findBuildInfo`: reads Foundry, Hardhat 2 and Hardhat 3 build info. The build is chosen by content: only one whose target and imports still match the files on disk, so the stale files Foundry keeps after each edit are never used. Its `input` holds only what the target imports. +- ERC-7201 namespaces are compared by the slot the code really uses, not only the annotation: a namespace whose slot moves fails, whatever the annotations say, and a corrected annotation over an unchanged slot passes (`SourceAnalysis.namespaces`, `checkNamespaces`). +- `analyzeSource`: `layoutFromSource` plus OpenZeppelin's safety check from one compile, as the CLI runs it; `safety({ kind })` takes the proxy kind being upgraded. +- Explorer errors no longer repeat fetch's error text, which can quote the request URL and with it the API key. +- CLI colors and a banner on an interactive terminal only (on runs, `--help` and `--version`); in a pipe or CI the output stays plain and `--version` prints just the version. `NO_COLOR` and `FORCE_COLOR` are honored. Built on Node's `util.styleText`, so no new dependency. +- **Breaking:** Node 22.13+ (was 22.12+), for `util.styleText`'s terminal detection. +- Releases are published from GitHub Actions through npm trusted publishing, staged for a maintainer's 2FA approval, with a provenance attestation linking each version to its commit. +- `SECURITY.md`: how to report a vulnerability privately, and what slotdiff touches (network, processes, files, environment). The README and `--help` now say plainly that a pass is no guarantee and that slotdiff comes without warranty. + ## 0.1.0-alpha.1 First release. -- `resolveProxyLayout` / `resolveLayout`: the storage layout of the code a proxy (ERC-1967 or - beacon) runs now, rebuilt from verified standard-json source on any Etherscan-compatible explorer - and proven against the deployed code. +- `resolveProxyLayout` / `resolveLayout`: the storage layout of the code a proxy (ERC-1967 or beacon) runs now, rebuilt from verified standard-json source on any Etherscan-compatible explorer and proven against the deployed code. - `proveLocalBuild`: a layout record from your own build, only when it is the deployed build. - `diffLayouts`: OpenZeppelin's storage comparison, including ERC-7201 namespaced storage. -- `LayoutRecord`s are plain JSON that slotdiff never writes. Load them back with `verifyRecord` - (checked against the chain id and deployed code) or `parseRecord` (offline). -- A match proves a layout only when the build embeds solc's metadata hash. Builds with - `bytecodeHash: "none"` or `appendCBOR: false` are reported as `code-only` and refused. -- `getSolc`: native solc from a cache (Hardhat's included) or a checksum-verified download. - Accepts release versions such as `0.8.24`, as Foundry's build info records them. +- `LayoutRecord`s are plain JSON that slotdiff never writes. Load them back with `verifyRecord` (checked against the chain id and deployed code) or `parseRecord` (offline). +- A match proves a layout only when the build embeds solc's metadata hash. Builds with `bytecodeHash: "none"` or `appendCBOR: false` are reported as `code-only` and refused. +- `getSolc`: native solc from a cache (Hardhat's included) or a checksum-verified download. Accepts release versions such as `0.8.24`, as Foundry's build info records them. - `layoutFromSource` accepts build-info input from Foundry and Hardhat. - Loads from ESM and CommonJS. Node 22.12+. diff --git a/README.md b/README.md index 4d89790..95e87a8 100644 --- a/README.md +++ b/README.md @@ -1,30 +1,39 @@ # slotdiff -Storage layout diff for upgradeable Solidity contracts, where either side can be a local build or -the code a proxy actually runs on chain. +[![npm](https://img.shields.io/npm/v/slotdiff)](https://www.npmjs.com/package/slotdiff) [![CI](https://github.com/rokso/slotdiff/actions/workflows/ci.yml/badge.svg)](https://github.com/rokso/slotdiff/actions/workflows/ci.yml) [![OpenSSF Scorecard](https://api.scorecard.dev/projects/github.com/rokso/slotdiff/badge)](https://scorecard.dev/viewer/?uri=github.com/rokso/slotdiff) [![License: MIT](https://img.shields.io/npm/l/slotdiff)](LICENSE) + +Storage layout diff for upgradeable Solidity contracts, where either side can be a local build or the code a proxy actually runs on chain. + +```sh +npx slotdiff 0xProxy src/Vault.sol:Vault --rpc $RPC_URL # what the proxy runs vs your build +``` + +For example, two local versions: -```ts -const live = await resolveProxyLayout(provider, proxy, { explorer: { apiKey } }); -const report = diffLayouts( - live.record.layout, - await layoutFromSource(input, solc, "src/Vault.sol:Vault"), -); -if (!report.ok) console.log(report.explain()); +``` +$ npx slotdiff src/Vault.sol:Vault src/VaultBad.sol:VaultBad +old src/Vault.sol:Vault + out/build-info/90f317b6c0a81633.json +new src/VaultBad.sol:VaultBad + out/build-info/90f317b6c0a81633.json + +✗ Storage layout incompatible + src/VaultBad.sol:4: Inserted `inserted` + > New variables should be placed after all existing inherited variables +✓ Safety rules no problems found (transparent proxy) + +Not safe to upgrade: the storage layout is incompatible. ``` ## Why -An upgrade is only safe relative to what the proxy runs **now**. Under lazy or multisig-queued -upgrades that is often not your latest build, and the tools you have compare the wrong thing: +An upgrade is only safe relative to what the proxy runs **now**. Under lazy or multisig-queued upgrades that is often not your latest build, and the tools you have compare the wrong thing: - OpenZeppelin's `validate` compares two local builds. It has no way to know what is live. - `cast storage
` prints a live layout but does not compare it with anything. - Deployment artifacts describe the last build you deployed, not necessarily what the proxy runs. -slotdiff reads the implementation behind the proxy, rebuilds its layout from verified source, and -**proves** that source compiles to the deployed code before trusting it. It then compares with -OpenZeppelin's own rules, including ERC-7201 namespaced storage. Framework-agnostic: Foundry, -Hardhat 2 and 3, or a plain script. +slotdiff reads the implementation behind the proxy, rebuilds its layout from verified source, and **proves** that source compiles to the deployed code before trusting it. It then compares with OpenZeppelin's own rules, including ERC-7201 namespaced storage. Framework-agnostic: Foundry, Hardhat 2 and 3, or a plain script. ## Install @@ -32,16 +41,94 @@ Hardhat 2 and 3, or a plain script. npm install slotdiff ``` -Node 22.12+. Works from ESM and CommonJS. +Node 22.13+. Works from ESM and CommonJS. + +## Command line + +```sh +slotdiff [--rpc ] +``` + +Each side is a contract in your build info (`src/Vault.sol:Vault`, or just `Vault` when only one contract has that name) or an address: a proxy, read as the implementation it runs now, or an implementation, such as one waiting in a multisig queue. Any mix works. An address needs `--rpc` and `ETHERSCAN_API_KEY` (or `--explorer-url` for another explorer), since its source comes from the explorer. The RPC is a flag because it picks the chain for each run; the key lives in the environment because one Etherscan key covers every chain. It is sent only to Etherscan: with `--explorer-url`, put that explorer's own key in the URL's query if it needs one. + +A local side is read from the build info your tool already wrote; slotdiff does not compile your project. Build first, from the project root or with `--root`. Foundry writes build info only with `build_info = true` in `foundry.toml` or `forge build --build-info`. A build older than the sources on disk is refused, never used. `--build-info ` names one build-info file to use as is, such as a CI artifact from another checkout; the output then says it was not compared with the sources on disk. + +Three checks run: + +- **Storage layout**, by OpenZeppelin's rules, as above, with ERC-7201 namespaces matched by the slot the code really uses. +- **ERC-7201 slots**: each namespace keeps its data in place and ``'s annotations match its code (see below). +- **Safety rules** for ``, OpenZeppelin's checks that it can run behind a proxy: constructors, `selfdestruct`, `delegatecall`, initial values, immutables and initializers (an initializer order problem is only a warning, as in OpenZeppelin). Source exceptions (`@custom:oz-upgrades-unsafe-allow`) are honored, and `--unsafe-allow ` accepts more. For an address, the verified source is checked. `--no-safety` skips it. + +OpenZeppelin matches ERC-7201 namespaces by their `@custom:storage-location erc7201:` annotation alone. slotdiff also reads the slot the code assigns (`assembly { $.slot := LOCATION }`, with `LOCATION` a hex constant, the ERC-7201 formula, or `erc7201("id")`) and compares namespaces by that slot. The check fails when `` would read a namespace from another slot than the deployed code wrote it to, whatever the annotations say, and when an annotation in `` disagrees with its code. In ``, already deployed, a disagreement is reported and the namespace is matched by its real slot, so correcting a wrong annotation is not reported as a deleted namespace. A slot slotdiff cannot read (computed at run time, say) means the upgrade could not be checked: exit code 2. + +For example, after correcting an annotation that never matched the deployed code: + +``` +ERC-7201 namespaces + annotation slot from annotation slot in code + old example.vault 0xd192…a100 0x0d90…d100 ✗ annotation does not match code + new example.vault.real 0x0d90…d100 0x0d90…d100 ✓ + Same slot in code on both sides: the storage did not move. Compared as one namespace. + +✓ Storage layout compatible +✓ ERC-7201 slots compared by the slot in code (see above) +✓ Safety rules no problems found (transparent proxy) + +Safe to upgrade. +``` + +Changing the constant to match the wrong annotation instead would move the data: + +``` +ERC-7201 namespaces + annotation slot from annotation slot in code + old example.vault 0xd192…a100 0x0d90…d100 ✗ annotation does not match code + new example.vault 0xd192…a100 0xd192…a100 ✓ + The new code moves this namespace from 0x0d90…d100 to 0xd192…a100: its data would be left behind. Keep the slot the deployed code uses, under an annotation that matches it. + +✓ Storage layout compatible +✗ ERC-7201 slots problems found in (see above) +✓ Safety rules no problems found (transparent proxy) + +Not safe to upgrade: an ERC-7201 namespace fails the slot check. +``` + +The proxy kind comes from ``'s implementation: UUPS when it has `upgradeTo` or `upgradeToAndCall`, beacon for a beacon proxy, transparent otherwise. So a UUPS implementation that lost its upgrade function, which would leave the proxy unable to upgrade again, is caught: + +``` +$ npx slotdiff src/UupsV1.sol:UupsV1 src/UupsV2Lost.sol:UupsV2Lost +old src/UupsV1.sol:UupsV1 + out/build-info/fb5d5362dc531baa.json +new src/UupsV2Lost.sol:UupsV2Lost + out/build-info/fb5d5362dc531baa.json + +✓ Storage layout compatible +✗ Safety rules problems found (uups proxy) + src/UupsV2Lost.sol:3: Implementation is missing a public `upgradeTo(address)` or `upgradeToAndCall(address,bytes)` function + Inherit UUPSUpgradeable to include one or both of these functions in your contract + @openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol + https://zpl.in/upgrades/error-008 + +Not safe to upgrade: breaks the safety rules. +``` + +Exit codes: `0` every check ran and passed (or was skipped with `--no-safety`), `1` a check failed, `2` something could not be checked (never reported as compatible). `slotdiff --help` lists every option. + +Colors and the banner appear only on an interactive terminal, never in a pipe, a log or CI. `NO_COLOR` turns colors off; `FORCE_COLOR` turns them on, for CI logs that render them. ## Usage ### Live proxy vs a local build ```ts -import { readFileSync } from "node:fs"; import { JsonRpcProvider } from "ethers"; -import { diffLayouts, getSolc, layoutFromSource, resolveProxyLayout } from "slotdiff"; +import { + diffLayouts, + findBuildInfo, + getSolc, + layoutFromSource, + resolveProxyLayout, +} from "slotdiff"; // Any object with send(method, params): an ethers JsonRpcProvider or Hardhat's network provider. const provider = new JsonRpcProvider(process.env.RPC_URL); @@ -50,10 +137,10 @@ const apiKey = process.env.ETHERSCAN_API_KEY; // 1. The layout of whatever the proxy runs now, proven against its deployed code. const live = await resolveProxyLayout(provider, "0xProxy", { explorer: { apiKey } }); -// 2. The layout of the new version, from your build info (see below for each tool). -const buildInfo = JSON.parse(readFileSync("out/build-info/.json", "utf8")); -const solc = await getSolc(buildInfo.solcLongVersion); -const next = await layoutFromSource(buildInfo.input, solc, "src/Vault.sol:Vault"); +// 2. The layout of the new version, from your build info (Foundry, Hardhat 2 or 3). +const build = await findBuildInfo(process.cwd(), "src/Vault.sol:Vault"); +const solc = await getSolc(build.solcVersion); +const next = await layoutFromSource(build.input, solc, build.contract); // 3. Compare. const report = diffLayouts(live.record.layout, next); @@ -63,15 +150,15 @@ if (!report.ok) { } ``` -Where the build info lives: +`findBuildInfo` picks the build whose sources still match the files on disk, so the stale build info Foundry keeps after each edit is never used, and its `input` holds only the target and what it imports. Pass `{ file }` to use one file as is (for example a CI artifact). Where the build info lives: -| Tool | Build info | Contract name | -| --------- | ------------------------------------------------------------------------------------------------------ | ----------------------------------- | -| Foundry | `out/build-info/*.json`, only with `build_info = true` in `foundry.toml` or `forge build --build-info` | `src/Vault.sol:Vault` | -| Hardhat 2 | `artifacts/build-info/*.json` | `contracts/Vault.sol:Vault` | -| Hardhat 3 | `artifacts/build-info/.json`; the compiler output is in `.output.json` next to it | `project/contracts/Vault.sol:Vault` | +| Tool | Build info | Contract name | +| --- | --- | --- | +| Foundry | `out/build-info/*.json`, only with `build_info = true` in `foundry.toml` or `forge build --build-info` | `src/Vault.sol:Vault` | +| Hardhat 2 | `artifacts/build-info/*.json` | `contracts/Vault.sol:Vault` | +| Hardhat 3 | `artifacts/build-info/.json`; the compiler output is in `.output.json` next to it | `project/contracts/Vault.sol:Vault` | -Each holds `input` and `solcLongVersion`. Use the source name exactly as it appears in `input.sources`. +With `findBuildInfo`, Hardhat 3's `project/` prefix is optional. `explain()` gives OpenZeppelin's report, for example: @@ -80,14 +167,36 @@ src/Vault.sol:7: Inserted `inserted` > New variables should be placed after all existing inherited variables ``` -The explorer defaults to Etherscan's v2 API, where one key covers every chain it indexes. For another -Etherscan-compatible explorer, such as Blockscout, pass `{ apiUrl }` (a key is optional there). +The explorer defaults to Etherscan's v2 API, where one key covers every chain it indexes. For another Etherscan-compatible explorer, such as Blockscout, pass `{ apiUrl }` (a key is optional there). + +### Safety rules and ERC-7201 slots too + +`analyzeSource` is `layoutFromSource` plus OpenZeppelin's safety check and the slot each ERC-7201 namespace really uses, from one compile, as the command line runs them. Pass the kind of the proxy being upgraded, not the new contract's own: a UUPS implementation that lost its upgrade function would otherwise look transparent and pass. + +```ts +import { analyzeSource, checkNamespaces, diffLayouts, findBuildInfo, getSolc } from "slotdiff"; + +async function analyze(contract: string) { + const build = await findBuildInfo(process.cwd(), contract); + return analyzeSource(build.input, await getSolc(build.solcVersion), build.contract); +} +const old = await analyze("src/Vault.sol:Vault"); +const next = await analyze("src/VaultV2.sol:VaultV2"); + +// Pair ERC-7201 namespaces by the slot the code uses, as the command line does. +const slots = checkNamespaces(old, next); // result: "pass", "fail" or "unread" +const layout = diffLayouts(slots.layout, next.layout); +const safety = next.safety({ kind: old.kind }); // the kind of the proxy being upgraded +if (!layout.ok || slots.result !== "pass" || !safety.ok) { + console.error(layout.explain(), slots.pairs, safety.explain()); +} +``` + +`safety` also takes `unsafeAllow` (error kinds to accept, listed in `SAFETY_ERROR_KINDS`), on top of `@custom:oz-upgrades-unsafe-allow` in the source. ### Keep records, skip the rebuild next time -Rebuilding means fetching source and compiling. A `LayoutRecord` is plain JSON: store it wherever -suits you (committed next to your deployments, a CI cache) and load it back with `verifyRecord`, -which refuses it unless this chain still runs the exact code it was proven against. +Rebuilding means fetching source and compiling. A `LayoutRecord` is plain JSON: store it wherever suits you (committed next to your deployments, a CI cache) and load it back with `verifyRecord`, which refuses it unless this chain still runs the exact code it was proven against. ```ts import { readFileSync, writeFileSync } from "node:fs"; @@ -105,109 +214,91 @@ try { } ``` -Key records by **implementation**, and read the implementation from the proxy every time. A record -for an old implementation still verifies after the proxy is upgraded, because that code is still -deployed at its own address. +Key records by **implementation**, and read the implementation from the proxy every time. A record for an old implementation still verifies after the proxy is upgraded, because that code is still deployed at its own address. -Without an RPC, `parseRecord(value, address)` checks a record's shape and proof offline. It cannot -tell whether the proxy has moved on, so prefer `verifyRecord` whenever you can reach the chain. +Without an RPC, `parseRecord(value, address)` checks a record's shape and proof offline. It cannot tell whether the proxy has moved on, so prefer `verifyRecord` whenever you can reach the chain. -A record is bound to the deployed code, not to its own `layout` field: an edited layout in a saved -record still verifies. Treat saved records as trusted input and review changes to them like code. +A record is bound to the deployed code, not to its own `layout` field: an edited layout in a saved record still verifies. Treat saved records as trusted input and review changes to them like code. ### No explorer: prove your own build -If you deployed it yourself, your build can serve as the source. `proveLocalBuild` compares the -deployed code with your compiler output and returns a record only when they prove to be the same -build. +If you deployed it yourself, your build can serve as the source. `proveLocalBuild` compares the deployed code with your compiler output and returns a record only when they prove to be the same build. ```ts -import { proveLocalBuild } from "slotdiff"; +import { findBuildInfo, getSolc, layoutFromSource, proveLocalBuild } from "slotdiff"; -// Hardhat 3: read `output` from `.output.json` instead. -const compiled = buildInfo.output.contracts["src/Vault.sol"].Vault.evm.deployedBytecode; +const build = await findBuildInfo(process.cwd(), "src/Vault.sol:Vault"); +const solc = await getSolc(build.solcVersion); const { bytecodeMatch, record } = await proveLocalBuild(provider, impl, { - contract: "src/Vault.sol:Vault", + contract: build.contract, compiler: solc.longVersion, - layout: await layoutFromSource(buildInfo.input, solc, "src/Vault.sol:Vault"), - deployedBytecode: compiled.object, - immutableReferences: compiled.immutableReferences, + layout: await layoutFromSource(build.input, solc, build.contract), + deployedBytecode: build.deployedBytecode!, + immutableReferences: build.immutableReferences, }); if (record === undefined) throw new Error(`Not the deployed build (${bytecodeMatch})`); ``` ## What counts as proof -A layout is only trusted when the compiled code matches the deployed code as `exact` or -`immutables-only` (identical once immutables and library addresses are masked), **and** the build -embeds solc's metadata hash. That hash covers every source file and setting, so matching code then -means matching source. +A layout is only trusted when the compiled code matches the deployed code as `exact` or `immutables-only` (identical once immutables and library addresses are masked), **and** the build embeds solc's metadata hash. That hash covers every source file and setting, so matching code then means matching source. -Anything weaker is refused, because variables no code reads, retyped fields and gap sizes never -reach the bytecode, so a different layout can compile to the same code: +Anything weaker is refused, because variables no code reads, retyped fields and gap sizes never reach the bytecode, so a different layout can compile to the same code: -| Match | Meaning | Proof? | -| ----------------- | ----------------------------------------------------------------------- | ------ | -| `exact` | Byte-identical | Yes | -| `immutables-only` | Identical once immutables and library links are masked | Yes | -| `metadata-only` | Identical only after stripping metadata: the source differed | No | -| `code-only` | Identical, but built with `bytecodeHash: "none"` or `appendCBOR: false` | No | -| `none` | Different code | No | +| Match | Meaning | Proof? | +| --- | --- | --- | +| `exact` | Byte-identical | Yes | +| `immutables-only` | Identical once immutables and library links are masked | Yes | +| `metadata-only` | Identical only after stripping metadata: the source differed | No | +| `code-only` | Identical, but built with `bytecodeHash: "none"` or `appendCBOR: false` | No | +| `none` | Different code | No | -Contracts built without a metadata hash (common for deterministic CREATE2 deployments) therefore -cannot be proven. For explorer source, the verified compiler settings are checked as well, so a -build without a hash is refused even if its code happens to end in bytes shaped like one. With -`proveLocalBuild`, the hash is read from your own compiler output. +Contracts built without a metadata hash (common for deterministic CREATE2 deployments) therefore cannot be proven. For explorer source, the verified compiler settings are checked as well, so a build without a hash is refused even if its code happens to end in bytes shaped like one. With `proveLocalBuild`, the hash is read from your own compiler output. ## Errors slotdiff fails closed: it never treats "could not check" as "safe". -- `LayoutUnavailableError`: no answer. Not a proxy, unverified source, explorer outage or rate - limit, no compiler for the platform. Falling back to another source is reasonable. -- `LayoutIntegrityError`: an answer that cannot be trusted. Source that does not compile to the - deployed code, a weak match, a record from another chain or for other code. Never fall back on - this one. +- `LayoutUnavailableError`: no answer. Not a proxy, unverified source, explorer outage or rate limit, no compiler for the platform. Falling back to another source is reasonable. +- `LayoutIntegrityError`: an answer that cannot be trusted. Source that does not compile to the deployed code, a weak match, a record from another chain or for other code. Never fall back on this one. -A few problems that are yours to fix throw a plain `Error`: a rejected explorer API key, a compiler -binary that fails to run (for example the wrong architecture), and a record in a format from a -newer slotdiff. +A few problems that are yours to fix throw a plain `Error`: a rejected explorer API key, a compiler binary that fails to run (for example the wrong architecture), and a record in a format from a newer slotdiff. ## API -| Export | Purpose | -| ------------------------------------------------------------ | ------------------------------------------------------------------------ | -| `diffLayouts(before, after, { unsafeAllowRenames? })` | OpenZeppelin's storage comparison. `ok` and `explain()` | -| `resolveProxyLayout(provider, proxy, options)` | Proven layout of what a proxy (ERC-1967 or beacon) runs now | -| `resolveLayout(provider, address, options)` | Proven layout of the code at an implementation address | -| `proveLocalBuild(provider, address, build)` | A record from your own build, only when it is the deployed build | -| `verifyRecord(provider, address, value)` | Load a saved record, checked against the chain | -| `parseRecord(value, address?)` | Load a saved record, checked offline | -| `layoutFromSource(input, solc, "file.sol:Name")` | Layout from standard-json input, including ERC-7201 namespaces. No proof | -| `getSolc(version)` | Native solc, from a cache or downloaded and checksum-verified | -| `readProxy`, `readImplementation`, `readCode`, `readChainId` | Chain reads | -| `fetchVerifiedSource(chainId, address, explorer)` | Verified standard-json source from an Etherscan-compatible explorer | -| `compareDeployedBytecode`, `isProvingMatch` | The match classification above | - -`getSolc` takes a long (`0.8.24+commit.e11b9ed9`) or release (`0.8.24`) version. Compilers are -cached under your OS cache directory (`slotdiff/compilers`, or `cacheDir`), and Hardhat's compiler -cache is reused when present. - -Renames and retypes follow OpenZeppelin's NatSpec tags, `@custom:oz-renamed-from` and -`@custom:oz-retyped-from`. +| Export | Purpose | +| --- | --- | +| `diffLayouts(before, after, { unsafeAllowRenames? })` | OpenZeppelin's storage comparison. `ok` and `explain()` | +| `resolveProxyLayout(provider, proxy, options)` | Proven layout of what a proxy (ERC-1967 or beacon) runs now | +| `resolveLayout(provider, address, options)` | Proven layout of the code at an implementation address | +| `proveLocalBuild(provider, address, build)` | A record from your own build, only when it is the deployed build | +| `verifyRecord(provider, address, value)` | Load a saved record, checked against the chain | +| `parseRecord(value, address?)` | Load a saved record, checked offline | +| `layoutFromSource(input, solc, "file.sol:Name")` | Layout from standard-json input, including ERC-7201 namespaces. No proof | +| `analyzeSource(input, solc, "file.sol:Name")` | The layout, proxy `kind`, ERC-7201 `namespaces` and `safety({ kind })` | +| `checkNamespaces(old, next)` | ERC-7201 namespaces paired by real slot; a moved one fails | +| `findBuildInfo(root, contract, { file? })` | Your build info for a contract: input, compiler version, deployed code | +| `getSolc(version)` | Native solc, from a cache or downloaded and checksum-verified | +| `readProxy`, `readImplementation`, `readCode`, `readChainId` | Chain reads | +| `fetchVerifiedSource(chainId, address, explorer)` | Verified standard-json source from an Etherscan-compatible explorer | +| `compareDeployedBytecode`, `isProvingMatch` | The match classification above | + +`getSolc` takes a long (`0.8.24+commit.e11b9ed9`) or release (`0.8.24`) version. Compilers are cached under your OS cache directory (`slotdiff/compilers`, or `cacheDir`), and Hardhat's compiler cache is reused when present. + +Renames and retypes follow OpenZeppelin's NatSpec tags, `@custom:oz-renamed-from` and `@custom:oz-retyped-from`. ## Limitations -- Explorer source must be verified as standard-json. Flattened and multi-file verifications do not - record every compiler setting, so they are refused. +- Explorer source must be verified as standard-json. Flattened and multi-file verifications do not record every compiler setting, so they are refused. - Solidity only. -- Struct members cannot be renamed or retyped: Solidity has no NatSpec on struct members, so - OpenZeppelin's tags cannot reach them. -- ERC-7201 namespaces are located by their `@custom:storage-location erc7201:` annotation, as - OpenZeppelin does. The `bytes32` slot constant your code actually uses is not checked against - it, so moving the constant while leaving the annotation unchanged is not detected. Keep the two in - sync, for example by deriving the constant with the ERC-7201 formula in a test. -- Library only for now. A CLI is planned. +- Struct members cannot be renamed or retyped: Solidity has no NatSpec on struct members, so OpenZeppelin's tags cannot reach them. +- An ERC-7201 slot is read only from a `$.slot := ...` assignment whose value is a constant (a hex literal, the ERC-7201 formula, or `erc7201("id")`), directly or through a local variable set once or a function with no arguments that returns one. Any other slot (computed at run time, a library's constant) is not read, and the upgrade is reported as not checked. + +## No guarantee + +slotdiff is provided as is, without warranty of any kind (see [LICENSE](LICENSE)). It checks what its rules and OpenZeppelin's can check. "Safe to upgrade" means those checks found no problem, not that the upgrade is safe in every way. It does not replace review, a test upgrade on a fork, or an audit. The authors accept no liability for any loss, including lost or locked funds, arising from using slotdiff or relying on its results. + +To report a security problem, or to see what slotdiff touches on your machine (network, processes, files, environment), read [SECURITY.md](SECURITY.md). ## License diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 0000000..91f0448 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,27 @@ +# Security policy + +slotdiff decides whether an upgrade is safe, so a wrong "Safe to upgrade" is a security bug. So is anything that leaks an RPC URL or an API key. + +## Reporting a vulnerability + +Report it privately through GitHub: **Security** tab of [rokso/slotdiff](https://github.com/rokso/slotdiff/security/advisories/new) → **Report a vulnerability**. Please do not open a public issue. + +Include what you ran (the command or code, the contracts or their shape) and what slotdiff said against what it should have said. I aim to reply within a week. + +## What slotdiff touches + +For anyone reviewing slotdiff, or the alerts a supply-chain scanner raises about it: + +- **Network:** the explorer (Etherscan's v2 API, or `--explorer-url`) for verified source, the RPC endpoint you pass, and `binaries.soliditylang.org` for compilers, each checked against the published checksum. Nothing else; no telemetry. +- **Processes:** it runs the native solc binary it downloaded, found in Hardhat's cache, or that a library caller names with `solc.resolvePath`. +- **Files:** it reads your build info and sources. It writes only the compiler cache (`slotdiff/compilers` under your OS cache directory, or `cacheDir`). +- **Environment:** `ETHERSCAN_API_KEY` (sent only to Etherscan), `XDG_CACHE_HOME` or `LOCALAPPDATA` for the cache directory, and, through Node's own color detection, `NO_COLOR`, `FORCE_COLOR`, `NODE_DISABLE_COLORS`, `TERM` and `COLORTERM`. +- **Install:** slotdiff has no install scripts, and one dependency, `@openzeppelin/upgrades-core`. Two of its transitive dependencies (`keccak` and `secp256k1`, through `ethereumjs-util`) build optional native code at install (`node-gyp-build || exit 0`). + +## No guarantee + +slotdiff comes without warranty and its authors accept no liability for any loss, including funds; see the README's "No guarantee" section and [LICENSE](LICENSE). Reporting a problem helps everyone, but a report does not make anyone liable for an upgrade. + +## Supported versions + +Fixes go into the latest release only. diff --git a/package.json b/package.json index 37f4e9e..4536941 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "slotdiff", - "version": "0.1.0-alpha.1", + "version": "0.2.0-alpha.1", "description": "Storage layout diff for upgradeable Solidity contracts: compare a local build or a live deployment against another", "license": "MIT", "author": "Rohit Solia (https://github.com/rokso)", @@ -15,6 +15,9 @@ "type": "module", "main": "./dist/index.js", "types": "./dist/index.d.ts", + "bin": { + "slotdiff": "./dist/bin.js" + }, "exports": { ".": { "types": "./dist/index.d.ts", @@ -56,7 +59,7 @@ "vitest": "^4.1.0" }, "engines": { - "node": ">=22.12.0" + "node": ">=22.13.0" }, "keywords": [ "ethereum", @@ -64,7 +67,7 @@ "upgradeable", "proxy", "storage-layout", - "upgrade-safety", + "erc7201", "foundry", "hardhat" ], diff --git a/src/bin.ts b/src/bin.ts new file mode 100644 index 0000000..8ce1283 --- /dev/null +++ b/src/bin.ts @@ -0,0 +1,25 @@ +#!/usr/bin/env node +import { styleText } from "node:util"; +import { run } from "./cli.js"; + +// A reader that stops early, such as `| head`, is not an error. +process.stdout.on("error", (e: NodeJS.ErrnoException) => { + if (e.code !== "EPIPE") throw e; + process.exit(); +}); + +run(process.argv.slice(2), { + stdout: (line) => console.log(line), + stderr: (line) => console.error(line), + tty: process.stdout.isTTY === true, + // Node's own rules for color: a terminal that has them, NO_COLOR, FORCE_COLOR. + color: styleText("red", "x", { stream: process.stdout }) !== "x", +}).then( + (code) => { + process.exitCode = code; + }, + (e: unknown) => { + console.error(e); + process.exitCode = 2; + }, +); diff --git a/src/build-info.ts b/src/build-info.ts new file mode 100644 index 0000000..d879f82 --- /dev/null +++ b/src/build-info.ts @@ -0,0 +1,405 @@ +/** + * Reads a project's own build info: Foundry, Hardhat 2 and Hardhat 3. + * + * The build is chosen by content, never by modification time or file count. + * Foundry keeps every build-info file it ever wrote, so after a few edits + * several hold the target, most of them describing older sources. Only a build + * whose sources still match the files on disk is used. + */ + +import type { SolcInput } from "@openzeppelin/upgrades-core"; +import { readFile, readdir, stat } from "node:fs/promises"; +import { isAbsolute, join, relative, resolve } from "node:path"; +import type { ImmutableReferences } from "./bytecode.js"; + +export interface BuildInfoOptions { + /** + * One build-info file to use as is, relative to the project root. Its + * sources are not compared with the files on disk, so it may come from + * another checkout, such as a CI artifact. + */ + file?: string; +} + +export interface BuildInfoContract { + /** The build-info file it was read from. */ + file: string; + /** + * Fully qualified name as solc saw it, `source.sol:Name`. Hardhat 3 prefixes + * project sources with `project/`. + */ + contract: string; + /** As recorded: a long version, or a release version for Foundry. Both work with `getSolc`. */ + solcVersion: string; + /** + * The compiler input, reduced to the target and what it imports, so that a + * layout compiles only those sources rather than the whole project. + */ + input: SolcInput; + /** Absent when the build info holds no compiler output. */ + deployedBytecode?: string; + immutableReferences?: ImmutableReferences; +} + +// Foundry, then Hardhat. +const DIRS = ["out/build-info", "artifacts/build-info"]; + +const NO_INPUT_HINT = + " Foundry writes the compiler input only with `build_info = true` in foundry.toml or " + + "`forge build --build-info`."; + +interface RawBuildInfo { + _format?: string; + solcLongVersion?: string; + input?: SolcInput & { sources?: Record }; + output?: { + contracts?: Record>; + sources?: Record< + string, + { ast?: { nodes?: Array<{ nodeType?: string; absolutePath?: string }> } } + >; + }; + userSourceNameMap?: Record; + source_id_to_path?: unknown; +} + +interface CompiledContract { + evm?: { deployedBytecode?: { object?: string; immutableReferences?: ImmutableReferences } }; +} + +interface Candidate { + file: string; + info: RawBuildInfo; + contract: string; +} + +/** + * Finds `contract` in the build info under `root` and returns what a layout + * or a bytecode proof needs. `contract` is `path/File.sol:Name`, with the path + * as written in the project (Hardhat 3's `project/` prefix is optional), or a + * bare `Name` when only one contract has it. + */ +export async function findBuildInfo( + root: string, + contract: string, + options: BuildInfoOptions = {}, +): Promise { + const target = parseTarget(contract); + if (target.path !== undefined && isAbsolute(target.path)) { + target.path = relative(root, target.path); + } + + if (options.file !== undefined) { + const file = resolve(root, options.file); + let text: string; + try { + text = await readFile(file, "utf8"); + } catch (e) { + throw new Error(`Cannot read ${options.file} (${(e as NodeJS.ErrnoException).code}).`, { + cause: e, + }); + } + const info = await readBuildInfo(file, text); + if (info === undefined) + throw new Error(`${options.file} holds no compiler input.${NO_INPUT_HINT}`); + const matches = findContract(info, target); + if (matches.length === 0) throw new Error(`${options.file} has no contract ${contract}.`); + if (matches.length > 1) throw ambiguousName(contract, matches); + return toResult({ file, info, contract: matches[0] }); + } + + const files = await listBuildInfoFiles(root); + if (files.length === 0) { + const isDir = await stat(root).then( + (st) => st.isDirectory(), + () => false, + ); + if (!isDir) throw new Error(`The project root ${root} is not a directory.`); + throw new Error( + `No build info found in ${DIRS.join(" or ")} under ${root}. Run from the project root ` + + `(or pass its path as the root), after a build.${NO_INPUT_HINT}`, + ); + } + + const holding: Candidate[] = []; + let withoutInput = false; + for (const file of files) { + // Cheap text check first: build-info files can be large, and most do not hold the target. + const text = await readFile(file, "utf8"); + if (target.path !== undefined && !text.includes(`${target.path}"`)) continue; + const info = await readBuildInfo(file, text); + if (info === undefined) { + withoutInput = true; + continue; + } + for (const name of findContract(info, target)) holding.push({ file, info, contract: name }); + } + if (holding.length === 0) { + throw new Error( + `No build info holds ${contract}. ` + + (withoutInput + ? `Some hold no compiler input.${NO_INPUT_HINT}` + : "Check the name (it is case-sensitive) and give the source path as the compiler " + + "saw it (for a remapped import, the path under lib/), or just the contract name. " + + "Or build first."), + ); + } + + // A build is fresh when the target's file exists and nothing it imports has changed. + const fresh: Candidate[] = []; + const stale: Array<{ candidate: Candidate; target: string | undefined }> = []; + for (const candidate of holding) { + const disk = await checkDisk(root, candidate); + if (disk === "gone") continue; + if (disk.fresh) fresh.push(candidate); + else stale.push({ candidate, target: disk.target }); + } + + // A bare name is ambiguous across every build that holds it, not only the + // fresh ones: an incremental build holds just the files it recompiled, so the + // freshest build can miss a second definition. A stale definition counts only + // while its file still declares the name (it may have been renamed since). + const name = target.name.replace(/\$/g, "\\$"); + const declares = new RegExp(`\\b(?:contract|interface|library)\\s+${name}(?![\\w$])`); + const present = [ + ...fresh, + ...stale + .filter(({ target }) => target === undefined || declares.test(target)) + .map(({ candidate }) => candidate), + ]; + const names = [...new Set(present.map((c) => c.contract))]; + if (names.length > 1) throw ambiguousName(contract, names); + + if (fresh.length === 0) { + throw new Error( + stale.length > 0 + ? `Every build info holding ${contract} is older than the sources on disk. Build again.` + : `${contract} is no longer on disk (moved or deleted). Build again.`, + ); + } + + // Several fresh builds of the same sources are interchangeable only when the + // compiler and its settings agree (Foundry profiles can differ in both). + const byBuild = new Map(); + for (const candidate of fresh.sort((a, b) => a.file.localeCompare(b.file))) { + const key = buildKey(candidate.info); + if (!byBuild.has(key)) byBuild.set(key, candidate); + } + if (byBuild.size > 1) { + const files = [...byBuild.values()].map((c) => relative(root, c.file)); + throw new Error( + `${contract} was built with different compilers or settings in ${files.join(", ")} ` + + `(a project with several solc versions compiles shared files once per version). ` + + `Pick one with the \`file\` option (\`--build-info\` on the command line); ` + + `\`forge clean\` drops leftover builds.`, + ); + } + return toResult([...byBuild.values()][0]); +} + +interface Target { + /** Source path as given, without Hardhat 3's `project/` prefix. */ + path?: string; + name: string; +} + +function parseTarget(contract: string): Target { + const colon = contract.lastIndexOf(":"); + const name = contract.slice(colon + 1); + if (!/^[A-Za-z_$][\w$]*$/.test(name)) { + // Not quoted: a URL passed here by mistake can carry an API key. + throw new Error("Expected a contract name or `path/File.sol:Name`."); + } + if (colon === -1) return { name }; + const path = contract.slice(0, colon).replace(/^\.\//, ""); + return { path: path.replace(/^project\//, ""), name }; +} + +async function listBuildInfoFiles(root: string): Promise { + const files: string[] = []; + for (const dir of DIRS) { + let entries: string[]; + try { + entries = await readdir(resolve(root, dir)); + } catch (e) { + if ((e as NodeJS.ErrnoException).code === "ENOENT") continue; + throw e; + } + for (const entry of entries.sort()) { + // Hardhat 3 keeps the compiler output next to the build info, in `.output.json`. + if (entry.endsWith(".json") && !entry.endsWith(".output.json")) { + files.push(join(resolve(root, dir), entry)); + } + } + } + return files; +} + +/** + * Undefined for a file with no compiler input: Foundry writes one per build + * (only `id` and `source_id_to_path`) unless asked for full build info. + */ +async function readBuildInfo(file: string, text: string): Promise { + let info: RawBuildInfo; + try { + info = JSON.parse(text) as RawBuildInfo; + } catch (e) { + throw new Error(`${file} is not JSON: ${(e as Error).message}`, { cause: e }); + } + if (info?.input === undefined && info?.source_id_to_path !== undefined) return undefined; + if ( + typeof info?.solcLongVersion !== "string" || + typeof info.input?.sources !== "object" || + info.input.sources === null + ) { + throw new Error(`${file} is not a build-info file (no solcLongVersion or input sources).`); + } + if (info.output === undefined) { + const outputFile = file.replace(/\.json$/, ".output.json"); + try { + info.output = (JSON.parse(await readFile(outputFile, "utf8")) as RawBuildInfo).output; + } catch (e) { + if ((e as NodeJS.ErrnoException).code !== "ENOENT") { + throw new Error(`Cannot read ${outputFile}: ${(e as Error).message}`, { cause: e }); + } + } + } + return info; +} + +// Fully qualified names in `info` that match the target. +function findContract(info: RawBuildInfo, target: Target): string[] { + const sources = Object.keys(info.input!.sources); + if (target.path !== undefined) { + const path = target.path; + const source = sources.find((s) => s === path || s === `project/${path}`); + if (source === undefined) return []; + const compiled = info.output?.contracts?.[source]; + // Without output the name cannot be checked here; solc checks it when the layout is built. + if (compiled !== undefined && !Object.hasOwn(compiled, target.name)) return []; + return [`${source}:${target.name}`]; + } + if (info.output?.contracts === undefined) return []; + return Object.entries(info.output.contracts) + .filter(([, contracts]) => Object.hasOwn(contracts, target.name)) + .map(([source]) => `${source}:${target.name}`); +} + +/** + * Compares the target and every source it imports, directly or not, with the + * disk. A changed base contract changes the target's layout; a changed + * unrelated file does not, and must not make a build stale: an incremental + * build holds only the files it recompiled, so the build that holds the target + * is often older than files the target never imports. `gone` when the target's + * file no longer exists; a dependency not found on disk (hoisted node_modules, + * say) is not checked. `target` is the target's text on disk, when found. + */ +async function checkDisk( + root: string, + { info, contract }: Candidate, +): Promise<"gone" | { fresh: boolean; target: string | undefined }> { + const targetSource = sourceOf(contract); + let target: string | undefined; + let fresh = true; + // The target comes first. + for (const source of dependencies(info, targetSource)) { + const onDisk = await readSource(root, info, source); + if (source === targetSource) { + if (onDisk === null) return "gone"; + target = onDisk; + } + const content = info.input!.sources[source]?.content; + if (typeof onDisk === "string" && typeof content === "string" && onDisk !== content) { + fresh = false; + } + } + return { fresh, target }; +} + +/** + * `source` and everything it imports, from the import directives in the + * output's ASTs. Without them, every source in the build (the cautious answer). + */ +function dependencies(info: RawBuildInfo, source: string): string[] { + // The target first, as checkDisk expects. + const all = [source, ...Object.keys(info.input!.sources).filter((s) => s !== source)]; + const asts = info.output?.sources; + const seen = new Set([source]); + const queue = [source]; + while (queue.length > 0) { + const nodes = asts?.[queue.pop()!]?.ast?.nodes; + if (nodes === undefined) return all; + for (const node of nodes) { + const imported = node.nodeType === "ImportDirective" ? node.absolutePath : undefined; + if (imported !== undefined && !seen.has(imported)) { + seen.add(imported); + queue.push(imported); + } + } + } + return [...seen]; +} + +/** + * The file on disk behind a build's source name: undefined when it cannot be + * located by name (Hardhat 3's versioned `npm/` paths), null when it should be + * there and is not. + */ +async function readSource( + root: string, + info: RawBuildInfo, + source: string, +): Promise { + const user = Object.entries(info.userSourceNameMap ?? {}).find(([, s]) => s === source)?.[0]; + if (user === undefined && source.startsWith("npm/")) return undefined; + const names = + user !== undefined + ? [user] + : [source, ...(source.startsWith("project/") ? [source.slice("project/".length)] : [])]; + const paths = names.flatMap((name) => [resolve(root, name), resolve(root, "node_modules", name)]); + return (await readFirst(paths)) ?? null; +} + +async function readFirst(paths: string[]): Promise { + for (const path of paths) { + try { + return await readFile(path, "utf8"); + } catch (e) { + const code = (e as NodeJS.ErrnoException).code; + if (code !== "ENOENT" && code !== "ENOTDIR") throw e; + } + } + return undefined; +} + +function buildKey(info: RawBuildInfo): string { + const settings = { ...(info.input!.settings as Record | undefined) }; + delete settings.outputSelection; + return JSON.stringify([info.solcLongVersion, settings]); +} + +function ambiguousName(contract: string, names: string[]): Error { + return new Error(`${contract} is ambiguous: ${names.join(", ")}. Use \`path/File.sol:Name\`.`); +} + +function sourceOf(contract: string): string { + return contract.slice(0, contract.lastIndexOf(":")); +} + +function toResult({ file, info, contract }: Candidate): BuildInfoContract { + const source = sourceOf(contract); + const deployed = + info.output?.contracts?.[source]?.[contract.slice(source.length + 1)]?.evm?.deployedBytecode; + const sources = info.input!.sources; + return { + file, + contract, + solcVersion: info.solcLongVersion!, + input: { + ...info.input!, + sources: Object.fromEntries(dependencies(info, source).map((s) => [s, sources[s]])), + }, + deployedBytecode: deployed?.object, + immutableReferences: deployed?.immutableReferences, + }; +} diff --git a/src/cli.ts b/src/cli.ts new file mode 100644 index 0000000..16621eb --- /dev/null +++ b/src/cli.ts @@ -0,0 +1,423 @@ +/** + * `slotdiff `: each side is a contract in the project's build info + * or an address: a proxy (read as the implementation it runs now) or an + * implementation. + * + * Exit codes: 0 every check ran and passed (or was skipped with --no-safety), + * 1 a check failed, 2 something could not be checked. Failing to check is + * never reported as compatible. + */ + +import { readFile } from "node:fs/promises"; +import { relative, resolve } from "node:path"; +import { parseArgs, styleText } from "node:util"; +import { findBuildInfo } from "./build-info.js"; +import { readCode, readProxy, type EthProvider } from "./chain.js"; +import { diffLayouts } from "./diff.js"; +import { checkNamespaces } from "./erc7201.js"; +import { checkLines, namespaceLines, plain, type Check, type Paint } from "./report.js"; +import { errorCode, LayoutUnavailableError } from "./errors.js"; +import { canQueryExplorer } from "./explorer.js"; +import { rebuild } from "./resolve.js"; +import { + analyzeSource, + SAFETY_ERROR_KINDS, + type ProxyKind, + type SafetyErrorKind, + type SourceAnalysis, +} from "./reconstruct.js"; +import { getSolc } from "./solc.js"; + +const USAGE = `Usage: slotdiff [options] + +Checks that can replace behind a proxy: OpenZeppelin's storage layout +rules, that each ERC-7201 namespace stays in the slot the code really uses, and +OpenZeppelin's safety rules for (constructors, initializers, selfdestruct...). +Each side is either: + a contract in this project's build info src/Vault.sol:Vault, or Vault when unique + an address a proxy (the implementation it runs now), + or an implementation + +Options: + --rpc JSON-RPC endpoint, required when a side is an address + --root Project root (default: the current directory) + --build-info Read local sides from this build-info file, as is + (not compared with the sources on disk) + --explorer-url Another Etherscan-compatible API, such as Blockscout + --unsafe-allow-renames Accept renames without @custom:oz-renamed-from + --unsafe-allow Comma-separated safety errors to accept, e.g. constructor; + prefer @custom:oz-upgrades-unsafe-allow in the source + --no-safety Skip the safety rules + -h, --help Show this help + -v, --version Show the version + +An address needs verified source: set ETHERSCAN_API_KEY once (one key covers every +chain), or use --explorer-url, with that explorer's key in its query if it needs one +(ETHERSCAN_API_KEY is sent only to Etherscan). --rpc picks the chain for each run. + +Exit codes: 0 every check passed (or was skipped with --no-safety), 1 a check +failed, 2 something could not be checked. + +A pass means these checks found no problem, not that an upgrade is safe in every +way. No warranty; see LICENSE.`; + +const HELP_HINT = "Run slotdiff --help for usage."; + +// "slot" and "diff" in block letters, split where the two words meet. +const BANNER: Array<[string, string]> = [ + [" _ _ ", " _ _ __ __"], + [" ___| | ___ | |_", " __| (_)/ _|/ _|"], + ["/ __| |/ _ \\| __", "/ _` | | |_| |_"], + ["\\__ \\ | (_) | |", "| (_| | | _| _|"], + ["|___/_|\\___/ \\__", "\\__,_|_|_| |_|"], +]; + +const ADDRESS = /^0x[0-9a-fA-F]{40}$/; + +type FetchLike = (url: string, init: RequestInit) => Promise; + +export interface CliIo { + stdout: (line: string) => void; + stderr: (line: string) => void; + cwd?: string; + env?: Record; + fetch?: FetchLike; + /** stdout is an interactive terminal: show the banner. */ + tty?: boolean; + /** stdout takes colors (Node's rules: a terminal, `NO_COLOR`, `FORCE_COLOR`). */ + color?: boolean; +} + +interface Side { + /** Two lines: what it is, then where it came from. Addresses in full, to copy. */ + description: [string, string]; + /** Inferred from the implementation's code, or `beacon` for a beacon proxy. */ + kind: ProxyKind; + analysis: SourceAnalysis; +} + +/** Runs the CLI and resolves to its exit code. */ +export async function run(argv: string[], io: CliIo): Promise { + let parsed; + try { + parsed = parseArgs({ + args: argv, + allowPositionals: true, + options: { + rpc: { type: "string" }, + root: { type: "string" }, + "build-info": { type: "string" }, + "explorer-url": { type: "string" }, + "unsafe-allow-renames": { type: "boolean" }, + "unsafe-allow": { type: "string" }, + "no-safety": { type: "boolean" }, + help: { type: "boolean", short: "h" }, + version: { type: "boolean", short: "v" }, + }, + }); + } catch (e) { + // Node's text for an unknown option runs on about `--`; the option is enough. + const message = (e as Error).message; + const unknown = /^Unknown option '([^']+)'/.exec(message)?.[1]; + const text = unknown !== undefined ? `Unknown option ${unknown}` : message; + io.stderr(`${text.replace(/\.?$/, ".")} ${HELP_HINT}`); + return 2; + } + const { values, positionals } = parsed; + const paint: Paint = io.color + ? (format, text) => styleText(format, text, { validateStream: false }) + : plain; + // Only for a person at a terminal: never in a pipe, a log or CI. + const banner = async () => + [ + ...BANNER.map(([slot, diff]) => paint("blueBright", slot) + paint("magenta", diff)), + ` ${paint("dim", `storage layout diff for upgradeable contracts · ${await packageVersion()}`)}`, + ].join("\n"); + if (values.help) { + io.stdout(io.tty ? `${await banner()}\n\n${USAGE}` : USAGE); + return 0; + } + if (values.version) { + io.stdout(io.tty ? await banner() : await packageVersion()); + return 0; + } + if (positionals.length !== 2) { + io.stderr(`Expected and , got ${positionals.length} argument(s). ${HELP_HINT}`); + return 2; + } + if (positionals.some((p) => p.includes("://"))) { + // Not echoed: an RPC URL usually carries an API key. + io.stderr("A side looks like a URL. Pass the endpoint with --rpc ."); + return 2; + } + const short = positionals.find((p) => /^0x[0-9a-fA-F]*$/.test(p) && !ADDRESS.test(p)); + if (short !== undefined) { + io.stderr(`${short} is not an address: an address is 0x and 40 hex digits.`); + return 2; + } + const hasAddress = positionals.some((p) => ADDRESS.test(p)); + if (values.rpc === undefined && hasAddress) { + io.stderr("An address needs --rpc ."); + return 2; + } + + const safety = !values["no-safety"]; + if (!safety && values["unsafe-allow"] !== undefined) { + io.stderr("--unsafe-allow does nothing with --no-safety."); + return 2; + } + const unsafeAllow = (values["unsafe-allow"]?.split(",") ?? []) + .map((k) => k.trim()) + .filter((k) => k !== "") as SafetyErrorKind[]; + const unknownKind = unsafeAllow.find( + (k) => !(SAFETY_ERROR_KINDS as readonly string[]).includes(k), + ); + if (unknownKind !== undefined) { + io.stderr( + `--unsafe-allow: unknown error kind "${unknownKind}". Known: ${SAFETY_ERROR_KINDS.join(", ")}.`, + ); + return 2; + } + + // Neither URL is ever echoed: endpoints often carry an API key. + let rpcUrl: URL | undefined; + try { + rpcUrl = values.rpc !== undefined ? httpUrl(values.rpc, "--rpc") : undefined; + const explorerUrl = values["explorer-url"]; + if (explorerUrl !== undefined && hasCredentials(httpUrl(explorerUrl, "--explorer-url"))) { + throw new Error("--explorer-url must not include a user name or password."); + } + } catch (e) { + io.stderr((e as Error).message); + return 2; + } + + const cwd = io.cwd ?? process.cwd(); + const root = resolve(cwd, values.root ?? "."); + const buildInfoFile = + values["build-info"] !== undefined ? resolve(cwd, values["build-info"]) : undefined; + const env = io.env ?? process.env; + // The Etherscan key goes to Etherscan only, never to another explorer's URL. + const explorer = + values["explorer-url"] !== undefined + ? { apiUrl: values["explorer-url"] } + : { apiKey: env.ETHERSCAN_API_KEY }; + const provider = rpcUrl !== undefined ? jsonRpcProvider(rpcUrl, io.fetch ?? fetch) : undefined; + + if (io.tty) io.stdout(`${await banner()}\n`); + + const readSide = async (spec: string): Promise => { + if (ADDRESS.test(spec)) { + // A proxy stands for what it runs now; any other address is an implementation. + const proxy = await readProxy(provider!, spec); + const implementation = proxy?.implementation ?? spec.toLowerCase(); + if (!canQueryExplorer(explorer)) { + await readCode(provider!, implementation); // a wrong address is reported as such first + throw new Error( + `${implementation} needs an explorer key to fetch its verified source: set ` + + "ETHERSCAN_API_KEY (or use --explorer-url for another explorer).", + ); + } + const { record, analysis } = await rebuild(provider!, implementation, { explorer }); + return { + description: [ + proxy === undefined + ? `${implementation} (implementation)` + : `${spec.toLowerCase()} → implementation ${implementation}` + + (proxy.beacon !== undefined ? ` (via beacon ${proxy.beacon})` : ""), + `${record.contract} (verified source, ${record.bytecodeMatch} match)`, + ], + kind: proxy?.beacon !== undefined ? "beacon" : analysis.kind, + analysis, + }; + } + const build = await findBuildInfo(root, spec, { file: buildInfoFile }); + const solc = await getSolc(build.solcVersion); + const analysis = await analyzeSource(build.input, solc, build.contract); + return { + description: [ + build.contract, + relative(root, build.file) + + (buildInfoFile !== undefined ? " (as given, not compared with the sources on disk)" : ""), + ], + kind: analysis.kind, + analysis, + }; + }; + + let old: Side; + let next: Side; + try { + old = await readSide(positionals[0]); + // The same side twice (e.g. a proxy against itself) is fetched and compiled once. + next = positionals[1] === positionals[0] ? old : await readSide(positionals[1]); + } catch (e) { + io.stderr(`Could not check: ${(e as Error).message}`); + return 2; + } + + for (const [label, side] of [ + ["old", old], + ["new", next], + ] as const) { + io.stdout( + `${paint("bold", label)} ${side.description[0]}\n ${paint("dim", side.description[1])}`, + ); + } + + // OpenZeppelin matches namespaces by annotation; slotdiff pairs them by the + // slot the code uses, and OpenZeppelin then compares the structs of each pair. + const namespaces = checkNamespaces(old.analysis, next.analysis); + const grid = namespaceLines(namespaces.pairs, paint); + if (grid.length > 0) io.stdout(`\n${grid.join("\n")}`); + + const report = diffLayouts(namespaces.layout, next.analysis.layout, { + unsafeAllowRenames: values["unsafe-allow-renames"], + }); + const renames = values["unsafe-allow-renames"] ? " (renames accepted)" : ""; + const checks: Check[] = [ + report.ok + ? { name: "Storage layout", status: "pass", summary: `compatible${renames}` } + : { + name: "Storage layout", + status: "fail", + summary: `incompatible${renames}`, + details: report.explain(), + }, + ]; + + if (namespaces.pairs.length > 0) { + checks.push({ + name: "ERC-7201 slots", + ...(namespaces.result === "fail" + ? { status: "fail", summary: "problems found in (see above)" } + : namespaces.result === "unread" + ? { status: "skip", summary: "not checked: some slots could not be read (see above)" } + : grid.length > 0 + ? { status: "pass", summary: "compared by the slot in code (see above)" } + : { status: "pass", summary: "match their annotations" }), + }); + } + + // The kind comes from , the proxy being upgraded: inferring it from + // would miss a UUPS implementation that lost its upgrade function. + const accepted = unsafeAllow.length > 0 ? `; accepted: ${unsafeAllow.join(", ")}` : ""; + const safetyCheck = safety ? next.analysis.safety({ kind: old.kind, unsafeAllow }) : undefined; + checks.push( + safetyCheck === undefined + ? { name: "Safety rules", status: "skip", summary: "skipped (--no-safety)" } + : safetyCheck.ok + ? { + name: "Safety rules", + status: "pass", + summary: `no problems found (${old.kind} proxy${accepted})`, + } + : { + name: "Safety rules", + status: "fail", + summary: `problems found (${old.kind} proxy${accepted})`, + details: safetyCheck.explain(), + }, + ); + io.stdout(`\n${checkLines(checks, paint).join("\n")}`); + + // One verdict, so a passing check next to a failing one is never read as safe. + const problems = [ + ...(report.ok ? [] : ["the storage layout is incompatible"]), + ...(namespaces.result === "fail" ? ["an ERC-7201 namespace fails the slot check"] : []), + ...(safetyCheck?.ok === false ? [" breaks the safety rules"] : []), + ]; + if (problems.length > 0) { + io.stdout(`\n${paint(["bold", "red"], "Not safe to upgrade:")} ${problems.join(" and ")}.`); + return 1; + } + if (namespaces.result === "unread") { + io.stdout( + `\n${paint(["bold", "yellow"], "Could not check:")} some ERC-7201 slots could not be read.`, + ); + return 2; + } + io.stdout( + safetyCheck === undefined + ? `\n${paint("yellow", "Storage layout compatible; safety rules not checked (--no-safety).")}` + : `\n${paint(["bold", "green"], "Safe to upgrade.")}`, + ); + return 0; +} + +function httpUrl(value: string, flag: string): URL { + let url: URL; + try { + url = new URL(value); + } catch { + throw new Error(`${flag} is not a valid URL; it needs a scheme, such as https://.`); + } + if (url.protocol !== "http:" && url.protocol !== "https:") { + throw new Error(`${flag} must be an http or https URL.`); + } + return url; +} + +function hasCredentials(url: URL): boolean { + return url.username !== "" || url.password !== ""; +} + +/** + * A minimal JSON-RPC client. Errors never include the URL: endpoints often + * carry an API key in the path or query. Credentials in the URL, which fetch + * refuses, are sent as basic auth instead. + */ +function jsonRpcProvider(url: URL, fetchImpl: FetchLike): EthProvider { + const headers: Record = { "content-type": "application/json" }; + if (hasCredentials(url)) { + const user = `${decodeURIComponent(url.username)}:${decodeURIComponent(url.password)}`; + headers.authorization = `Basic ${Buffer.from(user).toString("base64")}`; + url = new URL(url); + url.username = ""; + url.password = ""; + } + const endpoint = url.href; + let id = 0; + return { + async send(method, params) { + let response: Response; + try { + response = await fetchImpl(endpoint, { + method: "POST", + headers, + body: JSON.stringify({ jsonrpc: "2.0", id: ++id, method, params }), + }); + } catch (e) { + throw new LayoutUnavailableError(`Cannot reach the RPC endpoint (${errorCode(e)}).`); + } + if (!response.ok) { + throw new LayoutUnavailableError(`The RPC endpoint answered HTTP ${response.status}.`); + } + let body: { result?: unknown; error?: { message?: string } }; + try { + body = (await response.json()) as typeof body; + } catch { + body = {}; + } + if (typeof body !== "object" || body === null || !("result" in body || "error" in body)) { + throw new LayoutUnavailableError(`The RPC endpoint did not answer JSON-RPC.`); + } + if (body.error !== undefined) { + // The server's text, without control characters (terminal escapes) and capped. + const message = String(body.error.message ?? "unknown error") + // eslint-disable-next-line no-control-regex + .replace(/[\u0000-\u001f\u007f-\u009f]/g, " ") + .slice(0, 300); + throw new Error(`RPC ${method} failed: ${message}`); + } + return body.result; + }, + }; +} + +async function packageVersion(): Promise { + const pkg = JSON.parse(await readFile(new URL("../package.json", import.meta.url), "utf8")) as { + version: string; + }; + return pkg.version; +} diff --git a/src/erc7201.ts b/src/erc7201.ts new file mode 100644 index 0000000..02f5ce0 --- /dev/null +++ b/src/erc7201.ts @@ -0,0 +1,412 @@ +/** + * ERC-7201 namespaces by the slot the code really uses. + * + * OpenZeppelin matches namespaces by their `@custom:storage-location + * erc7201:` annotation and never checks the `bytes32` slot the code assigns + * (`assembly { $.slot := LOCATION }`). The two can disagree. A changed + * constant behind an unchanged annotation moves the storage while the layout + * check passes; a corrected annotation over an unchanged constant leaves the + * storage in place while the check reports a deleted namespace. Reading the + * slot from the AST catches the first and fixes the second. + */ + +import type { SolcOutput, StorageLayout } from "@openzeppelin/upgrades-core"; +// Not exported from the package index; the package has no exports map, and +// all exist since 1.46.0, the lowest version slotdiff accepts. +import { getStorageLocationAnnotation } from "@openzeppelin/upgrades-core/dist/storage/namespace.js"; +import { calculateERC7201StorageLocation } from "@openzeppelin/upgrades-core/dist/utils/erc7201.js"; +import { resolveBaseSlot } from "@openzeppelin/upgrades-core/dist/utils/erc7201-builtin.js"; +import { + normalizeUint256Literal, + toBytes32Hex, +} from "@openzeppelin/upgrades-core/dist/utils/integer-literals.js"; + +export interface NamespaceSlot { + /** `erc7201:`, as annotated. */ + namespace: string; + /** The slot the annotation implies. */ + expected: string; + /** The slot the code assigns, when it could be read and is unique. */ + actual?: string; + /** + * `match`: the code uses the annotated slot. `mismatch`: it uses `actual` + * instead. `conflict`: it assigns different slots in different places. + * `unread`: no assignment was found, or one could not be evaluated. + */ + status: "match" | "mismatch" | "conflict" | "unread"; +} + +interface AstNode { + nodeType?: string; + id?: number; + [key: string]: unknown; +} + +/** + * How each of `namespaces` (`erc7201:` keys of `contract`'s layout, + * `source.sol:Name`) is assigned in `output`. Only structs declared in the + * contract or its bases count, as for OpenZeppelin: a sibling contract in the + * same build (a V1 next to its V2, a test copy) may declare the same id. + */ +export function namespaceSlots( + output: SolcOutput, + contract: string, + namespaces: string[], +): NamespaceSlot[] { + const colon = contract.lastIndexOf(":"); + // Most derived first, as solc lists them: the order virtual calls resolve in. + let linearized: number[] = []; + walk(output.sources?.[contract.slice(0, colon)]?.ast, (node) => { + if (node.nodeType === "ContractDefinition" && node.name === contract.slice(colon + 1)) { + linearized = node.linearizedBaseContracts as number[]; + } + }); + const bases = new Set(linearized); + + // Only code the contract can run counts: its bases (minus the functions they + // override), libraries and free functions, not a sibling contract (a test + // harness) in the same files. + const byId = new Map(); + const touched = new Set(); + const overridden = new Set(); + for (const source of Object.values(output.sources ?? {})) { + walk(source.ast as unknown as AstNode, (node) => { + if (typeof node.id === "number") byId.set(node.id, node); + // Any Solidity use of a local after its declaration, so a local is read + // only when nothing but assembly reads it. + if (node.nodeType === "Identifier") touched.add(node.referencedDeclaration as number); + if (node.nodeType === "FunctionDefinition" && bases.has(node.scope as number)) { + for (const base of (node.baseFunctions ?? []) as number[]) overridden.add(base); + } + }); + } + const assemblies: AstNode[] = []; + for (const source of Object.values(output.sources ?? {})) { + walk(source.ast as unknown as AstNode, (node) => { + const sibling = + node.nodeType === "ContractDefinition" && + node.contractKind !== "library" && + !bases.has(node.id as number); + if (sibling || overridden.has(node.id as number)) return false; + if (node.nodeType === "InlineAssembly") assemblies.push(node); + }); + } + const ctx = { byId, linearized, bases, touched }; + + // Struct id → every slot value assigned to a pointer of that struct (undefined: unreadable). + const assigned = new Map>(); + for (const assembly of assemblies) { + const refs = (assembly.externalReferences ?? []) as Array<{ + declaration: number; + src: string; + suffix?: string; + }>; + const declarationAt = (src: unknown) => refs.find((r) => r.src === src)?.declaration ?? -1; + walk(assembly.AST as AstNode, (node) => { + if (node.nodeType !== "YulAssignment") return; + for (const target of node.variableNames as AstNode[]) { + if (typeof target.name !== "string") continue; + if (!target.name.endsWith(".slot")) { + touched.add(declarationAt(target.src)); + continue; + } + const pointer = byId.get(declarationAt(target.src)); + const struct = (pointer?.typeName as AstNode | undefined)?.referencedDeclaration; + if (typeof struct !== "number") continue; + const value = node.value as AstNode; + const slot = + value.nodeType === "YulLiteral" + ? value.kind === "number" + ? normalize(value.value) + : undefined + : resolveDecl(ctx, byId.get(declarationAt(value.src))); + assigned.set(struct, [...(assigned.get(struct) ?? []), slot]); + } + }); + } + + return namespaces.map((namespace) => { + const expected = calculateERC7201StorageLocation(namespace.slice("erc7201:".length)); + const slots = [...byId.values()] + .filter( + (n) => + n.nodeType === "StructDefinition" && + bases.has(n.scope as number) && + getStorageLocationAnnotation(n as never) === namespace, + ) + .flatMap((n) => assigned.get(n.id!) ?? []); + const known = [...new Set(slots.filter((s) => s !== undefined))]; + if (known.length > 1) return { namespace, expected, status: "conflict" }; + if (known.length === 0 || slots.includes(undefined)) { + return { namespace, expected, status: "unread" }; + } + const actual = known[0]; + return actual === expected + ? { namespace, expected, actual, status: "match" } + : { namespace, expected, actual, status: "mismatch" }; + }); +} + +export interface NamespacePair { + old?: NamespaceSlot; + next?: NamespaceSlot; + /** + * Why this pair fails the check: `` assigns several slots (`conflict`), + * leaves the old data behind (`moved`), or keeps the data in place under an + * annotation that does not match (`mismatch`). `unread`: a slot could not be + * read, so the pair could not be checked. + */ + problem?: "conflict" | "moved" | "mismatch" | "unread"; +} + +export interface NamespaceCheck { + /** + * `old`'s layout with each paired namespace under its name in ``, for + * `diffLayouts`: OpenZeppelin then compares the structs of the same storage. + * An unpaired namespace keeps its name, so one that is really gone is still + * reported as deleted. + */ + layout: StorageLayout; + /** Every namespace of either side, old and new paired when they are the same storage. */ + pairs: NamespacePair[]; + /** `fail` when any pair has a problem other than `unread`; `unread` when a slot could not be read. */ + result: "pass" | "fail" | "unread"; +} + +/** + * Checks ERC-7201 namespaces by the slot the code uses, not the annotation + * OpenZeppelin goes by. An old and a new namespace are the same storage when + * the code uses the same slot (under the same annotation first), failing that + * when they share an annotation, and failing that when the new annotation + * names the slot where the old data is. + * Pass `layout` to `diffLayouts` in place of `old.layout`. + */ +export function checkNamespaces( + old: { layout: StorageLayout; namespaces: NamespaceSlot[] }, + next: { namespaces: NamespaceSlot[] }, +): NamespaceCheck { + const unpaired = new Set(old.namespaces); + const pairs: NamespacePair[] = next.namespaces.map((n) => ({ next: n })); + const sameSlot = (o: NamespaceSlot, n: NamespaceSlot) => + o.actual !== undefined && n.actual !== undefined && same(o.actual, n.actual); + const rules: Array<(o: NamespaceSlot, n: NamespaceSlot) => boolean> = [ + (o, n) => sameSlot(o, n) && o.namespace === n.namespace, + sameSlot, + (o, n) => o.namespace === n.namespace, + (o, n) => o.actual !== undefined && same(o.actual, n.expected), + ]; + // Rule by rule over every namespace, so a weaker rule never takes a pair from a stronger one. + for (const rule of rules) { + for (const pair of pairs) { + const o = pair.old ?? [...unpaired].find((o) => rule(o, pair.next!)); + if (o !== undefined && unpaired.delete(o)) pair.old = o; + } + } + for (const o of unpaired) pairs.push({ old: o }); + for (const pair of pairs) pair.problem = problemOf(pair); + + // Renamed all at once, so two namespaces can swap names. + const renames = new Map( + pairs.flatMap((p) => + p.old !== undefined && p.next !== undefined ? [[p.old.namespace, p.next.namespace]] : [], + ), + ); + const oldNames = Object.keys(old.layout.namespaces ?? {}); + const renamed = oldNames.map((name) => renames.get(name) ?? name); + // On a clash (an old namespace that stays keeps the name another would take) + // nothing is renamed: OpenZeppelin then reports the moved one as deleted. + const layout = + new Set(renamed).size < renamed.length || renamed.every((to, i) => to === oldNames[i]) + ? old.layout + : { + ...old.layout, + namespaces: Object.fromEntries( + oldNames.map((name, i) => [renamed[i], old.layout.namespaces![name]]), + ), + }; + + const problems = pairs.map((p) => p.problem); + return { + layout, + pairs, + result: problems.some((p) => p !== undefined && p !== "unread") + ? "fail" + : problems.includes("unread") + ? "unread" + : "pass", + }; +} + +function problemOf({ old, next }: NamespacePair): NamespacePair["problem"] { + if (next?.status === "conflict") return "conflict"; + if (old?.actual !== undefined && next?.actual !== undefined && !same(old.actual, next.actual)) { + return "moved"; + } + if (next?.status === "mismatch") return "mismatch"; + if ((old !== undefined && old.actual === undefined) || next?.status === "unread") { + return "unread"; + } + return undefined; +} + +function same(a: string, b: string): boolean { + return BigInt(a) === BigInt(b); +} + +// Visits every node; a visitor returning false skips the node's children. +function walk(node: unknown, visit: (node: AstNode) => boolean | void): void { + if (Array.isArray(node)) { + for (const child of node) walk(child, visit); + } else if (typeof node === "object" && node !== null) { + if (visit(node as AstNode) === false) return; + for (const child of Object.values(node)) walk(child, visit); + } +} + +interface Context { + byId: Map; + linearized: number[]; + bases: Set; + /** Declarations that Solidity code uses, or assembly assigns. */ + touched: Set; +} + +/** + * The value of a constant, an immutable set where it is declared, or a local + * that only assembly reads. A state variable's initial value is not read: it + * lives in the implementation's storage, not the proxy's. + */ +function resolveDecl(ctx: Context, decl: AstNode | undefined, depth = 0): string | undefined { + if (decl === undefined) return undefined; + if (decl.stateVariable === true) { + if (decl.mutability !== "constant" && decl.mutability !== "immutable") return undefined; + return resolveExpr(ctx, decl.value as AstNode | undefined, depth); + } + if (ctx.touched.has(decl.id as number)) return undefined; + return resolveExpr(ctx, initialValue(ctx.byId, decl) as AstNode | undefined, depth); +} + +/** + * The value of an expression: a reference to a constant or local, a call to a + * no-argument function that returns one (OpenZeppelin's Initializable reads + * its slot from `_initializableStorageSlot()`), the ERC-7201 formula, a + * number literal or `erc7201("id")`. + */ +function resolveExpr(ctx: Context, e: AstNode | undefined, depth: number): string | undefined { + if (e === undefined || depth > 8) return undefined; + if (e.nodeType === "Identifier") { + return resolveDecl(ctx, ctx.byId.get(e.referencedDeclaration as number), depth + 1); + } + if (e.nodeType === "FunctionCall" && e.kind === "functionCall") { + const callee = e.expression as AstNode; + const fn = ctx.byId.get(callee.referencedDeclaration as number); + if (callee.nodeType !== "Identifier" || fn?.nodeType !== "FunctionDefinition") return undefined; + if ((e.arguments as unknown[]).length > 0) return undefined; + return resolveExpr(ctx, returned(override(ctx, fn)), depth + 1); + } + const id = formulaId(e); + if (id !== undefined) return calculateERC7201StorageLocation(id); + // OpenZeppelin would read any other literal, such as `hex"..."`, as a number. + if (e.nodeType === "Literal" && e.kind !== "number") return undefined; + try { + return resolveBaseSlot(e as never); + } catch { + return undefined; + } +} + +/** + * The implementation an internal call to `fn` reaches: the most derived + * override of a virtual function in the bases, or `fn` itself (a free or + * library function, which nothing overrides). + */ +function override(ctx: Context, fn: AstNode): AstNode { + if (fn.virtual !== true || !ctx.bases.has(fn.scope as number)) return fn; + for (const contract of ctx.linearized) { + for (const node of ctx.byId.values()) { + if ( + node.nodeType === "FunctionDefinition" && + node.scope === contract && + node.name === fn.name && + (node.parameters as { parameters: unknown[] }).parameters.length === 0 && + node.body + ) { + return node; + } + } + } + return fn; +} + +// The expression of a function whose body is a single `return ;` +// and that has no modifier, which could skip it. +function returned(fn: AstNode): AstNode | undefined { + if ((fn.modifiers as unknown[] | undefined)?.length) return undefined; + const statements = (fn.body as { statements?: AstNode[] } | null)?.statements; + return statements?.length === 1 && statements[0].nodeType === "Return" + ? (statements[0].expression as AstNode) + : undefined; +} + +// A local variable's value is on the statement that declares it. +function initialValue(byId: Map, decl: AstNode): unknown { + for (const node of byId.values()) { + if (node.nodeType !== "VariableDeclarationStatement") continue; + const declarations = node.declarations as Array; + if (declarations.length === 1 && declarations[0]?.id === decl.id) return node.initialValue; + } + return undefined; +} + +/** + * The id in the ERC-7201 formula written out in Solidity, + * `keccak256(abi.encode(uint256(keccak256("id")) - 1)) & ~bytes32(uint256(0xff))`, + * with an optional `bytes("id")`. Anything else is not recognized. + */ +function formulaId(e: AstNode): string | undefined { + if (e.nodeType !== "BinaryOperation" || e.operator !== "&") return undefined; + const mask = e.rightExpression as AstNode; + const masked = arg( + arg( + mask.nodeType === "UnaryOperation" ? (mask.subExpression as AstNode) : undefined, + "bytes32", + ), + "uint256", + ); + if (masked?.nodeType !== "Literal" || normalize(masked.value) !== toBytes32Hex(0xffn)) { + return undefined; + } + const minus = arg(arg(e.leftExpression as AstNode, "keccak256"), "abi.encode"); + if (minus?.nodeType !== "BinaryOperation" || minus.operator !== "-") return undefined; + const one = minus.rightExpression as AstNode; + if (one.nodeType !== "Literal" || one.value !== "1") return undefined; + let id = arg(arg(minus.leftExpression as AstNode, "uint256"), "keccak256"); + if (id?.nodeType === "FunctionCall") id = arg(id, "bytes"); + return id?.nodeType === "Literal" && id.kind === "string" ? (id.value as string) : undefined; +} + +// The single argument of a call to `name` (a function, `abi.encode`, or a type conversion). +function arg(call: AstNode | undefined, name: string): AstNode | undefined { + if (call?.nodeType !== "FunctionCall") return undefined; + const callee = call.expression as AstNode; + const calleeName = + callee.nodeType === "Identifier" + ? callee.name + : callee.nodeType === "MemberAccess" + ? `${(callee.expression as AstNode).name}.${callee.memberName}` + : callee.nodeType === "ElementaryTypeNameExpression" + ? (callee.typeName as AstNode).name + : undefined; + const args = call.arguments as AstNode[]; + return calleeName === name && args.length === 1 ? args[0] : undefined; +} + +// A number literal as a slot, or undefined when it is not one. +function normalize(value: unknown): string | undefined { + if (typeof value !== "string") return undefined; + try { + return normalizeUint256Literal(value); + } catch { + return undefined; + } +} diff --git a/src/errors.ts b/src/errors.ts index c2ab1c0..4475bde 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -22,3 +22,12 @@ export class LayoutIntegrityError extends Error { this.name = "LayoutIntegrityError"; } } + +/** + * A network error's code (ECONNREFUSED, ENOTFOUND), never its message: fetch + * quotes the URL in some messages, and URLs carry API keys. + */ +export function errorCode(e: unknown): string { + const code = (e as { cause?: { code?: unknown } }).cause?.code; + return typeof code === "string" ? code : "network error"; +} diff --git a/src/explorer.ts b/src/explorer.ts index 13db0f6..641d9ee 100644 --- a/src/explorer.ts +++ b/src/explorer.ts @@ -8,7 +8,7 @@ */ import type { SolcInput } from "@openzeppelin/upgrades-core"; -import { LayoutUnavailableError } from "./errors.js"; +import { errorCode, LayoutUnavailableError } from "./errors.js"; export const ETHERSCAN_V2_API_URL = "https://api.etherscan.io/v2/api"; @@ -87,7 +87,7 @@ async function requestWithRetry(url: string, fetchImpl: FetchLike): Promise { + return (await analyzeSource(input, solc, contract)).layout; +} + +export type ProxyKind = ProxyDeployment["kind"]; + +/** + * OpenZeppelin's safety error kinds, without the deprecated struct-definition + * and enum-definition, which the check never reports. + */ +export type SafetyErrorKind = Exclude< + ValidationError["kind"], + "struct-definition" | "enum-definition" +>; + +export const SAFETY_ERROR_KINDS: readonly SafetyErrorKind[] = errorKinds.filter( + (k): k is SafetyErrorKind => k !== "struct-definition" && k !== "enum-definition", +); + +export interface SafetyError { + /** OpenZeppelin's error kind, such as `constructor`. */ + kind: SafetyErrorKind; + /** Where, as `file.sol:line`. */ + src: string; +} + +export interface SafetyOptions { + /** + * The proxy being upgraded; only `uups` requires an `upgradeToAndCall`. + * Required: the new contract's own `kind` would let a UUPS implementation + * that lost its upgrade function pass. + */ + kind: ProxyKind; + /** Error kinds to accept anyway (OZ's `unsafeAllow`). Prefer NatSpec exceptions in the source. */ + unsafeAllow?: SafetyErrorKind[]; +} + +export interface SafetyReport { + ok: boolean; + errors: SafetyError[]; + /** OpenZeppelin's description of each error. */ + explain(): string; +} + +export interface SourceAnalysis { + layout: StorageLayout; + /** `uups` when the contract has `upgradeTo` or `upgradeToAndCall`, else `transparent`. */ + kind: Exclude; + /** + * The slot each ERC-7201 namespace really uses, read from the code + * (`$.slot := LOCATION`); OpenZeppelin trusts the annotation alone. + */ + namespaces: NamespaceSlot[]; + /** + * OpenZeppelin's safety check (constructors, `selfdestruct`, + * `delegatecall`, initial values, immutables, initializers), honoring + * `@custom:oz-upgrades-unsafe-allow` in the source. + */ + safety(options: SafetyOptions): SafetyReport; +} + +/** `layoutFromSource` plus OpenZeppelin's safety check, from one compile. */ +export async function analyzeSource( + input: SolcInput, + solc: SolcRunner, + contract: string, +): Promise { const selected = withOutputSelection(input, FULL_SELECTION); - const output = await compileOrThrow(solc, selected, "source"); - return extractLayout(selected, output, solc, contract); + return analyze(selected, await compileOrThrow(solc, selected, "source"), solc, contract); } -async function extractLayout( +async function analyze( input: SolcInput, output: SolcOutput, solc: SolcRunner, contract: string, -): Promise { +): Promise { + const { data, version } = await validateContract(input, output, solc, contract); + const layout = getStorageLayout(data, version); + return { + layout, + // Typed as any ProxyKind, but only ever uups or transparent. + kind: inferProxyKind(data, version) as SourceAnalysis["kind"], + namespaces: namespaceSlots(output, contract, Object.keys(layout.namespaces ?? {})), + safety({ kind, unsafeAllow = [] }) { + // A copy: OpenZeppelin appends to the array it is given. + const opts = { kind, unsafeAllow: [...unsafeAllow] }; + // The deprecated kinds are never reported, so every error has a SafetyErrorKind. + const errors = getErrors(data, version, opts) as Array; + return { + ok: errors.length === 0, + errors, + explain: () => new UpgradeableContractErrorReport(errors).explain(), + }; + }, + }; +} + +async function validateContract( + input: SolcInput, + output: SolcOutput, + solc: SolcRunner, + contract: string, +): Promise<{ data: ValidationRunData; version: Version }> { const solcVersion = solc.longVersion.split("+")[0]; let namespacedOutput: SolcOutput | undefined; @@ -116,13 +216,14 @@ async function extractLayout( const version = runData[contract]?.version; if (version === undefined) { throw new LayoutIntegrityError( - `${contract} compiled to no bytecode; cannot derive its layout.`, + `${contract} compiled to no bytecode (an abstract contract or interface?); ` + + `cannot derive its layout.`, ); } // OZ looks the contract up by bytecode version and takes the first match, so // two contracts with identical code (possible without a source hash) could - // swap layouts. Listing the target first makes the lookup land on it. - return getStorageLayout({ [contract]: runData[contract], ...runData }, version); + // swap layouts or safety errors. Listing the target first makes every lookup land on it. + return { data: { [contract]: runData[contract], ...runData }, version }; } /** diff --git a/src/report.ts b/src/report.ts new file mode 100644 index 0000000..aac9493 --- /dev/null +++ b/src/report.ts @@ -0,0 +1,155 @@ +/** + * The CLI's report: the ERC-7201 namespace grid and the check list. Pure + * functions from results to lines, so the layout is tested as text. Colors + * come from `paint`, applied after alignment so they never shift a column. + */ + +import type { NamespacePair, NamespaceSlot } from "./erc7201.js"; + +/** Node's `util.styleText`, or `plain` when colors are off. */ +export type Paint = ( + format: Parameters[0], + text: string, +) => string; + +export const plain: Paint = (_, text) => text; + +export type CheckStatus = "pass" | "fail" | "skip"; + +export interface Check { + name: string; + status: CheckStatus; + summary: string; + /** Multi-line explanation shown under the check, such as OpenZeppelin's report. */ + details?: string; +} + +const MARK: Record = { pass: "✓", fail: "✗", skip: "-" }; +const TONE = { pass: "green", fail: "red", skip: "yellow" } as const; + +/** One line per check, names aligned, details indented under their check. */ +export function checkLines(checks: Check[], paint: Paint = plain): string[] { + const width = Math.max(...checks.map((c) => c.name.length)); + return checks.flatMap((c) => [ + `${paint(TONE[c.status], MARK[c.status])} ${paint("bold", c.name.padEnd(width))} ${c.summary}`, + ...(c.details?.split("\n").map((line) => ` ${line}`) ?? []), + ]); +} + +/** + * A grid for each namespace whose slot is in question on either side: the + * annotation, the slot it implies and the slot the code uses, for old and new, + * with a sentence saying what that means. Namespaces that line up are left out. + */ +export function namespaceLines(pairs: NamespacePair[], paint: Paint = plain): string[] { + const blocks = pairs + // A side that is absent is not a problem here; OpenZeppelin reports a deleted namespace. + .filter( + (p) => + p.problem !== undefined || + [p.old, p.next].some((s) => s !== undefined && s.status !== "match"), + ) + .map((p) => { + const fmt = formatter([p.old, p.next]); + return { + rows: [ + ...(p.old !== undefined ? [row("old", p.old, fmt)] : []), + ...(p.next !== undefined ? [row("new", p.next, fmt)] : []), + ], + meaning: meaning(p, fmt), + tone: (p.problem === undefined + ? "dim" + : p.problem === "unread" + ? "yellow" + : "red") as Row["tone"], + }; + }); + if (blocks.length === 0) return []; + + const header = { + side: "", + id: "annotation", + implied: "slot from annotation", + inCode: "slot in code", + mark: "", + tone: "dim", + } as Row; + const all = [header, ...blocks.flatMap((b) => b.rows)]; + const width = (key: "id" | "implied" | "inCode") => Math.max(...all.map((r) => r[key].length)); + const cells = (r: Row) => + ` ${r.side.padEnd(5)}${r.id.padEnd(width("id"))} ${r.implied.padEnd(width("implied"))} ` + + r.inCode.padEnd(width("inCode")); + const line = (r: Row) => `${cells(r)} ${paint(r.tone, r.mark)}`; + + return [ + paint("bold", "ERC-7201 namespaces"), + paint("dim", cells(header).trimEnd()), + ...blocks.flatMap((b) => [...b.rows.map(line), ` ${paint(b.tone, b.meaning)}`]), + ]; +} + +interface Row { + side: string; + id: string; + implied: string; + inCode: string; + mark: string; + tone: "green" | "red" | "yellow" | "dim"; +} + +function row(side: string, s: NamespaceSlot, fmt: (slot: string) => string): Row { + return { + side, + id: s.namespace.slice("erc7201:".length), + implied: fmt(s.expected), + inCode: + s.status === "conflict" ? "several" : s.actual !== undefined ? fmt(s.actual) : "not read", + mark: { + match: "✓", + mismatch: "✗ annotation does not match code", + conflict: "✗ code uses more than one slot", + unread: "? slot in code not read", + }[s.status], + tone: ({ match: "green", mismatch: "red", conflict: "red", unread: "yellow" } as const)[ + s.status + ], + }; +} + +function meaning({ old, next, problem }: NamespacePair, fmt: (slot: string) => string): string { + switch (problem) { + case "conflict": + return "The new code assigns more than one slot to this namespace; it must use one."; + case "moved": + return ( + `The new code moves this namespace from ${fmt(old!.actual!)} to ${fmt(next!.actual!)}: ` + + "its data would be left behind. Keep the slot the deployed code uses, under an annotation that matches it." + ); + case "mismatch": + return old === undefined + ? "The new code keeps this namespace in a different slot than its annotation says. Fix one of the two before deploying." + : "The storage did not move, but the new annotation does not match its code. Fix it before deploying."; + case "unread": + return "The slot in code could not be read (computed at run time?), so this namespace could not be checked."; + } + return next !== undefined + ? "Same slot in code on both sides: the storage did not move. Compared as one namespace." + : "The deployed annotation does not match its code, and no namespace in the new code uses its slot."; +} + +/** + * Short slots for a block, unless two different slots in it would then look + * the same: a hand-written constant can share both ends with another. + */ +function formatter(sides: Array): (slot: string) => string { + const slots = new Set( + sides.flatMap((s) => (s === undefined ? [] : [s.expected, s.actual ?? s.expected])), + ); + const shorts = new Set([...slots].map(short)); + return shorts.size === slots.size ? short : (slot) => slot; +} + +/** `0x5ea2…5c00`: enough to tell slots apart; slotdiff does the comparing. */ +function short(slot: string): string { + return `${slot.slice(0, 6)}…${slot.slice(-4)}`; +} diff --git a/src/resolve.ts b/src/resolve.ts index be3c5f9..4a82597 100644 --- a/src/resolve.ts +++ b/src/resolve.ts @@ -18,7 +18,7 @@ import { import { codeSha256, readChainId, readCode, readProxy, type EthProvider } from "./chain.js"; import { LayoutUnavailableError } from "./errors.js"; import { canQueryExplorer, fetchVerifiedSource, type ExplorerConfig } from "./explorer.js"; -import { reconstructLayout } from "./reconstruct.js"; +import { reconstructLayout, type ReconstructedLayout } from "./reconstruct.js"; import { parseRecord, type LayoutRecord } from "./record.js"; import { getSolc, type SolcOptions } from "./solc.js"; @@ -37,6 +37,20 @@ export async function resolveLayout( address: string, options: ResolveOptions, ): Promise { + return (await rebuild(provider, address, options)).record; +} + +/** + * `resolveLayout` plus the analysis of the verified source: the proxy kind, + * inferred from the code as OpenZeppelin recommends (not from the admin slot, + * which a UUPS proxy can have set), the ERC-7201 slots and the safety check. + * Internal, for the CLI. + */ +export async function rebuild( + provider: EthProvider, + address: string, + options: ResolveOptions, +): Promise<{ record: LayoutRecord; analysis: ReconstructedLayout }> { const lower = address.toLowerCase(); if (!canQueryExplorer(options.explorer)) { throw new LayoutUnavailableError( @@ -49,7 +63,7 @@ export async function resolveLayout( const solc = await getSolc(source.solcLongVersion, options.solc); const rebuilt = await reconstructLayout(source, solc, code); - return { + const record: LayoutRecord = { format: 1, address: lower, chainId, @@ -61,6 +75,7 @@ export async function resolveLayout( recordedAt: new Date().toISOString(), layout: rebuilt.layout, }; + return { record, analysis: rebuilt }; } export interface ProxyLayout { diff --git a/test/analyze.test.ts b/test/analyze.test.ts new file mode 100644 index 0000000..89627a4 --- /dev/null +++ b/test/analyze.test.ts @@ -0,0 +1,74 @@ +/** + * `analyzeSource` with a real solc: the safety check must judge the contract + * asked for, and infer its proxy kind through inheritance. + */ +import { describe, it, expect, beforeAll } from "vitest"; +import type { SolcInput } from "@openzeppelin/upgrades-core"; +// From the public entry point: library users get the same check as the CLI. +import { analyzeSource, type SafetyErrorKind } from "../src/index.js"; +import { getSolc, type SolcRunner } from "../src/solc.js"; + +const SOLC = "0.8.24+commit.e11b9ed9"; +const header = "// SPDX-License-Identifier: MIT\npragma solidity 0.8.24;\n"; + +function input(sources: Record, settings: object = {}): SolcInput { + return { + language: "Solidity", + sources: Object.fromEntries(Object.entries(sources).map(([k, v]) => [k, { content: v }])), + settings: { optimizer: { enabled: false, runs: 200 }, ...settings }, + } as SolcInput; +} + +let solc: SolcRunner; + +beforeAll(async () => { + solc = await getSolc(SOLC); +}, 120_000); + +describe("analyzeSource", () => { + // Without CBOR metadata both compile to identical bytecode, and OZ looks + // contracts up by bytecode, taking the first match. Each file order puts the + // other contract first at least once. + it.each([ + ["src/A.sol", "src/B.sol"], + ["src/B.sol", "src/A.sol"], + ])( + "judges the contract asked for when another has identical code (%s first)", + async (...order) => { + const files: Record = { + "src/A.sol": `${header}contract Allowed { uint256 a; + /// @custom:oz-upgrades-unsafe-allow constructor + constructor() { a = 1; } }\n`, + "src/B.sol": `${header}contract Bad { uint256 a; constructor() { a = 1; } }\n`, + }; + const sources = Object.fromEntries(order.map((k) => [k, files[k]])); + const noCbor = { metadata: { appendCBOR: false } }; + const allowed = await analyzeSource(input(sources, noCbor), solc, "src/A.sol:Allowed"); + const bad = await analyzeSource(input(sources, noCbor), solc, "src/B.sol:Bad"); + expect(allowed.safety({ kind: "transparent" }).ok).toBe(true); + expect(bad.safety({ kind: "transparent" }).ok).toBe(false); + }, + 60_000, + ); + + it("infers UUPS through a base contract, and finds unsafe code in a base", async () => { + const sources = { + "src/Base.sol": `${header}abstract contract Base { + function upgradeToAndCall(address, bytes calldata) external payable {} + function call(address t) external { (bool ok, ) = t.delegatecall(""); ok; } }\n`, + "src/Impl.sol": `${header}import {Base} from "./Base.sol";\ncontract Impl is Base { uint256 a; }\n`, + }; + const analysis = await analyzeSource(input(sources), solc, "src/Impl.sol:Impl"); + expect(analysis.kind).toBe("uups"); + const safety = analysis.safety({ kind: analysis.kind }); + expect(safety.ok).toBe(false); + expect(safety.errors.map((e) => e.kind)).toEqual(["delegatecall"]); + expect(safety.errors[0].src).toMatch(/^src\/Base.sol:\d+$/); + expect(safety.explain()).toContain("delegatecall"); + expect(analysis.safety({ kind: "uups", unsafeAllow: ["delegatecall"] }).ok).toBe(true); + // OpenZeppelin appends to the list it gets (for external-library-linking); not to the caller's. + const allow: SafetyErrorKind[] = ["external-library-linking"]; + analysis.safety({ kind: "uups", unsafeAllow: allow }); + expect(allow).toEqual(["external-library-linking"]); + }, 60_000); +}); diff --git a/test/build-info.test.ts b/test/build-info.test.ts new file mode 100644 index 0000000..34976bf --- /dev/null +++ b/test/build-info.test.ts @@ -0,0 +1,390 @@ +/** + * The build-info reader against each tool's file shape, as written by real + * builds (forge 1.8.3, Hardhat 2.x, Hardhat 3.19): Foundry's inline output, + * short version, stale and input-less files; Hardhat 3's `project/` prefix and + * separate `.output.json`. + */ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { dirname, join } from "node:path"; +import { tmpdir } from "node:os"; +import { findBuildInfo } from "../src/build-info.js"; + +const VAULT = "contract Vault is Base { uint256 public total; }"; +const BASE = "contract Base { address public owner; }"; + +let root: string; + +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), "slotdiff-build-info-")); +}); + +afterEach(async () => { + await rm(root, { recursive: true }); +}); + +async function put(path: string, value: unknown): Promise { + await mkdir(dirname(join(root, path)), { recursive: true }); + await writeFile(join(root, path), typeof value === "string" ? value : JSON.stringify(value)); +} + +function compiled(object: string) { + return { evm: { deployedBytecode: { object, immutableReferences: { "3": [] } } } }; +} + +function foundry(sources: Record, settings: object = { optimizer: {} }) { + return { + id: "x", + source_id_to_path: {}, + language: "Solidity", + _format: "ethers-rs-sol-build-info-1", + solcVersion: "0.8.30", + solcLongVersion: "0.8.30", + input: { + version: "0.8.30", + language: "Solidity", + sources: Object.fromEntries(Object.entries(sources).map(([k, v]) => [k, { content: v }])), + settings, + allowPaths: [], + basePath: "", + includePaths: [], + }, + output: { + contracts: Object.fromEntries( + Object.keys(sources).map((k) => [ + k, + { [k.split("/").pop()!.replace(".sol", "")]: compiled(`0x${k.length}`) }, + ]), + ), + }, + }; +} + +// Adds solc's per-source ASTs, reduced to the import directives the reader follows. +function withImports>( + info: T, + imports: Record, +) { + const sources = Object.fromEntries( + Object.keys(info.input.sources).map((source) => [ + source, + { + ast: { + nodeType: "SourceUnit", + nodes: (imports[source] ?? []).map((absolutePath) => ({ + nodeType: "ImportDirective", + absolutePath, + })), + }, + }, + ]), + ); + return { ...info, output: { ...info.output, sources } }; +} + +describe("findBuildInfo", () => { + describe("Foundry", () => { + beforeEach(async () => { + await put("src/Vault.sol", VAULT); + await put("src/Base.sol", BASE); + }); + + it("picks the build whose sources match the disk, among stale and input-less files", async () => { + // Sorted before the fresh one, so neither order nor first match can pick it. + await put("out/build-info/a.json", foundry({ "src/Vault.sol": "old", "src/Base.sol": BASE })); + await put("out/build-info/b.json", { id: "b", source_id_to_path: { 0: "src/Vault.sol" } }); + await put("out/build-info/c.json", foundry({ "src/Vault.sol": VAULT, "src/Base.sol": BASE })); + + const found = await findBuildInfo(root, "src/Vault.sol:Vault"); + expect(found.file).toBe(join(root, "out/build-info/c.json")); + expect(found).toMatchObject({ + contract: "src/Vault.sol:Vault", + solcVersion: "0.8.30", + deployedBytecode: "0x13", + immutableReferences: { "3": [] }, + }); + expect(Object.keys(found.input.sources)).toEqual(["src/Vault.sol", "src/Base.sol"]); + }); + + it("takes an absolute source path", async () => { + await put("out/build-info/c.json", foundry({ "src/Vault.sol": VAULT, "src/Base.sol": BASE })); + const found = await findBuildInfo(root, `${join(root, "src/Vault.sol")}:Vault`); + expect(found.contract).toBe("src/Vault.sol:Vault"); + }); + + it("does not take a name Object.prototype has for a contract", async () => { + await put("out/build-info/c.json", foundry({ "src/Vault.sol": VAULT, "src/Base.sol": BASE })); + await expect(findBuildInfo(root, "toString")).rejects.toThrow(/No build info holds toString/); + await expect(findBuildInfo(root, "src/Vault.sol:toString")).rejects.toThrow(/No build info/); + }); + + it("treats a changed dependency as stale", async () => { + await put( + "out/build-info/a.json", + foundry({ "src/Vault.sol": VAULT, "src/Base.sol": "old" }), + ); + await expect(findBuildInfo(root, "src/Vault.sol:Vault")).rejects.toThrow(/older than the/); + }); + + it("says a build of a moved or deleted target is gone", async () => { + await put("out/build-info/a.json", foundry({ "src/Gone.sol": "x", "src/Base.sol": BASE })); + await expect(findBuildInfo(root, "src/Gone.sol:Gone")).rejects.toThrow(/no longer on disk/); + }); + + describe("after an incremental build", () => { + // forge recompiles only changed files and what imports them, into a new + // build-info file; the older file still holds everything else. + it("keeps an older build when only a file the target does not import changed", async () => { + await put("src/Other.sol", "new"); + await put( + "out/build-info/a.json", + withImports( + foundry({ "src/Vault.sol": VAULT, "src/Base.sol": BASE, "src/Other.sol": "old" }), + { + "src/Vault.sol": ["src/Base.sol"], + }, + ), + ); + await put("out/build-info/b.json", withImports(foundry({ "src/Other.sol": "new" }), {})); + const found = await findBuildInfo(root, "src/Vault.sol:Vault"); + expect(found.file).toBe(join(root, "out/build-info/a.json")); + // Only what the target imports is compiled again. + expect(Object.keys(found.input.sources)).toEqual(["src/Vault.sol", "src/Base.sol"]); + }); + + it("still treats a changed indirect import as stale", async () => { + await put("src/Mid.sol", "mid"); + await put("src/Base.sol", "changed"); + await put( + "out/build-info/a.json", + withImports( + foundry({ "src/Vault.sol": VAULT, "src/Mid.sol": "mid", "src/Base.sol": BASE }), + { + "src/Vault.sol": ["src/Mid.sol"], + "src/Mid.sol": ["src/Base.sol"], + }, + ), + ); + await expect(findBuildInfo(root, "src/Vault.sol:Vault")).rejects.toThrow(/older/); + }); + + it("still refuses a bare name a stale build also defines elsewhere", async () => { + await put("test/Vault.sol", "new mock"); + await put( + "out/build-info/a.json", + withImports( + foundry({ "src/Vault.sol": VAULT, "src/Base.sol": BASE, "test/Vault.sol": "old" }), + { + "src/Vault.sol": ["src/Base.sol"], + }, + ), + ); + await put( + "out/build-info/b.json", + withImports(foundry({ "test/Vault.sol": "new mock" }), {}), + ); + await expect(findBuildInfo(root, "Vault")).rejects.toThrow(/ambiguous/); + }); + + it("does not check a dependency it cannot find, such as hoisted node_modules", async () => { + await put( + "out/build-info/a.json", + withImports(foundry({ "src/Vault.sol": VAULT, "@oz/Base.sol": BASE }), { + "src/Vault.sol": ["@oz/Base.sol"], + }), + ); + expect((await findBuildInfo(root, "src/Vault.sol:Vault")).contract).toBe( + "src/Vault.sol:Vault", + ); + }); + + it("ignores a stale definition whose file no longer declares the name", async () => { + await put("src/Vault.sol", "contract VaultOld {}"); + await put("src/V2.sol", "contract Vault {}"); + await put( + "out/build-info/a.json", + withImports(foundry({ "src/Vault.sol": VAULT, "src/Base.sol": BASE }), {}), + ); + const v2 = foundry({ "src/V2.sol": "contract Vault {}" }); + v2.output.contracts["src/V2.sol"] = { Vault: compiled("0xcc") } as never; + await put("out/build-info/b.json", withImports(v2, {})); + expect((await findBuildInfo(root, "Vault")).contract).toBe("src/V2.sol:Vault"); + }); + + it("still counts a stale definition its file still declares", async () => { + await put("src/Vault.sol", "contract Vault {} // edited"); + await put("src/V2.sol", "contract Vault {}"); + await put( + "out/build-info/a.json", + withImports(foundry({ "src/Vault.sol": VAULT, "src/Base.sol": BASE }), {}), + ); + const v2 = foundry({ "src/V2.sol": "contract Vault {}" }); + v2.output.contracts["src/V2.sol"] = { Vault: compiled("0xcc") } as never; + await put("out/build-info/b.json", withImports(v2, {})); + await expect(findBuildInfo(root, "Vault")).rejects.toThrow(/ambiguous/); + }); + + it("ignores a second definition in a deleted file", async () => { + await put( + "out/build-info/a.json", + withImports( + foundry({ "src/Vault.sol": VAULT, "src/Base.sol": BASE, "test/Vault.sol": "gone" }), + { + "src/Vault.sol": ["src/Base.sol"], + }, + ), + ); + expect((await findBuildInfo(root, "Vault")).contract).toBe("src/Vault.sol:Vault"); + }); + }); + + it("resolves a bare name", async () => { + await put("out/build-info/a.json", foundry({ "src/Vault.sol": VAULT, "src/Base.sol": BASE })); + expect((await findBuildInfo(root, "Base")).contract).toBe("src/Base.sol:Base"); + }); + + it("refuses a bare name two sources define", async () => { + await put("lib/x/Vault.sol", "other"); + await put( + "out/build-info/a.json", + foundry({ "src/Vault.sol": VAULT, "src/Base.sol": BASE, "lib/x/Vault.sol": "other" }), + ); + await expect(findBuildInfo(root, "Vault")).rejects.toThrow( + /ambiguous: src\/Vault.sol:Vault, lib\/x\/Vault.sol:Vault/, + ); + }); + + it("refuses fresh builds with different settings", async () => { + const sources = { "src/Vault.sol": VAULT, "src/Base.sol": BASE }; + await put("out/build-info/a.json", foundry(sources, { optimizer: { enabled: true } })); + await put("out/build-info/b.json", foundry(sources, { optimizer: { enabled: false } })); + await expect(findBuildInfo(root, "src/Vault.sol:Vault")).rejects.toThrow( + /different compilers or settings in out\/build-info\/a.json, out\/build-info\/b.json .*--build-info/, + ); + }); + + it("takes either of two fresh builds that differ only in output selection", async () => { + const sources = { "src/Vault.sol": VAULT, "src/Base.sol": BASE }; + await put("out/build-info/a.json", foundry(sources, { outputSelection: { a: 1 } })); + await put("out/build-info/b.json", foundry(sources, { outputSelection: { b: 2 } })); + expect((await findBuildInfo(root, "src/Vault.sol:Vault")).file).toBe( + join(root, "out/build-info/a.json"), + ); + }); + + it("says how to get full build info when only input-less files exist", async () => { + await put("out/build-info/b.json", { id: "b", source_id_to_path: { 0: "src/Vault.sol" } }); + await expect(findBuildInfo(root, "src/Vault.sol:Vault")).rejects.toThrow( + /no compiler input.*forge build --build-info/, + ); + }); + + it("does not find a name the source does not define", async () => { + await put("out/build-info/a.json", foundry({ "src/Vault.sol": VAULT, "src/Base.sol": BASE })); + await expect(findBuildInfo(root, "src/Vault.sol:Base")).rejects.toThrow(/No build info/); + }); + + it("uses an explicit file as is, without comparing it with the disk", async () => { + await put("ci/a.json", foundry({ "src/Vault.sol": "old", "src/Base.sol": BASE })); + const found = await findBuildInfo(root, "src/Vault.sol:Vault", { file: "ci/a.json" }); + expect(found.file).toBe(join(root, "ci/a.json")); + }); + }); + + describe("Hardhat 2", () => { + it("reads the build info and checks dependencies in node_modules", async () => { + await put("contracts/Vault.sol", VAULT); + await put("node_modules/@oz/Base.sol", BASE); + const info = { + _format: "hh-sol-build-info-1", + solcVersion: "0.8.24", + solcLongVersion: "0.8.24+commit.e11b9ed9", + input: { + language: "Solidity", + sources: { "contracts/Vault.sol": { content: VAULT }, "@oz/Base.sol": { content: BASE } }, + settings: {}, + }, + output: { contracts: { "contracts/Vault.sol": { Vault: compiled("0xaa") } } }, + }; + await put("artifacts/build-info/a.json", info); + expect(await findBuildInfo(root, "contracts/Vault.sol:Vault")).toMatchObject({ + contract: "contracts/Vault.sol:Vault", + solcVersion: "0.8.24+commit.e11b9ed9", + deployedBytecode: "0xaa", + }); + + await put("node_modules/@oz/Base.sol", "changed"); + await expect(findBuildInfo(root, "contracts/Vault.sol:Vault")).rejects.toThrow(/older/); + }); + }); + + describe("Hardhat 3", () => { + beforeEach(async () => { + await put("contracts/Vault.sol", VAULT); + await put("contracts/Base.sol", BASE); + await put("artifacts/build-info/solc-1.json", { + _format: "hh3-sol-build-info-1", + id: "solc-1", + solcVersion: "0.8.24", + solcLongVersion: "0.8.24+commit.e11b9ed9", + compilerType: "solc", + userSourceNameMap: { "contracts/Vault.sol": "project/contracts/Vault.sol" }, + input: { + language: "Solidity", + sources: { + "project/contracts/Vault.sol": { content: VAULT }, + // Not in userSourceNameMap, as for a file the build did not name directly. + "project/contracts/Base.sol": { content: BASE }, + // Versioned npm paths have no file on disk to compare with. + "npm/@oz/contracts@5.0.0/Base.sol": { content: BASE }, + }, + settings: {}, + }, + }); + await put("artifacts/build-info/solc-1.output.json", { + _format: "hh3-sol-build-info-output-1", + id: "solc-1", + output: { contracts: { "project/contracts/Vault.sol": { Vault: compiled("0xbb") } } }, + }); + }); + + it.each(["contracts/Vault.sol:Vault", "project/contracts/Vault.sol:Vault", "Vault"])( + "finds %s with its output from .output.json", + async (name) => { + expect(await findBuildInfo(root, name)).toMatchObject({ + file: join(root, "artifacts/build-info/solc-1.json"), + contract: "project/contracts/Vault.sol:Vault", + deployedBytecode: "0xbb", + }); + }, + ); + + it("maps project sources back to the disk", async () => { + await put("contracts/Vault.sol", "changed"); + await expect(findBuildInfo(root, "Vault")).rejects.toThrow(/older/); + }); + + it("maps project sources missing from userSourceNameMap", async () => { + await put("contracts/Base.sol", "changed"); + await expect(findBuildInfo(root, "Vault")).rejects.toThrow(/older/); + }); + }); + + it("explains where build info comes from when there is none", async () => { + await expect(findBuildInfo(root, "Vault")).rejects.toThrow( + /No build info found.*project root.*build_info/, + ); + }); + + it("names a project root that does not exist", async () => { + await expect(findBuildInfo(join(root, "nope"), "Vault")).rejects.toThrow( + `The project root ${join(root, "nope")} is not a directory.`, + ); + }); + + it("rejects a malformed contract name without quoting it", async () => { + const failing = findBuildInfo(root, "https://rpc.test/v2/SECRET"); + await expect(failing).rejects.toThrow(/Expected a contract name/); + await expect(failing).rejects.not.toThrow(/SECRET/); + }); +}); diff --git a/test/cli.test.ts b/test/cli.test.ts new file mode 100644 index 0000000..673a659 --- /dev/null +++ b/test/cli.test.ts @@ -0,0 +1,656 @@ +/** + * The CLI end to end with a real solc: local sides come from build info + * written to a temporary project, address sides from a mock chain behind the + * CLI's own JSON-RPC client and a stubbed explorer. + */ +import { describe, it, expect, vi, beforeAll, afterAll, beforeEach, afterEach } from "vitest"; +import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { dirname, join } from "node:path"; +import { tmpdir } from "node:os"; +import { stripVTControlCharacters } from "node:util"; +import type { SolcInput } from "@openzeppelin/upgrades-core"; +import { calculateERC7201StorageLocation as slotOf } from "@openzeppelin/upgrades-core/dist/utils/erc7201.js"; +import { run } from "../src/cli.js"; +import { getSolc, type SolcRunner } from "../src/solc.js"; +import { makeMockChain } from "./helpers/mock-chain.js"; + +const ADMIN = "0x00000000000000000000000000000000000000cc"; +const BEACON = "0x00000000000000000000000000000000000000dd"; + +const SOLC = "0.8.24+commit.e11b9ed9"; +const PROXY = "0x00000000000000000000000000000000000000aa"; +const IMPL = "0x00000000000000000000000000000000000000bb"; +const RPC = "http://rpc.test/secret-key"; + +const header = "// SPDX-License-Identifier: MIT\npragma solidity 0.8.24;\n"; +const UPGRADE = "function upgradeToAndCall(address, bytes calldata) external payable {}"; +const bump = (slot: string) => "0x" + (BigInt(slot) + 1n).toString(16).padStart(64, "0"); + +// A contract with one ERC-7201 namespace annotated `id`, stored at `slot`. +function namespaced(name: string, id: string, slot: string, fields = "uint256 shutdown;"): string { + return `${header}contract ${name} { + /// @custom:storage-location erc7201:${id} + struct S { ${fields} } + bytes32 private constant LOCATION = ${slot}; + function _s() private pure returns (S storage $) { assembly { $.slot := LOCATION } } + function shutdown() external view returns (uint256) { return _s().shutdown; } +}\n`; +} +const SOURCES: Record = { + "src/V1.sol": `${header}contract V1 { uint256 public a; address public owner; }\n`, + "src/V2.sol": `${header}contract V2 { uint256 public a; address public owner; uint256 public added; }\n`, + "src/Bad.sol": `${header}contract Bad { uint256 public inserted; uint256 public a; address public owner; }\n`, + "src/Renamed.sol": `${header}contract Renamed { uint256 public b; address public owner; }\n`, + // For --safety: a UUPS implementation, one that lost its upgrade function, constructors. + "src/Uups.sol": `${header}contract Uups { uint256 public a; address public owner; ${UPGRADE} }\n`, + "src/Lost.sol": `${header}contract Lost { uint256 public a; address public owner; }\n`, + "src/Ctor.sol": `${header}contract Ctor { uint256 public a; address public owner; constructor() { a = 1; } }\n`, + "src/BadCtor.sol": `${header}contract BadCtor { uint256 public inserted; uint256 public a; address public owner; constructor() { a = 1; } }\n`, + // The annotation was wrong in the deployed version and fixed later; the slot never moved. + "src/OldShut.sol": namespaced("OldShut", "x.shutdown", slotOf("x.shutdown.real")), + "src/NewShut.sol": namespaced("NewShut", "x.shutdown.real", slotOf("x.shutdown.real")), + // The slot moved behind an unchanged annotation. + "src/NewMoved.sol": namespaced("NewMoved", "x.shutdown.real", slotOf("x.elsewhere")), + // The annotation corrected to where the old data is, but the slot then moved by one. + "src/NewOff.sol": namespaced("NewOff", "x.shutdown.real", bump(slotOf("x.shutdown.real"))), + "src/NewOffInsert.sol": namespaced( + "NewOffInsert", + "x.shutdown.real", + bump(slotOf("x.shutdown.real")), + "uint256 inserted; uint256 shutdown;", + ), + // OldShut's constant changed to match its wrong annotation: the data is left behind. + "src/FixedConst.sol": namespaced("FixedConst", "x.shutdown", slotOf("x.shutdown")), + // A slot slotdiff cannot read. + "src/Unread.sol": namespaced("Unread", "x.shutdown.real", slotOf("x.shutdown.real")).replace( + "assembly { $.slot := LOCATION }", + 'bytes32 l = keccak256("x"); assembly { $.slot := l }', + ), + "src/NoNs.sol": `${header}contract NoNs { uint256 private x; }\n`, + "src/CtorAllowed.sol": `${header}contract CtorAllowed { uint256 public a; address public owner; + /// @custom:oz-upgrades-unsafe-allow constructor + constructor() { a = 1; } }\n`, +}; + +function input(sources: Record): SolcInput { + return { + language: "Solidity", + sources: Object.fromEntries(Object.entries(sources).map(([k, v]) => [k, { content: v }])), + settings: { optimizer: { enabled: false, runs: 200 }, outputSelection: {} }, + } as SolcInput; +} + +let solc: SolcRunner; +let root: string; +let v1Code: string; +let uupsCode: string; +let oldShutCode: string; + +async function deployedCode(path: string, name: string): Promise { + const out = await solc.compile({ + ...input({ [path]: SOURCES[path] }), + settings: { + optimizer: { enabled: false, runs: 200 }, + outputSelection: { "*": { "*": ["evm.deployedBytecode.object"] } }, + }, + } as SolcInput); + const compiled = out.contracts[path][name] as unknown as { + evm: { deployedBytecode: { object: string } }; + }; + return "0x" + compiled.evm.deployedBytecode.object; +} + +// The explorer verifies `name` from `path` as the implementation's source. +function serveVerified(path: string, name: string) { + explorer = vi.fn(async () => ({ + ok: true, + status: 200, + json: async () => ({ + status: "1", + result: [ + { + SourceCode: `{${JSON.stringify(input({ [path]: SOURCES[path] }))}}`, + ContractName: name, + CompilerVersion: `v${SOLC}`, + }, + ], + }), + })); + vi.stubGlobal("fetch", explorer); +} + +beforeAll(async () => { + solc = await getSolc(SOLC); + v1Code = await deployedCode("src/V1.sol", "V1"); + uupsCode = await deployedCode("src/Uups.sol", "Uups"); + oldShutCode = await deployedCode("src/OldShut.sol", "OldShut"); + + // A Hardhat 2 shaped project: the reader only needs input, version and output contract names. + root = await mkdtemp(join(tmpdir(), "slotdiff-cli-")); + for (const [path, content] of Object.entries(SOURCES)) { + await mkdir(dirname(join(root, path)), { recursive: true }); + await writeFile(join(root, path), content); + } + await mkdir(join(root, "artifacts/build-info"), { recursive: true }); + await writeFile( + join(root, "artifacts/build-info/a.json"), + JSON.stringify({ + solcLongVersion: SOLC, + input: input(SOURCES), + output: { + contracts: Object.fromEntries( + Object.keys(SOURCES).map((p) => [p, { [p.slice(4, -4)]: {} }]), + ), + }, + }), + ); +}, 120_000); + +afterAll(async () => { + await rm(root, { recursive: true }); +}); + +let explorer: ReturnType; +// Node's own fetch, whose error messages quote the URL in some cases. +const realFetch = globalThis.fetch; + +beforeEach(() => { + serveVerified("src/V1.sol", "V1"); +}); + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +// The mock chain behind a JSON-RPC endpoint, as the CLI's client sees it. +function rpcFetch( + chain = makeMockChain({ code: { [IMPL]: v1Code }, implementations: { [PROXY]: IMPL } }), +) { + return vi.fn(async (_url: string, init: RequestInit) => { + const { id, method, params } = JSON.parse(String(init.body)); + try { + return Response.json({ jsonrpc: "2.0", id, result: await chain.send(method, params) }); + } catch (e) { + return Response.json({ + jsonrpc: "2.0", + id, + error: { code: -32000, message: (e as Error).message }, + }); + } + }); +} + +async function cli( + args: string[], + fetchImpl: ReturnType = rpcFetch(), + env: Record = { ETHERSCAN_API_KEY: "KEY" }, + terminal: { tty?: boolean; color?: boolean } = {}, +) { + const stdout: string[] = []; + const stderr: string[] = []; + const code = await run(args, { + stdout: (l) => stdout.push(l), + stderr: (l) => stderr.push(l), + cwd: root, + env, + fetch: fetchImpl as never, + ...terminal, + }); + return { code, stdout: stdout.join("\n"), stderr: stderr.join("\n") }; +} + +describe("slotdiff CLI", () => { + it("passes a compatible local upgrade", async () => { + const r = await cli(["src/V1.sol:V1", "V2"]); + expect(r.code).toBe(0); + expect(r.stdout).toContain("old src/V1.sol:V1\n artifacts/build-info/a.json"); + expect(r.stdout).toContain("✓ Storage layout compatible"); + expect(r.stdout).toContain("✓ Safety rules no problems found (transparent proxy)"); + expect(r.stdout).toMatch(/\nSafe to upgrade.$/); + }, 60_000); + + it("fails an incompatible local upgrade with OpenZeppelin's explanation", async () => { + const r = await cli(["V1", "Bad"]); + expect(r.code).toBe(1); + expect(r.stdout).toContain("✗ Storage layout incompatible\n src/Bad.sol:"); + expect(r.stdout).toMatch(/src\/Bad.sol:\d+: Inserted `inserted`/); + // Safe code does not rescue a broken layout, and the verdict says so. + expect(r.stdout).toContain("✓ Safety rules no problems found (transparent proxy)"); + expect(r.stdout).toMatch(/\nNot safe to upgrade: the storage layout is incompatible.$/); + }, 60_000); + + it("passes renames through only with --unsafe-allow-renames", async () => { + expect((await cli(["V1", "Renamed"])).code).toBe(1); + expect((await cli(["V1", "Renamed", "--unsafe-allow-renames"])).code).toBe(0); + }, 60_000); + + it("compares what a proxy runs with a local build", async () => { + const r = await cli([PROXY, "V2", "--rpc", RPC]); + expect(r.stderr).toBe(""); + expect(r.code).toBe(0); + expect(r.stdout).toContain( + `old ${PROXY} → implementation ${IMPL}\n src/V1.sol:V1 (verified source, exact match)`, + ); + expect(String(explorer.mock.calls[0][0])).toContain("apikey=KEY"); + }, 60_000); + + it("never sends the Etherscan key to another explorer", async () => { + const r = await cli([PROXY, "V2", "--rpc", RPC, "--explorer-url", "https://other.test/api"]); + expect(r.code).toBe(0); + const url = String(explorer.mock.calls[0][0]); + expect(url).toMatch(/^https:\/\/other\.test\/api\?/); + expect(url).not.toContain("KEY"); + }, 60_000); + + it("exits 2 when the RPC endpoint answers JSON that is not JSON-RPC", async () => { + const r = await cli( + [PROXY, "V2", "--rpc", RPC], + vi.fn(async () => Response.json({ hello: "world" })), + ); + expect(r.code).toBe(2); + expect(r.stderr).toBe("Could not check: The RPC endpoint did not answer JSON-RPC."); + }); + + it("fails a proxy against an incompatible local build", async () => { + const r = await cli([PROXY, "Bad", "--rpc", RPC]); + expect(r.code).toBe(1); + }, 60_000); + + it("takes an address that is not a proxy as an implementation", async () => { + const r = await cli([IMPL, "V2", "--rpc", RPC]); + expect(r.stderr).toBe(""); + expect(r.code).toBe(0); + expect(r.stdout).toContain( + `old ${IMPL} (implementation)\n src/V1.sol:V1 (verified source, exact match)`, + ); + }, 60_000); + + it("exits 2 for an address with no code", async () => { + const r = await cli([ADMIN, "V2", "--rpc", RPC]); + expect(r.code).toBe(2); + expect(r.stderr).toMatch(/Could not check: No code at/); + }); + + it("resolves the same proxy once when it is both sides, and checks its safety rules", async () => { + const r = await cli([PROXY, PROXY, "--rpc", RPC]); + expect(r.code).toBe(0); + expect(explorer).toHaveBeenCalledTimes(1); + expect(r.stdout).toContain("✓ Safety rules no problems found (transparent proxy)"); + expect(r.stdout).toMatch(/\nSafe to upgrade.$/); + }, 60_000); + + it("checks the safety rules of verified source when is an address", async () => { + const r = await cli(["Bad", PROXY, "--rpc", RPC]); + expect(r.code).toBe(1); + expect(r.stdout).toContain("✓ Safety rules no problems found"); + expect(r.stdout).toMatch(/\nNot safe to upgrade: the storage layout is incompatible.$/); + }, 60_000); + + it("names both problems when both checks fail", async () => { + const r = await cli(["V1", "BadCtor"]); + expect(r.code).toBe(1); + expect(r.stdout).toMatch( + /\nNot safe to upgrade: the storage layout is incompatible and breaks the safety rules.$/, + ); + }, 60_000); + + describe("ERC-7201 slots", () => { + const short = (slot: string) => `${slot.slice(0, 6)}…${slot.slice(-4)}`; + + it("matches namespaces by the slot the code uses, not the annotation", async () => { + const r = await cli(["OldShut", "NewShut"]); + const lines = r.stdout.split("\n"); + const at = lines.indexOf("ERC-7201 namespaces"); + expect(lines.slice(at, at + 5)).toEqual([ + "ERC-7201 namespaces", + " annotation slot from annotation slot in code", + ` old x.shutdown ${short(slotOf("x.shutdown"))} ${short(slotOf("x.shutdown.real"))} ✗ annotation does not match code`, + ` new x.shutdown.real ${short(slotOf("x.shutdown.real"))} ${short(slotOf("x.shutdown.real"))} ✓`, + " Same slot in code on both sides: the storage did not move. Compared as one namespace.", + ]); + expect(r.stdout).toContain("✓ ERC-7201 slots compared by the slot in code (see above)"); + expect(r.stdout).toContain("✓ Storage layout compatible"); + expect(r.code).toBe(0); + }, 60_000); + + it("does the same when is what a proxy runs", async () => { + serveVerified("src/OldShut.sol", "OldShut"); + const chain = makeMockChain({ + code: { [IMPL]: oldShutCode }, + implementations: { [PROXY]: IMPL }, + }); + const r = await cli([PROXY, "NewShut", "--rpc", RPC], rpcFetch(chain)); + expect(r.stderr).toBe(""); + expect(r.stdout).toContain("Same slot in code on both sides"); + expect(r.code).toBe(0); + }, 60_000); + + it("fails a slot that moved behind an unchanged annotation", async () => { + const r = await cli(["NewShut", "NewMoved"]); + expect(r.stdout).toContain( + ` new x.shutdown.real ${short(slotOf("x.shutdown.real"))} ${short(slotOf("x.elsewhere"))} ✗ annotation does not match code`, + ); + expect(r.stdout).toContain( + `The new code moves this namespace from ${short(slotOf("x.shutdown.real"))} to ${short(slotOf("x.elsewhere"))}`, + ); + expect(r.stdout).toContain("✗ ERC-7201 slots problems found in (see above)"); + expect(r.stdout).toMatch( + /\nNot safe to upgrade: an ERC-7201 namespace fails the slot check.$/, + ); + expect(r.code).toBe(1); + }, 60_000); + + // Where the slot check owns the namespace, OpenZeppelin judges only the struct. + it("reports a moved slot once, as a slot problem, not a deleted namespace", async () => { + const r = await cli(["OldShut", "NewOff"]); + expect(r.stdout).toContain("The new code moves this namespace from"); + expect(r.stdout).toContain("✓ Storage layout compatible"); + expect(r.stdout).not.toContain("Deleted namespace"); + expect(r.code).toBe(1); + }, 60_000); + + it("still lets OpenZeppelin catch a change inside a moved namespace", async () => { + const r = await cli(["OldShut", "NewOffInsert"]); + expect(r.stdout).toContain("✗ Storage layout incompatible"); + expect(r.stdout).toMatch(/Inserted `inserted`/); + expect(r.code).toBe(1); + }, 60_000); + + it("fails a constant changed to match an annotation the deployed code did not follow", async () => { + const r = await cli(["OldShut", "FixedConst"]); + expect(r.stdout).toContain( + `The new code moves this namespace from ${short(slotOf("x.shutdown.real"))} to ${short(slotOf("x.shutdown"))}`, + ); + expect(r.stdout).toMatch( + /\nNot safe to upgrade: an ERC-7201 namespace fails the slot check.$/, + ); + expect(r.code).toBe(1); + }, 60_000); + + it("exits 2 when a slot cannot be read, never reporting it safe", async () => { + const r = await cli(["NewShut", "Unread"]); + expect(r.stdout).toContain("? slot in code not read"); + expect(r.stdout).toContain( + "- ERC-7201 slots not checked: some slots could not be read (see above)", + ); + expect(r.stdout).toMatch(/\nCould not check: some ERC-7201 slots could not be read.$/); + expect(r.code).toBe(2); + }, 60_000); + + it("leaves a namespace that is really gone to OpenZeppelin", async () => { + const r = await cli(["NewShut", "NoNs"]); + expect(r.stdout).toContain("Deleted namespace `erc7201:x.shutdown.real`"); + expect(r.stdout).not.toContain("ERC-7201 namespaces"); + expect(r.code).toBe(1); + }, 60_000); + + it("shows no grid when every slot matches", async () => { + const r = await cli(["NewShut", "NewShut"]); + expect(r.stdout).not.toContain("ERC-7201 namespaces"); + expect(r.stdout).toContain("✓ ERC-7201 slots match their annotations"); + expect(r.code).toBe(0); + }, 60_000); + + it("leaves the ERC-7201 check out for a contract with no namespaces", async () => { + const r = await cli(["V1", "V2"]); + expect(r.stdout).not.toContain("ERC-7201"); + }, 60_000); + }); + + describe("safety rules", () => { + it("is skipped with --no-safety", async () => { + const r = await cli(["Uups", "Lost", "--no-safety"]); + expect(r.code).toBe(0); + expect(r.stdout).toContain("- Safety rules skipped (--no-safety)"); + expect(r.stdout).toMatch( + /\nStorage layout compatible; safety rules not checked \(--no-safety\).$/, + ); + }, 60_000); + + it("takes the kind from , so a UUPS implementation that lost upgradeToAndCall fails", async () => { + const r = await cli(["Uups", "Lost"]); + expect(r.code).toBe(1); + expect(r.stdout).toContain("✓ Storage layout compatible"); + expect(r.stdout).toContain( + "✗ Safety rules problems found (uups proxy)\n src/Lost.sol:", + ); + expect(r.stdout).toMatch(/src\/Lost.sol:\d+: .*upgradeTo/); + expect(r.stdout).toMatch(/\nNot safe to upgrade: breaks the safety rules.$/); + }, 60_000); + + it("fails a constructor, unless the source or --unsafe-allow allows it", async () => { + const r = await cli(["V1", "Ctor"]); + expect(r.code).toBe(1); + expect(r.stdout).toMatch(/has a constructor/); + expect((await cli(["V1", "CtorAllowed"])).code).toBe(0); + expect((await cli(["V1", "Ctor", "--unsafe-allow", "delegatecall, constructor"])).code).toBe( + 0, + ); + }, 60_000); + + // OZ's detectProxyKind reads the admin slot, which a UUPS proxy can have set + // (and it is deprecated for that); the kind comes from the implementation's code. + it.each([ + ["no admin slot", {}], + ["an admin slot set", { admins: { [PROXY]: ADMIN } }], + ])( + "infers UUPS from what the proxy runs, with %s", + async (_, extra) => { + serveVerified("src/Uups.sol", "Uups"); + const chain = makeMockChain({ + code: { [IMPL]: uupsCode }, + implementations: { [PROXY]: IMPL }, + ...extra, + }); + const r = await cli([PROXY, "Lost", "--rpc", RPC], rpcFetch(chain)); + expect(r.code).toBe(1); + expect(r.stdout).toContain("✗ Safety rules problems found (uups proxy)"); + }, + 60_000, + ); + + it("infers transparent when what the proxy runs has no upgrade function", async () => { + const r = await cli([PROXY, "Lost", "--rpc", RPC]); + expect(r.code).toBe(0); + expect(r.stdout).toContain("✓ Safety rules no problems found (transparent proxy)"); + }, 60_000); + + it("reads a beacon proxy as beacon, whatever its implementation", async () => { + serveVerified("src/Uups.sol", "Uups"); + const chain = makeMockChain({ + code: { [IMPL]: uupsCode }, + beacons: { [PROXY]: BEACON }, + beaconImplementations: { [BEACON]: IMPL }, + }); + const r = await cli([PROXY, "Lost", "--rpc", RPC], rpcFetch(chain)); + expect(r.stderr).toBe(""); + expect(r.code).toBe(0); + expect(r.stdout).toContain(`→ implementation ${IMPL} (via beacon ${BEACON})`); + expect(r.stdout).toContain("✓ Safety rules no problems found (beacon proxy)"); + }, 60_000); + + it.each([ + [ + ["V1", "V2", "--no-safety", "--unsafe-allow", "constructor"], + /does nothing with --no-safety/, + ], + [["V1", "V2", "--unsafe-allow", "nope"], /unknown error kind "nope"/], + ])("exits 2 for %j", async (args, message) => { + const r = await cli(args); + expect(r.code).toBe(2); + expect(r.stderr).toMatch(message); + }); + }); + + it("names the key it needs to fetch verified source", async () => { + const r = await cli([PROXY, "V2", "--rpc", RPC], rpcFetch(), {}); + expect(r.code).toBe(2); + expect(r.stderr).toContain(`Could not check: ${IMPL} needs an explorer key`); + expect(r.stderr).toContain("set ETHERSCAN_API_KEY"); + }); + + it("reports an address with no code before asking for an explorer key", async () => { + const r = await cli([ADMIN, "V2", "--rpc", RPC], rpcFetch(), {}); + expect(r.code).toBe(2); + expect(r.stderr).toMatch(/Could not check: No code at/); + }); + + it("refuses an address that is too short, as an address", async () => { + const r = await cli(["0x1234", "V2"]); + expect(r.code).toBe(2); + expect(r.stderr).toBe("0x1234 is not an address: an address is 0x and 40 hex digits."); + }); + + it("exits 2 for a wrong number of arguments", async () => { + const r = await cli(["V1"]); + expect(r.code).toBe(2); + expect(r.stderr).toMatch(/^Expected and , got 1 argument/); + }); + + it("needs --rpc for an address", async () => { + const r = await cli([PROXY, "V2"]); + expect(r.code).toBe(2); + expect(r.stderr).toContain("--rpc"); + }); + + it("exits 2 without the RPC URL when the endpoint is unreachable", async () => { + const r = await cli( + [PROXY, "V2", "--rpc", RPC], + vi.fn().mockRejectedValue(new TypeError("fetch failed")), + ); + expect(r.code).toBe(2); + expect(r.stderr).toContain("Cannot reach the RPC endpoint"); + expect(r.stderr).not.toContain("secret-key"); + }); + + it("exits 2 with the endpoint's error text, without control characters", async () => { + const failing = { send: vi.fn().mockRejectedValue(new Error("bad\u001b[31m red")) }; + const r = await cli([PROXY, "V2", "--rpc", RPC], rpcFetch(failing as never)); + expect(r.code).toBe(2); + expect(r.stderr).toMatch(/RPC eth_\w+ failed: bad \[31m red/); + }); + + describe("never prints a URL or key", () => { + it.each([ + ["no scheme", "eth.example.com/v2/SECRET", /--rpc is not a valid URL/], + ["credentials, unreachable", "http://user:SECRET@127.0.0.1:59999/v2/SECRET", /ECONNREFUSED/], + ["key in the path, unreachable", "http://127.0.0.1:59999/v2/SECRET", /ECONNREFUSED/], + ])("--rpc with %s", async (_, url, message) => { + const r = await cli([PROXY, "V2", "--rpc", url], realFetch as never); + expect(r.code).toBe(2); + expect(r.stderr).toMatch(message); + expect(r.stderr).not.toContain("SECRET"); + }); + + it("a URL given as a side", async () => { + const r = await cli(["https://rpc.test/v2/SECRET", "V2"]); + expect(r.code).toBe(2); + expect(r.stderr).toContain("--rpc"); + expect(r.stderr).not.toContain("SECRET"); + }); + + it("--explorer-url with credentials", async () => { + const r = await cli([ + PROXY, + "V2", + "--rpc", + RPC, + "--explorer-url", + "https://u:SECRET@x.test/api", + ]); + expect(r.code).toBe(2); + expect(r.stderr).toContain("--explorer-url must not include"); + expect(r.stderr).not.toContain("SECRET"); + }); + }); + + it("sends credentials in the --rpc URL as basic auth", async () => { + const fetchImpl = rpcFetch(); + const r = await cli([PROXY, "V2", "--rpc", "https://alice:p%40ss@rpc.test/x"], fetchImpl); + expect(r.code).toBe(0); + const [url, init] = fetchImpl.mock.calls[0] as [string, RequestInit]; + expect(url).toBe("https://rpc.test/x"); + expect((init.headers as Record).authorization).toBe( + `Basic ${Buffer.from("alice:p@ss").toString("base64")}`, + ); + }, 60_000); + + it("reads --build-info relative to the current directory, not --root", async () => { + const r = await cli([ + "V1", + "V2", + "--root", + "elsewhere", + "--build-info", + "artifacts/build-info/a.json", + ]); + expect(r.stderr).toBe(""); + expect(r.code).toBe(0); + }, 60_000); + + it("says a missing --build-info file is missing", async () => { + const r = await cli(["V1", "V2", "--build-info", "nope.json"]); + expect(r.code).toBe(2); + expect(r.stderr).toContain("Cannot read"); + expect(r.stderr).toContain("ENOENT"); + }); + + it("exits 2 for an unknown contract", async () => { + const r = await cli(["V1", "Nope"]); + expect(r.code).toBe(2); + expect(r.stderr).toContain("Could not check"); + }); + + it.each([[["V1"]], [["V1", "V2", "V3"]], [["V1", "V2", "--nope"]]])( + "exits 2 with a one-line pointer to --help for %j", + async (args) => { + const r = await cli(args); + expect(r.code).toBe(2); + expect(r.stderr).toMatch(/^[^\n]+\. Run slotdiff --help for usage\.$/); + }, + ); + + describe("on a terminal", () => { + const BANNER_LINE = "|___/_|\\___/ \\__\\__,_|_|_| |_|"; + + it("colors the report without moving a column", async () => { + const args = ["NewShut", "NewMoved"]; + const plainRun = await cli(args); + const colored = await cli(args, undefined, undefined, { color: true }); + // A red ✗ on the failing check and a red verdict; plain text is what is aligned. + expect(colored.stdout).toContain("\x1b[31m✗\x1b[39m \x1b[1mERC-7201 slots\x1b[22m"); + expect(colored.stdout).toContain("\x1b[31mNot safe to upgrade:"); + expect(stripVTControlCharacters(colored.stdout)).toBe(plainRun.stdout); + expect(colored.code).toBe(plainRun.code); + }, 60_000); + + it("shows the banner only on a terminal, and never colors without color", async () => { + const shown = await cli(["V1", "V2"], undefined, undefined, { tty: true }); + expect(shown.stdout.split("\n")[4]).toBe(BANNER_LINE); + expect(shown.stdout).toContain(" storage layout diff for upgradeable contracts · "); + expect(shown.stdout).not.toContain("\x1b["); + expect((await cli(["V1", "V2"])).stdout).not.toContain(BANNER_LINE); + }, 60_000); + + it('draws "slot" and "diff" in two colors, the same letters as without color', async () => { + const banner = (color: boolean) => + cli(["--version"], undefined, undefined, { tty: true, color }).then((r) => r.stdout); + const colored = await banner(true); + const [slot, diff] = [String.raw`|___/_|\___/ \__`, String.raw`\__,_|_|_| |_|`]; + expect(colored).toContain(`\x1b[94m${slot}\x1b[39m\x1b[35m${diff}\x1b[39m`); + expect(stripVTControlCharacters(colored)).toBe(await banner(false)); + }); + + it("puts the banner on --help and --version, but a bare version in a pipe", async () => { + const tty = { tty: true }; + expect((await cli(["--help"], undefined, undefined, tty)).stdout).toContain(BANNER_LINE); + expect((await cli(["--version"], undefined, undefined, tty)).stdout).toContain(BANNER_LINE); + expect((await cli(["--version"])).stdout).toMatch(/^\d+\.\d+\.\d+(-[\w.]+)?$/); + }); + }); + + it("prints help and the version", async () => { + expect((await cli(["--help"])).stdout).toContain("Usage: slotdiff"); + expect((await cli(["-v"])).stdout).toMatch(/^\d+\.\d+\.\d+/); + }); +}); diff --git a/test/erc7201.test.ts b/test/erc7201.test.ts new file mode 100644 index 0000000..12c7dba --- /dev/null +++ b/test/erc7201.test.ts @@ -0,0 +1,544 @@ +/** + * ERC-7201 slots read from real solc output: each way a namespace's slot is + * written, and the comparison by real slot that OpenZeppelin's annotation-only + * check cannot make. + */ +import { describe, it, expect, beforeAll } from "vitest"; +import type { SolcInput } from "@openzeppelin/upgrades-core"; +import { calculateERC7201StorageLocation } from "@openzeppelin/upgrades-core/dist/utils/erc7201.js"; +import { analyzeSource } from "../src/reconstruct.js"; +import { checkNamespaces, type NamespaceSlot } from "../src/erc7201.js"; +import { diffLayouts } from "../src/diff.js"; +import type { StorageLayout } from "@openzeppelin/upgrades-core"; +import { getSolc, type SolcRunner } from "../src/solc.js"; + +const SOLC = "0.8.24+commit.e11b9ed9"; +const slot = (id: string) => calculateERC7201StorageLocation(id); + +// A contract with one namespace `annotated`, whose getter body is `getter`. +function contract(annotated: string, getter: string, extra = ""): string { + return `// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; +contract C { + /// @custom:storage-location erc7201:${annotated} + struct S { uint256 a; } + ${extra} + function _s() private pure returns (S storage $) { ${getter} } + function a() external view returns (uint256) { return _s().a; } +} +`; +} + +let solc: SolcRunner; + +beforeAll(async () => { + solc = await getSolc(SOLC); +}, 120_000); + +async function analyze(content: string) { + const input = { + language: "Solidity", + sources: { "src/C.sol": { content } }, + settings: { optimizer: { enabled: false, runs: 200 } }, + } as SolcInput; + return analyzeSource(input, solc, "src/C.sol:C"); +} + +describe("namespace slots", () => { + it.each([ + [ + "a hex constant", + `bytes32 private constant L = ${slot("x.main")};`, + "assembly { $.slot := L }", + ], + [ + "a constant referencing another", + `bytes32 private constant L0 = ${slot("x.main")}; bytes32 private constant L = L0;`, + "assembly { $.slot := L }", + ], + ["a literal in the assembly", "", `assembly { $.slot := ${slot("x.main")} }`], + [ + "the formula, through a local variable", + `bytes32 private constant L = keccak256(abi.encode(uint256(keccak256("x.main")) - 1)) & ~bytes32(uint256(0xff));`, + "bytes32 l = L; assembly { $.slot := l }", + ], + [ + "the formula with bytes(id)", + `bytes32 private constant L = keccak256(abi.encode(uint256(keccak256(bytes("x.main"))) - 1)) & ~bytes32(uint256(0xff));`, + "bytes32 l = L; assembly { $.slot := l }", + ], + ])( + "reads %s", + async (_, extra, getter) => { + const { namespaces } = await analyze(contract("x.main", getter, extra)); + expect(namespaces).toEqual([ + { + namespace: "erc7201:x.main", + expected: slot("x.main"), + actual: slot("x.main"), + status: "match", + }, + ]); + }, + 60_000, + ); + + it("reads a namespace declared in a base contract", async () => { + const content = `// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; +abstract contract Base { + /// @custom:storage-location erc7201:x.base + struct S { uint256 a; } + function _s() internal pure returns (S storage $) { assembly { $.slot := ${slot("x.base")} } } +} +contract C is Base { + function a() external view returns (uint256) { return _s().a; } +} +`; + const { namespaces } = await analyze(content); + expect(namespaces).toEqual([ + { + namespace: "erc7201:x.base", + expected: slot("x.base"), + actual: slot("x.base"), + status: "match", + }, + ]); + }, 60_000); + + it("reads a slot returned by a no-argument function, as OpenZeppelin's Initializable does", async () => { + const extra = `bytes32 private constant L = ${slot("x.main")}; + function _slot() internal pure virtual returns (bytes32) { return L; }`; + const { namespaces } = await analyze( + contract("x.main", "bytes32 s = _slot(); assembly { $.slot := s }", extra), + ); + expect(namespaces[0].status).toBe("match"); + }, 60_000); + + it("follows the most derived override of that function", async () => { + const content = `// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; +contract Base { + /// @custom:storage-location erc7201:x.main + struct S { uint256 a; } + function _slot() internal pure virtual returns (bytes32) { return ${slot("x.main")}; } + function _s() internal pure returns (S storage $) { bytes32 s = _slot(); assembly { $.slot := s } } + function a() external view returns (uint256) { return _s().a; } +} +contract C is Base { + function _slot() internal pure override returns (bytes32) { return ${slot("x.moved")}; } +} +`; + const { namespaces } = await analyze(content); + expect(namespaces[0]).toMatchObject({ actual: slot("x.moved"), status: "mismatch" }); + }, 60_000); + + it("does not follow a call with arguments, whose override it cannot pick", async () => { + const content = `// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; +contract Base { + /// @custom:storage-location erc7201:x.main + struct S { uint256 a; } + function _slot(uint256) internal pure virtual returns (bytes32) { return ${slot("x.main")}; } + function _s() internal pure returns (S storage $) { bytes32 s = _slot(1); assembly { $.slot := s } } + function a() external view returns (uint256) { return _s().a; } +} +contract C is Base { + function _slot(uint256) internal pure override returns (bytes32) { return ${slot("x.moved")}; } +} +`; + const { namespaces } = await analyze(content); + expect(namespaces[0].status).toBe("unread"); + }, 60_000); + + it("reports the slot the code uses when the annotation disagrees", async () => { + const { namespaces } = await analyze( + contract( + "x.wrong", + "assembly { $.slot := L }", + `bytes32 private constant L = ${slot("x.real")};`, + ), + ); + expect(namespaces).toEqual([ + { + namespace: "erc7201:x.wrong", + expected: slot("x.wrong"), + actual: slot("x.real"), + status: "mismatch", + }, + ]); + }, 60_000); + + it("reports two different slots for one namespace as a conflict", async () => { + const extra = `bytes32 private constant L = ${slot("x.main")}; + function _t() private pure returns (S storage $) { assembly { $.slot := ${slot("x.other")} } }`; + const { namespaces } = await analyze(contract("x.main", "assembly { $.slot := L }", extra)); + expect(namespaces[0].status).toBe("conflict"); + }, 60_000); + + it("follows a getter that forwards to the one that assigns", async () => { + const extra = `function _t() private pure returns (S storage $) { assembly { $.slot := ${slot("x.main")} } }`; + const { namespaces } = await analyze(contract("x.main", "return _t();", extra)); + expect(namespaces[0].status).toBe("match"); + }, 60_000); + + it("leaves a slot computed at run time unread", async () => { + const getter = 'bytes32 l = keccak256("other"); assembly { $.slot := l }'; + const { namespaces } = await analyze(contract("x.main", getter)); + expect(namespaces[0]).toEqual({ + namespace: "erc7201:x.main", + expected: slot("x.main"), + status: "unread", + }); + }, 60_000); + + it("leaves a namespace unread when any of its assignments cannot be read", async () => { + const extra = `function _t() private pure returns (S storage $) { bytes32 l = keccak256("x"); assembly { $.slot := l } }`; + const { namespaces } = await analyze( + contract("x.main", `assembly { $.slot := ${slot("x.main")} }`, extra), + ); + expect(namespaces[0].status).toBe("unread"); + }, 60_000); + + it("leaves a namespace the code never assigns unread", async () => { + const content = `// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; +contract C { + /// @custom:storage-location erc7201:x.main + struct S { uint256 a; } +} +`; + const { namespaces } = await analyze(content); + expect(namespaces[0].status).toBe("unread"); + }, 60_000); +}); + +describe("namespace slots the code may change", () => { + it.each([ + ["in Solidity", "loc = Y;"], + ["in assembly", "assembly { loc := Y }"], + ["by delete", "delete loc;"], + ])( + "leaves a local assigned again %s unread", + async (_, reassign) => { + const extra = `bytes32 private constant L = ${slot("x.main")}; + bytes32 private constant Y = ${slot("y.main")};`; + const getter = `bytes32 loc = L; ${reassign} assembly { $.slot := loc }`; + const { namespaces } = await analyze(contract("x.main", getter, extra)); + expect(namespaces[0].status).toBe("unread"); + }, + 60_000, + ); + + it("leaves a counter changed by ++ unread", async () => { + const extra = `uint256 private constant L = ${slot("x.main")};`; + const getter = "uint256 loc = L; loc++; assembly { $.slot := loc }"; + const { namespaces } = await analyze(contract("x.main", getter, extra)); + expect(namespaces[0].status).toBe("unread"); + }, 60_000); + + it("still reads a constant that is also used as a mapping key", async () => { + const content = `// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; +contract C { + /// @custom:storage-location erc7201:x.main + struct S { mapping(bytes32 => uint256) m; } + bytes32 private constant L = ${slot("x.main")}; + function _s() private pure returns (S storage $) { assembly { $.slot := L } } + function f() external { _s().m[L] = 1; } +} +`; + const { namespaces } = await analyze(content); + expect(namespaces[0].status).toBe("match"); + }, 60_000); + + it("does not follow a function with a modifier, which could skip its body", async () => { + const extra = `bool private constant F = false; + modifier m() { if (F) { _; } } + function _loc() internal pure m returns (bytes32) { return ${slot("x.main")}; }`; + const getter = "bytes32 l = _loc(); assembly { $.slot := l }"; + const { namespaces } = await analyze(contract("x.main", getter, extra)); + expect(namespaces[0].status).toBe("unread"); + }, 60_000); + + it('leaves a hex"..." constant unread rather than reading it as slot 0', async () => { + const extra = `bytes32 private constant L = hex"${slot("x.main").slice(2)}";`; + const getter = "bytes32 l = L; assembly { $.slot := l }"; + const { namespaces } = await analyze(contract("x.main", getter, extra)); + expect(namespaces[0].status).toBe("unread"); + }, 60_000); + + it("reads an immutable set where it is declared, not one set in the constructor", async () => { + const at = (init: string, ctor: string) => `// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; +contract C { + /// @custom:storage-location erc7201:x.main + struct S { uint256 a; } + bytes32 private immutable L${init}; + constructor() { ${ctor} } + function _s() private view returns (S storage $) { bytes32 l = L; assembly { $.slot := l } } + function a() external view returns (uint256) { return _s().a; } +} +`; + expect((await analyze(at(` = ${slot("x.main")}`, ""))).namespaces[0].status).toBe("match"); + expect((await analyze(at("", `L = ${slot("x.main")};`))).namespaces[0].status).toBe("unread"); + }, 60_000); + + it("gives up on getters that call each other, instead of recursing forever", async () => { + const extra = `function _p() internal pure returns (bytes32) { return _q(); } + function _q() internal pure returns (bytes32) { return _p(); }`; + const getter = "bytes32 l = _p(); assembly { $.slot := l }"; + const { namespaces } = await analyze(contract("x.main", getter, extra)); + expect(namespaces[0].status).toBe("unread"); + }, 60_000); + + it("ignores a base's storage getter that the contract overrides", async () => { + const content = `// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; +abstract contract B { + /// @custom:storage-location erc7201:x.main + struct S { uint256 a; } + function _s() internal pure virtual returns (S storage $) { assembly { $.slot := ${slot("y.main")} } } +} +contract C is B { + function _s() internal pure override returns (S storage $) { assembly { $.slot := ${slot("x.main")} } } + function a() external view returns (uint256) { return _s().a; } +} +`; + const { namespaces } = await analyze(content); + expect(namespaces[0].status).toBe("match"); + }, 60_000); + + it("reads a slot a library assigns", async () => { + const content = `// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; +contract C { + /// @custom:storage-location erc7201:x.main + struct S { uint256 a; } + function a() external view returns (uint256) { return L.s().a; } +} +library L { + function s() internal pure returns (C.S storage $) { assembly { $.slot := ${slot("y.main")} } } +} +`; + const { namespaces } = await analyze(content); + expect(namespaces[0]).toMatchObject({ actual: slot("y.main"), status: "mismatch" }); + }, 60_000); + + it("ignores a sibling contract's struct with the same annotation", async () => { + const content = `// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; +contract C { + /// @custom:storage-location erc7201:x.main + struct S { uint256 a; } + function _s() private pure returns (S storage $) { assembly { $.slot := ${slot("x.main")} } } + function a() external view returns (uint256) { return _s().a; } +} +contract Other { + /// @custom:storage-location erc7201:x.main + struct S { uint256 a; } +} +// Free, so its assembly counts; it assigns Other's struct, not C's. +function otherS() pure returns (Other.S storage $) { assembly { $.slot := ${slot("y.main")} } } +`; + const { namespaces } = await analyze(content); + expect(namespaces[0].status).toBe("match"); + }, 60_000); + + it("does not read a state variable's initial value, which the proxy never stores", async () => { + const extra = `bytes32 private loc = ${slot("x.main")};`; + const getter = "bytes32 l = loc; assembly { $.slot := l }"; + const { namespaces } = await analyze( + contract("x.main", getter, extra).replace( + "private pure returns (S", + "private view returns (S", + ), + ); + expect(namespaces[0].status).toBe("unread"); + }, 60_000); + + it("follows a free function, not a contract function of the same name", async () => { + const content = `// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; +function _loc() pure returns (bytes32) { return ${slot("y.main")}; } +contract C { + /// @custom:storage-location erc7201:x.main + struct S { uint256 a; } + function _loc() internal pure virtual returns (bytes32) { return ${slot("x.main")}; } + function a() external view returns (uint256) { return getS().a + uint256(_loc()); } +} +function getS() pure returns (C.S storage $) { bytes32 l = _loc(); assembly { $.slot := l } } +`; + const { namespaces } = await analyze(content); + expect(namespaces[0]).toMatchObject({ actual: slot("y.main"), status: "mismatch" }); + }, 60_000); + + it("ignores assembly in a contract the target does not inherit", async () => { + const content = `// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; +contract B { + /// @custom:storage-location erc7201:x.main + struct S { uint256 a; } + function _s() internal pure returns (S storage $) { assembly { $.slot := ${slot("x.main")} } } +} +contract C is B { + function a() external view returns (uint256) { return _s().a; } +} +contract Harness is B { + function at() external view returns (uint256) { S storage $; assembly { $.slot := 0 } return $.a; } +} +`; + const { namespaces } = await analyze(content); + expect(namespaces[0].status).toBe("match"); + }, 60_000); +}); + +describe("checkNamespaces", () => { + it("compares an old namespace by the slot it really used", async () => { + // The old annotation was wrong; the new one is fixed and the slot is unchanged. + const old = await analyze( + contract( + "x.wrong", + "assembly { $.slot := L }", + `bytes32 private constant L = ${slot("x.real")};`, + ), + ); + const next = await analyze( + contract( + "x.real", + "assembly { $.slot := L }", + `bytes32 private constant L = ${slot("x.real")};`, + ), + ); + expect(diffLayouts(old.layout, next.layout).ok).toBe(false); // OpenZeppelin alone + const checked = checkNamespaces(old, next); + expect(checked.result).toBe("pass"); + expect(Object.keys(checked.layout.namespaces!)).toEqual(["erc7201:x.real"]); + expect(diffLayouts(checked.layout, next.layout).ok).toBe(true); + }, 60_000); + + it("fails a namespace whose data the new code leaves behind", async () => { + // The old annotation was wrong; the new code changed the constant to match it. + const old = await analyze( + contract( + "x.main", + "assembly { $.slot := L }", + `bytes32 private constant L = ${slot("y.main")};`, + ), + ); + const next = await analyze( + contract( + "x.main", + "assembly { $.slot := L }", + `bytes32 private constant L = ${slot("x.main")};`, + ), + ); + expect(diffLayouts(old.layout, next.layout).ok).toBe(true); // OpenZeppelin alone + const checked = checkNamespaces(old, next); + expect(checked.result).toBe("fail"); + expect(checked.pairs).toEqual([ + { old: old.namespaces[0], next: next.namespaces[0], problem: "moved" }, + ]); + }, 60_000); + + it("swaps two namespaces whose annotations were swapped", () => { + const old = { + layout: layoutWith({ "erc7201:a": "A", "erc7201:b": "B" }), + namespaces: [mismatch("a", slot("b")), mismatch("b", slot("a"))], + }; + const checked = checkNamespaces(old, { namespaces: [match("a"), match("b")] }); + expect(checked.result).toBe("pass"); + expect(checked.layout.namespaces).toEqual({ "erc7201:b": "A", "erc7201:a": "B" }); + }); + + it("renames nothing onto a namespace the old layout keeps", () => { + // Old `a` really used `b`'s slot, but old also has a `b` of its own. + const old = { + layout: layoutWith({ "erc7201:a": "A", "erc7201:b": "B" }), + namespaces: [mismatch("a", slot("b")), mismatch("b", slot("c"))], + }; + const checked = checkNamespaces(old, { namespaces: [match("b")] }); + expect(checked.layout).toBe(old.layout); + }); + + it.each([ + ["the new slot", [match("a")], [unread("a")]], + ["the old slot", [unread("a")], [match("a")]], + ])("cannot check a namespace when %s is not read", (_, oldSlots, nextSlots) => { + const old = { layout: layoutWith({ "erc7201:a": "A" }), namespaces: oldSlots }; + const checked = checkNamespaces(old, { namespaces: nextSlots }); + expect(checked.result).toBe("unread"); + expect(checked.pairs[0].problem).toBe("unread"); + }); + + it("pairs by slot before it pairs by annotation", () => { + // New `x` would take old `x` by name; new `z` holds old `x`'s data by slot. + const old = { + layout: layoutWith({ "erc7201:x": "X" }), + namespaces: [mismatch("x", slot("s1"))], + }; + const next = { namespaces: [mismatch("x", slot("s9")), mismatch("z", slot("s1"))] }; + const { pairs } = checkNamespaces(old, next); + expect(pairs.find((p) => p.old !== undefined)?.next?.namespace).toBe("erc7201:z"); + }); + + it("pairs a namespace under the same annotation and slot first", () => { + // New `b` uses `a`'s slot too; `a` stays `a`, and `b` is seen to move. + const old = { + layout: layoutWith({ "erc7201:a": "A", "erc7201:b": "B" }), + namespaces: [match("a"), match("b")], + }; + const next = { namespaces: [mismatch("b", slot("a")), match("a")] }; + const { pairs } = checkNamespaces(old, next); + expect(pairs.map((p) => [p.old?.namespace, p.next?.namespace, p.problem])).toEqual([ + ["erc7201:b", "erc7201:b", "moved"], + ["erc7201:a", "erc7201:a", undefined], + ]); + }); + + it("fails a new namespace that assigns several slots", () => { + const conflict: NamespaceSlot = { ...unread("a"), status: "conflict" }; + const checked = checkNamespaces( + { layout: layoutWith({}), namespaces: [] }, + { namespaces: [conflict] }, + ); + expect(checked.result).toBe("fail"); + expect(checked.pairs[0].problem).toBe("conflict"); + }); + + it("fails before it gives up: a failing namespace outranks an unread one", () => { + const old = { layout: layoutWith({ "erc7201:a": "A", "erc7201:b": "B" }), namespaces: [] }; + const next = { namespaces: [unread("a"), mismatch("b", slot("c"))] }; + expect(checkNamespaces(old, next).result).toBe("fail"); + }); + + it("renames nothing when the slots already line up", async () => { + const c = await analyze( + contract( + "x.main", + "assembly { $.slot := L }", + `bytes32 private constant L = ${slot("x.main")};`, + ), + ); + const checked = checkNamespaces(c, c); + expect(checked.result).toBe("pass"); + expect(checked.layout).toBe(c.layout); + }, 60_000); +}); + +// Synthetic layouts: only the namespace keys matter here, so members are a label. +function layoutWith(namespaces: Record): StorageLayout { + return { storage: [], types: {}, namespaces } as unknown as StorageLayout; +} + +function match(id: string): NamespaceSlot { + return { namespace: `erc7201:${id}`, expected: slot(id), actual: slot(id), status: "match" }; +} + +function mismatch(id: string, actual: string): NamespaceSlot { + return { namespace: `erc7201:${id}`, expected: slot(id), actual, status: "mismatch" }; +} + +function unread(id: string): NamespaceSlot { + return { namespace: `erc7201:${id}`, expected: slot(id), status: "unread" }; +} diff --git a/test/explorer.test.ts b/test/explorer.test.ts index bee3b29..92e0086 100644 --- a/test/explorer.test.ts +++ b/test/explorer.test.ts @@ -243,4 +243,14 @@ describe("explorer edge cases", () => { solcLongVersion: nightly.slice(1), }); }); + + it("never quotes the URL, which carries the key, when fetch fails", async () => { + // Node's fetch quotes the whole URL when refusing one with credentials. + const failing = fetchVerifiedSource(1, "0x00000000000000000000000000000000000000bb", { + apiUrl: "https://user:pass@explorer.test/api", + apiKey: "SECRET", + }); + await expect(failing).rejects.toThrow(LayoutUnavailableError); + await expect(failing).rejects.not.toThrow(/SECRET/); + }); }); diff --git a/test/helpers/mock-chain.ts b/test/helpers/mock-chain.ts index ff1b20d..595fbb3 100644 --- a/test/helpers/mock-chain.ts +++ b/test/helpers/mock-chain.ts @@ -8,6 +8,7 @@ import { vi } from "vitest"; export const IMPL_SLOT = "0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc"; export const BEACON_SLOT = "0xa3f0ad74e5423aebfd80d3ef4346578335a9a72aeaee59ff6cb3582b35133d50"; +export const ADMIN_SLOT = "0xb53127684a568b3173ae13b9f8a6016e243e63b6e8ee1178d6a717850b5d6103"; const IMPLEMENTATION_SELECTOR = "0x5c60da1b"; // implementation() const ZERO_WORD = "0x" + "0".repeat(64); @@ -21,6 +22,8 @@ export interface MockChainState { beacons?: Record; /** beacon address → what its implementation() returns */ beaconImplementations?: Record; + /** transparent proxy address → admin (ERC-1967 admin slot) */ + admins?: Record; } export function makeMockChain(state: MockChainState = {}) { @@ -28,6 +31,7 @@ export function makeMockChain(state: MockChainState = {}) { const implementations = lowerKeys(state.implementations ?? {}); const beacons = lowerKeys(state.beacons ?? {}); const beaconImplementations = lowerKeys(state.beaconImplementations ?? {}); + const admins = lowerKeys(state.admins ?? {}); const send = vi.fn(async (method: string, params: unknown[] = []) => { switch (method) { case "eth_chainId": @@ -41,7 +45,9 @@ export function makeMockChain(state: MockChainState = {}) { ? implementations[address.toLowerCase()] : slot.toLowerCase() === BEACON_SLOT ? beacons[address.toLowerCase()] - : undefined; + : slot.toLowerCase() === ADMIN_SLOT + ? admins[address.toLowerCase()] + : undefined; return target === undefined ? ZERO_WORD : word(target); } case "eth_call": { diff --git a/test/report.test.ts b/test/report.test.ts new file mode 100644 index 0000000..c13c6dc --- /dev/null +++ b/test/report.test.ts @@ -0,0 +1,117 @@ +/** + * The CLI report as text: the ERC-7201 grid for each kind of slot result, and + * the check list. The CLI tests cover the common paths end to end. + */ +import { describe, it, expect } from "vitest"; +import { checkLines, namespaceLines } from "../src/report.js"; +import { checkNamespaces, type NamespaceSlot } from "../src/erc7201.js"; +import type { StorageLayout } from "@openzeppelin/upgrades-core"; + +const A = "0xaaaa000000000000000000000000000000000000000000000000000000001100"; +const B = "0xbbbb000000000000000000000000000000000000000000000000000000002200"; + +// The grid for old and new slots, paired as the CLI pairs them. +function grid(old: NamespaceSlot[], next: NamespaceSlot[]): string[] { + const layout = { storage: [], types: {}, namespaces: {} } as unknown as StorageLayout; + return namespaceLines(checkNamespaces({ layout, namespaces: old }, { namespaces: next }).pairs); +} + +function ns(id: string, expected: string, rest: Partial): NamespaceSlot { + return { namespace: `erc7201:${id}`, expected, status: "match", actual: expected, ...rest }; +} + +describe("namespaceLines", () => { + it("shows nothing when every slot matches", () => { + expect(grid([ns("x", A, {})], [ns("x", A, {})])).toEqual([]); + }); + + it("shows nothing for a matching namespace that only one side has", () => { + // A namespace gone from the new version is OpenZeppelin's to report, as deleted. + expect(grid([ns("x", A, {})], [])).toEqual([]); + expect(grid([], [ns("x", A, {})])).toEqual([]); + }); + + it("marks a slot that could not be read, and says only annotations were compared", () => { + const unread = ns("x", A, { status: "unread", actual: undefined }); + expect(grid([unread], [unread])).toEqual([ + "ERC-7201 namespaces", + " annotation slot from annotation slot in code", + " old x 0xaaaa…1100 not read ? slot in code not read", + " new x 0xaaaa…1100 not read ? slot in code not read", + " The slot in code could not be read (computed at run time?), so this namespace could not be checked.", + ]); + }); + + it("marks a namespace given more than one slot", () => { + const lines = grid([], [ns("x", A, { status: "conflict", actual: undefined })]); + expect(lines[2]).toBe( + " new x 0xaaaa…1100 several ✗ code uses more than one slot", + ); + expect(lines[3]).toContain("assigns more than one slot"); + }); + + it("explains a new mismatch with no old namespace to compare", () => { + const lines = grid([], [ns("x", A, { status: "mismatch", actual: B })]); + expect(lines).toHaveLength(4); + expect(lines[3]).toContain("in a different slot than its annotation says"); + }); + + it("explains a new annotation that is wrong where the storage did not move", () => { + const lines = grid( + [ns("x", A, { status: "mismatch", actual: B })], + [ns("x", A, { status: "mismatch", actual: B })], + ); + expect(lines.at(-1)).toContain("The storage did not move, but the new annotation"); + }); + + it("explains an old mismatch that no new namespace shares", () => { + const lines = grid([ns("x", A, { status: "mismatch", actual: B })], []); + expect(lines.at(-1)).toContain("The deployed annotation does not match its code"); + }); +}); + +describe("namespaceLines pairing", () => { + // The old annotation was wrong (its code used B); the new annotation names B, + // but the new code moved the data to B + 1. One namespace, shown as one block. + const C = "0xbbbb000000000000000000000000000000000000000000000000000000002201"; + + it("pairs a new annotation with the old namespace whose data it points at", () => { + expect( + grid( + [ns("old.id", A, { status: "mismatch", actual: B })], + [ns("new.id", B, { status: "mismatch", actual: C })], + ), + ).toEqual([ + "ERC-7201 namespaces", + " annotation slot from annotation slot in code", + " old old.id 0xaaaa…1100 0xbbbb…2200 ✗ annotation does not match code", + " new new.id 0xbbbb…2200 0xbbbb…2201 ✗ annotation does not match code", + " The new code moves this namespace from 0xbbbb…2200 to 0xbbbb…2201: its data would be left behind. Keep the slot the deployed code uses, under an annotation that matches it.", + ]); + }); + + it("shows slots in full when two different ones would shorten alike", () => { + const lookalike = "0xbbbb111111111111111111111111111111111111111111111111111111112200"; + const lines = grid([], [ns("x", B, { status: "mismatch", actual: lookalike })]); + expect(lines[2]).toContain(B); + expect(lines[2]).toContain(lookalike); + }); +}); + +describe("checkLines", () => { + it("aligns check names and indents details under their check", () => { + expect( + checkLines([ + { name: "Storage layout", status: "fail", summary: "incompatible", details: "a\n b" }, + { name: "Safety rules", status: "skip", summary: "skipped" }, + { name: "Short", status: "pass", summary: "fine" }, + ]), + ).toEqual([ + "✗ Storage layout incompatible", + " a", + " b", + "- Safety rules skipped", + "✓ Short fine", + ]); + }); +});