Document use of retyped comments for incompatible enums

[soccerarenadev]

When upgrading a "enum" type in my contract, I have these cases:

Base enum enum MyEnum { AA, BB }

• Keep enum size the same but change the names. Example: new enum: enum MyEnum { AA, BC } This is marked as invalid though it should be valid.
• Shrink enum size. Example: new enum: enum MyEnum { AA } This is marked as invalid. I "could" be valid
• Grow enum size. Example: new enum: enum MyEnum { AA, BB, CC } This is marked as valid.

However, I think that being all uint8 internally, there is no incompatible types, and they should all be valid.

[frangio]

Can you provide two sample contracts A and B that are marked as invalid when you try to upgrade from A to B?

[frangio]

The problem at the core here is that our definition of "valid" is not simply that an upgrade will not corrupt the storage layout, we also try to catch problems that might lead to corruption of the storage semantics now or in the future. The reason we don't allow removal or reordering of enums is that instances of those values may already be in storage. If you have a mapping (uint => Status) with enum Status { Open, Closed }, and you upgrade a contract where the enum Status is reordered to enum Status { Closed, Open }, all of the Status instances that are already in storage will change to the opposite meaning!

[soccerarenadev]

The problem at the core here is that our definition of "valid" is not simply that an upgrade will not corrupt the storage layout, we also try to catch problems that might lead to corruption of the storage semantics now or in the future. The reason we don't allow removal or reordering of enums is that instances of those values may already be in storage. If you have a mapping (uint => Status) with enum Status { Open, Closed }, and you upgrade a contract where the enum Status is reordered to enum Status { Closed, Open }, all of the Status instances that are already in storage will change to the opposite meaning!

But they are "compatible types". Maybe this should be called a different way, and allow a different unsafeSkipOption, other than unsafeSkipStorageCheck.

[frangio]

Here's a trick, you can use the "retyped" feature for layout-compatible types. In the case of enums, if you're getting an error from the following variable:

contract C {
    enum MyEnum { AA, BC }

    MyEnum e; // error here

You can write this to pass the checks:

contract C {
    enum MyEnum { AA, BC }

    /// @custom:oz-retyped-from enum C.MyEnum
    MyEnum e;

In the retyped comment, enum C.MyEnum should describe the original type. Here I'm assuming the original type had the same name and was defined inside a contract C. You will see this type in the validation report in the console. If you see:

Bad upgrade from enum C.MyEnum to enum C.MyEnum

This means that you should write the comment as I put in the example above.