Automatic checking of storage compatibility of structs and enums

docs/modules/ROOT/pages/api-hardhat-upgrades.adoc

@@ -10,8 +10,8 @@ Creates a proxy given an ethers contract factory to use as implementation, and r
 Options are:
 
 * `initializer`: set a different initializer function to call, or specify `false` to disable initialization
-* `unsafeAllowCustomTypes`: allows a deployment where structs or enums are used in the implementation contract (required since xref:faq.adoc#what-does-it-mean-for-an-implementation-to-be-compatible[storage compatibility validations] do not handle custom types, so make sure the change you are introducing is safe)
 * `unsafeAllowLinkedLibraries`: allows a deployment with external libraries linked to the implementation contract (required since the plugins do not support xref:faq.adoc#why-cant-i-use-external-libraries[external libraries] yet)
+* `unsafeAllowCustomTypes`: an override for a few situations where structs or enums cannot be automatically verified for safety
 
 [source,ts]
 ----
@@ -29,8 +29,8 @@ Upgrades a proxy at a specified address to a new implementation contract, and re
 
 Options are:
 
-* `unsafeAllowCustomTypes`: allows an upgrade where structs or enums are used in the implementation contract (required since xref:faq.adoc#what-does-it-mean-for-an-implementation-to-be-compatible[storage compatibility validations] do not handle custom types, so make sure the change you are introducing is safe)
 * `unsafeAllowLinkedLibraries`: allows an upgrade with external libraries linked to the implementation contract (required since the plugins do not support xref:faq.adoc#why-cant-i-use-external-libraries[external libraries] yet)
+* `unsafeAllowCustomTypes`: an override for a few situations where structs or enums cannot be automatically verified for safety
 
 [source,ts]
 ----
@@ -48,8 +48,8 @@ Validates and deploys a new implementation contract, and returns its address. Us
 
 Options are:
 
-* `unsafeAllowCustomTypes`: allows an upgrade where structs or enums are used in the implementation contract (required since xref:faq.adoc#what-does-it-mean-for-an-implementation-to-be-compatible[storage compatibility validations] do not handle custom types, so make sure the change you are introducing is safe)
 * `unsafeAllowLinkedLibraries`: allows an upgrade with external libraries linked to the implementation contract (required since the plugins do not support xref:faq.adoc#why-cant-i-use-external-libraries[external libraries] yet)
+* `unsafeAllowCustomTypes`: an override for a few situations where structs or enums cannot be automatically verified for safety
 
 [source,ts]
 ----

docs/modules/ROOT/pages/api-truffle-upgrades.adoc

@@ -13,8 +13,8 @@ Options for this function are:
 
 * `initializer`: set a different initializer function to call, or specify `false` to disable initialization
 * `deployer`: set as the Truffle migration deployer during migrations
-* `unsafeAllowCustomTypes`: allows a deployment where structs or enums are used in the implementation contract (required since xref:faq.adoc#what-does-it-mean-for-an-implementation-to-be-compatible[storage compatibility validations] do not handle custom types, so make sure the change you are introducing is safe)
 * `unsafeAllowLinkedLibraries`: allows a deployment with external libraries linked to the implementation contract (required since the plugins do not support xref:faq.adoc#why-cant-i-use-external-libraries[external libraries] yet)
+* `unsafeAllowCustomTypes`: an override for a few situations where structs or enums cannot be automatically verified for safety
 
 [source,ts]
 ----
@@ -33,8 +33,8 @@ Upgrades a proxy at a specified address to a new implementation contract, and re
 Options for this function are:
 
 * `deployer`: set as the Truffle migration deployer during migrations
-* `unsafeAllowCustomTypes`: allows an upgrade where structs or enums are used in the implementation contract (required since (required since xref:faq.adoc#what-does-it-mean-for-an-implementation-to-be-compatible[storage compatibility validations] do not handle custom types, so make sure the change you are introducing is safe)
 * `unsafeAllowLinkedLibraries`: allows an upgrade with external libraries linked to the implementation contract (required since the plugins do not support xref:faq.adoc#why-cant-i-use-external-libraries[external libraries] yet)
+* `unsafeAllowCustomTypes`: an override for a few situations where structs or enums cannot be automatically verified for safety
 
 [source,ts]
 ----
@@ -53,8 +53,8 @@ Validates and deploys a new implementation contract, and returns its address. Us
 Options are:
 
 * `deployer`: set as the Truffle migration deployer during migrations
-* `unsafeAllowCustomTypes`: allows an upgrade where structs or enums are used in the implementation contract (required since (required since xref:faq.adoc#what-does-it-mean-for-an-implementation-to-be-compatible[storage compatibility validations] do not handle custom types, so make sure the change you are introducing is safe)
 * `unsafeAllowLinkedLibraries`: allows an upgrade with external libraries linked to the implementation contract (required since the plugins do not support xref:faq.adoc#why-cant-i-use-external-libraries[external libraries] yet)
+* `unsafeAllowCustomTypes`: an override for a few situations where structs or enums cannot be automatically verified for safety
 
 [source,ts]
 ----

docs/modules/ROOT/pages/faq.adoc

@@ -41,7 +41,7 @@ Both plugins will validate that the contract you are trying to deploy complies w
 
 When upgrading a proxy from one implementation to another, the _storage layout_ of both implementations must be compatible. This means that, even though you can completely change the code of the implementation, you cannot modify the existing contract state variables. The only operation allowed is to append new state variables after the ones already declared.
 
-Both plugins will validate that the new implementation contract is compatible with the previous one. However, the plugins currently do not support validating custom types (enums or structs). To force a deployment where custom types are involved, set the `unsafeAllowCustomTypes` flag to true in the `deployProxy` or `upgradeProxy` calls.
+Both plugins will validate that the new implementation contract is compatible with the previous one.
 
 You can read more about how to make storage-compatible changes to an implementation contract xref:upgrades::writing-upgradeable.adoc#modifying-your-contracts.adoc[here].
 
@@ -86,13 +86,11 @@ In the meantime you can deploy upgradeable contracts linked to external librarie
 You can follow or contribute to https://github.com/OpenZeppelin/openzeppelin-upgrades/issues/52[this issue in Github].
 
 [[why-cant-i-use-custom-types]]
-== Why can't I use custom types like structs and enums?
+== Can I use custom types like structs and enums?
 
-At the moment the plugins do not support upgradeable contracts that implement or make use of custom types like structs or enums in their code or linked libraries. This is because of the additional complexity of checking for storage layout incompatibilities during an upgrade. (See <<what-does-it-mean-for-an-implementation-to-be-compatible,"What does it mean for an implementation to be compatible?">>.)
+Past versions of the plugins did not support upgradeable contracts that used custom types like structs or enums in their code or linked libraries. This is no longer the case for current versions of the plugins, and structs and enums will be automatically checked for compatibility when upgrading a contract.
 
-In the meantime, we encourage users to either avoid using these kind of types or to manually check for xref:upgrades::writing-upgradeable.adoc#modifying-your-contracts[storage incompatibilities] and make use of the `unsafeAllowCustomTypes` flag available for the `deployProxy`, `upgradeProxy` and `prepareUpgrade` functions. If you're unsure about how to do this manual check, we'll be happy to help out with your situation if you https://forum.openzeppelin.com[post in the forum].
-
-You can follow or contribute to https://github.com/OpenZeppelin/openzeppelin-upgrades/issues/95[this issue in Github].
+Some users who have already deployed proxies with structs and/or enums and who need to upgrade those proxies may need to use the override flag `unsafeAllowCustomTypes` for their next upgrade, after which it will no longer be necessary. If the project contains the source code for the implementation currently in use by the proxy, the plugin will attempt to recover the metadata that it needs before the upgrade, falling back to the override flag if this is not possible.
 
 [[why-do-i-have-to-recompile-all-contracts-for-truffle]]
 == Why do I have to recompile all contracts for Truffle?

packages/core/ava.config.js

@@ -2,4 +2,7 @@ export default {
   verbose: true,
   ignoredByWatcher: ['**/*.{ts,map,tsbuildinfo}', 'artifacts', 'cache'],
   typescript: { rewritePaths: { 'src/': 'dist/' } },
+  environmentVariables: {
+    FORCE_COLOR: '0',
+  },
 };

packages/core/contracts/test/ManifestMigrate.sol

@@ -0,0 +1,73 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.6.8;
+
+contract ManifestMigrateLayout {
+    struct T {
+        bool b;
+    }
+
+    struct S {
+        uint x;
+        string s;
+        T t;
+    }
+
+    enum E {
+        E1,
+        E2
+    }
+
+    uint x1;
+    T t1;
+    S s1;
+    string s2;
+    bool b1;
+    E e1;
+}
+
+contract ManifestMigrateUnique is ManifestMigrateLayout {
+    function useAll() external {
+        x1 = 5;
+        t1 = T({ b: true });
+        s1 = S({ x: 6, s: "ok", t: t1 });
+        s2 = "no";
+        b1 = false;
+        e1 = E.E2;
+    }
+}
+
+contract ManifestMigrateUnambiguous0 is ManifestMigrateLayout {
+    function useAll() external {
+        x1 = 10;
+        t1 = T({ b: false });
+        s1 = S({ x: 6, s: "string", t: t1 });
+        s2 = "ok";
+        b1 = false;
+        e1 = E.E1;
+    }
+}
+
+// These two are expected to have the same bytecode modulo metadata.
+contract ManifestMigrateUnambiguous1 is ManifestMigrateUnambiguous0 { }
+contract ManifestMigrateUnambiguous2 is ManifestMigrateUnambiguous0 { }
+
+// These two are expected to have the same bytecode modulo metadata and similar
+// layout, but different types (see struct D members);
+contract ManifestMigrateAmbiguous1 is ManifestMigrateUnique {
+    struct D {
+        uint w;
+    }
+    D d1;
+    function test() external {
+        d1.w += 1;
+    }
+}
+contract ManifestMigrateAmbiguous2 is ManifestMigrateUnique {
+    struct D {
+        uint z;
+    }
+    D d1;
+    function test() external {
+        d1.z += 1;
+    }
+}