How to setup a Factory for an upgradeable smart contract deployed with UUPS Proxy?

I am trying to create a Factory for the following smart contract:

SmartWallet.sol

// SPDX-License-Identifier: Apache-2.0
pragma solidity ^0.8.11;

//  ==========  External imports    ==========

import "@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721EnumerableUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/interfaces/IERC2981Upgradeable.sol";

//  ==========  Internal imports    ==========

import "../interfaces/ISmartWallet.sol";
import "../openzeppelin-presets/metatx/ERC2771ContextUpgradeable.sol";

//  ==========  Features    ==========

import "../extension/ContractMetadata.sol";
import "../extension/Royalty.sol";
import "../extension/ThirdOwnable.sol";
import "../extension/PermissionsEnumerable.sol";
import { TokenStore, ERC1155ReceiverUpgradeable, IERC1155ReceiverUpgradeable } from "../extension/TokenStore.sol";

//  ==========  Proxy    ==========

import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";

contract SmartWallet is
    Initializable,
    ContractMetadata,
    Royalty,
    ThirdOwnable,
    PermissionsEnumerable,
    TokenStore,
    ReentrancyGuardUpgradeable,
    ERC2771ContextUpgradeable,
    ERC721EnumerableUpgradeable,
    ISmartWallet,
    UUPSUpgradeable
{
    /*///////////////////////////////////////////////////////////////
                            State variables
    //////////////////////////////////////////////////////////////*/

    bytes32 private constant MODULE_TYPE = bytes32("Multiwrap");
    uint256 private constant VERSION = 1;

    /// @dev Only transfers to or from TRANSFER_ROLE holders are valid, when transfers are restricted.
    bytes32 private constant TRANSFER_ROLE = keccak256("TRANSFER_ROLE");
    /// @dev Only MINTER_ROLE holders can wrap tokens, when wrapping is restricted.
    bytes32 private constant MINTER_ROLE = keccak256("MINTER_ROLE");
    /// @dev Only UNWRAP_ROLE holders can unwrap tokens, when unwrapping is restricted.
    bytes32 private constant UNWRAP_ROLE = keccak256("UNWRAP_ROLE");
    /// @dev Only assets with ASSET_ROLE can be wrapped, when wrapping is restricted to particular assets.
    bytes32 private constant ASSET_ROLE = keccak256("ASSET_ROLE");

    /// @dev The next token ID of the NFT to mint.
    uint256 public nextTokenIdToMint;

    /*///////////////////////////////////////////////////////////////
                    Constructor + initializer logic
    //////////////////////////////////////////////////////////////*/

    /// @custom:oz-upgrades-unsafe-allow constructor
    constructor(address _nativeTokenWrapper) TokenStore(_nativeTokenWrapper) initializer {} // 0x9c3C9283D3e44854697Cd22D3Faa240Cfb032889

    /// @dev Initiliazes the contract, like a constructor.
    function initialize(
        address _defaultAdmin,
        string memory _name,
        string memory _symbol,
        string memory _contractURI,
        address[] memory _trustedForwarders,
        address _royaltyRecipient,
        uint256 _royaltyBps
    ) external initializer {
        // Initialize inherited contracts, most base-like -> most derived.
        __ReentrancyGuard_init();
        __ERC2771Context_init(_trustedForwarders);
        __ERC721_init(_name, _symbol);

        // Initialize this contract's state.
        _setupDefaultRoyaltyInfo(_royaltyRecipient, _royaltyBps);
        _setupOwner(_defaultAdmin);
        _setupContractURI(_contractURI);

        _setupRole(DEFAULT_ADMIN_ROLE, _defaultAdmin);
        _setupRole(MINTER_ROLE, _defaultAdmin);
        _setupRole(TRANSFER_ROLE, _defaultAdmin);

        // note: see `_beforeTokenTransfer` for TRANSFER_ROLE behaviour.
        _setupRole(TRANSFER_ROLE, address(0));

        // note: see `onlyRoleWithSwitch` for UNWRAP_ROLE behaviour.
        _setupRole(UNWRAP_ROLE, address(0));

        // note: see `onlyRoleWithSwitch` for UNWRAP_ROLE behaviour.
        _setupRole(ASSET_ROLE, address(0));

        // note: mints empty Smart NFT
        mintSmartNFT(_defaultAdmin);

    }

    ///@dev required by the OZ UUPS module
   function _authorizeUpgrade(address) internal override onlyOwner {}

    /*///////////////////////////////////////////////////////////////
                            Modifiers
    //////////////////////////////////////////////////////////////*/

    modifier onlyRoleWithSwitch(bytes32 role) {
        _checkRoleWithSwitch(role, _msgSender());
        _;
    }

    /*///////////////////////////////////////////////////////////////
                        Generic contract logic
    //////////////////////////////////////////////////////////////*/

    /// @dev Returns the type of the contract.
    function contractType() external pure returns (bytes32) {
        return MODULE_TYPE;
    }

    /// @dev Returns the version of the contract.
    function contractVersion() external pure returns (uint8) {
        return uint8(VERSION);
    }

    /// @dev Lets the contract receive ether to unwrap native tokens.
    receive() external payable {
        require(msg.sender == nativeTokenWrapper, "caller not native token wrapper.");
    }

    /*///////////////////////////////////////////////////////////////
                        ERC 165 / 721 / 2981 logic
    //////////////////////////////////////////////////////////////*/

    /// @dev Returns the URI for a given tokenId.
    function tokenURI(uint256 _tokenId) public view override returns (string memory) {
        return getUriOfBundle(_tokenId);
    }

    /// @dev See ERC 165
    function supportsInterface(bytes4 interfaceId)
        public
        view
        virtual
        override(ERC1155ReceiverUpgradeable, ERC721EnumerableUpgradeable, IERC165Upgradeable)
        returns (bool)
    {
        return
            super.supportsInterface(interfaceId) ||
            interfaceId == type(IERC721Upgradeable).interfaceId ||
            interfaceId == type(IERC1155ReceiverUpgradeable).interfaceId ||
            interfaceId == type(IERC2981Upgradeable).interfaceId;
    }

    /*///////////////////////////////////////////////////////////////
                    Minting logic
    //////////////////////////////////////////////////////////////*/

    /// @dev Mint an empty ERC721 NFT that can later wrap multiple ERC1155, ERC721, ERC20 tokens.
    function mintSmartNFT(
        address _recipient
    ) public nonReentrant onlyRoleWithSwitch(MINTER_ROLE) returns (uint256 tokenId) {

        Token[] memory noTokens;
        tokenId = nextTokenIdToMint;
        nextTokenIdToMint += 1;
        
        _safeMint(_recipient, tokenId);

        emit SmartWalletCreated(_msgSender(), _recipient, tokenId, noTokens);
    }

    /*///////////////////////////////////////////////////////////////
                    Loading / Unloading logic
    //////////////////////////////////////////////////////////////*/

    /// @dev Load multiple ERC1155, ERC721, ERC20 tokens into empty Smart NFT
    function initialLoad(
        Token[] calldata _tokensToLoad,
        uint256 _smartNFTId,
        string calldata _uriForLoadedToken
    ) external payable nonReentrant onlyRoleWithSwitch(MINTER_ROLE) {

        require(getTokenCountOfBundle(_smartNFTId) == 0, "Smart NFT not empty. Call loadTokens instead.");

        if (!hasRole(ASSET_ROLE, address(0))) {
            for (uint256 i = 0; i < _tokensToLoad.length; i += 1) {
                _checkRole(ASSET_ROLE, _tokensToLoad[i].assetContract);
            }
        }

        // check that Smart NFT exists
        require(_smartNFTId < nextTokenIdToMint, "Smart NFT DNE.");

        _storeTokens(_msgSender(), _tokensToLoad, _uriForLoadedToken, _smartNFTId);

        emit TokensLoaded(_msgSender(), _smartNFTId, _tokensToLoad);

    }

    /// @dev Load multiple ERC1155, ERC721, ERC20 tokens into a non-empty Smart NFT (bundle already created).
    function loadTokens(
        Token[] calldata _tokensToLoad,
        uint256 _smartNFTId
    ) external payable nonReentrant onlyRoleWithSwitch(MINTER_ROLE) {

        require(getTokenCountOfBundle(_smartNFTId) > 0, "Smart NFT is empty. Call initialLoad instead.");
        require(_smartNFTId < nextTokenIdToMint, "Smart NFT DNE.");
        require(_isApprovedOrOwner(_msgSender(), _smartNFTId), "caller not approved for loading this Smart NFT.");

        if (!hasRole(ASSET_ROLE, address(0))) {
            for (uint256 i = 0; i < _tokensToLoad.length; i += 1) {
                _checkRole(ASSET_ROLE, _tokensToLoad[i].assetContract);
            }
        }

        for (uint256 i = 0; i < _tokensToLoad.length; i += 1) {
            _addTokenInBundle(_tokensToLoad[i], _smartNFTId);
            _transferToken(_msgSender(), address(this), _tokensToLoad[i]);
        }

        emit TokensLoaded(_msgSender(), _smartNFTId, _tokensToLoad);
    }

    /// @dev Unload from Smart Wallet to retrieve one underlying ERC1155, ERC721, ERC20 token.
    function unloadToken(uint256 _smartNFTId, Token calldata _tokenToRetrieve, address _recipient) external nonReentrant onlyRoleWithSwitch(UNWRAP_ROLE) {
        require(_smartNFTId < nextTokenIdToMint, "Smart NFT DNE.");
        require(_isApprovedOrOwner(_msgSender(), _smartNFTId), "caller not approved for unloading.");

        _releaseToken(_recipient, _smartNFTId, _tokenToRetrieve);

        emit TokenUnloaded(_msgSender(), _recipient, _smartNFTId, _tokenToRetrieve);

    }

    /// @dev Unload from Smart Wallet to retrieve all underlying ERC1155, ERC721, ERC20 tokens.
    function unloadAll(uint256 _smartNFTId, address _recipient) external nonReentrant onlyRoleWithSwitch(UNWRAP_ROLE) {
        require(_smartNFTId < nextTokenIdToMint, "Smart NFT DNE.");
        require(_isApprovedOrOwner(_msgSender(), _smartNFTId), "caller not approved for unloading.");

        _releaseTokens(_recipient, _smartNFTId);

        emit AllTokensUnloaded(_msgSender(), _recipient, _smartNFTId);

    }

    /*///////////////////////////////////////////////////////////////
                        Getter functions
    //////////////////////////////////////////////////////////////*/

    /// @dev Returns the underlying contents of a wrapped NFT.
    function getWrappedContents(uint256 _tokenId) external view returns (Token[] memory contents) {
        uint256 total = getTokenCountOfBundle(_tokenId);
        contents = new Token[](total);

        for (uint256 i = 0; i < total; i += 1) {
            contents[i] = getTokenOfBundle(_tokenId, i);
        }
    }

    /*///////////////////////////////////////////////////////////////
                        Internal functions
    //////////////////////////////////////////////////////////////*/

    /// @dev Returns whether owner can be set in the given execution context.
    function _canSetOwner() internal view override returns (bool) {
        return hasRole(DEFAULT_ADMIN_ROLE, _msgSender());
    }

    /// @dev Returns whether royalty info can be set in the given execution context.
    function _canSetRoyaltyInfo() internal view override returns (bool) {
        return hasRole(DEFAULT_ADMIN_ROLE, _msgSender());
    }

    /// @dev Returns whether contract metadata can be set in the given execution context.
    function _canSetContractURI() internal view override returns (bool) {
        return hasRole(DEFAULT_ADMIN_ROLE, _msgSender());
    }

    /*///////////////////////////////////////////////////////////////
                        Miscellaneous
    //////////////////////////////////////////////////////////////*/

    /**
     * @dev See {ERC721-_beforeTokenTransfer}.
     */
    function _beforeTokenTransfer(
        address from,
        address to,
        uint256 tokenId
    ) internal virtual {
        super._beforeTokenTransfer(from, to, tokenId, 1);

        // if transfer is restricted on the contract, we still want to allow burning and minting
        if (!hasRole(TRANSFER_ROLE, address(0)) && from != address(0) && to != address(0)) {
            require(hasRole(TRANSFER_ROLE, from) || hasRole(TRANSFER_ROLE, to), "!TRANSFER_ROLE");
        }
    }

    function _msgSender()
        internal
        view
        virtual
        override(ContextUpgradeable, ERC2771ContextUpgradeable)
        returns (address sender)
    {
        return ERC2771ContextUpgradeable._msgSender();
    }

    function _msgData()
        internal
        view
        virtual
        override(ContextUpgradeable, ERC2771ContextUpgradeable)
        returns (bytes calldata)
    {
        return ERC2771ContextUpgradeable._msgData();
    }
}

The contract is working as expected and I am able to deploy it correctly using Hardhat. This is the script I am using:

DeploySmartWallet.js

const { ethers, upgrades } = require("hardhat");

async function main() {
   const gas = await ethers.provider.getGasPrice()
   const V1contract = await ethers.getContractFactory("SmartWallet");
   console.log("Deploying V1contract...");
   const v1contract = await upgrades.deployProxy(
      V1contract, 
      [
         "0xF8e6A72C0e72E0Eb510bD20097b0522Eae4B24E4", // _defaultAdmin
         "Smart NFT", // _name
         "SNFT", // _symbol
         "My Smart Wallet", // _contractURI
         [], // _trustedForwarders
         "0xF8e6A72C0e72E0Eb510bD20097b0522Eae4B24E4", // _royaltyRecipient
         0 // _royaltyBps */
      ], {
         gasPrice: gas, 
         initializer: "initialize",
         kind: "uups",
         constructorArgs: ["0x9c3C9283D3e44854697Cd22D3Faa240Cfb032889"]
   });
   await v1contract.deployed();
   console.log("V1 SmartWallet Contract deployed to: ", v1contract.address);
}

main().catch((error) => {
   console.error(error);
   process.exitCode = 1;
 });

But of course this only deploys one instance of my contract. What I need to do is to create a Factory that deploys many instances of SmartWallet.sol on demand. The code I have to do that is the following, but it is not working:

SmartWalletFactory.sol

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

import "../multiwrap/SmartWallet.sol";
import "../proxy/SmartWalletProxy.sol";

contract SmartWalletFactory {
    mapping(uint256 => address) private smartWallets;
    TransparentUpgradeableProxy immutable proxy;

    constructor(address _initialImpl) {
        proxy = new TransparentUpgradeableProxy(_initialImpl, address(this), "0x");
    }

    function createSmartWallet(
        uint256 _smartWalletId,
        address _defaultAdmin,
        string memory _name,
        string memory _symbol,
        string memory _contractURI,
        address[] memory _trustedForwarders,
        address _royaltyRecipient,
        uint256 _royaltyBps
    ) public {
        TransparentUpgradeableProxy smartWallet = new TransparentUpgradeableProxy(address(proxy), address(this), abi.encodeCall(SmartWallet(payable(address(0))).initialize, (_defaultAdmin, _name, _symbol, _contractURI, _trustedForwarders, _royaltyRecipient, _royaltyBps)));
        smartWallets[_smartWalletId] = address(smartWallet);
    }
}

Does anybody know what's the best solution to achieve this behaviour?

The error I am getting when I try to deploy SmartWalletFactory.sol using the address of SmartWallet.sol as initial implementation is the following:

{
"code": 3,
"message": "execution reverted: Address: low-level delegate call failed",
"data": "0x08c379a000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000027416464726573733a206c6f772d6c6576656c2064656c65676174652063616c6c206661696c656400000000000000000000000000000000000000000000000000"
}

The problem is that the first argument to constructor TransparentUpgradeableProxy in the function createSmartWallet should be the address of the implementation contract. You are instead passing the address of a proxy.

SmartWalletFactory should be more like this:

contract SmartWalletFactory {
    address impl;

    constructor(address _initialImpl) {
        impl = _initialImpl;
    }

And in the function createSmartWallet you can write:

TransparentUpgradeableProxy smartWallet = new TransparentUpgradeableProxy(impl, admin, abi.encodeCall(...));

I.e., the first argument is impl. The second argument should be the initial proxy admin, not sure what value you want to use for that, but address(this) sounds like it may not be the right value?

1 Like

I'm noticing something weird though. You asked about UUPS, but you're deploying a TransparentUpgradeableProxy. When you want a UUPS system, you should deploy an ERC1967Proxy instead.

1 Like

Yes, good catch as well. I have this solution working already and this is something that I had to change.