Skip to content

Change the Layout of Storage Variables for Upgradeable Proxy Contracts #8353

Closed as not planned

Description

@mudgen

Abstract

Upgradegradable proxy contracts that delegate to other contracts (called delegate or logic contracts) for functionality currently have second-class variable storage layout support in Solidity.

The problem is that storage variable declarations must match/line up between a proxy contract and all delegate contracts that are delegated to by the proxy contract.

In the case of a proxy contract with multiple delegate contracts the delegate contract variable declarations must match up with the proxy contract and with all the other delegate contracts that are used.

Alternatively, proxy contracts have used unstructured storage which resorts to manually specifying and manipulating storage with assembly.

Some examples of the proxy contract pattern in use:

  1. ERC1538: Transparent Contract Standard
  2. OpenZepplin's Proxies
  3. AragonOS
  4. Genosis Proxies
  5. DecentralLand
  6. EIP1822: Universal Upgradeable Proxy Standard
  7. EIP1967: Standard Proxy Storage Slots

The proxy contract pattern is used by people and it would be really great if it had first-class variable storage layout support!

Motivation

Currently the layout of storage variables is documented here: https://solidity.readthedocs.io/en/v0.6.3/miscellaneous.html

I propose that the layout works the same with one change: Instead of starting at position 0 make a keccak256 hash of the name of the contract and start the storage for the contract at the position given by the hash. This way the storage space is separate for each contract that has a different name.

With this approach a delegate contract can create its own storage variables without clashing with the proxy contract or other delegate contracts. And delegate contracts no longer need to import and inherit classes it doesn't need in order to maintain storage variable alignment.

A syntactic construct can exist for one delegate contract to access the storage specified by the proxy contract or other delegate contract. I propose that a delegate contract simply imports the code of the delegate/proxy its wants to access and then use the contract name to access the contract's storage like this:

import "./DelegateA.sol"
// imagine that DelegateA.sol has a storage variable called num2.

contract DelegateB {
  uint256 internal num1;

  // function that returns storage variable data defined within the contract
  function getNum1() external view returns(uint256) {
    return num1; 
  }

  // function that returns storage variable data defined in another delegate contract
  function getNum2() external view returns(uint256) {
    return DelegateA.num2; 
  }
}

This is the same syntax that is used by libraries, now being used by contracts.

Perhaps this could be an optional feature that is enabled with a pragma directive. Because this may only be useful for the proxy contract pattern. Also, I am really interested in other possible solutions.

Backwards Compatibility

This is a not a backwards compatible change. This functionality could be used by new proxy contracts and delegate contracts. Perhaps, to help with transition, this mode of storage variable layout could have its own pragma directive.

Activity

  1. axic commented on Feb 20, 2020

    @axic
    Contributor

    See also #597.

    It took me a while to read because the description is quite big, but the actual proposal is in the middle:

    I propose that the layout works the same with one change: Instead of starting at position 0 make a keccak256 hash of the name of the contract and start the storage for the contract at the position given by the hash. This way the storage space is separate for each contract that has a different name.

    While this seem like an elegant solution, please note that this addition means either doing an extra round of hashing (and storing the input in the contract) or storing the hash in the contract, both of which are quite expensive.

  2. mudgen commented on Feb 20, 2020

    @mudgen
    Author

    Thanks @axic. Perhaps this could be an optional feature that is enabled with a pragma directive. Because this may only be useful for the proxy contract pattern. Also, I am really interested in other possible solutions.

  3. leonardoalt commented on Feb 20, 2020

    @leonardoalt

    We have been using the JSON interface for different options and configurations. I think this would be encouraged instead of a pragma directive.

  4. chriseth commented on Feb 20, 2020

    @chriseth
    Contributor

    Oh wow, I just wanted to propose the following:

    struct Data {
      uint a;
      uint b;
    }
    
    contract DelegateB {
      function data() internal returns (Data storage r) {
        uint s = keccak256("DelegateB");
        assembly { r_slot := s }
      }
    
      function doSomething() external view returns(uint256) {
        data().a = 8;
      }
    }
    

    when I realized that we have no way to assign to storage pointer variables in assembly. Did nobody ever try that?

  5. chriseth commented on Feb 20, 2020

    @chriseth
    Contributor

    Regardless of the above, I would be happier with a more explicit notation like for example

    struct Data {
      uint a;
      uint b;
    }
    
    contract DelegateB {
      Data data internal at keccak256("DelegateB");
      function doSomething() external view returns(uint256) {
        data.a = 8;
      }
    }
    
  6. mudgen commented on Feb 20, 2020

    @mudgen
    Author

    @chriseth I see, so structs could be used for the storage layout of proxy and/or delegate contracts. I also like explicitness. This is brilliant, I love it!

    I love the syntax you gave for defining storage pointers: Data data internal at keccak256("DelegateB"); How quickly could this be implemented and released?

  7. mudgen commented on Feb 20, 2020

    @mudgen
    Author

    @chriseth Could there be a way to specify a struct to use for the storage layout for a contract? Here is an example:

    struct DataDelegateB {
      uint a;
      uint b;
    }
    
    contract DelegateB {
      using storage layout DataDelegateB at keccak256("DataDelegateB");
    
      function doSomething() external view returns(uint256) {
        a = 8;
        b = 7;
      }
    }

    Notice that I did not need to prepend a struct variable name to access the "a" and "b" members of the struct.

    Using the storage pointer definition syntax other delegate contracts can access the storage layout of DelegateB. Here is an example:

    struct DataDelegateB {
      uint a;
      uint b;
    }
    
    struct DataDelegateC {
      uint first;
      uint second;
    }
    
    contract DelegateC {
      using storage layout DataDelegateC at keccak256("DataDelegateC");
      
      DataDelegateB dataDelegateB internal at keccak256("DataDelegateB");
      
      function getFirst() external view returns(uint256) {
        first = 7;
        second = 10;
        return dataDelegateB.a + first + second
      }
    }

    I think this is pretty clean!

    But I think the at keccak256("DelegateC") part could be optional so this syntax could be used:

    using storage layout DataDelegateC;

    If the at part is omitted then automatically the keccak256 of the name of the struct is used as the storage pointer value.

  8. mudgen commented on Feb 21, 2020

    @mudgen
    Author

    @chriseth I eagerly await your feedback.

    I am planning to launch a new and improved proxy contract upgrade standard based on ERC1538 and recent solidity features. Having the above storage layout mechanisms we are discussing would make it so much more developer friendly and better.

  9. chriseth commented on Feb 24, 2020

    @chriseth
    Contributor

    @mudgen we need more feedback on this, unfortunately. In my opinion, something like using storage layout DataDelegateC at keccak256("DataDelegateC"); that applies to the whole contract is too dangerous and too easy to miss. Furthermore, it would be very complicated with regards to inheritance.

    What I think could find better consensus is a single state variable (which might be of struct type) that is "dislocated". For proxy contracts, wouldn't it even be better if the part needed by the proxy would be dislocated instead of the actual storage? Or is the point that you could move to a different "fresh" location when upgrading?

  10. mudgen commented on Feb 24, 2020

    @mudgen
    Author

    Hi @chriseth.

    That is a good point about inheritance.

    Being able to create a dislocated state variable, like how you showed before: Data data internal at keccak256("DelegateB"); would be very useful for most proxy contracts.

    To answer your question regarding having a dislocated storage variable or having dislocated storage layouts for proxy contracts, it depends. There are two different use cases:

    1. A proxy contract that only ever has one delegate/logic contract that it delegates to at a time, so that any upgrade is simply replacing the one logic contract with a different logic contract would benefit the most by using a dislocated state variable like you suggested. Each logic contract would use the same storage layout so there would be no need for dislocated storage layouts.
    2. It is different for proxy contracts that use multiple delegate/logic contracts at the same time. In this case the proxy contract will delegate to a logic contract based on what function is being called. In this approach sharing the same storage layout for each logic contract is a burden, because each logic contract handles different things and does not need to access the same state variables. This approach would benefit a lot if each logic contract could use its own dislocated storage layout space, so that each logic contract is separate from each other but still able to access each other's state variables if needed. Check out ERC1538 for this multiple delegate/logic contract approach: https://eips.ethereum.org/EIPS/eip-1538

    No matter what having the ability to create a dislocated state variable (such as a struct) is very useful for both approaches.

    The multiple delegate/logic contract proxy approach would benefit from being able to create dislocated state variable like you mentioned. Each logic contract could use one or more dislocated state variables for its data.

    I understand that creating dislocated storage layouts for contracts would require more work and greater consensus. But are you able to move forward now in creating what you have suggested to create dislocated state variable, something like this: Data data internal at keccak256("DelegateB"); ?
    Because I want to start using that in proxy contracts and teaching others about this awesome new capability.

  11. chriseth commented on Mar 12, 2020

    @chriseth
    Contributor

    I have some thoughts on all this, but I think this needs a dedicated brainstorming session to get everything right. I currently aim for the online solidity summit to do this.

  12. mudgen commented on Mar 12, 2020

    @mudgen
    Author

    Sounds good.

  13. mudgen commented on Mar 12, 2020

    @mudgen
    Author

    I just applied to go to the online solidity summit.

  14. 7 remaining items

  15. mudgen commented on Oct 13, 2021

    @mudgen
    Author

    The problem is that some projects can fit in the first slots and will want to.

    Do we know any projects? Every project I know of stores most of their data in mappings and/or dynamic arrays.

  16. mudgen commented on Oct 13, 2021

    @mudgen
    Author

    Well, the pattern does have some downsides as pointed out above so it's not such a no-brainer I thought it was. Doesn't mean we won't implement it but right now at least there's still no consensus that it's the best approach.

    Understood, I personally like the idea of letting the user choose the slot or the data that is hashed to choose the slot.
    For example I like this method of choosing a slot location:

    bytes32 constant MYSTRUCT_POSITION = 
        keccak256(abi.encodePacked(
          ERC1155.interfaceId, 
          ERC1155.name, 
          address(this)
        ));

    That would choose a slot location for a struct that contains state variables for an ERC1155 implementation.

    @cyotee showed me this.

  17. cameel commented on Oct 13, 2021

    @cameel
    Collaborator

    Do we know any projects? Every project I know of stores most of their data in mappings and/or dynamic arrays.

    Someone mentioned one on the call. @hrkrshnn @axic, was that Uniswap?

  18. cyotee commented on Oct 13, 2021

    @cyotee

    Hi, thanks for the call out @mudgen.

    @cameel, If you're interested I can comment on some of the points you've brought up.

    STL;DR

    Unstructured storage powerful. Language limits exercising power. Give us more power.

    TL;DR

    Being able to dynamically define storage pointers for variables has expanded my ability to write more flexible contracts. Main issues have come from structs not having a standard way to identify them that is inherent to their code, like interfaceId for interfaces. I have to go through a lot of work to write code to ensure it's all being done in a consistent way. Language and compiler support would make this a lot easier, can could standardize the usage.

    BUT, still allow for developer defined storage pointers so we can manipulate them as needed. So possible an addition to instantiating structs to let us set the storage pointer at the start.

    Promoting structs to a type, somehow, seems like the optimal solution. Baring limitations, as writing compilers is beyond me.

    P.S.

    All this is really only relevant in the context of dynamic proxies as this whole option facilitates being able to use delegatecall dynamically based on mapping transaction properties to target addresses. And that is limited due to the inherent surcharge. So, really, this would be most beneficial if something can be done about the surcharge for using external libraries so we can leverage this further. Which would have the side effect of potentially reducing chain size as we wouldn't need to deploy as much redundant code to get around the surcharge.

    Pontification Mode Active /

    I agree that a storage pointer standard should rely on an inherent property of the struct to minimize any human errors from having an arbitrary definition. I've settled on using this to calculate my storage slots.

    bytes32 private constant STORAGE_SLOT = keccak256(type(ServiceProxyStorage).creationCode);

    Where the storage library code consists of

    library ServiceProxyStorage {
    
      struct Layout {
        mapping(bytes4 => address) serviceForFunctionSelector;
      }
    
      function _layout(bytes32 slot) pure internal returns (Layout storage layout) {
        assembly{ layout.slot := slot }
      }
      
    }
    

    The default storage slot is defined and used in a library where I implement the desired CRUD operations for the struct defined in the storage library.

    By using the creation code for the struct defining library I can ensure the default slot is intrinsic to the definition of the struct. By including a function that effectively casts a storage pointer to the struct i also provide an option to define a different slot if needed.

    I've settled on defining structs in libraries with their _layout casting function. Implementing the CRUD functions in a library where I can actually access the creation code of the struct definition library. And then using that CRUD library in an abstract contract to define the "business logic" for a contract as an atomic unit of functionality using only internal functions. Then expose those internal function with a contract that only implements external functions and a constructor if needed.

    With that background out of the way, It would be helpful if struct could have implicit properties, like how interfaces have selector properties, or like type offers. Then we could have an inherent identifier to use when dynamically defining storage slots. Maybe there's something available in the ABI encoder when it's encoding structs for passing between contracts that could give that inherent identifier?

    Also, one point I didn't explain on why I implement my storage usage as I do is that I do have times where I don't want to use the default storage slot. So I made sure to leave the ability for me to reuse my struct definitions, and CRUD libraries with a different storage slot. An example of how I use these libraries in an abstract contract is here.

    function _setOwner(
        address owner
      ) internal {
        OwnableLib._layout(OwnableLib._getDefaultSlot() )._setOwner(owner);
     }
    

    The CRUD library exposes a _layout function that passes through to the struct defining library to facilitate loading the right struct without the contract inheriting the abstract having to know anything about the underlying structs or their usage. Which helps maintain a separation of concerns between the presentation layer and the internal logic. So I'm kind fo treating the storage pointer like a database connection, and my struct and CRUD libraries like database driver libraries.

    So, it would be nice to also have an option when instantiating a struct to define its storage slot so we don't have to go through all these hoops, and can eliminate the usage of assembly to access the slot property of a storage variable.

  19. cyotee commented on Oct 14, 2021

    @cyotee

    Another matter I've been considering is the potential benefit of including a modifier when calling a function using an interface to designate that a delegatecall should be used. I was thinking of 3 options, again, no idea how it might be best to implement. just examples to explain the idea.

    IContract(address).delegate.externalFunction();

    IContract(address).externalFunction{delegate}();

    or maybe

    delegate{
      IContract(address).externalFunction();
    }
    

    Either way, I raise the point here as I see the primary usage of unstructured / dynamically assigned storage being to facilitate using delegatecall by resolving the storage slot expectation mismatch problem. So it might be relevant when considering a storage slot standard.

    Also, it does work to use external libraries as the implementation for a proxy. It might be appropriate to tie this to the relevant library when it's external. If there is some way to introduce an externally accessible contract metadata system that would work for libraries, then the blockchain could build up a standard set of available data types and logic accessible more easily, and reliably. Like how ERC165 was baked into the compiler.

    But all of this might not be very useful since we don't have true polymorhpism and reflection in the EVM. But from some experimentation, I think it would be possible to realize fuller functionality. The reflection could be handled with a contract metadata standard baked into the compiler so developer can more easily expose those properties to other contracts.

    And with a standardized set of external libraries, we might be able to leverage more implementations like in ERC1167. Making it possible to store parts of a contracts bytecode, and compose them with external libraries so developers wouldn't need to write as much code. And users could know that proven,, immutable code was being reused from the blockchain. Almost making it possible to treat the blockchain like a byte code file path for loading the relevant libraries. Which might make it possible to implement a declarative framework for Solidity that will handle composing the appropriate bytecode based on deployed external libraries declared like dependencies.

    Something like

    import <ERC20 Mintable Library Address>
    
    @ERC20Mintable{
      name: "Token",
      symbol: "T",
      decimals: 18,
      totalSupply: 1000**decimals
    }
    

    Then the compiler can check the metadata for the library address, like the interface ID and function selectors, and compile bytecode similar to ERC1167 mapping the library function selectors to the library address in the new proxy. Using a standardized storage slot for the function to library mapping that could be overriden if needed. So, just like being able to drop down to assembly, developers could opt to drop down to base Solidity when they need to do something that isn't already available. But a reverse lookup system could be implemented to make it easier to find available external libraries to import. Treating libraries on chain like packages. And this could then be extended so part of a contract's metadata would be able other contracts that it's dependent upon. So if other libraries need to be included they can be pulled in as more mapped functions in the new proxy. Which means it might be possible to actually find implementations you can import using just the interface id for the functionality you want.

    I'm not really capable of writing a compiler, far too rusty. But I was meaning to try implementing a declarative build tool to trial this idea. Initially writing it all as a Hardhat plugin that would start by including my repo as the available libraries, and use my on-chain proxy factory system to identify available modules and simply deploy a proxy using those already deployed contracts. Then see if I can figure out the bytecode to handle it with a more direction insertion by splicing the strings resulting in immutable proxies created at deployment. Then there simply wouldn't be any actual source code. just declarations and bytecode that handles the delegatecall to the library for that function.

  20. hrkrshnn commented on Oct 14, 2021

    @hrkrshnn
    Contributor

    @cyotee Maybe you would be interested in #12096

  21. novaknole commented on Nov 5, 2021

    @novaknole

    @mudgen @cameel

    Thanks for such a great discussion. I also am arriving on this and love the topic.

    Below is @cameel 's example.

    contract DelegateB {
        struct Data {
            uint a;
            uint b;
        }
    
        mapping (string => Data) delegateData;
    
        function data() internal view returns (Data storage) {
            return delegateData["DelegateB"];
        }
    
        function doSomething() external {
            data().a = 8;
            data().b = 7;
        }
    }

    I don't understand how good this way can be when there's only proxy and only one logic contract at any time. Let's say for the proxy contract, first version was DelegateB contract above. You all know how variables and values will be stored in this case. let's say a=10, b=20. Time for the update. So we write new DelegateC contract and the main point is I should not be forced to append variables in the struct, if I am forced, then what's the point of this pattern in the first place ? So new Data gets changed to

    struct Data {
        uint k; // NOTE: we put this in the beginning.
        uint a; 
        uint b;
     }

    If the key in the mapping stays DelegateB, we are screwed as collision happens (k gets put in the same slot as a ), so we gotta change the key as well to DelegateC. Now, we are safe because collisions don't happen, but we got another problem. Since new key appeared in the mapping, if we do delegateData[delegateC].a, we won't get the value of a, which I really need otherwise, what's the point of upgrade ? the point of upgrade is i change state variables, add some features, but I also need to know the values of old variables, otherwise state is lost. Okay, so the way of solving this is that in the new upgraded contract DelegateC, I should start writing/using delegateData[delegateB] alongside delegateData[delegateC]. And the story will go on and on. In the new updated version - delegateD contract, I would need to use delegateData[delegateB], delegateData[delegateC] and delegateData[delegateD] all three. So changing orders of variables or their types, this pattern doesn't seem to work.

    am I missing something ?

  22. added
    high impactChanges are very prominent and affect users or the project in a major way.
    on Sep 14, 2022
  23. github-actions commented on Mar 2, 2023

    @github-actions

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

  24. added
    staleThe issue/PR was marked as stale because it has been open for too long.
    on Mar 2, 2023
  25. github-actions commented on Mar 10, 2023

    @github-actions

    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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    breaking change ⚠️closed due inactivityThe issue/PR was automatically closed due to inactivity.high impactChanges are very prominent and affect users or the project in a major way.language design :rage4:Any changes to the language, e.g. new featuresmedium effortDefault level of effortstaleThe issue/PR was marked as stale because it has been open for too long.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions