Contract factory for upgradeable erc721

I am trying to find some way for users to deploy an erc721 contract and would like to clone an implementation contract through a contract factory like eip-1167. However when the contract factory deploys a new contract, the interaction with the transparent proxy and erc721 do not work. The interaction schema i am trying to implement is below:


My thought is that a user would be able to call the factory to create a new proxy contract and that contract would point to the transparent proxy of my erc721. I want the transparent proxy so that I can upgrade the erc721 if needed. When my contracts are deployed you are unable to interact with the proxies created from the factory. I think this is because the erc721 has an initializer that only allows it to be called once but I am not sure how to get around that…

I’m pretty new to all of this but I think my questions are:

Are two layer proxies possible?
Is metadata stored in the transparent proxy or in the contract itself?
Is there a pattern to clone erc721 contracts but modify name and symbol for each proxy created by the factory? (eip-3448?)

I am not set on this layout or implementation and would like suggestions for a better schema to achieve the same result or fixes to my current code.
I would also appreciate any clarifying explanation on the topic of proxies as I am sure I have more to learn. I also put my code below as it might help understand what I am trying to do.

ERC721

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

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/utils/CountersUpgradeable.sol";

contract ERC721ControlledUpgradeable is ERC721Upgradeable, ERC721URIStorageUpgradeable, 
                    PausableUpgradeable, AccessControlUpgradeable, ERC721BurnableUpgradeable {
    using CountersUpgradeable for CountersUpgradeable.Counter;

    bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE");
    bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");
    bytes32 public constant BURNER_ROLE = keccak256("BURNER_ROLE");
    CountersUpgradeable.Counter private _tokenIdCounter;

    function initialize(string memory name, string memory symbol) public {
        __ERC721_init(name, symbol);
        _setupRole(DEFAULT_ADMIN_ROLE, _msgSender());
        _setupRole(PAUSER_ROLE, _msgSender());
        _setupRole(MINTER_ROLE, _msgSender());
        _setupRole(BURNER_ROLE, _msgSender());
    }

    function safeMint(address to) public {
        require(hasRole(MINTER_ROLE, _msgSender()));
        _safeMint(to, _tokenIdCounter.current());
        _tokenIdCounter.increment();
    }

    function pause() public {
        require(hasRole(PAUSER_ROLE, _msgSender()));
        _pause();
    }

    function unpause() public {
        require(hasRole(PAUSER_ROLE, _msgSender()));
        _unpause();
    }

    function _baseURI() internal pure override returns (string memory) {
        return "";
    }

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

    function _burn(uint256 tokenId) internal override(ERC721Upgradeable, ERC721URIStorageUpgradeable) {
        require(hasRole(BURNER_ROLE, _msgSender()));
        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);
    }
}

Contract Factory

// USES ERC 1167
pragma solidity  >=0.8.0;
 
import "./ERC721ControlledUpgradeable.sol";

contract Factory{
    address public implementation;
    address[] public clonedContracts;


    constructor(address _implementationAddress) {
        implementation = _implementationAddress;
    }


    function createProxy() internal returns (address result) {
        bytes20 targetBytes = bytes20(implementation);
        assembly {
            let clone := mload(0x40)
            mstore(clone, 0x3d602d80600a3d3981f3363d3d373d3d3d363d73000000000000000000000000)
            mstore(add(clone, 0x14), targetBytes)
            mstore(add(clone, 0x28), 0x5af43d82803e903d91602b57fd5bf30000000000000000000000000000000000)
            result := create(0, clone, 0x37)
        }
    }

    function createERC721Controlled(string memory name, string memory symbol) public {
        address clone = createProxy(); 
        ERC721ControlledUpgradeable(clone).initialize(name, symbol); // THIS IS A LITTLE FUNNY 
        //emit ClonedContract(clone)
        clonedContracts.push(clone);
    }
    
    function getAddress(uint i) view external returns(address) {
        return clonedContracts[i];
    }
  
}

I am using truffle to deploy the erc721 and then the factory.

This architecture is not possible with the contracts that we have available. The problem is that the implementation address for the transparent proxy is kept in the storage of the transparent proxy, but when the user goes through the clone it will be using the storage of the clone, so it doesn’t know where to delegate.

I would recommend looking into beacon proxies. Check it out and let me know if it fits your use case.

1 Like

Thank you. The beacons and beacon proxies worked and I was able to deploy them manually. Can the beacon proxies be deployed through a factory? On my initial attempt with the factory the beacon proxy deployed from the factory but I was unable to mint a coin through the beacon proxy by calling safeMint.

The code for the factory is below:

pragma solidity >=0.8.0;

import './ERC721ControlledUpgradeable.sol';
import '@openzeppelin/contracts/proxy/beacon/BeaconProxy.sol';

contract Factory {
    address public implementation;
    address[] public clonedContracts;

    constructor(address _impl) {


        implementation = _impl;
    }
    function createToken(string memory name, string memory symbol) external returns (address){
        // address clone = Clones.clone(implementation);
        // ERC721ControlledUpgradeable(clone).initialize(name, symbol);

        // clonedContracts.push(clone);
        // return clone;

        BeaconProxy proxy = new BeaconProxy(
            implementation, 
            abi.encodeWithSelector(ERC721ControlledUpgradeable.initialize.selector, name, symbol)
        );
        clonedContracts.push(address(proxy));
        return address(proxy);
    }
}

Any ideas?

The BeaconProxy works by holding the implementation address in a beacon contract. If you see the documentation for the constructor of BeaconProxy you can see that the first argument is not an implementation address but a beacon address.

You need to deploy an instance of UpgradeableBeacon, and then use that when you deploy each BeaconProxy.

@Lorenzo_Melendez , i am interested to know ur solution since am also trying to implement the same pattern :

  • Minial Proxy using Clones.sol
  • ERC721 upgradeable

the question is do i need to use erc721upgradeable or i can use the non upgradeable ... either ways a proxy can upgrade the implementation no ?? so where is difference between upgradeable contracts or not in our usecase??

@abcoathup please can u advice a solution for my scenario ..

pretty similar to the above schema :

erc721 that might be upgraded in future , a factory to clone implementation and create proxies ( same code as the erc721 but different storage ) ... whenever the erc721 is upgraded , is it possible to upgrade all proxies ??

@frangio , if you can help me understand and advice a schema

If you want to deploy minimal proxies you need to use the "Upgradeable" variant of the contracts. Even though the clones are not really upgradeable, they have most of the same requirements that upgradeable contracts have: initializer function, no immutable variables.

@frangio I have implemented the OpenZeppelin EIP-1167 Clone contract factory here:

I understand, as you have mentioned that you need to use the Upgradeable variant of all contracts (initializer function instead of contructor, no immutable variables).

However, I find that many of these contracts have a "storage gap" for future upgradeability.

uint256[42] private __gap;

Shouldn't this be removed from all the Upgradeable contracts when using them as Clones since they are not truly upgradeable and this storage gap will never be utilized. From what I understand, this is just a waste of gas and Solidity storage slots.

Please advise, want to make sure I get this perfect before deploying expensive contracts to mainnet.

Thanks,
Zaboomafoo

@zaboomafoo These storage gaps do not cost additional gas if their slots are not read/written to. You can test this out by deploying a simple contract with and without gaps, and see that the gas cost is the same.

But if you are using Clones and will never be upgrading your contracts, then there is no need to have storage gaps in your own contracts.

Continuing the discussion from Contract factory for upgradeable erc721:

Thanks @ericglau, everything is working well on my end except for the fact that owner() was resolving to the zero address for my deployed contracts.

I saw your thread here: https://github.com/OpenZeppelin/openzeppelin-upgrades/issues/514

So I added __Ownable_init(); to my initializer

However, this now sets the factory contract as the owner of the contract rather than the EOA that calls the create721A function.

Do you know how to fix this?

Here is my open-source repository: https://github.com/Candy-Labs/CandyContracts/tree/factory-withoutgov/contracts

In Base, you'll find the contract that I'm cloning through the factory
In Factories, you'll find the factory

Hi @zaboomafoo, you can call transferOwnership to transfer the owner to the EOA. See this example.

@ericglau I have added one more parameter (_msgSender()) to the line in the factory function that creates a new Clone:

function create721A(
        string memory name,
        string memory symbol,
        string memory _placeholderURI,
        uint256 _mintPrice,
        uint256 _mintSize,
        address[] memory splitAddresses,
        uint256[] memory splitShares,
        bytes32 _whitelistMerkleRoot,
        uint256 _maxPublicMints,
        uint64 _maxWhitelistMints
    )
    external returns (address) {
        address payable clone = payable(Clones.clone(tokenImplementation));
        CandyCreator721AUpgradeable(clone).initialize(name, symbol, _placeholderURI, _mintPrice, _mintSize, splitAddresses, splitShares, _whitelistMerkleRoot, _maxPublicMints, _maxWhitelistMints, candyWallet, _msgSender());
        emit CandyCreator721ACreated(clone);
        return clone;
    }

Then in the initialization function for the CandyCreator721AUpgradeable contract:

function initialize(
        // 32 bytes
        string memory name,
        // 32 bytes
        string memory symbol,
        // 32 bytes
        string memory _placeholderURI,
        // 32 bytes
        uint256 _mintPrice,
        // 32 bytes 
        uint256 _mintSize,
        // 32 bytes per element
        address[] memory splitAddresses,
        // 32 bytes per element
        // Can use uint16 (max value 65535) since an element in splitBasisPoints cannot be greater than 9,500.
        uint256[] memory splitBasisPoints,
        // 32 bytes
        bytes32 _whitelistMerkleRoot,
        // 32 bytes 
        uint256 _maxPublicMints,
        // 8 bytes
        uint64 _maxWhitelistMints,
        // 20 bytes
        address candyWallet
        // 20 bytes 
        address owner
    )   public initializer {
        __ERC721A_init(name, symbol);
        __Ownable_init();
        transferOwnership(owner);
        setupPaymentSplit(candyWallet, splitAddresses, splitBasisPoints);
        if (_whitelistMerkleRoot != 0) {
            whitelistMerkleRoot = _whitelistMerkleRoot;
            enableWhitelist();
        }
        placeholderURI = _placeholderURI;
        mintPrice = _mintPrice;
        mintSize = _mintSize;
        maxPublicMints = _maxPublicMints;
        maxWhitelistMints = _maxWhitelistMints;
    }

Does this look correct?

In addition, as I'm new to the Upgradeable variant of OZ contracts, is there any need to call init on the two abstract contracts my contract inherits from? Do these abstract contracts even need to inherit Initializable since they don't have initializer functions?

  1. PaymentSplitter
    https://github.com/Candy-Labs/CandyContracts/blob/factory-withoutgov/contracts/Base/PaymentSplitter/CandyPaymentSplitterUpgradeable.sol

  2. 2981 Royalties
    https://github.com/Candy-Labs/CandyContracts/blob/factory-withoutgov/contracts/Base/Royalties/CandyCollection2981RoyaltiesUpgradeable.sol

I think with these questions answered we will finally be able to upgrade our platform to the Clone Factory pattern. Thank you so much for your help.

That usage looks right.

The general recommendation is that if you have an initializer, then you need to inherit Initializable so you can use the initializer modifier. And in that initializer, call the _init method of every inherited contract from the @openzeppelin/contracts-upgradeable library.

If your abstract contract examples, the parent libraries do not have _init functions anyways (except the first link which inherits ContextUpgradeable but is not using it).

(Also, to clarify my earlier statement, if you will not be upgrading your contract since you are just using Clones, then you don't need storage gaps (although it doesn't cost more gas to have them)).