openzeppelin-sdk icon indicating copy to clipboard operation
openzeppelin-sdk copied to clipboard

Storage layout validation errors are shown on create instead of upgrade

Open spalladino opened this issue 5 years ago • 3 comments

Reported by @abcoathup. Storage layout errors are thrown when running create on the new version on the contract, instead on upgrading. Steps to reproduce:

Run npx oz create A

pragma solidity ^0.5.0;
contract A {
    uint256 public a;
}

Edit the contract to add a variable

pragma solidity ^0.5.0;
contract A {
    uint256 public b;
    uint256 public a;
}

Running npx oz create again gives validation errors, shouldn't errors only be given for oz upgrade?

$ npx oz create
✓ Compiled contracts with solc 0.5.16 (commit.9c3226ce)
? Pick a contract to instantiate A
? Pick a network development
- New variable 'uint256 b' was inserted in contract A in contracts/A.sol:1. You should only add new variables at the end of your contract.
See https://docs.openzeppelin.com/sdk/2.6/writing-contracts#modifying-your-contracts for more info.
One or more contracts have validation errors. Please review the items listed above and fix them, or run this command again with the --force option.

This is caused by all validation checks being done on push, which is called behind the scenes to update all implementation contracts whenever create or upgrade is called. We should review this issue for 3.0, in case we effectively deprecate push.

spalladino avatar Feb 06 '20 17:02 spalladino

Hi!

Experiencing this currently during development.

  1. Created a contract. Deployed upgradeable contract to dev network using deploy.
  2. Turn off dev chain.
  3. Add new variable to contract (not at the end).
  4. Create new dev chain.
  5. deploy throws the validation error that a new variable was added.

To my mind, this shouldn't happen, same as create since it is a clean deploy to a fresh chain. Should only happen on upgrade as mentioned above.

The only way to fix this is to delete .openzeppelin and then init again. Are there other ways to work around this for the time being?

simondlr avatar May 08 '20 09:05 simondlr

Are there other ways to work around this for the time being?

If you spin up a new dev chain with a different id, then oz will recognize it as a different chain altogether, and not throw that warning. In case you want to reuse that particular id, instead of nuking the entire .openzeppelin folder, you can just remove the file .openzeppelin/dev-NETWORKID.json.

Alternatively, if you want for oz to just forget about the storage layout of a deployed contract, you can remove the storage info for that contract in the .openzeppelin/dev-NETWORKID.json file. Hope this helps!

spalladino avatar May 11 '20 13:05 spalladino

Thanks @spalladino. This helps. :)

simondlr avatar May 12 '20 14:05 simondlr