How to make Solidity Contracts Upgradable with a mapping and state variables?

Question

While making a contract upgradable, I am facing issues with the approach where we split up the code implementation and storage.

As of this approach, I have created storage slots for every state variable that are either of type uint256 and address. These variables are static and don't extent with time, I define the slots as

For uint256 _totalSupply

bytes32 internal constant _TOTAL_SUPPLY_SLOT = 0x6cac11967c5738fe648c7b461acdbb3138d8619be340b9043646ca5ab64ec74b;

Getting the slot with eip1967

_TOTAL_SUPPLY_SLOT == bytes32(uint256(keccak256("eip1967.tokenGeyserStorage.totalSupply")) - 1)

And correspondingly

    function _totalSupply() internal view returns (uint256) {
    return getUint256(_TOTAL_SUPPLY_SLOT);
    }

    function _setTotalSupply(uint256 _value) internal {
        setUint256(_TOTAL_SUPPLY_SLOT, _value);
    }

where

    function setUint256(bytes32 slot, uint256 _value) private {
        // solhint-disable-next-line no-inline-assembly
            assembly {
                sstore(slot, _value)
            }
        }

and getter as 

    function getUint256(bytes32 slot) private view returns (uint256 str) {
        // solhint-disable-next-line no-inline-assembly
        assembly {
            str := sload(slot)
        }
    } 

What about data-types like

mapping(address => uint256) private _balances;

and

IERC20 public token;

As we know the mappings can grow, how to allot a slot for them, and what would be the setters and getters look like?

As IERC20 is also a data type that is different, how shall we go forward with those state variables while making the contract upgradable retaining these in states?

Any concepts like diamonds and facets that can help in better implementation of proxy?

Answer 1

First the short answers:

As we know the mappings can grow, how to allot a slot for them, and what would be the setters and getters look like?

You could define a hash function that uses both a "slot" and an "index", e.g. (pseudo), hash(abi.encodePacked("myMapping", key)). That will generate a unique location for each value that you can then go fetch or set.

As IERC20 is also a data type that is different, how shall we go forward with those state variables while making the contract upgradable retaining these in states?

All you need is an address. Your "token" is IERC20(<address>). The implementation details, i.e. the function, are a "type" defined at compile-time. You don't store that as "data".

Now the long answer. I think you are on the wrong track and it's making it hard on you.

Your approach seems to follow "eternal storage" which is generally not the preferred, modern way to create an upgradeable contract. There are also elements of Proxy and it seems to lead to the worst of all worlds. For example, you should not need assembly to do eternal storage. Why you are doing that is unclear.

Consider:

contract Storage {

  function getUint(bytes32 key) public view returns(unit) {
    return datastoreUint[key];
  }

contract Logic }

  Storage storage; // instantiated in constructor

  function getSomething(address user) public view returns(unit) {
    bytes32 key = hashFunction(arguments); // pseudo
    return storage.getUint(key);
  }

In Eternal Storage, the general idea is the implementation should know the slots it is using for the important variables. The storage contract requires no assembler. It just has mappings of key/value pairs for each scaler data type and returns whatever.

You can make it work with the suggestions, above that address the mappings and contract types and also work for arrays. Structs will devolve into multiple fetches for the individual members and another argument for a hashing scheme.

You can probably see that the pattern forces you to write very abstract and busy code to deal with variables. It's error-prone and not especially nice to read. It also involves a lot of communication between "implementation" and "storage" which will add gas overhead at runtime.

The Proxy approach lets you write in normal syntax and, subject to a little self-discipline, safely upgrade the contact. It's a big topic to unpack and beyond the scope of the original question so I won't attempt to explain how it all works, here. There are some good posts about "Proxy" on this site and elsewhere.

The upgradable Proxy contract will use assembler to store one or two variables. That's all. Assembler is used to avoid a storage collision with the implementation contract. That is to say, the proxy (forwarder) contract needs to write something down, the address of the implementation, and it needs to write it in a location that certainly won't be overwritten by an implementation that has no awareness of it. The solution to that problem is to rely on a collision-resistant location - usually, a key generated by a hash function - a far-away pseudo-random place that you can only get to with assembler.

How do Proxy Upgraded Logic Contracts Share Data

Have a look over here for an example of a well-solved implementation. https://github.com/OpenZeppelin/openzeppelin-contracts/tree/solc-0.6/contracts/proxy. (I chose this version for simplicity, so mind the releases before using).

It may seem a little strange, at first but it is worth studying.

• The Proxy delegates to the implementation. Context is the Proxy and data is stored in the Proxy.
• The Proxy locates the implementation with assembler and includes a function to change the value (address) of the implementation.
• The Proxy uses an "Ownable" pattern to guard the upgrade function.

The upgradeable contract might also use an "Ownable" pattern and that causes a bit of a challenge - what if the Proxy itself and the Implementation both implement "changeOwner"? Which function should execute?

They solve this problem with a simple rule. There is ProxyAdmin that exists solely to manage the upgrade process. The Proxy's rule is simple: Everything from ProxyAdmin runs "here" and everything not from ProxyAdmin is for the Implementation. The ProxyAdmin is "owned" by the deployer/governance. The Implementation does whatever it does without the possibility of collision or interference with the Proxy/upgrade concern.

Hope it helps.