Upgrading an ERC721Upgradable without Enumerability to have Enumerability

Hey OZ Community!

I am trying to upgrade an ERC721Upgradeable contract without enumerability to extend it to have the enumerability features. Running into deploy time errors on the upgradeProxy. See below for code examples and logs from the console on deploy.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.4;

import "@openzeppelin/contracts-upgradeable/token/ERC721/ERC721Upgradeable.sol";
import "@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721URIStorageUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/security/PausableUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721BurnableUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";

contract ERC721Non is Initializable, ERC721Upgradeable, ERC721URIStorageUpgradeable, PausableUpgradeable, AccessControlUpgradeable, ERC721BurnableUpgradeable, UUPSUpgradeable {
    bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE");
    bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");
    bytes32 public constant UPGRADER_ROLE = keccak256("UPGRADER_ROLE");

    /// @custom:oz-upgrades-unsafe-allow constructor
    constructor() initializer {}

    function initialize() initializer public {
        __ERC721_init("ERC721Non", "NON");
        __ERC721URIStorage_init();
        __Pausable_init();
        __AccessControl_init();
        __ERC721Burnable_init();
        __UUPSUpgradeable_init();

        _grantRole(DEFAULT_ADMIN_ROLE, msg.sender);
        _grantRole(PAUSER_ROLE, msg.sender);
        _grantRole(MINTER_ROLE, msg.sender);
        _grantRole(UPGRADER_ROLE, msg.sender);
    }

    function pause() public onlyRole(PAUSER_ROLE) {
        _pause();
    }

    function unpause() public onlyRole(PAUSER_ROLE) {
        _unpause();
    }

    function safeMint(address to, uint256 tokenId, string memory uri)
        public
        onlyRole(MINTER_ROLE)
    {
        _safeMint(to, tokenId);
        _setTokenURI(tokenId, uri);
    }

    function _beforeTokenTransfer(address from, address to, uint256 tokenId)
        internal
        whenNotPaused
        override
    {
        super._beforeTokenTransfer(from, to, tokenId);
    }

    function _authorizeUpgrade(address newImplementation)
        internal
        onlyRole(UPGRADER_ROLE)
        override
    {}

    // The following functions are overrides required by Solidity.

    function _burn(uint256 tokenId)
        internal
        override(ERC721Upgradeable, ERC721URIStorageUpgradeable)
    {
        super._burn(tokenId);
    }

    function tokenURI(uint256 tokenId)
        public
        view
        override(ERC721Upgradeable, ERC721URIStorageUpgradeable)
        returns (string memory)
    {
        return super.tokenURI(tokenId);
    }

    function supportsInterface(bytes4 interfaceId)
        public
        view
        override(ERC721Upgradeable, AccessControlUpgradeable)
        returns (bool)
    {
        return super.supportsInterface(interfaceId);
    }
}
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.4;

import "@openzeppelin/contracts-upgradeable/token/ERC721/ERC721Upgradeable.sol";
import "@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721EnumerableUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721URIStorageUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/security/PausableUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721BurnableUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";

contract ERC721Is is Initializable, ERC721Upgradeable, ERC721EnumerableUpgradeable, ERC721URIStorageUpgradeable, PausableUpgradeable, AccessControlUpgradeable, ERC721BurnableUpgradeable, UUPSUpgradeable {
    bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE");
    bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");
    bytes32 public constant UPGRADER_ROLE = keccak256("UPGRADER_ROLE");

    /// @custom:oz-upgrades-unsafe-allow constructor
    constructor() initializer {}

    function initialize() initializer public {
        __ERC721_init("ERC721Is", "IS");
        __ERC721Enumerable_init();
        __ERC721URIStorage_init();
        __Pausable_init();
        __AccessControl_init();
        __ERC721Burnable_init();
        __UUPSUpgradeable_init();

        _grantRole(DEFAULT_ADMIN_ROLE, msg.sender);
        _grantRole(PAUSER_ROLE, msg.sender);
        _grantRole(MINTER_ROLE, msg.sender);
        _grantRole(UPGRADER_ROLE, msg.sender);
    }

    function pause() public onlyRole(PAUSER_ROLE) {
        _pause();
    }

    function unpause() public onlyRole(PAUSER_ROLE) {
        _unpause();
    }

    function safeMint(address to, uint256 tokenId, string memory uri)
        public
        onlyRole(MINTER_ROLE)
    {
        _safeMint(to, tokenId);
        _setTokenURI(tokenId, uri);
    }

    function _beforeTokenTransfer(address from, address to, uint256 tokenId)
        internal
        whenNotPaused
        override(ERC721Upgradeable, ERC721EnumerableUpgradeable)
    {
        super._beforeTokenTransfer(from, to, tokenId);
    }

    function _authorizeUpgrade(address newImplementation)
        internal
        onlyRole(UPGRADER_ROLE)
        override
    {}

    // The following functions are overrides required by Solidity.

    function _burn(uint256 tokenId)
        internal
        override(ERC721Upgradeable, ERC721URIStorageUpgradeable)
    {
        super._burn(tokenId);
    }

    function tokenURI(uint256 tokenId)
        public
        view
        override(ERC721Upgradeable, ERC721URIStorageUpgradeable)
        returns (string memory)
    {
        return super.tokenURI(tokenId);
    }

    function supportsInterface(bytes4 interfaceId)
        public
        view
        override(ERC721Upgradeable, ERC721EnumerableUpgradeable, AccessControlUpgradeable)
        returns (bool)
    {
        return super.supportsInterface(interfaceId);
    }
}

Here is the console output on upgradeProxy.


StorageUpgradeErrors: New storage layout is incompatible

@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721EnumerableUpgradeable.sol:22: Inserted `_ownedTokens`
  > New variables should be placed after all existing inherited variables

@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721EnumerableUpgradeable.sol:25: Inserted `_ownedTokensIndex`
  > New variables should be placed after all existing inherited variables

@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721EnumerableUpgradeable.sol:28: Inserted `_allTokens`
  > New variables should be placed after all existing inherited variables

@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721EnumerableUpgradeable.sol:31: Inserted `_allTokensIndex`
  > New variables should be placed after all existing inherited variables

@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721EnumerableUpgradeable.sol:175: Inserted `__gap`
  > New variables should be placed after all existing inherited variables
    at assertStorageUpgradeSafe (/Users/nolanjacobson/DraftKings-ERC721-Upgradeable-DK-Demo/node_modules/@openzeppelin/upgrades-core/dist/storage/index.js:27:15)
    at deployImpl (/Users/nolanjacobson/DraftKings-ERC721-Upgradeable-DK-Demo/node_modules/@openzeppelin/hardhat-upgrades/dist/utils/deploy-impl.js:49:58)
    at async Proxy.upgradeProxy (/Users/nolanjacobson/DraftKings-ERC721-Upgradeable-DK-Demo/node_modules/@openzeppelin/hardhat-upgrades/dist/upgrade-proxy.js:9:36)
    at async main (/Users/nolanjacobson/DraftKings-ERC721-Upgradeable-DK-Demo/scripts/deploy_upgradable.js:13:20) {
  report: LayoutCompatibilityReport {
    ops: [
      {
        kind: 'insert',
        totalCost: 2,
        predecessor: {
          kind: 'nop',
          totalCost: 0,
          predecessor: { kind: 'nop', totalCost: 0, predecessor: [Object] }
        },
        updated: {
          label: '_ownedTokens',
          offset: 0,
          slot: '151',
          type: {
            id: 't_mapping(t_address,t_mapping(t_uint256,t_uint256))',
            head: 't_mapping',
            args: [Array],
            tail: undefined,
            rets: undefined,
            item: [Object]
          },
          contract: 'ERC721EnumerableUpgradeable',
          src: '@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721EnumerableUpgradeable.sol:22'
        }
      },
      {
        kind: 'insert',
        totalCost: 4,
        predecessor: {
          kind: 'insert',
          totalCost: 2,
          predecessor: { kind: 'nop', totalCost: 0, predecessor: [Object] },
          updated: {
            label: '_ownedTokens',
            offset: 0,
            slot: '151',
            type: [Object],
            contract: 'ERC721EnumerableUpgradeable',
            src: '@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721EnumerableUpgradeable.sol:22'
          }
        },
        updated: {
          label: '_ownedTokensIndex',
          offset: 0,
          slot: '152',
          type: {
            id: 't_mapping(t_uint256,t_uint256)',
            head: 't_mapping',
            args: [Array],
            tail: undefined,
            rets: undefined,
            item: [Object]
          },
          contract: 'ERC721EnumerableUpgradeable',
          src: '@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721EnumerableUpgradeable.sol:25'
        }
      },
      {
        kind: 'insert',
        totalCost: 6,
        predecessor: {
          kind: 'insert',
          totalCost: 4,
          predecessor: {
            kind: 'insert',
            totalCost: 2,
            predecessor: [Object],
            updated: [Object]
          },
          updated: {
            label: '_ownedTokensIndex',
            offset: 0,
            slot: '152',
            type: [Object],
            contract: 'ERC721EnumerableUpgradeable',
            src: '@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721EnumerableUpgradeable.sol:25'
          }
        },
        updated: {
          label: '_allTokens',
          offset: 0,
          slot: '153',
          type: {
            id: 't_array(t_uint256)dyn_storage',
            head: 't_array',
            args: [Array],
            tail: 'dyn_storage',
            rets: undefined,
            item: [Object]
          },
          contract: 'ERC721EnumerableUpgradeable',
          src: '@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721EnumerableUpgradeable.sol:28'
        }
      },
      {
        kind: 'insert',
        totalCost: 8,
        predecessor: {
          kind: 'insert',
          totalCost: 6,
          predecessor: {
            kind: 'insert',
            totalCost: 4,
            predecessor: [Object],
            updated: [Object]
          },
          updated: {
            label: '_allTokens',
            offset: 0,
            slot: '153',
            type: [Object],
            contract: 'ERC721EnumerableUpgradeable',
            src: '@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721EnumerableUpgradeable.sol:28'
          }
        },
        updated: {
          label: '_allTokensIndex',
          offset: 0,
          slot: '154',
          type: {
            id: 't_mapping(t_uint256,t_uint256)',
            head: 't_mapping',
            args: [Array],
            tail: undefined,
            rets: undefined,
            item: [Object]
          },
          contract: 'ERC721EnumerableUpgradeable',
          src: '@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721EnumerableUpgradeable.sol:31'
        }
      },
      {
        kind: 'insert',
        totalCost: 10,
        predecessor: {
          kind: 'insert',
          totalCost: 8,
          predecessor: {
            kind: 'insert',
            totalCost: 6,
            predecessor: [Object],
            updated: [Object]
          },
          updated: {
            label: '_allTokensIndex',
            offset: 0,
            slot: '154',
            type: [Object],
            contract: 'ERC721EnumerableUpgradeable',
            src: '@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721EnumerableUpgradeable.sol:31'
          }
        },
        updated: {
          label: '__gap',
          offset: 0,
          slot: '155',
          type: {
            id: 't_array(t_uint256)46_storage',
            head: 't_array',
            args: [Array],
            tail: '46_storage',
            rets: undefined,
            item: [Object]
          },
          contract: 'ERC721EnumerableUpgradeable',
          src: '@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721EnumerableUpgradeable.sol:175'
        }
      }
    ]
  }
}

This is because state variables cannot be inserted before existing state variables during an upgrade, and by adding the ERC721EnumerableUpgradeable inheritance before some other parents, that is equivalent to inserting its variables. (Because storage slots for parents are used in the order they are declared.)

The solution is to add the ERC721EnumerableUpgradeable inheritance at the end of the list instead, e.g.:
contract ERC721Is is Initializable, ERC721Upgradeable, ERC721URIStorageUpgradeable, PausableUpgradeable, AccessControlUpgradeable, ERC721BurnableUpgradeable, UUPSUpgradeable, ERC721EnumerableUpgradeable {

2 Likes