Bad storage gap resize when adding variables and reducing gap

Hi! Which version of @openzeppelin/hardhat-upgrades is supporting storage gaps? I'm using version 1.28.0 but I still see this error when try to upgrade a contract already deployed.
I've added two uint256 variables and reduced the __gap size by 2 but I'm getting the error

Error: New storage layout is incompatible

contracts/IdleCDOStorage.sol:120: Inserted `lossToleranceBps`
  > New variables should be placed after all existing inherited variables

contracts/IdleCDOStorage.sol:136: Upgraded `__gap` to an incompatible type
  - Bad storage gap resize from 48 to 46
    Size decrease must match with corresponding variable inserts

the reference storage file is https://github.com/Idle-Labs/idle-tranches/blob/master/contracts/IdleCDOStorage.sol#L120-L136 and I'm trying to upgrade this contract https://github.com/Idle-Labs/idle-tranches/blob/master/contracts/polygon-zk/IdleCDOPolygonZK.sol which have no storage on its own and is inheriting another contract with no additional storage declared after IdleCDOStorage.sol. This is the OZ manifest entry for that contract saved in the last deployment https://github.com/Idle-Labs/idle-tranches/blob/master/.openzeppelin/unknown-1101.json#L491-L809

I'm trying to do the upgrade via the hre.upgrades.prepareUpgrade method

I managed to deploy the contract regardless using the unsafeSkipStorageCheck: true option but I'm waiting for the actual upgrade. I'm pretty sure it's safe, but would be great to have a couple more eyes on this @ericglau @frangio. I checked the storage configuration with foundry's cast storage command and seems fine there too, any thoughts on why this is happening?

Hi @bugduino,

  • Do you have the source code available for the previous version of the contract? (I understand that you only added 2 variables, but would suggest double-checking whether there are any other storage changes such as in inherited contracts).
  • Can you share the manifest entry for the new version of the contract? (This could be from a local dev network where the manifest is in a temp directory).
1 Like

Yeah here you have both @ericglau

OLD version of the code: https://zkevm.polygonscan.com/address/0x2361130282a24421D9fdf2d1072C8edE2a79F108#code
NEW version of the code: https://zkevm.polygonscan.com/address/0x5d4E705315ACa451Db40bf7c067077C768B3FFd0#code

OLD manifest entry: https://github.com/Idle-Labs/idle-tranches/blob/master/.openzeppelin/unknown-1101.json#L496-L810
NEW manifest entry: https://github.com/Idle-Labs/idle-tranches/blob/master/.openzeppelin/unknown-1101.json#L850-L1176 (this was generated with the unsafeSkipStorageCheck: true option and refers to the new code above)

There is only a constant in between but this should be inlined in the bytecode at compile time and not affect storage layout right?

Thanks for sharing those. I don't see any storage layout issues between those versions.

I tried the following:

  1. Copied the verified source code of OLD version into a new file contracts/V1.sol
  2. Copied the verified source code of NEW version into a new file contracts/V2.sol
  3. Ran a Hardhat test with the following:
    const V1 = await ethers.getContractFactory("contracts/V1.sol:IdleCDOPolygonZK");   
    const V2 = await ethers.getContractFactory("contracts/V2.sol:IdleCDOPolygonZK");
    await upgrades.validateUpgrade(V1, V2);

This passed the storage layout checks. (And if I change the gap in V2 to be the wrong size, it would give an error).

1 Like

which version of the packages are you using? Thanks for the test btw

I tried your script and is not raising any errors but If I do the prepareUpgrade with this

    const V1 = await hre.ethers.getContractFactory("contracts/old.sol:IdleCDOPolygonZK", signer);
    const V2 = await hre.ethers.getContractFactory("contracts/new.sol:IdleCDOPolygonZK", signer);
    // const res = await hre.upgrades.validateUpgrade(V1, V2); // this does not raise any error
    let impl = await hre.upgrades.prepareUpgrade('0x8890957F80d7D771337f4ce42e15Ec40388514f1', V2, { kind: 'transparent' });

where 0x8890957F80d7D771337f4ce42e15Ec40388514f1 is an address for a proxy with V1 implementation (OLD) that we deployed on polygon zkEVM I'm still getting the initial error. Is there any differences in the underlying methods used in prepareUpgrade ?

I see the same error when following your steps above. We will look into this and get back to you.

1 Like

This is caused by a bug in the plugin where solidity settings overrides in the Hardhat configuration resulted in storage layout details to be missing from the network file. For example, we can see that the storage layouts in your network file are missing slot and offset fields, and the storage gap validation cannot be performed correctly without these. Opened GitHub issue https://github.com/OpenZeppelin/openzeppelin-upgrades/issues/850 -- we will work on a fix and provide steps afterwards to correct your network file.

However, the passing validateUpgrade results mentioned above are correct, and your new version is indeed using the storage gap correctly.

1 Like

@bugduino This issue is fixed in @openzeppelin/hardhat-upgrades@2.0.2 so that any new entries written to network files will contain the required storage layout details. Note that v2.x of the plugin uses Ethers v6, while your project is on Ethers v5 so this will require some updates in your project to use the latest version of Ethers. Let me know if that is a concern.

After updating the plugin to the above version, please do the following in order to update your unknown-1101.json network file. This will allow future upgrades to work properly, using this network file for storage layout comparisons.

  1. Delete the "632bff08239289c26b134bde9903cd0c9c7d97eb54b169d869888e49bf7f3475" and "06d417120603d72bbe5849b26dab0b3da36ff8e0ba67fc308988730685eb7b75" JSON entries from the unknown-1101.json file, including all of their child elements.
    These entries correspond to your implementations at addresses 0x2361130282a24421D9fdf2d1072C8edE2a79F108 and 0x5d4E705315ACa451Db40bf7c067077C768B3FFd0, respectively.

  2. In a new Hardhat script, run the following:

await upgrades.forceImport('0x2361130282a24421D9fdf2d1072C8edE2a79F108', V1);
await upgrades.forceImport('0x5d4E705315ACa451Db40bf7c067077C768B3FFd0', V2);

where V1 and V2 are the Ethers contract factories of your implementation contracts using the source code that was deployed to those corresponding addresses.

(I noticed that you already upgraded your proxy to the V2 contract, so if you are no longer using the V1 contract anywhere, then it's sufficient to only import the V2 contract above.)

  1. Commit the updated unknown-1101.json file to source control.

Feel free to share the updated unknown-1101.json here if you'd like me to confirm whether it looks correct. Thanks!

1 Like

Thanks for the update @ericglau much appreciated!

You are correct we use ethers v5, I'm not sure what we will need to update to v6 (I'm a bit worried :sweat_smile:) will check out if it's straightforward or not.
I'll let you know if we manage to update and share the manifest file.

Will mark your comment as a solution in the meantime.

1 Like