Validate storage layout

[spalladino]

This is a new iteration of #92, which github is not allowing me to reopen.

---

When running zos-push, validate that the storage layout is compatible with the previous version of the contract, and abort push otherwise. All warnings are displayed to the user, eg:

$ zos push
Validating contract AnotherImplV1 before push
Variable 'uint256 value22' in contract ImplV1 was renamed to foo in contracts/mocks/ImplV1.sol:1.
Note that the new variable foo will have the value of value22 after upgrading. If this is not the desired behavior, add a new variable foo at the end of your contract instead.
New variable 'uint256 bar' was added in contract ImplV1 in contracts/mocks/ImplV1.sol:1
This pushes all variables after bar to a higher position in storage, causing the updated contract to read incorrect initial values. Only add new variables at the end of your contract to prevent this issue.

Storage layout is extracted from the contract's AST by the Storage class in lib, and is stored along with other contract data (such as bytecode hash) in the zos.network.json file. It is then retrieved for comparison on the next zos push. The format is a tuple (storage, types), similar to the format discussed in ethereum/solidity#4017, where storage contains the array of variables, with references to the types dictionary with detailed info on each type.

Comparison is handled by the Layout class in lib, and uses a modified levenshtein algorithm for calculating the minimum change between both storages. Changes are identified as additions (either at the end or mid-contract, with additions at the end computed at zero cost for the algorithm), deletions (end or mid-contract), or substitutions (renames, typechanges, or both).

Fixes #37
Spawns issues #112, #113, and #114

TBD: Should we add a feature toggle to disable it until the 3 issues above are ready?

[facuspagnuolo]

@spalladino I think we should add a warning message in case we indentify that an enum or a struct is being used, at least until #113 and #114 are solved. Do you agree?

[spalladino]

I'm afraid it could be a bit annoying, since any warnings halt the push operation altogether. How about showing the warning, but letting the operation go through? And it's always reversible if something is indeed broken.

[facuspagnuolo]

@spalladino yes, I meant to do it that way, I think that simply printing a message to let the user know we are not performing full validations on structs and enums is ok for now, otherwise it could be a false positive

[spalladino]

@facuspagnuolo added the following warning:

- Variable foo4 (AnotherImplV1) contain a struct or enum type, which are not being compared for layout changes in this version. Double-check that the storage layout of these types was not modified in the updated contract. Read more at https://docs.zeppelinos.org/docs/advanced.html#preserving-the-storage-structure.

[facuspagnuolo]

LGTM. Left some minor comments

[facuspagnuolo]

this would mean that contracts of different versions are going to be always pushed?

[spalladino]

Yes, at least until we implement zeppelinos/zos-cli#94

[facuspagnuolo]

If this works for every scenario where this function is called, I'll remove the artifacts parameter.

[spalladino]

TBH I'd rather keep it for simplicity of use. I wouldn't rule out extracting these methods for external usage, so I'd prefer the API to be as easy to use as possible.

[facuspagnuolo]

fs cannot solve this right?

[spalladino]

Not at the moment. I could move it to fs later, though.

[facuspagnuolo]

@spalladino I think there are some integration tests failing, would you mind looking at them?

[spalladino]

It was the StatusFetcher/StatusComparator, due to the PR we merged recently that re-enabled them. I'll look into this later, but I really want to get this PR merged!