Adding ERC20PermitUpgradeable to an existing ERC20 contract

Smart Contracts
, ,
1

I have an existing already deployed upgradeable ERC20 contract.

contract Token is 
Initializable,
ERC20Upgradeable,
PausableUpgradeable,
AccessControlUpgradeable,
UUPSUpgradeable
{
}

I wanted to add permit functionality to it. I have to do this carefully so that existing storage slots don't get overwritten and mess up the entire token.

I was thinking of just adding the ERC20PermitUpgradeable inheritance to the end of the contract so that it doesn't mess with the storage slots from the previously inherited contracts. Note the Token contract doesn't have any state variables itself so this should work fine.

import "@openzeppelin/contracts-upgradeable/token/ERC20/extensions/draft-ERC20PermitUpgradeable.sol";

contract Token is
Initializable,
ERC20Upgradeable,
PausableUpgradeable,
AccessControlUpgradeable,
UUPSUpgradeable,
ERC20PermitUpgradeable
{
}

But then I noticed that the ERC20PermitUpgradeable contract inherits from already inherited classes like ERC20Upgradeable.

My upgrade tests all fail because storage layout changed. How do I do this safely :sob:

EDIT: My tests pass. It was some mistake with my test. Openzeppelin also doesn't complain about this upgrade. Is there something else I should do to make sure I am not messing stuff up? I am reading https://docs.soliditylang.org/en/develop/contracts.html#multiple-inheritance-and-linearization now.

2

You can manually port the required ERC20PermitUpgradeable functionality into your contract.

I'd do it if I were you, even just in order to ensure that your tests are failing because of the reason that you've mentioned, and not because of something else.

Once your tests are all completing successfully, you can start dwelling on a better solution (or whether you even care about a better solution).

1 Like
3

My tests failed because they were actually upgrading from the newer version to the older version. It made sense to fail because it was removing previously used storage slots and is obviously wrong.

But your general advice makes sense to me. I'll try manually porting at least to just compare it.

1 Like