Skip to content
This repository has been archived by the owner on Jul 11, 2019. It is now read-only.

Provide a way to check two contracts share storage layout #63

Closed
frangio opened this issue Apr 23, 2018 · 7 comments
Closed

Provide a way to check two contracts share storage layout #63

frangio opened this issue Apr 23, 2018 · 7 comments
Assignees
Labels
source:user-feedback topic:security Security reviews and considerations topic:tools Off-chain tooling in general topic:upgradeability Upgreadeability for user contracts
Milestone

Comments

@frangio
Copy link
Contributor

frangio commented Apr 23, 2018

One of the prerequisites for an upgrade to be correct is that the underlying Solidity implementations have the same storage layout. This means that the contracts have the same state variables in the same places, and if there are new state variables in the upgrade they shouldn't conflict with the storage of the state variables in the previous implementation.

We should provide a script to check this. I've begun working on it but I'm blocked by some things about the inheritance graph linearization that I'm waiting to discuss with a few people.

@frangio frangio added topic:tools Off-chain tooling in general topic:upgradeability Upgreadeability for user contracts topic:security Security reviews and considerations labels Apr 23, 2018
@frangio frangio self-assigned this Apr 23, 2018
@frangio frangio added this to the v0.1.0 (Kernel MVP) milestone Apr 24, 2018
@maraoz maraoz modified the milestones: v0.1.0 (Kernel MVP), v0.2.0 May 14, 2018
@maraoz
Copy link
Contributor

maraoz commented May 14, 2018

@frangio moved this out of MVP, unless it's close to being done.

@frangio
Copy link
Contributor Author

frangio commented May 15, 2018

I had a script that was almost done but I accidentally deleted it two weeks ago. 😢

In any case I had stopped working on it after @spalladino created ethereum/solidity#4017 which I think is a better approach.

@spalladino
Copy link
Contributor

That issue seems to be a bit on hold. We may want to resume the discussion with the Solidity maintainers to make a case for that feature to be accepted.

@maraoz
Copy link
Contributor

maraoz commented May 15, 2018

@frangio you should have said "I accidentally killed it"

@spalladino
Copy link
Contributor

We are not getting much feedback from the Solidity folks, so we may need to implement this outside the compiler.

@frangio frangio modified the milestones: Backlog, v2.0.0 Jul 23, 2018
@frangio
Copy link
Contributor Author

frangio commented Jul 23, 2018

We've found that the AST includes the linearization of the inheritance graph. This is info that we can already use for this check. In fact with the storage layout of each separate contract plus the linearization we have nearly everything we need.

@spalladino
Copy link
Contributor

Closing in favour of OpenZeppelin/openzeppelin-sdk#37

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
source:user-feedback topic:security Security reviews and considerations topic:tools Off-chain tooling in general topic:upgradeability Upgreadeability for user contracts
Projects
None yet
Development

No branches or pull requests

3 participants