Replace unused variable in struct with others

Support Upgrades
1

I have a struct which has various members and takes up a few storage slots, the first one is packed with many members, but I simplified it below to demonstrate it easier. I have an unused variable version that is always not set so should be able to replace the packed storage with another variable but have been unable to do so:

:1234: Code to reproduce

Before:

struct Player {
  // These variables pack
  uint8 var1;
  uint8 version; // Not used currently (always 0)
  uint[] arr; // New storage slot
}

enum Skill { ... }

Desired:

  uint8 var1;
  Skill currentActionProcessedSkill3; // an enum
  uint24 currentActionProcessedXPGained3;
  uint8 version;
  uint[] arr;

Error produced:

  - In struct Player
    - Inserted `currentActionProcessedSkill3`
      > New struct members should be placed after existing ones

I have tried removing version:

  uint8 var1;
  Skill currentActionProcessedSkill3;
  uint24 currentActionProcessedXPGained3;
  uint[] arr;

Error produced:

  - In struct Player
    - Inserted `currentActionProcessedSkill3`
      > New struct members should be placed after existing ones
    - Replaced `version` with `currentActionProcessedXPGained3` of incompatible type

I have tried removing version and using /// @custom:oz-retyped-from uint8 for the Skill:

  uint8 var1;
  /// @custom:oz-retyped-from uint8
  Skill currentActionProcessedSkill3;
  uint24 currentActionProcessedXPGained3;
  uint[] arr;

Error produced:

  - In struct Player
    - Inserted `currentActionProcessedSkill3`
      > New struct members should be placed after existing ones
    - Replaced `version` with `currentActionProcessedXPGained3` of incompatible type

The only thing that works is this, adding them afterwards, but this is not desirable because this storage slot is very packed and I may not use version so have been leaving it till the end so that it can be replaced if there's something else I want to use:

  uint8 var1;
  uint8 version;
  Skill currentActionProcessedSkill3; // an enum
  uint24 currentActionProcessedXPGained3;
  uint[] arr;

:computer: Environment

Hardhat 0.8.19 solidity version

2

The way I solved it was to modify the .openzeppelin generated files (a hack) and remove version but would like to know if there is a proper way

3

The proper way is to do the following in the new implementation:

  uint8 var1;
  /// @custom:oz-renamed-from version
  /// @custom:oz-retyped-from uint8
  Skill currentActionProcessedSkill3; // an enum
  uint24 currentActionProcessedXPGained3;
  uint8 version;
  uint[] arr;

This says that you want to rename and retype your existing version variable to convert it into currentActionProcessedSkill3. Then the plugin allows packing additional variables into unused space within a storage slot, so the currentActionProcessedXPGained3 and version variables after that are considered new variables within the same slot.

4

Thanks for the reply, so I did try that before without success, I think the issue is because it is part of a struct, you can see the errors reported above are actually referring to the variable below it (currentActionProcessedXPGained3) for some reason.

I have managed to simplify this:

V1:

    struct S {
        uint64 a;
        uint8 bFails; // <--- Type cannot be changed
        uint8 cRenameFails; // <--- Variable cannot be renamed
    }

V2

    struct S {
        uint64 a;
        /// @custom:oz-retyped-from uint8
        bool bFails; // <--- Type cannot be changed
        /// @custom:oz-renamed-from cRenameFails
        uint8 cChangedName; // <--- Variable cannot be renamed
    }

Trying to upgrade results in this error:

  - Bad upgrade from struct Lock.S to struct LockV2.S
  - In struct LockV2.S
    - Upgraded `bFails` to an incompatible type
      - Bad upgrade from uint8 to bool
    - Renamed `cRenameFails` to `cChangedName`

But doing the same with the same variables as normal state variable works ok with the oz-* comments.

Please see this repo which should show the reproducible issue:

5

Hi @0xSamWitch, thanks for those details. The plugin does not support rename/retype annotations within structs at the moment, due to a limitation of solidity: https://github.com/ethereum/solidity/issues/12295. But you can use a retype annotation on the actual variable where you use the struct.

For example:

contract Lock is UUPSUpgradeable, OwnableUpgradeable {
    struct S {
        uint64 a;
        uint8 bFails;
        uint8 cRenameFails;
    }

    S s;
    ...

contract LockV2 is UUPSUpgradeable, OwnableUpgradeable {
    struct S {
        uint64 a;
        bool bFails;
        uint8 cChangedName;
    }

    /// @custom:oz-retyped-from Lock.S
    S s;
    ...

Note that by doing this, it does not perform storage layout comparisons within the struct, so you should manually ensure that they are compatible.

I've also opened an issue here to track this.

6

ah ok, I'll probably just continue editing .openzeppelin network json file directly file in that case :smiley: thanks for the info