OpenZeppelin · ericglau · Oct 30, 2023 · Oct 26, 2023 · Oct 26, 2023 · Oct 26, 2023
diff --git a/docs/modules/ROOT/pages/api-core.adoc b/docs/modules/ROOT/pages/api-core.adoc
@@ -118,7 +118,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. If specified, requires either the `--reference` option to be provided or the contract to have a `@custom:oz-upgrades-from` annotation.
+* `--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.
 * `--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`
 * `--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.
@@ -167,7 +167,7 @@ Note that this function does not throw validation errors directly. Instead, you
 ** `unsafeAllow`
 ** `unsafeAllowRenames`
 ** `unsafeSkipStorageCheck`
-** `requireReference` - Can only be used when the `contract` argument is also provided. If specified, requires either the `reference` argument to be provided or the contract to have a `@custom:oz-upgrades-from` annotation.
+** `requireReference` - Can only be used when the `contract` argument is also provided. Not compatible with the `unsafeSkipStorageCheck` option. If specified, requires either the `reference` argument to be provided or the contract to have a `@custom:oz-upgrades-from` annotation.
 
 *Returns:*
 

diff --git a/packages/core/CHANGELOG.md b/packages/core/CHANGELOG.md
@@ -1,5 +1,9 @@
 # Changelog
 
+## Unreleased
+
+- CLI: Throw error if `--requireReference` and `--unsafeSkipStorageCheck` are both enabled. ([#913](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/913))
+
 ## 1.31.0 (2023-10-23)
 
 - CLI: Add `--requireReference` option. ([#900](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/900))

diff --git a/packages/core/src/cli/cli.test.ts b/packages/core/src/cli/cli.test.ts
@@ -188,6 +188,20 @@ test('validate - requireReference - no reference, no upgradesFrom', async t => {
   t.true(error?.message.includes('does not specify what contract it upgrades from'), error?.message);
 });
 
+test('validate - requireReference and unsafeSkipStorageCheck', async t => {
+  const temp = await getTempDir(t);
+  const buildInfo = await artifacts.getBuildInfo(`contracts/test/cli/Validate.sol:StorageV1`);
+  await fs.writeFile(path.join(temp, 'validate.json'), JSON.stringify(buildInfo));
+
+  const error = await t.throwsAsync(
+    execAsync(`${CLI} validate ${temp} --contract StorageV1 --requireReference --unsafeSkipStorageCheck`),
+  );
+  t.true(
+    error?.message.includes('The requireReference and unsafeSkipStorageCheck options cannot be used at the same time.'),
+    error?.message,
+  );
+});
+
 test('validate - requireReference - no reference, has upgradesFrom - safe', async t => {
   const temp = await getTempDir(t);
   const buildInfo = await artifacts.getBuildInfo(`contracts/test/cli/Validate.sol:BecomesSafe`);

diff --git a/packages/core/src/cli/cli.test.ts.md b/packages/core/src/cli/cli.test.ts.md
@@ -18,7 +18,7 @@ Generated by [AVA](https://avajs.dev).
     Options:␊
       --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. If specified, requires either the --reference option to be provided or the contract to have a @custom:oz-upgrades-from annotation.␊
+      --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.␊
       --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␊
       --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.␊
@@ -38,7 +38,7 @@ Generated by [AVA](https://avajs.dev).
     Options:␊
       --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. If specified, requires either the --reference option to be provided or the contract to have a @custom:oz-upgrades-from annotation.␊
+      --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.␊
       --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␊
       --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/src/cli/cli.test.ts.snap b/packages/core/src/cli/cli.test.ts.snap
diff --git a/packages/core/src/cli/validate.ts b/packages/core/src/cli/validate.ts
@@ -15,7 +15,7 @@ Arguments:
 Options:
   --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. If specified, requires either the --reference option to be provided or the contract to have a @custom:oz-upgrades-from annotation.
+  --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.
   --unsafeAllow "<VALIDATION_ERRORS>"  Selectively disable one or more validation errors. Comma-separated list with one or more of the following: ${errorKinds.join(
     ', ',
   )}

diff --git a/packages/core/src/cli/validate/validate-upgrade-safety.ts b/packages/core/src/cli/validate/validate-upgrade-safety.ts
@@ -66,6 +66,9 @@ export function findSpecifiedContracts(
 }
 
 export function withCliDefaults(opts: ValidateUpgradeSafetyOptions): Required<ValidateUpgradeSafetyOptions> {
+  if (opts.requireReference && opts.unsafeSkipStorageCheck) {
+    throw new Error(`The requireReference and unsafeSkipStorageCheck options cannot be used at the same time.`);
+  }
   return {
     ...withValidationDefaults(opts),
     requireReference: opts.requireReference ?? false,

diff --git a/packages/core/src/storage.test.ts.snap b/packages/core/src/storage.test.ts.snap