Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unexpected storage layout caused by interfaces #13332

Closed
cyraxred opened this issue Jul 29, 2022 · 7 comments
Closed

Unexpected storage layout caused by interfaces #13332

cyraxred opened this issue Jul 29, 2022 · 7 comments
Labels
breaking change ⚠️ closed due inactivity The issue/PR was automatically closed due to inactivity. high impact Changes are very prominent and affect users or the project in a major way. medium effort Default level of effort stale The issue/PR was marked as stale because it has been open for too long.
Projects

Comments

@cyraxred
Copy link

Description

Addition of an interface causes reordering of fields in the storage layout.

Environment

  • Compiler version: reproduced on 0.8.0, 0.8.4, 0.8.10. 0.8.15
  • Framework/IDE (e.g. Truffle or Remix): Remix, VisualStudio

Steps to Reproduce

Compiling of the code below gives following storage layouts:

  • V1: p11, p12
  • V2: p21, p11, p12, p22

Expected layout is:

  • V1: p11, p12
  • V2: p11, p12, p21, p22

When ITest is removed from P22, then the layout is generated as expected.
This effect depends on position of interfaces on the inheritance list, especially for contracts with complex composition.

interface ITest {}

contract P11 is ITest {
    uint8 public p11 = 11;
}

contract P12 is P11 {
    uint8 public p12 = 12;
}

contract V1 is P12 {}

contract P21 {
    uint8 public p21 = 21;
}

contract P22 is P21, ITest {
    uint8 public p22 = 22;
}

contract V2 is V1, P22 {}
@cyraxred
Copy link
Author

cyraxred commented Aug 3, 2022

@ekpyron
Copy link
Member

ekpyron commented Aug 5, 2022

The layout of storage in the presence of inheritance is determined by the C3 linearization of the inheritance graph.
Adding the interface changes the C3 linearization, so the change in the storage layout is expected, conforms to the current specification and is in that sense not a bug.

That being said, I'm actually wondering, whether interfaces should be exempt from being treated the same as contracts in the C3 linearization - out of my head, it seems to me that this would be possible and might actually be beneficial. This would be a gravely breaking change, though, so at least for the time being, the change in storage layout is expected and this behaviour cannot be changed in the short term...

@ekpyron ekpyron added this to New issues in Solidity via automation Aug 5, 2022
@ekpyron ekpyron moved this from New issues to Design Backlog in Solidity Aug 5, 2022
@cyraxred
Copy link
Author

cyraxred commented Aug 5, 2022

yeah, ideally, interfaces should be exempt from the linearization

@Elyx0
Copy link

Elyx0 commented Aug 8, 2022

Is there any tool to preview the solidity C3 output of a given contract?

@montyly
Copy link

montyly commented Aug 9, 2022

@Elyx0 : you can use slither's inheritance graph: https://github.com/crytic/slither/wiki/Printer-documentation#inheritance-graph

slither file.sol --print inheritance-graph

@cameel cameel added medium effort Default level of effort high impact Changes are very prominent and affect users or the project in a major way. labels Sep 26, 2022
@github-actions
Copy link

This issue has been marked as stale due to inactivity for the last 90 days.
It will be automatically closed in 7 days.

@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Mar 30, 2023
@github-actions
Copy link

github-actions bot commented Apr 8, 2023

Hi everyone! This issue has been automatically closed due to inactivity.
If you think this issue is still relevant in the latest Solidity version and you have something to contribute, feel free to reopen.
However, unless the issue is a concrete proposal that can be implemented, we recommend starting a language discussion on the forum instead.

@github-actions github-actions bot added the closed due inactivity The issue/PR was automatically closed due to inactivity. label Apr 8, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 8, 2023
Solidity automation moved this from Design Backlog to Done Apr 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change ⚠️ closed due inactivity The issue/PR was automatically closed due to inactivity. high impact Changes are very prominent and affect users or the project in a major way. medium effort Default level of effort stale The issue/PR was marked as stale because it has been open for too long.
Projects
No open projects
Solidity
  
Done
Development

No branches or pull requests

5 participants