Upgrade plugin reports unsafe upgrade between t_string_memory and t_string_memory_ptr

I am currently trying to upgrade an old TransparentProxy smart contract (not UUPS or Beacon) on Ethereum, the last deployed logic contract used solidity version 0.5.16, and the new versions I am about to deploy use 0.8.17. While mocking the upgrade using hardhat with mainnet forking, I got an Error: New storage layout is incompatible message from the openzeppelin upgrades module. It reported for four different values the following error:

Upgraded `controlledBridgeTokens` to an incompatible type
  - In key of mapping(string => address)
    - Bad upgrade from undefined to string

The only change to these variables was to move them from public to private. The order of the variables has not changed, and the types were not changed. In the old contract, the code was as follows:

    /**
    * @notice mapping of symbols to token addresses
    */
    mapping(string => address) controlledBridgeTokens;
    

and on the new contract, it looks like this:

  /**
   * @dev {DEPRECATED}
   */
  mapping(string => address) private controlledBridgeTokens;

The ordering of inheritance has not changed; there are values before and after these variables that do not show an incompatible storage layout error.

To try and understand what was happening, I debugged the plugin code and saw it was grabbing the type t_string_memory from the manifest file, but the type it was getting from the newly compiled contract was t_string_memory_ptr. It appears to me that these are the same types and have just been renamed. Replacing all instances of t_string_memory with t_string_memory_ptr in the manifest removes the error from the plugin. There do not appear to be any storage layout changes that I can see from manually checking the values. Still, I have not seen this documented anywhere and wanted to confirm that what I am doing is safe.
The way I see it, there are two possibilities, either the types were renamed at some point, and I have an old manifest file, or the type t_string_memory and t_string_memory_ptr is different. If the type was renamed, was this change documented somewhere? If, instead, they are incompatible and have different types, what is likely to cause solidity to generate a different type here?

:computer: Environment

Using Hardhat 2.8.4 with @openzeppelin/hardhat-upgrades 1.11.0. Running on Debian Sid.

These type identifiers come from Solidity and look identical in terms of storage layout (they both have "numberOfBytes": "32"). You can further confirm/test in your project that they are functionally the same.

Opened issue https://github.com/OpenZeppelin/openzeppelin-upgrades/issues/689 to address this in the plugins.

This is now fixed in the upgrades plugins. Please update the @openzeppelin/upgrades-core package to the latest version e.g.:

npm install --save-dev @openzeppelin/upgrades-core@latest
1 Like