Merge branch 'master' into namespaced

OpenZeppelin · Sep 20, 2023 · 108c2b0 · 108c2b0
2 parents 2769f38 + d5394ba
commit 108c2b0
Show file tree

Hide file tree

Showing 18 changed files with 45 additions and 22 deletions.
diff --git a/docs/modules/ROOT/pages/api-hardhat-upgrades.adoc b/docs/modules/ROOT/pages/api-hardhat-upgrades.adoc
@@ -15,7 +15,7 @@ The following options are common to some functions.
 ** `"state-variable-immutable"`: Allows use of immutable variables, which are not unsafe
 ** `"constructor"`: Allows defining a constructor. See `constructorArgs`.
 ** `"delegatecall"`, `"selfdestruct"`: Allow the use of these operations. Incorrect use of this option can put funds at risk of permanent loss. See xref:faq.adoc#delegatecall-selfdestruct[Can I safely use `delegatecall` and `selfdestruct`?]
-** `"missing-public-upgradeto"`: Allow UUPS implementations that do not contain a public `upgradeTo` function. Enabling this option is likely to cause a revert due to the built-in UUPS safety mechanism.
+** `"missing-public-upgradeto"`: Allow UUPS implementations that do not contain a public `upgradeTo` or `upgradeToAndCall` function. Enabling this option is likely to cause a revert due to the built-in UUPS safety mechanism.
 * `unsafeAllowRenames`: (`boolean`) Configure storage layout check to allow variable renaming.
 * `unsafeSkipStorageCheck`: (`boolean`) upgrades the proxy or beacon without first checking for storage layout compatibility errors. This is a dangerous option meant to be used as a last resort.
 * `constructorArgs`: (`unknown[]`) Provide arguments for the constructor of the implementation contract. Note that these are different from initializer arguments, and will be used in the deployment of the implementation contract itself. Can be used to initialize immutable variables.

diff --git a/docs/modules/ROOT/pages/api-truffle-upgrades.adoc b/docs/modules/ROOT/pages/api-truffle-upgrades.adoc
@@ -16,7 +16,7 @@ The following options are common to some functions.
 ** `"state-variable-immutable"`: Allows use of immutable variables, which are not unsafe
 ** `"constructor"`: Allows defining a constructor. See `constructorArgs`.
 ** `"delegatecall"`, `"selfdestruct"`: Allow the use of these operations. Incorrect use of this option can put funds at risk of permanent loss. See xref:faq.adoc#delegatecall-selfdestruct[Can I safely use `delegatecall` and `selfdestruct`?]
-** `"missing-public-upgradeto"`: Allow UUPS implementations that do not contain a public `upgradeTo` function. Enabling this option is likely to cause a revert due to the built-in UUPS safety mechanism.
+** `"missing-public-upgradeto"`: Allow UUPS implementations that do not contain a public `upgradeTo` or `upgradeToAndCall` function. Enabling this option is likely to cause a revert due to the built-in UUPS safety mechanism.
 * `unsafeAllowRenames`: (`boolean`) Configure storage layout check to allow variable renaming.
 * `unsafeSkipStorageCheck`: (`boolean`) upgrades the proxy or beacon without first checking for storage layout compatibility errors. This is a dangerous option meant to be used as a last resort.
 * `constructorArgs`: (`unknown[]`) Provide arguments for the constructor of the implementation contract. Note that these are different from initializer arguments, and will be used in the deployment of the implementation contract itself. Can be used to initialize immutable variables.

diff --git a/docs/modules/ROOT/pages/faq.adoc b/docs/modules/ROOT/pages/faq.adoc
@@ -187,11 +187,11 @@ In the meantime, you can deploy upgradeable contracts linked to external librari
 You can follow or contribute to https://github.com/OpenZeppelin/openzeppelin-upgrades/issues/52[this issue in GitHub].
 
 [[why-public-upgradeto]]
-== Why do I need a public `upgradeTo` function?
+== Why do I need a public `upgradeTo` or `upgradeToAndCall` function?
 
-When using UUPS proxies (through the `kind: 'uups'` option), the implementation contract must include the public function `upgradeTo(address newImplementation)`. This is because in the UUPS pattern the proxy does not contain an upgrading function itself, and the entire upgradeability mechanism lives on the implementation side. Thus, on every deploy and upgrade we have to make sure to include it, otherwise we may permanently disable the upgradeability of the contract.
+When using UUPS proxies (through the `kind: 'uups'` option), the implementation contract must include one or both of the public functions `upgradeTo(address newImplementation)` or `upgradeToAndCall(address newImplementation, bytes memory data)`. This is because in the UUPS pattern the proxy does not contain an upgrading function itself, and the entire upgradeability mechanism lives on the implementation side. Thus, on every deploy and upgrade we have to make sure to include it, otherwise we may permanently disable the upgradeability of the contract.
 
-The recommended way to include this function is by inheriting the `UUPSUpgradeable` contract provided in OpenZeppelin Contracts, as shown below. This contract adds the required `upgradeTo` function, but also contains a built-in mechanism that will check on-chain, at the time of an upgrade, that the new implementation proposed also inherits `UUPSUpgradeable` or implements the same interface. In this way, when using the Upgrades Plugins there are two layers of mitigations to prevent accidentally disabling upgradeability: an off-chain check by the plugins, and an on-chain fallback in the contract itself.
+The recommended way to include one or both of these functions is by inheriting the `UUPSUpgradeable` contract provided in OpenZeppelin Contracts, as shown below. This contract adds the required function(s), but also contains a built-in mechanism that will check on-chain, at the time of an upgrade, that the new implementation proposed also inherits `UUPSUpgradeable` or implements the same interface. In this way, when using the Upgrades Plugins there are two layers of mitigations to prevent accidentally disabling upgradeability: an off-chain check by the plugins, and an on-chain fallback in the contract itself.
 
 ```solidity
 import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";

diff --git a/packages/core/CHANGELOG.md b/packages/core/CHANGELOG.md
@@ -4,6 +4,10 @@
 
 - Add validations for namespaced storage layout. ([#876](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/876))
 
+## 1.29.0 (2023-09-19)
+
+- Support implementations with upgradeTo or upgradeToAndCall. ([#880](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/880))
+
 ## 1.28.0 (2023-08-03)
 
 - Support `contract` and `reference` options for CLI. ([#856](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/856))

diff --git a/packages/core/contracts/test/Validations.sol b/packages/core/contracts/test/Validations.sol
@@ -169,9 +169,20 @@ contract HasUpgradeToFunction {
   function upgradeTo(address) public {}
 }
 
+contract HasInternalUpgradeToAndCallFunction {
+  function upgradeToAndCall(address newImplementation, bytes memory data) internal {}
+}
+
+contract HasUpgradeToAndCallFunction {
+  function upgradeToAndCall(address newImplementation, bytes memory data) public {}
+}
+
 contract ParentHasUpgradeToFunction is HasUpgradeToFunction {
 }
 
+contract ParentHasUpgradeToAndCallFunction is HasUpgradeToAndCallFunction {
+}
+
 contract HasInlineAssembly {
   function unsafe() public pure {
     assembly {

diff --git a/packages/core/package.json b/packages/core/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@openzeppelin/upgrades-core",
-  "version": "1.28.0",
+  "version": "1.29.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.md b/packages/core/src/cli/cli.test.ts.md
@@ -82,8 +82,8 @@ Generated by [AVA](https://avajs.dev).
     ␊
      ✘  contracts/test/cli/Validate.sol:UpgradesFromUUPS (upgrades from contracts/test/cli/Validate.sol:HasUpgradeTo)␊
     ␊
-          contracts/test/cli/Validate.sol:72: Implementation is missing a public \`upgradeTo(address)\` function␊
-              Inherit UUPSUpgradeable to include this function in your contract␊
+          contracts/test/cli/Validate.sol:72: 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␊
     ␊
@@ -174,8 +174,8 @@ Generated by [AVA](https://avajs.dev).
 
     `Stdout:  ✘  contracts/test/cli/Validate.sol:BecomesBadLayout (upgrades from contracts/test/cli/Validate.sol:HasUpgradeTo)␊
     ␊
-          contracts/test/cli/Validate.sol:111: Implementation is missing a public \`upgradeTo(address)\` function␊
-              Inherit UUPSUpgradeable to include this function in your contract␊
+          contracts/test/cli/Validate.sol:111: 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␊
     ␊

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/project-report.test.ts.md b/packages/core/src/cli/validate/project-report.test.ts.md
@@ -10,8 +10,8 @@ Generated by [AVA](https://avajs.dev).
 
     ` ✘  mypath/MyContract.sol:MyContract␊
     ␊
-          MyContract.sol:10: Implementation is missing a public \`upgradeTo(address)\` function␊
-              Inherit UUPSUpgradeable to include this function in your contract␊
+          MyContract.sol:10: 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␊
           ␊
@@ -30,8 +30,8 @@ Generated by [AVA](https://avajs.dev).
 
     ` ✘  mypath/MyContract.sol:MyContract1␊
     ␊
-          MyContract.sol:10: Implementation is missing a public \`upgradeTo(address)\` function␊
-              Inherit UUPSUpgradeable to include this function in your contract␊
+          MyContract.sol:10: 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␊
     ␊

diff --git a/packages/core/src/cli/validate/project-report.test.ts.snap b/packages/core/src/cli/validate/project-report.test.ts.snap
diff --git a/packages/core/src/cli/validate/validate-upgrade-safety.test.ts.md b/packages/core/src/cli/validate/validate-upgrade-safety.test.ts.md
@@ -44,8 +44,8 @@ Generated by [AVA](https://avajs.dev).
     ␊
      ✘  contracts/test/cli/Validate.sol:UpgradesFromUUPS (upgrades from contracts/test/cli/Validate.sol:HasUpgradeTo)␊
     ␊
-          contracts/test/cli/Validate.sol:72: Implementation is missing a public \`upgradeTo(address)\` function␊
-              Inherit UUPSUpgradeable to include this function in your contract␊
+          contracts/test/cli/Validate.sol:72: 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␊
     ␊

diff --git a/packages/core/src/cli/validate/validate-upgrade-safety.test.ts.snap b/packages/core/src/cli/validate/validate-upgrade-safety.test.ts.snap
diff --git a/packages/core/src/cli/validate/validations.test.ts.md b/packages/core/src/cli/validate/validations.test.ts.md
@@ -50,8 +50,8 @@ Generated by [AVA](https://avajs.dev).
 
     ` ✘  contracts/test/cli/Validate.sol:UpgradesFromUUPS (upgrades from contracts/test/cli/Validate.sol:HasUpgradeTo)␊
     ␊
-          contracts/test/cli/Validate.sol:72: Implementation is missing a public \`upgradeTo(address)\` function␊
-              Inherit UUPSUpgradeable to include this function in your contract␊
+          contracts/test/cli/Validate.sol:72: 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`
 

diff --git a/packages/core/src/cli/validate/validations.test.ts.snap b/packages/core/src/cli/validate/validations.test.ts.snap
diff --git a/packages/core/src/validate.test.ts b/packages/core/src/validate.test.ts
@@ -99,7 +99,10 @@ testOverride('UsesExplicitSafeExternalLibrary', 'transparent', { unsafeAllow: ['
 testValid('HasEmptyConstructor', 'uups', false);
 testValid('HasInternalUpgradeToFunction', 'uups', false);
 testValid('HasUpgradeToFunction', 'uups', true);
+testValid('HasInternalUpgradeToAndCallFunction', 'uups', false);
+testValid('HasUpgradeToAndCallFunction', 'uups', true);
 testValid('ParentHasUpgradeToFunction', 'uups', true);
+testValid('ParentHasUpgradeToAndCallFunction', 'uups', true);
 testValid('ChildOfProxiable', 'uups', true);
 
 testValid('HasNonEmptyConstructorNatspec1', 'transparent', true);

diff --git a/packages/core/src/validate/overrides.ts b/packages/core/src/validate/overrides.ts
@@ -73,7 +73,7 @@ export const ValidationErrorUnsafeMessages: Record<ValidationError['kind'], stri
   selfdestruct: [`You are using the \`unsafeAllow.selfdestruct\` flag.`],
   'missing-public-upgradeto': [
     `You are using the \`unsafeAllow.missing-public-upgradeto\` flag with uups proxy.`,
-    `Not having a public upgradeTo function in your implementation can break upgradeability.`,
+    `Not having a public upgradeTo or upgradeToAndCall function in your implementation can break upgradeability.`,
     `Some implementation might check that onchain, and cause the upgrade to revert.`,
   ],
 };

diff --git a/packages/core/src/validate/query.ts b/packages/core/src/validate/query.ts
@@ -8,6 +8,7 @@ import { ValidationData, normalizeValidationData } from './data';
 import { ProxyDeployment } from '../manifest';
 
 const upgradeToSignature = 'upgradeTo(address)';
+const upgradeToAndCallSignature = 'upgradeToAndCall(address,bytes)';
 
 export function assertUpgradeSafe(data: ValidationData, version: Version, opts: ValidationOptions): void {
   const dataV3 = normalizeValidationData(data);
@@ -149,7 +150,10 @@ export function getErrors(data: ValidationData, version: Version, opts: Validati
 
   const selfAndInheritedMethods = getAllMethods(runValidation, fullContractName);
 
-  if (!selfAndInheritedMethods.includes(upgradeToSignature)) {
+  if (
+    !selfAndInheritedMethods.includes(upgradeToSignature) &&
+    !selfAndInheritedMethods.includes(upgradeToAndCallSignature)
+  ) {
     errors.push({
       src: c.src,
       kind: 'missing-public-upgradeto',

diff --git a/packages/core/src/validate/report.ts b/packages/core/src/validate/report.ts
@@ -57,9 +57,10 @@ const errorInfo: ErrorDescriptions<ValidationError> = {
     hint: () => `Update your dependency and run again`,
   },
   'missing-public-upgradeto': {
-    msg: () => `Implementation is missing a public \`upgradeTo(address)\` function`,
+    msg: () =>
+      `Implementation is missing a public \`upgradeTo(address)\` or \`upgradeToAndCall(address,bytes)\` function`,
     hint: () =>
-      `Inherit UUPSUpgradeable to include this function in your contract\n` +
+      `Inherit UUPSUpgradeable to include one or both of these functions in your contract\n` +
       `    @openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol`,
     link: 'https://zpl.in/upgrades/error-008',
   },