Skip to content

Commit

Permalink
CLI: Support --referenceBuildInfoDirs option (#1062)
Browse files Browse the repository at this point in the history
Co-authored-by: Hadrien Croubois <[email protected]>
  • Loading branch information
ericglau and Amxx authored Aug 21, 2024
1 parent b1d92cb commit a79ccda
Show file tree
Hide file tree
Showing 22 changed files with 593 additions and 32 deletions.
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/api-core.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ If any errors are found, the command will exit with a non-zero exit code and pri
* `--contract <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 <REFERENCE_CONTRACT>` 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 `<dirName>:` before the contract name or fully qualified name in the `--reference` option or `@custom:oz-upgrades-from` annotation, where `<dirName>` is the directory short name. Each directory short name must be unique.
* `--unsafeAllow "<VALIDATION_ERRORS>"` - 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.
Expand Down
3 changes: 2 additions & 1 deletion packages/core/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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)

Expand Down
8 changes: 8 additions & 0 deletions packages/core/contracts/test/cli/ValidateBuildInfoV1.sol
Original file line number Diff line number Diff line change
@@ -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;
}
Original file line number Diff line number Diff line change
@@ -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;
}
Original file line number Diff line number Diff line change
@@ -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;
}
7 changes: 7 additions & 0 deletions packages/core/contracts/test/cli/ValidateBuildInfoV2_Bad.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.9;

contract MyContract {
uint256 public x;
uint256[49] private __gap;
}
Original file line number Diff line number Diff line change
@@ -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;
}
10 changes: 10 additions & 0 deletions packages/core/contracts/test/cli/ValidateBuildInfoV2_Branch_Ok.sol
Original file line number Diff line number Diff line change
@@ -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;
}
9 changes: 9 additions & 0 deletions packages/core/contracts/test/cli/ValidateBuildInfoV2_Ok.sol
Original file line number Diff line number Diff line change
@@ -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;
}
2 changes: 1 addition & 1 deletion packages/core/package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
274 changes: 274 additions & 0 deletions packages/core/src/cli/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`);
Expand Down Expand Up @@ -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,
);
});
Loading

0 comments on commit a79ccda

Please sign in to comment.