From a79ccdac4027a88931ccf37bf3757e478015bf0c Mon Sep 17 00:00:00 2001 From: Eric Lau Date: Wed, 21 Aug 2024 16:22:28 -0400 Subject: [PATCH] CLI: Support `--referenceBuildInfoDirs` option (#1062) Co-authored-by: Hadrien Croubois --- docs/modules/ROOT/pages/api-core.adoc | 1 + packages/core/CHANGELOG.md | 3 +- .../test/cli/ValidateBuildInfoV1.sol | 8 + .../ValidateBuildInfoV2_Annotation_Bad.sol | 8 + .../cli/ValidateBuildInfoV2_Annotation_Ok.sol | 10 + .../test/cli/ValidateBuildInfoV2_Bad.sol | 7 + .../cli/ValidateBuildInfoV2_Branch_Bad.sol | 10 + .../cli/ValidateBuildInfoV2_Branch_Ok.sol | 10 + .../test/cli/ValidateBuildInfoV2_Ok.sol | 9 + packages/core/package.json | 2 +- packages/core/src/cli/cli.test.ts | 274 ++++++++++++++++++ packages/core/src/cli/cli.test.ts.md | 119 ++++++++ packages/core/src/cli/cli.test.ts.snap | Bin 2148 -> 2707 bytes packages/core/src/cli/validate.ts | 9 +- .../core/src/cli/validate/build-info-file.ts | 19 +- .../core/src/cli/validate/contract-report.ts | 19 +- .../core/src/cli/validate/find-contract.ts | 54 +++- .../cli/validate/upgradeability-assessment.ts | 5 +- .../validate/validate-upgrade-safety.test.ts | 2 +- .../cli/validate/validate-upgrade-safety.ts | 42 ++- .../core/src/cli/validate/validations.test.ts | 12 +- packages/core/src/cli/validate/validations.ts | 2 + 22 files changed, 593 insertions(+), 32 deletions(-) create mode 100644 packages/core/contracts/test/cli/ValidateBuildInfoV1.sol create mode 100644 packages/core/contracts/test/cli/ValidateBuildInfoV2_Annotation_Bad.sol create mode 100644 packages/core/contracts/test/cli/ValidateBuildInfoV2_Annotation_Ok.sol create mode 100644 packages/core/contracts/test/cli/ValidateBuildInfoV2_Bad.sol create mode 100644 packages/core/contracts/test/cli/ValidateBuildInfoV2_Branch_Bad.sol create mode 100644 packages/core/contracts/test/cli/ValidateBuildInfoV2_Branch_Ok.sol create mode 100644 packages/core/contracts/test/cli/ValidateBuildInfoV2_Ok.sol diff --git a/docs/modules/ROOT/pages/api-core.adoc b/docs/modules/ROOT/pages/api-core.adoc index ae6413edb..9522eff4f 100644 --- a/docs/modules/ROOT/pages/api-core.adoc +++ b/docs/modules/ROOT/pages/api-core.adoc @@ -119,6 +119,7 @@ If any errors are found, the command will exit with a non-zero exit code and pri * `--contract ` The name or fully qualified name of the contract to validate. If not specified, all upgradeable contracts in the build info directory will be validated. * `--reference ` Can only be used when the `--contract` option is also provided. The name or fully qualified name of the reference contract to use for storage layout comparisons. If not specified, uses the `@custom:oz-upgrades-from` annotation if it is defined in the contract that is being validated. * `--requireReference` Can only be used when the `--contract` option is also provided. Not compatible with `--unsafeSkipStorageCheck`. If specified, requires either the `--reference` option to be provided or the contract to have a `@custom:oz-upgrades-from` annotation. +* `--referenceBuildInfoDirs` Optional paths of additional build info directories from previous versions of the project to use for storage layout comparisons. When using this option, refer to one of these directories using prefix `:` before the contract name or fully qualified name in the `--reference` option or `@custom:oz-upgrades-from` annotation, where `` is the directory short name. Each directory short name must be unique. * `--unsafeAllow ""` - Selectively disable one or more validation errors. Comma-separated list with one or more of the following: `state-variable-assignment, state-variable-immutable, external-library-linking, struct-definition, enum-definition, constructor, delegatecall, selfdestruct, missing-public-upgradeto, internal-function-storage` * `--unsafeAllowRenames` - Configure storage layout check to allow variable renaming. * `--unsafeSkipStorageCheck` - Skips checking for storage layout compatibility errors. This is a dangerous option meant to be used as a last resort. diff --git a/packages/core/CHANGELOG.md b/packages/core/CHANGELOG.md index c895b61fa..7af4ee161 100644 --- a/packages/core/CHANGELOG.md +++ b/packages/core/CHANGELOG.md @@ -1,8 +1,9 @@ # Changelog -## Unreleased +## 1.36.0 (2024-08-21) - Update dependency on Slang. ([#1059](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/1059)) +- CLI: Support `--referenceBuildInfoDirs` option. ([#1062](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/1062)) ## 1.35.1 (2024-08-13) diff --git a/packages/core/contracts/test/cli/ValidateBuildInfoV1.sol b/packages/core/contracts/test/cli/ValidateBuildInfoV1.sol new file mode 100644 index 000000000..9b461c63c --- /dev/null +++ b/packages/core/contracts/test/cli/ValidateBuildInfoV1.sol @@ -0,0 +1,8 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.9; + +contract MyContract { + uint256 public x; + uint256 public y; + uint256[48] private __gap; +} diff --git a/packages/core/contracts/test/cli/ValidateBuildInfoV2_Annotation_Bad.sol b/packages/core/contracts/test/cli/ValidateBuildInfoV2_Annotation_Bad.sol new file mode 100644 index 000000000..e5c16e091 --- /dev/null +++ b/packages/core/contracts/test/cli/ValidateBuildInfoV2_Annotation_Bad.sol @@ -0,0 +1,8 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.9; + +/// @custom:oz-upgrades-from build-info-v1:MyContract +contract MyContract { + uint256 public x; + uint256[49] private __gap; +} diff --git a/packages/core/contracts/test/cli/ValidateBuildInfoV2_Annotation_Ok.sol b/packages/core/contracts/test/cli/ValidateBuildInfoV2_Annotation_Ok.sol new file mode 100644 index 000000000..7345522aa --- /dev/null +++ b/packages/core/contracts/test/cli/ValidateBuildInfoV2_Annotation_Ok.sol @@ -0,0 +1,10 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.9; + +/// @custom:oz-upgrades-from build-info-v1:MyContract +contract MyContract { + uint256 public x; + uint256 public y; + uint256 public z; + uint256[47] private __gap; +} diff --git a/packages/core/contracts/test/cli/ValidateBuildInfoV2_Bad.sol b/packages/core/contracts/test/cli/ValidateBuildInfoV2_Bad.sol new file mode 100644 index 000000000..53295fa89 --- /dev/null +++ b/packages/core/contracts/test/cli/ValidateBuildInfoV2_Bad.sol @@ -0,0 +1,7 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.9; + +contract MyContract { + uint256 public x; + uint256[49] private __gap; +} diff --git a/packages/core/contracts/test/cli/ValidateBuildInfoV2_Branch_Bad.sol b/packages/core/contracts/test/cli/ValidateBuildInfoV2_Branch_Bad.sol new file mode 100644 index 000000000..c55ea2fca --- /dev/null +++ b/packages/core/contracts/test/cli/ValidateBuildInfoV2_Branch_Bad.sol @@ -0,0 +1,10 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.9; + +/// @custom:oz-upgrades-from build-info-v2:MyContract +contract MyContract { + uint256 public x; + uint256 public y; + uint256 public zBranch; + uint256[47] private __gap; +} diff --git a/packages/core/contracts/test/cli/ValidateBuildInfoV2_Branch_Ok.sol b/packages/core/contracts/test/cli/ValidateBuildInfoV2_Branch_Ok.sol new file mode 100644 index 000000000..fc2232b4c --- /dev/null +++ b/packages/core/contracts/test/cli/ValidateBuildInfoV2_Branch_Ok.sol @@ -0,0 +1,10 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.9; + +/// @custom:oz-upgrades-from build-info-v1:MyContract +contract MyContract { + uint256 public x; + uint256 public y; + uint256 public zBranch; + uint256[47] private __gap; +} diff --git a/packages/core/contracts/test/cli/ValidateBuildInfoV2_Ok.sol b/packages/core/contracts/test/cli/ValidateBuildInfoV2_Ok.sol new file mode 100644 index 000000000..584798375 --- /dev/null +++ b/packages/core/contracts/test/cli/ValidateBuildInfoV2_Ok.sol @@ -0,0 +1,9 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.9; + +contract MyContract { + uint256 public x; + uint256 public y; + uint256 public z; + uint256[47] private __gap; +} diff --git a/packages/core/package.json b/packages/core/package.json index 88e27a910..d1ce44559 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -1,6 +1,6 @@ { "name": "@openzeppelin/upgrades-core", - "version": "1.35.1", + "version": "1.36.0", "description": "", "repository": "https://github.com/OpenZeppelin/openzeppelin-upgrades/tree/master/packages/core", "license": "MIT", diff --git a/packages/core/src/cli/cli.test.ts b/packages/core/src/cli/cli.test.ts index 1baa6aa71..cec9887d0 100644 --- a/packages/core/src/cli/cli.test.ts +++ b/packages/core/src/cli/cli.test.ts @@ -94,6 +94,21 @@ test('validate - single contract, reference', async t => { t.snapshot(expectation.join('\n')); }); +test('validate - single contract, reference with same build info dir', async t => { + const temp = await getTempDir(t); + const buildInfoDir = path.join(temp, 'build-info'); + await fs.mkdir(buildInfoDir); + + const buildInfo = await artifacts.getBuildInfo(`contracts/test/cli/Validate.sol:BecomesBadLayout`); + await fs.writeFile(path.join(buildInfoDir, 'validate.json'), JSON.stringify(buildInfo)); + + const error = await t.throwsAsync( + execAsync(`${CLI} validate ${buildInfoDir} --contract BecomesBadLayout --reference build-info:StorageV1`), + ); + const expectation: string[] = [`Stdout: ${(error as any).stdout}`, `Stderr: ${(error as any).stderr}`]; + t.snapshot(expectation.join('\n')); +}); + test('validate - single contract, reference, fully qualified names', async t => { const temp = await getTempDir(t); const buildInfo = await artifacts.getBuildInfo(`contracts/test/cli/Validate.sol:BecomesBadLayout`); @@ -298,3 +313,262 @@ test('validate - references fully qualified version of ambiguous contract name', ).stdout; t.snapshot(output); }); + +test('validate - references other build info dir by command - ok', async t => { + const temp = await getTempDir(t); + const referenceDir = path.join(temp, 'build-info-v1'); + await fs.mkdir(referenceDir); + + const referenceBuildInfo = await artifacts.getBuildInfo(`contracts/test/cli/ValidateBuildInfoV1.sol:MyContract`); + await fs.writeFile(path.join(referenceDir, 'validate.json'), JSON.stringify(referenceBuildInfo)); + + const updatedDir = path.join(temp, 'build-info'); + await fs.mkdir(updatedDir); + + const updatedBuildInfo = await artifacts.getBuildInfo(`contracts/test/cli/ValidateBuildInfoV2_Ok.sol:MyContract`); + await fs.writeFile(path.join(updatedDir, 'validate.json'), JSON.stringify(updatedBuildInfo)); + + const output = await execAsync( + `${CLI} validate ${updatedDir} --referenceBuildInfoDirs ${referenceDir} --contract MyContract --reference build-info-v1:MyContract`, + ); + t.snapshot(output); +}); + +test('validate - references other build info dir by command - reference not found', async t => { + const temp = await getTempDir(t); + const referenceDir = path.join(temp, 'build-info-v1'); + await fs.mkdir(referenceDir); + + const referenceBuildInfo = await artifacts.getBuildInfo(`contracts/test/cli/ValidateBuildInfoV1.sol:MyContract`); + await fs.writeFile(path.join(referenceDir, 'validate.json'), JSON.stringify(referenceBuildInfo)); + + const updatedDir = path.join(temp, 'build-info'); + await fs.mkdir(updatedDir); + + const updatedBuildInfo = await artifacts.getBuildInfo(`contracts/test/cli/ValidateBuildInfoV2_Ok.sol:MyContract`); + await fs.writeFile(path.join(updatedDir, 'validate.json'), JSON.stringify(updatedBuildInfo)); + + const error = await t.throwsAsync( + execAsync( + `${CLI} validate ${updatedDir} --referenceBuildInfoDirs ${referenceDir} --contract MyContract --reference build-info-not-found:MyContract`, + ), + ); + t.true(error?.message.includes('Could not find contract build-info-not-found:MyContract.'), error?.message); +}); + +test('validate - references other build info dir by command - dir not found', async t => { + const temp = await getTempDir(t); + + const updatedDir = path.join(temp, 'build-info'); + await fs.mkdir(updatedDir); + + const updatedBuildInfo = await artifacts.getBuildInfo(`contracts/test/cli/ValidateBuildInfoV2_Ok.sol:MyContract`); + await fs.writeFile(path.join(updatedDir, 'validate.json'), JSON.stringify(updatedBuildInfo)); + + const error = await t.throwsAsync( + execAsync( + `${CLI} validate ${updatedDir} --referenceBuildInfoDirs build-info-not-found --contract MyContract --reference build-info-not-found:MyContract`, + ), + ); + t.true( + error?.message.includes( + "The directory 'build-info-not-found' does not exist or does not contain any build info files.", + ), + error?.message, + ); +}); + +test('validate - references other build info dir by command with fully qualified names - ok', async t => { + const temp = await getTempDir(t); + const referenceDir = path.join(temp, 'build-info-v1'); + await fs.mkdir(referenceDir); + + const referenceBuildInfo = await artifacts.getBuildInfo(`contracts/test/cli/ValidateBuildInfoV1.sol:MyContract`); + await fs.writeFile(path.join(referenceDir, 'validate.json'), JSON.stringify(referenceBuildInfo)); + + const updatedDir = path.join(temp, 'build-info'); + await fs.mkdir(updatedDir); + + const updatedBuildInfo = await artifacts.getBuildInfo(`contracts/test/cli/ValidateBuildInfoV2_Ok.sol:MyContract`); + await fs.writeFile(path.join(updatedDir, 'validate.json'), JSON.stringify(updatedBuildInfo)); + + const output = await execAsync( + `${CLI} validate ${updatedDir} --referenceBuildInfoDirs ${referenceDir} --contract contracts/test/cli/ValidateBuildInfoV2_Ok.sol:MyContract --reference build-info-v1:contracts/test/cli/ValidateBuildInfoV1.sol:MyContract`, + ); + t.snapshot(output); +}); + +test('validate - references other build info dir by command - bad', async t => { + const temp = await getTempDir(t); + const referenceDir = path.join(temp, 'build-info-v1'); + await fs.mkdir(referenceDir); + + const referenceBuildInfo = await artifacts.getBuildInfo(`contracts/test/cli/ValidateBuildInfoV1.sol:MyContract`); + await fs.writeFile(path.join(referenceDir, 'validate.json'), JSON.stringify(referenceBuildInfo)); + + const updatedDir = path.join(temp, 'build-info'); + await fs.mkdir(updatedDir); + + const updatedBuildInfo = await artifacts.getBuildInfo(`contracts/test/cli/ValidateBuildInfoV2_Bad.sol:MyContract`); + await fs.writeFile(path.join(updatedDir, 'validate.json'), JSON.stringify(updatedBuildInfo)); + + const error = await t.throwsAsync( + execAsync( + `${CLI} validate ${updatedDir} --referenceBuildInfoDirs ${referenceDir} --contract MyContract --reference build-info-v1:MyContract`, + ), + ); + const expectation: string[] = [`Stdout: ${(error as any).stdout}`, `Stderr: ${(error as any).stderr}`]; + t.snapshot(expectation.join('\n')); +}); + +test('validate - references other build info dir by command with fully qualified names - bad', async t => { + const temp = await getTempDir(t); + const referenceDir = path.join(temp, 'build-info-v1'); + await fs.mkdir(referenceDir); + + const referenceBuildInfo = await artifacts.getBuildInfo(`contracts/test/cli/ValidateBuildInfoV1.sol:MyContract`); + await fs.writeFile(path.join(referenceDir, 'validate.json'), JSON.stringify(referenceBuildInfo)); + + const updatedDir = path.join(temp, 'build-info'); + await fs.mkdir(updatedDir); + + const updatedBuildInfo = await artifacts.getBuildInfo(`contracts/test/cli/ValidateBuildInfoV2_Bad.sol:MyContract`); + await fs.writeFile(path.join(updatedDir, 'validate.json'), JSON.stringify(updatedBuildInfo)); + + const error = await t.throwsAsync( + execAsync( + `${CLI} validate ${updatedDir} --referenceBuildInfoDirs ${referenceDir} --contract contracts/test/cli/ValidateBuildInfoV2_Bad.sol:MyContract --reference build-info-v1:contracts/test/cli/ValidateBuildInfoV1.sol:MyContract`, + ), + ); + const expectation: string[] = [`Stdout: ${(error as any).stdout}`, `Stderr: ${(error as any).stderr}`]; + t.snapshot(expectation.join('\n')); +}); + +test('validate - references multiple build info dirs by annotation', async t => { + const temp = await getTempDir(t); + + const v1Dir = path.join(temp, 'build-info-v1'); + await fs.mkdir(v1Dir); + const v1BuildInfo = await artifacts.getBuildInfo(`contracts/test/cli/ValidateBuildInfoV1.sol:MyContract`); + await fs.writeFile(path.join(v1Dir, 'validate.json'), JSON.stringify(v1BuildInfo)); + + const v2Dir = path.join(temp, 'build-info-v2'); + await fs.mkdir(v2Dir); + const v2BuildInfo = await artifacts.getBuildInfo(`contracts/test/cli/ValidateBuildInfoV2_Ok.sol:MyContract`); + await fs.writeFile(path.join(v2Dir, 'validate.json'), JSON.stringify(v2BuildInfo)); + + const v2BranchDir = path.join(temp, 'build-info-v2-branch'); + await fs.mkdir(v2BranchDir); + const v2BranchBuildInfoOk = await artifacts.getBuildInfo( + `contracts/test/cli/ValidateBuildInfoV2_Branch_Ok.sol:MyContract`, + ); + const v2BranchBuildInfoBad = await artifacts.getBuildInfo( + `contracts/test/cli/ValidateBuildInfoV2_Branch_Bad.sol:MyContract`, + ); + await fs.writeFile(path.join(v2BranchDir, 'ok.json'), JSON.stringify(v2BranchBuildInfoOk)); + await fs.writeFile(path.join(v2BranchDir, 'bad.json'), JSON.stringify(v2BranchBuildInfoBad)); + + const error = await t.throwsAsync( + execAsync(`${CLI} validate ${v2BranchDir} --referenceBuildInfoDirs ${v1Dir},${v2Dir}`), + ); + const expectation: string[] = [`Stdout: ${(error as any).stdout}`, `Stderr: ${(error as any).stderr}`]; + t.snapshot(expectation.join('\n')); +}); + +test('validate - references other build info dir by annotation - ok', async t => { + const temp = await getTempDir(t); + const referenceDir = path.join(temp, 'build-info-v1'); + await fs.mkdir(referenceDir); + + const referenceBuildInfo = await artifacts.getBuildInfo(`contracts/test/cli/ValidateBuildInfoV1.sol:MyContract`); + await fs.writeFile(path.join(referenceDir, 'validate.json'), JSON.stringify(referenceBuildInfo)); + + const updatedDir = path.join(temp, 'build-info'); + await fs.mkdir(updatedDir); + + const updatedBuildInfo = await artifacts.getBuildInfo( + `contracts/test/cli/ValidateBuildInfoV2_Annotation_Ok.sol:MyContract`, + ); + await fs.writeFile(path.join(updatedDir, 'validate.json'), JSON.stringify(updatedBuildInfo)); + + const output = await execAsync( + `${CLI} validate ${updatedDir} --referenceBuildInfoDirs ${referenceDir} --contract MyContract`, + ); + t.snapshot(output); +}); + +test('validate - references other build info dir by annotation - bad', async t => { + const temp = await getTempDir(t); + const referenceDir = path.join(temp, 'build-info-v1'); + await fs.mkdir(referenceDir); + + const referenceBuildInfo = await artifacts.getBuildInfo(`contracts/test/cli/ValidateBuildInfoV1.sol:MyContract`); + await fs.writeFile(path.join(referenceDir, 'validate.json'), JSON.stringify(referenceBuildInfo)); + + const updatedDir = path.join(temp, 'build-info'); + await fs.mkdir(updatedDir); + + t.assert(updatedDir !== referenceDir); + const updatedBuildInfo = await artifacts.getBuildInfo( + `contracts/test/cli/ValidateBuildInfoV2_Annotation_Bad.sol:MyContract`, + ); + await fs.writeFile(path.join(updatedDir, 'validate.json'), JSON.stringify(updatedBuildInfo)); + + const error = await t.throwsAsync( + execAsync(`${CLI} validate ${updatedDir} --referenceBuildInfoDirs ${referenceDir} --contract MyContract`), + ); + const expectation: string[] = [`Stdout: ${(error as any).stdout}`, `Stderr: ${(error as any).stderr}`]; + t.snapshot(expectation.join('\n')); +}); + +test('validate - contract must not have build info dir name', async t => { + const temp = await getTempDir(t); + const referenceDir = path.join(temp, 'build-info-v1'); + await fs.mkdir(referenceDir); + + const referenceBuildInfo = await artifacts.getBuildInfo(`contracts/test/cli/ValidateBuildInfoV1.sol:MyContract`); + await fs.writeFile(path.join(referenceDir, 'validate.json'), JSON.stringify(referenceBuildInfo)); + + const updatedDir = path.join(temp, 'build-info'); + await fs.mkdir(updatedDir); + + const updatedBuildInfo = await artifacts.getBuildInfo(`contracts/test/cli/ValidateBuildInfoV2_Ok.sol:MyContract`); + await fs.writeFile(path.join(updatedDir, 'validate.json'), JSON.stringify(updatedBuildInfo)); + + const error = await t.throwsAsync( + execAsync( + `${CLI} validate ${updatedDir} --referenceBuildInfoDirs ${referenceDir} --contract build-info:MyContract --reference build-info-v1:MyContract`, + ), + ); + t.true( + error?.message.includes('Contract build-info:MyContract must be specified without a build info directory name'), + error?.message, + ); +}); + +test('validate - contract must not have build info dir name - fully qualified', async t => { + const temp = await getTempDir(t); + const referenceDir = path.join(temp, 'build-info-v1'); + await fs.mkdir(referenceDir); + + const referenceBuildInfo = await artifacts.getBuildInfo(`contracts/test/cli/ValidateBuildInfoV1.sol:MyContract`); + await fs.writeFile(path.join(referenceDir, 'validate.json'), JSON.stringify(referenceBuildInfo)); + + const updatedDir = path.join(temp, 'build-info'); + await fs.mkdir(updatedDir); + + const updatedBuildInfo = await artifacts.getBuildInfo(`contracts/test/cli/ValidateBuildInfoV2_Ok.sol:MyContract`); + await fs.writeFile(path.join(updatedDir, 'validate.json'), JSON.stringify(updatedBuildInfo)); + + const error = await t.throwsAsync( + execAsync( + `${CLI} validate ${updatedDir} --referenceBuildInfoDirs ${referenceDir} --contract build-info:contracts/test/cli/ValidateBuildInfoV2_Ok.sol:MyContract --reference build-info-v1:MyContract`, + ), + ); + t.true( + error?.message.includes( + 'Contract build-info:contracts/test/cli/ValidateBuildInfoV2_Ok.sol:MyContract must be specified without a build info directory name', + ), + error?.message, + ); +}); diff --git a/packages/core/src/cli/cli.test.ts.md b/packages/core/src/cli/cli.test.ts.md index 86aa1f1ab..26f9ef3ea 100644 --- a/packages/core/src/cli/cli.test.ts.md +++ b/packages/core/src/cli/cli.test.ts.md @@ -19,6 +19,7 @@ Generated by [AVA](https://avajs.dev). --contract The name or fully qualified name of the contract to validate. If not specified, all upgradeable contracts in the build info directory will be validated.␊ --reference Can only be used when the --contract option is also provided. The name or fully qualified name of the reference contract to use for storage layout comparisons. If not specified, uses the @custom:oz-upgrades-from annotation if it is defined in the contract that is being validated.␊ --requireReference Can only be used when the --contract option is also provided. Not compatible with --unsafeSkipStorageCheck. If specified, requires either the --reference option to be provided or the contract to have a @custom:oz-upgrades-from annotation.␊ + --referenceBuildInfoDirs Optional paths of additional build info directories from previous versions of the project to use for storage layout comparisons. When using this option, refer to one of these directories using prefix ':' before the contract name or fully qualified name in the --reference option or @custom:oz-upgrades-from annotation, where is the directory short name. Each directory short name must be unique, including compared to the main build info directory.␊ --unsafeAllow "" Selectively disable one or more validation errors. Comma-separated list with one or more of the following: state-variable-assignment, state-variable-immutable, external-library-linking, struct-definition, enum-definition, constructor, delegatecall, selfdestruct, missing-public-upgradeto, internal-function-storage␊ --unsafeAllowRenames Configure storage layout check to allow variable renaming.␊ --unsafeSkipStorageCheck Skips checking for storage layout compatibility errors. This is a dangerous option meant to be used as a last resort.␊ @@ -39,6 +40,7 @@ Generated by [AVA](https://avajs.dev). --contract The name or fully qualified name of the contract to validate. If not specified, all upgradeable contracts in the build info directory will be validated.␊ --reference Can only be used when the --contract option is also provided. The name or fully qualified name of the reference contract to use for storage layout comparisons. If not specified, uses the @custom:oz-upgrades-from annotation if it is defined in the contract that is being validated.␊ --requireReference Can only be used when the --contract option is also provided. Not compatible with --unsafeSkipStorageCheck. If specified, requires either the --reference option to be provided or the contract to have a @custom:oz-upgrades-from annotation.␊ + --referenceBuildInfoDirs Optional paths of additional build info directories from previous versions of the project to use for storage layout comparisons. When using this option, refer to one of these directories using prefix ':' before the contract name or fully qualified name in the --reference option or @custom:oz-upgrades-from annotation, where is the directory short name. Each directory short name must be unique, including compared to the main build info directory.␊ --unsafeAllow "" Selectively disable one or more validation errors. Comma-separated list with one or more of the following: state-variable-assignment, state-variable-immutable, external-library-linking, struct-definition, enum-definition, constructor, delegatecall, selfdestruct, missing-public-upgradeto, internal-function-storage␊ --unsafeAllowRenames Configure storage layout check to allow variable renaming.␊ --unsafeSkipStorageCheck Skips checking for storage layout compatibility errors. This is a dangerous option meant to be used as a last resort.␊ @@ -187,6 +189,22 @@ Generated by [AVA](https://avajs.dev). ## validate - single contract, reference +> Snapshot 1 + + `Stdout: ✘ contracts/test/cli/Validate.sol:BecomesBadLayout (upgrades from contracts/test/cli/Validate.sol:StorageV1)␊ + ␊ + StorageV1: Deleted \`x\`␊ + > Keep the variable even if unused␊ + ␊ + StorageV1: Deleted \`__gap\`␊ + > Keep the variable even if unused␊ + ␊ + FAILED␊ + ␊ + Stderr: ` + +## validate - single contract, reference with same build info dir + > Snapshot 1 `Stdout: ✘ contracts/test/cli/Validate.sol:BecomesBadLayout (upgrades from contracts/test/cli/Validate.sol:StorageV1)␊ @@ -304,3 +322,104 @@ Generated by [AVA](https://avajs.dev). ␊ SUCCESS␊ ` + +## validate - references other build info dir by command - ok + +> Snapshot 1 + + { + stderr: '', + stdout: ` ✔ contracts/test/cli/ValidateBuildInfoV2_Ok.sol:MyContract (upgrades from build-info-v1:contracts/test/cli/ValidateBuildInfoV1.sol:MyContract)␊ + ␊ + SUCCESS␊ + `, + } + +## validate - references other build info dir by command with fully qualified names - ok + +> Snapshot 1 + + { + stderr: '', + stdout: ` ✔ contracts/test/cli/ValidateBuildInfoV2_Ok.sol:MyContract (upgrades from build-info-v1:contracts/test/cli/ValidateBuildInfoV1.sol:MyContract)␊ + ␊ + SUCCESS␊ + `, + } + +## validate - references other build info dir by command - bad + +> Snapshot 1 + + `Stdout: ✘ contracts/test/cli/ValidateBuildInfoV2_Bad.sol:MyContract (upgrades from build-info-v1:contracts/test/cli/ValidateBuildInfoV1.sol:MyContract)␊ + ␊ + MyContract: Deleted \`y\`␊ + > Keep the variable even if unused␊ + ␊ + contracts/test/cli/ValidateBuildInfoV2_Bad.sol:6: Upgraded \`__gap\` to an incompatible type␊ + - Bad storage gap resize from 48 to 49␊ + Size cannot increase␊ + ␊ + FAILED␊ + ␊ + Stderr: ` + +## validate - references other build info dir by command with fully qualified names - bad + +> Snapshot 1 + + `Stdout: ✘ contracts/test/cli/ValidateBuildInfoV2_Bad.sol:MyContract (upgrades from build-info-v1:contracts/test/cli/ValidateBuildInfoV1.sol:MyContract)␊ + ␊ + MyContract: Deleted \`y\`␊ + > Keep the variable even if unused␊ + ␊ + contracts/test/cli/ValidateBuildInfoV2_Bad.sol:6: Upgraded \`__gap\` to an incompatible type␊ + - Bad storage gap resize from 48 to 49␊ + Size cannot increase␊ + ␊ + FAILED␊ + ␊ + Stderr: ` + +## validate - references multiple build info dirs by annotation + +> Snapshot 1 + + `Stdout: ✘ contracts/test/cli/ValidateBuildInfoV2_Branch_Bad.sol:MyContract (upgrades from build-info-v2:contracts/test/cli/ValidateBuildInfoV2_Ok.sol:MyContract)␊ + ␊ + contracts/test/cli/ValidateBuildInfoV2_Branch_Bad.sol:8: Renamed \`z\` to \`zBranch\`␊ + ␊ + ✔ contracts/test/cli/ValidateBuildInfoV2_Branch_Ok.sol:MyContract (upgrades from build-info-v1:contracts/test/cli/ValidateBuildInfoV1.sol:MyContract)␊ + ␊ + FAILED (2 upgradeable contracts detected, 1 passed, 1 failed)␊ + ␊ + Stderr: ` + +## validate - references other build info dir by annotation - ok + +> Snapshot 1 + + { + stderr: '', + stdout: ` ✔ contracts/test/cli/ValidateBuildInfoV2_Annotation_Ok.sol:MyContract (upgrades from build-info-v1:contracts/test/cli/ValidateBuildInfoV1.sol:MyContract)␊ + ␊ + SUCCESS␊ + `, + } + +## validate - references other build info dir by annotation - bad + +> Snapshot 1 + + `Stdout: ✘ contracts/test/cli/ValidateBuildInfoV2_Annotation_Bad.sol:MyContract (upgrades from build-info-v1:contracts/test/cli/ValidateBuildInfoV1.sol:MyContract)␊ + ␊ + MyContract: Deleted \`y\`␊ + > Keep the variable even if unused␊ + ␊ + contracts/test/cli/ValidateBuildInfoV2_Annotation_Bad.sol:7: Upgraded \`__gap\` to an incompatible type␊ + - Bad storage gap resize from 48 to 49␊ + Size cannot increase␊ + ␊ + FAILED␊ + ␊ + Stderr: ` diff --git a/packages/core/src/cli/cli.test.ts.snap b/packages/core/src/cli/cli.test.ts.snap index 4cc73afe1253c420a04aa596b631e63f24d7fd4d..1b7118f66881adb3817d1c87b1a0714db1e380c6 100644 GIT binary patch literal 2707 zcmV;E3T*X3RzVIhi%0w0SA00000000B+Tg`JDNfoz}+J!ZhA`4td6-C!l;G{}pMUIm=TCXdPY*!SF zwOFIaN|O8@nNp(xIO0xxW0>7p6&l`uu}u z*Jr*qgFmJBWw?UfhM)cka((cEgSyqKL`V?OKnl0?I4+h(NWI=-OTY6-VbC}{`W zHVJnumzZ2Ye^(3$bD8`Oh!=BV(wh00dcat1CpM=V#6r&O3d>Jv}6zfc%K+Z@a>)^WoP91}!~ zJI<4aMFwsl;uaD}kdKm1AbkKdlw+t<89S3cJS5U%9c~Q>hvhQW2aFNU5HAI$VZ`<# z%k}2g&h~132N2x>03F(AD76*X_JF($z*-h(X4GyekdwgxMr=-{faAvEz>qSbwmpF) z+&RTs9EP=rd^G2k#DZC%htpwXd3$|heS3YYzP_&sU#AXn9grD@0(K_SD_lj>)>Rk~ z4#?WlMXZmw2`l7QeJF(v0<5R33Z@d^Ys98t9bfT@3N8WlEO?ZKani@3k!$XWk+@{& zMh*<2p~w~(7zH<(#T{nG5Dvva-of)W<4$)Ryq5t0yq!pYLAbWuNIRdSVU-&hM&O|5 zwhp=13W2EiSm#iwuBjc#A_?QM`=`tsEef!LXz`M0mr?5V=n(@C$${PogPtJm8o;_W zF62lck!WdZLOclDGAPPf6y-d@j<_30a>Rs0--=lh`*2Qb{0yNFB-+jI0oWk|<&bPR ziR;7!gxPd4!a*R1#gEC{GCbM>vaZzT06PnVdJ3~s3C^8KmVgmCFfJp5V8uiTqz%qE z^lV5|B9vmDtWyZdS&zb?hOBh>%YcyRXPc z%X_PvjkQ%wV*Bgc+s*CP%14B>m<=QN5d%C-E>)CLU<=aMaw4L@gm48ItM2yu)Q}7$ zLH{B)2XU1H=mR4nST3#(ve$qB(A_u!qvI+Dm6CTIOmF4UN4(z;eEd`_i{M8BSb@{QS&fLuqX=&^G#+?U-E}P94FH2t%40A<(@|xSSY2Q_jD*#7 zqm9N8uoSe&a1NxxB$tAC#BI#wvAK58p)i0ElR8}{&|M>5_ZfBkD9)%5NU@tu!JZJF zfRFRDQKi*&Kx)yI(_(8{Y)y-;X|Xjewx-3_wAh*!Thn6eEfiZXliJ9LCLS-lZahzw zUC(Z`d=tc~5%SgNzbC`oSAo*UuXJo)*^5)BbnRLTw4R;K*Y_!v4YV<|oiI^g)NW|l z9aoKcU2IH!$fjLoB$!0~oHfb4do^T57=sQ@o$>&Jx~h7H528izXHBI(h&hB0)rZg0 zXVldC^?cuxwMym0v*)!kx1vmE+`U`PtNu@*x(VrEO zcOmMVcm1WSU4L;gSziw`=U@L`UVAkhNI_5sPwn>KWw$6G$PFkdc6UE*k=yZHVWCwd zOx?+A?uQFC(&z(AFbUJR(2Y_d#Go)4l7k?g`8Je=kQn5h15ES7_EpEM132;Ha(e&^ zLZ>)oYKTPyC8*@Cwljhx6qY$j`O$WJ8kmx<~~v1(6(R|s7#Ls zmB8n=tc(bOpcF9fQr^6y8+S)gM`8^JiMP0owWN0^H?_hN#WACDVpA0h>ne#C6(O2p zubP`1Sm&&&?IP+#)cwyyZs+#Fwf4@t;1JE2QyM_A?K3jbBf*31A`+6r4#~>}+ z)y5-6RpjbI(q#ooXrHJ_2^E44@Ua97k;U8udBUJ{V-hQ%q83R9`rM?5R(wLu2%>_^ zY7!?@(AsKHsn`rR2M5Oo8dN`K%v0Nfr1oV;Ol|E02Mf@=;f+t2^8V_*hwp&%;p*VoAH z>f(fBKvSJ0KP?f9a+{d|Gwh&1^BReEVaK<$Z5SqLZLYEX*w4RAZO8sL zqix4>P$ums`7QqD4IY9x4u&xqjI!EAr4DMw_p7D9rvUnMsTe@hyn5x1HO`4oPUD1j zu6G2ZIGX1yj@(Ld?y`1wk)0wBLXM1?-NjaUTUUb zdtA_GW0zRi2jup*+~LWr`9 z`hm}CCEU0g)l>}n!Ipc*5k~T^xUVhZ9*$4^%hbeTB0NI~lwQNHiC*M(HB%_chtE$~hu0kZ(lj*D06zZMMtgk@^g0$(SNcd8tV<9H*A8 zyQ}VRr3+rtR`)3TczH6DMbAs$O4R*&>G}fvU4q{|`2BY2{SQm;H`{o3s#Hoo-n@Bp zrgUSbbZw^eP3Uv&JEiM@6g-wHennqrGM+-1jlN#{{OmxKTnkfCD69+y$;E{cXonX% zjiYKU*R0i%DKmiHJs;3&_mNrgPHj-w1|Of7>}_ghApMt=bN?xSv7$o(mCB`H5mpbw zCane>oPQHiP{{{15}lwVn-z6!46prYmrnhG$F!OVMkhDIu&3w}AJ%l|B0mb2;7kM@}(jFCv0;YHdEQv54Lim%Q|3PGJt zuOK%T@^K?0Sm)Ujog&^+jf7W9!Kf$7pa&;mmxCN{jx#2{q-+wu^ISOdmFKWFuTFmD z`AMyn78hL?WUm*8A>?7oy>Yg9=@JF7J9Lo!k zfNgT7yMDg@hR5>Nzezv?M(iax4P*&mV0Y2l(0bfBa$X`kMNz zUt9a*d!JwLOFSYq#JyAWPm2U3x;xL|Qs;|j0PcPHlo}S$fiE5 z7yblcZl`J_dO0S+nD_|(f?HrgED%ZTkd0EQ?vFy3@Icl|2+i7rP{#|X;Fuu~2V)cn zC1s*= z3MJTw(J+(&025(CICWwN%h3*5!3eE^G4W?x!@j}~;PWzx+ZyW%k z2LK?)E>TuHA!8HtB7|r;l-TigM}wSh1~8IvY6rMT%z-Usgc(~x63Wks7KLqtDIe_( zZLv=r@PiKt+U_>@n%!o*(L6MaZ(xoD2b*ClU}r0OMOqARg~k9;K$c0Nus)$SXvppQ z)Cv;>(5Iscq4FWu5W^6iK=TRvR08UY;L#CQLLWv!u8A`%P01*YIBda@$qocq8EzAY za$+YCPR&rfgZl$Q`Dh-z7a;)LO|8EqTx}717f=;goko@wa+SHgW9s!HAR1#bJl3uo zZpXGrL}-%!nefI+0ah?AQK{lGOT95ZA&?Om&ti1oV}Anw@?#LE_W0p2 zurxcUG7uK?(9K)`@SG(R?8TMcmiF8A8eH3$~)pqbw=eY>U?!bnp{fX){-lV#oAKJb`tM(@{;jQ4zj@4DT>qWC z@oI4>FwhsR;`aX)xA;Jidr-#v{V#gxW+E#ZFNz#fxAKPj-ewK8TwsYR!Z>Ms;!+4_ zP*e=jQJ6@+iESGSgS>U5%KY?to!bonC%Ii7Oh6$lN=l}ISWHl)oa`H&G87@ohb*+y z_G2JaN1?k2)ojM1bSJIHhQvEVcy`!J-M5UEyaN5z>7)_{l*!7B5ek$7!M&fi?wG+n z@G+OrIRWAqh_O-hZsn#{)S_@ZZYTD2v(VHDWh^N)u9D8u@cSxyxnjx9@lg3MR zKDO?ejt&n;*gFExp|jh(`{|ck51u1y2N^$arRG?u$Aqz8VN_LI-AtG4KnoopEfhw2 zA5YY62@yhDxe4-!KVS$yBzw^3CPl3A5w>H93aT|MPN`sAt3jv2h#C%#PLB+z zJ}1P}V?o;ck`tmwy^yO0G_UzRNM9DYT-5Tm;6}>l=5e+9;CHDFIoOfZ^!h;Rb%y!5 z!wqL?Jsl3^#gHqW{MD`7SC#$MJNMp!^xvzu_M5xtW_4@HV{LA(@7n-T6)#_@fy4+B;jAwtz#j^!Dd^ba* zPu?<(=Fj%@%7xqHXyG~HqQkgQ=e-Q}?Od>z7rsPV@RWEJjY~zNVMJr@9hU`%yBQL* z2~rOqR0N@l%(sf)m@!#Dd;3#suit1id%fg!!)f!9ZbmOr<{kZChJ3#i?1|smsd*V& zw$6u90p}aPm!S4+y_13MVZoeDQeyKEkUMOP=_L9#M$mU>4xEqAX0iAz=JcyfaQ?NH z^L1Mql+N)Ri^*&ne`xpzIcu~G^KMi#XV9P9;x!|4`fk!Sl!)Vb!Cz+t`%B@OLZJPc z{!IOXL4Ki8#u#5s$N1an7@wf=r{$G&iRn)=rf*-u^wOdUX|`lTV=ff~ph3j$fQ~}- zt~fPO74&%%|CTYavCzb)>3<@CbIv7g_@mCdc&v#o%v<_6lN@mt$Twp2_nDOWc5%w& avHCU1vOq*s@-jk<#Q6=B~ diff --git a/packages/core/src/cli/validate.ts b/packages/core/src/cli/validate.ts index 4953afe85..4e88b2adf 100644 --- a/packages/core/src/cli/validate.ts +++ b/packages/core/src/cli/validate.ts @@ -16,6 +16,7 @@ Options: --contract The name or fully qualified name of the contract to validate. If not specified, all upgradeable contracts in the build info directory will be validated. --reference Can only be used when the --contract option is also provided. The name or fully qualified name of the reference contract to use for storage layout comparisons. If not specified, uses the @custom:oz-upgrades-from annotation if it is defined in the contract that is being validated. --requireReference Can only be used when the --contract option is also provided. Not compatible with --unsafeSkipStorageCheck. If specified, requires either the --reference option to be provided or the contract to have a @custom:oz-upgrades-from annotation. + --referenceBuildInfoDirs Optional paths of additional build info directories from previous versions of the project to use for storage layout comparisons. When using this option, refer to one of these directories using prefix ':' before the contract name or fully qualified name in the --reference option or @custom:oz-upgrades-from annotation, where is the directory short name. Each directory short name must be unique, including compared to the main build info directory. --unsafeAllow "" Selectively disable one or more validation errors. Comma-separated list with one or more of the following: ${errorKinds.join( ', ', )} @@ -32,6 +33,7 @@ export async function main(args: string[]): Promise { functionArgs.contract, functionArgs.reference, functionArgs.opts, + functionArgs.referenceBuildInfoDirs, ); console.log(result.explain()); process.exitCode = result.ok ? 0 : 1; @@ -48,7 +50,7 @@ function parseArgs(args: string[]) { 'unsafeAllowLinkedLibraries', 'requireReference', ], - string: ['unsafeAllow', 'contract', 'reference'], + string: ['unsafeAllow', 'contract', 'reference', 'referenceBuildInfoDirs'], alias: { h: 'help' }, }); const extraArgs = parsedArgs._; @@ -71,6 +73,7 @@ interface FunctionArgs { contract?: string; reference?: string; opts: Required; + referenceBuildInfoDirs?: string[]; } /** @@ -90,6 +93,7 @@ export function getFunctionArgs(parsedArgs: minimist.ParsedArgs, extraArgs: stri const contract = getAndValidateString(parsedArgs, 'contract'); const reference = getAndValidateString(parsedArgs, 'reference'); const opts = withDefaults(parsedArgs); + const referenceBuildInfoDirs = getAndValidateString(parsedArgs, 'referenceBuildInfoDirs')?.split(/,+/); if (contract === undefined) { if (reference !== undefined) { @@ -98,7 +102,7 @@ export function getFunctionArgs(parsedArgs: minimist.ParsedArgs, extraArgs: stri throw new Error('The --requireReference option can only be used along with the --contract option.'); } } - return { buildInfoDir, contract, reference, opts }; + return { buildInfoDir, contract, reference, opts, referenceBuildInfoDirs }; } } @@ -125,6 +129,7 @@ function validateOptions(parsedArgs: minimist.ParsedArgs) { 'contract', 'reference', 'requireReference', + 'referenceBuildInfoDirs', ].includes(key), ); if (invalidArgs.length > 0) { diff --git a/packages/core/src/cli/validate/build-info-file.ts b/packages/core/src/cli/validate/build-info-file.ts index 597b3c28d..5922397bd 100644 --- a/packages/core/src/cli/validate/build-info-file.ts +++ b/packages/core/src/cli/validate/build-info-file.ts @@ -49,6 +49,11 @@ export interface BuildInfoFile { * The Solidity compiler output JSON object. */ output: SolcOutput; + + /** + * Short name of the build info dir containing this file. + */ + dirShortName: string; } /** @@ -57,13 +62,18 @@ export interface BuildInfoFile { * @param buildInfoDir Build info directory, or undefined to use the default Hardhat or Foundry build-info dir. * @returns The build info files with Solidity compiler input and output. */ -export async function getBuildInfoFiles(buildInfoDir?: string) { +export async function getBuildInfoFiles(buildInfoDir?: string): Promise { const dir = await findDir(buildInfoDir); + const shortName = path.basename(dir); const jsonFiles = await getJsonFiles(dir); - return await readBuildInfo(jsonFiles); + + return await readBuildInfo(jsonFiles, shortName); } -async function findDir(buildInfoDir: string | undefined) { +/** + * Finds the build info dir if provided, otherwise finds the default Hardhat or Foundry build info dir. Throws an error if no build info files were found in the expected dir. + */ +async function findDir(buildInfoDir?: string): Promise { if (buildInfoDir !== undefined && !(await hasJsonFiles(buildInfoDir))) { throw new ValidateCommandError( `The directory '${buildInfoDir}' does not exist or does not contain any build info files.`, @@ -123,7 +133,7 @@ async function getJsonFiles(dir: string): Promise { return jsonFiles.map(file => path.join(dir, file)); } -async function readBuildInfo(buildInfoFilePaths: string[]) { +async function readBuildInfo(buildInfoFilePaths: string[], dirShortName: string) { const buildInfoFiles: BuildInfoFile[] = []; for (const buildInfoFilePath of buildInfoFilePaths) { @@ -143,6 +153,7 @@ async function readBuildInfo(buildInfoFilePaths: string[]) { input: buildInfoJson.input, output: buildInfoJson.output, solcVersion: buildInfoJson.solcVersion, + dirShortName: dirShortName, }); } } diff --git a/packages/core/src/cli/validate/contract-report.ts b/packages/core/src/cli/validate/contract-report.ts index ae68329a6..21d474356 100644 --- a/packages/core/src/cli/validate/contract-report.ts +++ b/packages/core/src/cli/validate/contract-report.ts @@ -17,7 +17,7 @@ import { getUpgradeabilityAssessment } from './upgradeability-assessment'; import { SourceContract } from './validations'; import { LayoutCompatibilityReport } from '../../storage/report'; import { indent } from '../../utils/indent'; -import { SpecifiedContracts } from './validate-upgrade-safety'; +import { BuildInfoDictionary, SpecifiedContracts } from './validate-upgrade-safety'; /** * Report for an upgradeable contract. @@ -58,29 +58,30 @@ export class UpgradeableContractReport implements Report { } /** - * Gets upgradeble contract reports for the upgradeable contracts in the given set of source contracts. + * Gets upgradeble contract reports for the upgradeable contracts in the set of source contracts at dictionary key ''. + * Reference contracts can come from source contracts at the corresponding dictionary key. * Only contracts that are detected as upgradeable will be included in the reports. * Reports include upgradeable contracts regardless of whether they pass or fail upgrade safety checks. * - * @param sourceContracts The source contracts to check, which must include all contracts that are referenced by the given contracts. Can also include non-upgradeable contracts, which will be ignored. + * @param buildInfoDictionary Dictionary of build info directories and the source contracts they contain. * @param opts The validation options. * @param specifiedContracts If provided, only the specified contract (upgrading from its reference contract) will be reported. * @returns The upgradeable contract reports. */ export function getContractReports( - sourceContracts: SourceContract[], + buildInfoDictionary: BuildInfoDictionary, opts: Required, specifiedContracts?: SpecifiedContracts, ) { const upgradeableContractReports: UpgradeableContractReport[] = []; const contractsToReport: SourceContract[] = - specifiedContracts !== undefined ? [specifiedContracts.contract] : sourceContracts; + specifiedContracts !== undefined ? [specifiedContracts.contract] : buildInfoDictionary['']; for (const sourceContract of contractsToReport) { const upgradeabilityAssessment = getUpgradeabilityAssessment( sourceContract, - sourceContracts, + buildInfoDictionary, specifiedContracts?.reference, ); if (opts.requireReference && upgradeabilityAssessment.referenceContract === undefined) { @@ -129,7 +130,11 @@ function getUpgradeableContractReport( const referenceVersion = getContractVersion(referenceContract.validationData, referenceContract.fullyQualifiedName); const referenceLayout = getStorageLayout(referenceContract.validationData, referenceVersion); - reference = referenceContract.fullyQualifiedName; + if (referenceContract.buildInfoDirShortName !== contract.buildInfoDirShortName) { + reference = `${referenceContract.buildInfoDirShortName}:${referenceContract.fullyQualifiedName}`; + } else { + reference = referenceContract.fullyQualifiedName; + } storageLayoutReport = getStorageUpgradeReport(referenceLayout, layout, withValidationDefaults(opts)); } diff --git a/packages/core/src/cli/validate/find-contract.ts b/packages/core/src/cli/validate/find-contract.ts index 12605b332..8a705d1ff 100644 --- a/packages/core/src/cli/validate/find-contract.ts +++ b/packages/core/src/cli/validate/find-contract.ts @@ -1,4 +1,6 @@ +import { assert } from '../../utils/assert'; import { ValidateCommandError } from './error'; +import { BuildInfoDictionary } from './validate-upgrade-safety'; import { SourceContract } from './validations'; export class ReferenceContractNotFound extends Error { @@ -12,7 +14,12 @@ export class ReferenceContractNotFound extends Error { */ readonly origin?: string; - constructor(reference: string, origin?: string) { + /** + * Build info directories that were also searched. + */ + readonly buildInfoDirs?: string[]; + + constructor(reference: string, origin?: string, buildInfoDirs?: string[]) { const msg = origin !== undefined ? `Could not find contract ${reference} referenced in ${origin}.` @@ -20,11 +27,30 @@ export class ReferenceContractNotFound extends Error { super(msg); this.reference = reference; this.origin = origin; + this.buildInfoDirs = buildInfoDirs; } } -export function findContract(contractName: string, origin: SourceContract | undefined, allContracts: SourceContract[]) { - const foundContracts = allContracts.filter(c => c.fullyQualifiedName === contractName || c.name === contractName); +export function findContract( + contractName: string, + origin: SourceContract | undefined, + buildInfoDictionary: BuildInfoDictionary, + onlyMainBuildInfoDir = false, +) { + const foundContracts: SourceContract[] = []; + if (onlyMainBuildInfoDir) { + if (hasBuildInfoDirWithContractName(contractName) || hasBuildInfoDirWithFullyQualifiedName(contractName)) { + throw new ValidateCommandError( + `Contract ${contractName} must be specified without a build info directory name`, + () => `Build info directory names can only be specified for reference contracts.`, + ); + } + foundContracts.push(...buildInfoDictionary[''].filter(c => isMatchFound(contractName, c, ''))); + } else { + for (const [dir, contracts] of Object.entries(buildInfoDictionary)) { + foundContracts.push(...contracts.filter(c => isMatchFound(contractName, c, dir))); + } + } if (foundContracts.length > 1) { const msg = @@ -39,6 +65,26 @@ export function findContract(contractName: string, origin: SourceContract | unde } else if (foundContracts.length === 1) { return foundContracts[0]; } else { - throw new ReferenceContractNotFound(contractName, origin?.fullyQualifiedName); + throw new ReferenceContractNotFound(contractName, origin?.fullyQualifiedName, Object.keys(buildInfoDictionary)); + } +} + +function isMatchFound(contractName: string, foundContract: SourceContract, buildInfoDirShortName: string): boolean { + let prefix = ''; + if (buildInfoDirShortName.length > 0) { + assert(foundContract.buildInfoDirShortName === buildInfoDirShortName); + prefix = `${buildInfoDirShortName}:`; } + return ( + `${prefix}${foundContract.fullyQualifiedName}` === contractName || `${prefix}${foundContract.name}` === contractName + ); +} + +function hasBuildInfoDirWithContractName(contractName: string): boolean { + return contractName.split(':').length === 2 && !contractName.includes('.sol:'); +} + +function hasBuildInfoDirWithFullyQualifiedName(contractName: string): boolean { + const tokens = contractName.split(':'); + return tokens.length === 3 && tokens[1].endsWith('.sol'); } diff --git a/packages/core/src/cli/validate/upgradeability-assessment.ts b/packages/core/src/cli/validate/upgradeability-assessment.ts index 5e2e7c806..77f0f6294 100644 --- a/packages/core/src/cli/validate/upgradeability-assessment.ts +++ b/packages/core/src/cli/validate/upgradeability-assessment.ts @@ -2,6 +2,7 @@ import { getAnnotationArgs, getDocumentation, hasAnnotationTag } from '../../uti import { inferInitializable, inferUUPS } from '../../validate/query'; import { ValidateCommandError } from './error'; import { findContract } from './find-contract'; +import { BuildInfoDictionary } from './validate-upgrade-safety'; import { SourceContract } from './validations'; interface AnnotationAssessment { @@ -17,7 +18,7 @@ export interface UpgradeabilityAssessment { export function getUpgradeabilityAssessment( contract: SourceContract, - allContracts: SourceContract[], + buildInfoDictionary: BuildInfoDictionary, overrideReferenceContract?: SourceContract, ): UpgradeabilityAssessment { const fullContractName = contract.fullyQualifiedName; @@ -29,7 +30,7 @@ export function getUpgradeabilityAssessment( let referenceContract = overrideReferenceContract; if (referenceContract === undefined && annotationAssessment.referenceName !== undefined) { - referenceContract = findContract(annotationAssessment.referenceName, contract, allContracts); + referenceContract = findContract(annotationAssessment.referenceName, contract, buildInfoDictionary); } let isReferenceUUPS = false; diff --git a/packages/core/src/cli/validate/validate-upgrade-safety.test.ts b/packages/core/src/cli/validate/validate-upgrade-safety.test.ts index 149385ffb..4e085d91f 100644 --- a/packages/core/src/cli/validate/validate-upgrade-safety.test.ts +++ b/packages/core/src/cli/validate/validate-upgrade-safety.test.ts @@ -100,7 +100,7 @@ test('invalid annotation args - upgrades', async t => { test('findSpecifiedContracts - requireReference option without contract', async t => { try { - findSpecifiedContracts([], withCliDefaults({ requireReference: true })); + findSpecifiedContracts({}, withCliDefaults({ requireReference: true })); } catch (e: any) { t.true( e.message.includes( diff --git a/packages/core/src/cli/validate/validate-upgrade-safety.ts b/packages/core/src/cli/validate/validate-upgrade-safety.ts index 2f38e4934..a6042ee6b 100644 --- a/packages/core/src/cli/validate/validate-upgrade-safety.ts +++ b/packages/core/src/cli/validate/validate-upgrade-safety.ts @@ -1,3 +1,4 @@ +import path from 'path'; import { ValidationOptions, withValidationDefaults } from '../..'; import { getBuildInfoFiles } from './build-info-file'; @@ -22,10 +23,11 @@ export type SpecifiedContracts = { * Validates the upgrade safety of all contracts in the build info dir's build info files. * Only contracts that are detected as upgradeable will be validated. * - * @param buildInfoDir Absolute path of build info directory, or undefined to use the default Hardhat or Foundry build-info dir. + * @param buildInfoDir Path of build info directory, or undefined to use the default Hardhat or Foundry build-info dir. * @param contract The name or fully qualified name of the contract to validate. If not specified, all upgradeable contracts in the build info directory will be validated. * @param reference The name or fully qualified name of the reference contract to use for storage layout comparisons. Can only be used along with `contract`. If not specified, uses the `@custom:oz-upgrades-from` annotation in the contract that is being validated. * @param opts Validation options, or undefined to use the default validation options. + * @param referenceBuildInfoDirs Optional paths of additional build info directories from previous versions of the project to use for storage layout comparisons. When using this option, refer to one of these directories using prefix `:` before the contract name or fully qualified name in the `reference` param or `@custom:oz-upgrades-from` annotation, where `` is the directory short name. Each directory short name must be unique, including compared to the main build info directory. * @returns The project report. */ export async function validateUpgradeSafety( @@ -33,28 +35,56 @@ export async function validateUpgradeSafety( contract?: string, reference?: string, opts: ValidateUpgradeSafetyOptions = {}, + referenceBuildInfoDirs?: string[], ): Promise { const allOpts = withCliDefaults(opts); const buildInfoFiles = await getBuildInfoFiles(buildInfoDir); const sourceContracts = validateBuildInfoContracts(buildInfoFiles); - const specifiedContracts = findSpecifiedContracts(sourceContracts, allOpts, contract, reference); + const buildInfoDictionary: BuildInfoDictionary = {}; + buildInfoDictionary[''] = sourceContracts; + if (buildInfoFiles.length > 0) { + buildInfoDictionary[buildInfoFiles[0].dirShortName] = sourceContracts; + } + + if (referenceBuildInfoDirs !== undefined) { + for (const referenceBuildInfoDir of referenceBuildInfoDirs) { + const key = path.basename(referenceBuildInfoDir); + + if (buildInfoDictionary[key] !== undefined) { + throw new Error(`Reference build info directory short name '${key}' is not unique.`); + } + + const referenceBuildInfoFiles = await getBuildInfoFiles(referenceBuildInfoDir); + buildInfoDictionary[key] = validateBuildInfoContracts(referenceBuildInfoFiles); + } + } - const contractReports = getContractReports(sourceContracts, allOpts, specifiedContracts); + const specifiedContracts = findSpecifiedContracts(buildInfoDictionary, allOpts, contract, reference); + + const contractReports = getContractReports(buildInfoDictionary, allOpts, specifiedContracts); return getProjectReport(contractReports, specifiedContracts !== undefined); } +/** + * Dictionary of build info directories and the contracts they contain. + * Main build info directory can be found with the key '' and also with its short name. + */ +export interface BuildInfoDictionary { + [buildInfoDirName: string]: SourceContract[]; +} + export function findSpecifiedContracts( - sourceContracts: SourceContract[], + buildInfoDictionary: BuildInfoDictionary, opts: Required, contractName?: string, referenceName?: string, ): SpecifiedContracts | undefined { if (contractName !== undefined) { return { - contract: findContract(contractName, undefined, sourceContracts), - reference: referenceName !== undefined ? findContract(referenceName, undefined, sourceContracts) : undefined, + contract: findContract(contractName, undefined, buildInfoDictionary, true), // only search main build info dir for the specified contract + reference: referenceName !== undefined ? findContract(referenceName, undefined, buildInfoDictionary) : undefined, }; } else if (referenceName !== undefined) { throw new Error(`The reference option can only be specified when the contract option is also specified.`); diff --git a/packages/core/src/cli/validate/validations.test.ts b/packages/core/src/cli/validate/validations.test.ts index 413538903..2f047b804 100644 --- a/packages/core/src/cli/validate/validations.test.ts +++ b/packages/core/src/cli/validate/validations.test.ts @@ -14,13 +14,17 @@ const test = _test as TestFn; const SOURCE_FILE = 'contracts/test/cli/Validate.sol'; test.before(async t => { - const buildInfo = await artifacts.getBuildInfo(`${SOURCE_FILE}:Safe`); + const hardhatBuildInfo = await artifacts.getBuildInfo(`${SOURCE_FILE}:Safe`); - if (buildInfo === undefined) { + if (hardhatBuildInfo === undefined) { t.fail(); } else { - const sourceContracts = validateBuildInfoContracts([buildInfo]); - t.context.reports = getContractReports(sourceContracts, withCliDefaults({})); + const buildInfoFile = { + ...hardhatBuildInfo, + dirShortName: 'build-info', + }; + const sourceContracts = validateBuildInfoContracts([buildInfoFile]); + t.context.reports = getContractReports({ '': sourceContracts }, withCliDefaults({})); } }); diff --git a/packages/core/src/cli/validate/validations.ts b/packages/core/src/cli/validate/validations.ts index 7cfe84ce3..6201cc8b0 100644 --- a/packages/core/src/cli/validate/validations.ts +++ b/packages/core/src/cli/validate/validations.ts @@ -13,6 +13,7 @@ export interface SourceContract { name: string; fullyQualifiedName: string; validationData: ValidationRunData; + buildInfoDirShortName: string; } export function validateBuildInfoContracts(buildInfoFiles: BuildInfoFile[]): SourceContract[] { @@ -48,6 +49,7 @@ function addContractsFromBuildInfo( name: contractDef.name, fullyQualifiedName, validationData: validationData, + buildInfoDirShortName: buildInfoFile.dirShortName, }); } }