Wrong indication of incompatible layout after upgrade

Support Upgrades
1

Hi, I have situation like below, so I have

layout:

uint256
Data (struct)

I want to add to Data struct at the end, additional variable uint256 data3

Running this with upgrades raises error that layout is not compatible. Is this expected behaviour? As I see it would not affect any layout as information is added at the end of Data struct and Data struct is last variable in storage.

contracts/test/TestContractV2.sol:19: Upgraded `data` to an incompatible type
  - Bad upgrade from struct TestContract.Data to struct TestContractV2.Data
  - In struct TestContractV2.Data
    - Added `data3`

Thank you in advance.

:1234: Code to reproduce

// INITIAL CONTRACT LAYOUT
contract TestContract is OwnableUpgradeable {
    struct Data {
        uint256 data1;
        uint256 data2;
    }
    /// @custom:storage-location erc7201:openzeppelin.storage.TestContract
    struct TestContractStorage {
        string _version;
        Data data;
    }

    // keccak256(abi.encode(uint256(keccak256("f.storage.TestContract")) - 1)) & ~bytes32(uint256(0xff))
    bytes32 private constant TestContractStorageLocation =
        0xca9ab86016606c0de0fd52d033f885973b1d29b412e722f528ec4c1b4b023800;

    function _getTestContractStorage()
        private
        pure
        returns (TestContractStorage storage $)
    {
        assembly {
            $.slot := TestContractStorageLocation
        }
    }

}

// UPGRADE
contract TestContractV2 is OwnableUpgradeable {
    struct Data {
        uint256 data1;
        uint256 data2;
        uint256 data3;
    }
    /// @custom:storage-location erc7201:openzeppelin.storage.TestContract
    struct TestContractStorage {
        string _version;
        Data data;
    }

    // keccak256(abi.encode(uint256(keccak256("f.storage.TestContract")) - 1)) & ~bytes32(uint256(0xff))
    bytes32 private constant TestContractStorageLocation =
        0xca9ab86016606c0de0fd52d033f885973b1d29b412e722f528ec4c1b4b023800;

    function _getTestContractStorage()
        private
        pure
        returns (TestContractStorage storage $)
    {
        assembly {
            $.slot := TestContractStorageLocation
        }
    }

:computer: Environment

hardhat-upgrades 3.9.0 . transparent proxy

1 Like
2

If I understand you correctly, you want to add one more variable to one of your structures. That's a bad idea.
I would recommend that you add the variable at the end:

   struct Data {
        uint256 data1;
        uint256 data2;
    }

    /// @custom:storage-location erc7201:openzeppelin.storage.TestContract
    struct TestContractStorage {
        string _version;
        Data data;
    }

    // New variables should be added here
    uint256 public newVariable;

If you are in a development period, deploy new upgradeable contracts if you want to be in struct ones.

Therefore, when creating contracts, it is VERY IMPORTANT to be clear about what data structures you will need. You cannot add properties on demand.

In case the properties may change in the future, convert them into individual mappings, so you can easily add as many as you like.

1 Like
3

Thank you for reply. I understand what you are saying, but this is if I would go with _gap approach. But I am using erc7201 standard for creating proxy storage layouts and this standard should be compatible with upgrades plugin validation.

1 Like
4

I don't understand what you mean by ‘with _gap approach. ‘.
The ERC721 standard has nothing to do with the requirement to correlate new variables in an updateable contact.

Introducing a variable into an existing structure is not something that is recommended, nor does it say that you can't, although I have no recollection that this can be done.

Perhaps someone else can shed more light about.

5

I am referring to this:

which is what upgrades plugin supports

2 Likes
6

This looks like a valid scenario, and the error seems to be an issue with the validations. Opened an issue here: https://github.com/OpenZeppelin/openzeppelin-upgrades/issues/1136

7

I have checked my upgradable smart contracts and yes, I have added at the end of already stored structures a new variable without problems.

But I see what here is asking about is the use of @custom:storage-location which, if I understand correctly, one stores the variable outside the memory tree structure.

Now I open question.

Adding variables to the end of the structure has never given me problems but I have not used @custom:storage-location in those additions.
Is this necessary?
Why has it worked for me?
@ericglau