Deploying Upgradeable Proxies and Proxy Admin from Factory contract

Support Upgrades
1

HI!

Is it possible to deploy Upgradable Proxies from a Factory contract?

What we need to achieve is have a Factory Contract that can deploy the ProxyAdmin, an Upgradable Proxy (Transparent or UUPS), and then transfer ownership to the deployer, and I have to do this at runtime.

Is this something that can be achieved? Are there examples I can use?

I’ve looked at the OZ contracts and found the Clones library but that is for non upgradable proxies, so would not cover my use case because I need the user to be able to update the implementation of these proxies after deployment.

Any advise would be highly appreciated! Thanks!

Regards,
Julian

3 Likes
2

This can be achieved but we don’t have any examples. I believe it should be quite simple to put together by importing the relevant contracts and just using Solidity’s new operator create the contract instances. If you want to put together this code and share it here I can review it!

3

@frangio Here is my initial attempt. Please let me know if its in the right direction.

It looks too easy to be true. I created two contracts the UpgradeableToken and the Factory contract which deploys the Tokens. I realized I don’t need a ProxyAdmin for UUPS.

One of the doubts I had was which type of “Proxy” should I use for UUPS. I went for ERC1967Proxy not sure if it’s the correct choice.

MyTokenUpgradeable

// SPDX-License-Identifier: MIT

pragma solidity 0.8.4;

import "@openzeppelin/contracts-upgradeable/token/ERC20/presets/ERC20PresetFixedSupplyUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
import "@openzeppelin/contracts/proxy/utils/UUPSUpgradeable.sol";

contract MyTokenUpgradeable is
    ERC20PresetFixedSupplyUpgradeable,
    UUPSUpgradeable,
    OwnableUpgradeable
{
    function initialize(
        string memory name,
        string memory symbol,
        uint256 initialSupply,
        address owner
    ) public override initializer {
        __ERC20PresetFixedSupply_init(name, symbol, initialSupply, owner);
        __Ownable_init();
        transferOwnership(owner);
    }

    function _authorizeUpgrade(address newImplementation) internal override onlyOwner {}
}

MyTokenFactory

// SPDX-License-Identifier: MIT

pragma solidity 0.8.4;

import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
import "./MyTokenUpgradeable.sol";

contract MyTokenFactoryUUPS {
    address immutable tokenImplementation;

    event TokenDeployed(address tokenAddress);

    constructor() {
        tokenImplementation = address(new MyTokenUpgradeable());
    }

    function createToken(string calldata name, string calldata symbol, uint256 initialSupply, address owner) external returns (address) {
        ERC1967Proxy proxy = new ERC1967Proxy(
            tokenImplementation,
            abi.encodeWithSelector(MyTokenUpgradeable(address(0)).initialize.selector, name, symbol, initialSupply, owner)
        );
        emit TokenDeployed(address(proxy));
        return address(proxy);
    }
}

One thing I wasn’t sure is if there is a better way to set the new owner for the Token, but seems like I have to do __Ownable_init so I can then transfer it to the provided owner. Any workaround to avoid the 2-step process?

Thank you very much for all your support!

Regards,
Julian

4

because you inherited from OwnableUpgradeable and it does not have constructor so you must call it’s initialize function. (initializer functions are not called automatically).
so owner will not set and you can’t upgrade because you use onlyOwner modifier in _authorizeUpgrade if you ommit __Ownable_init(); .

2 Likes
5

Your code looks good to me!

There’s no way to avoid the 2-step ownership setup, but consider that the second time you set the owner the storage slot is warm so it will be cheaper to write. (Actually, does RSK have EIP-2929?)

2 Likes
6

Thanks Fran for looking at the code! Im working fully on Ethereum now so this is great to know. Not sure about RSK but I think they did something different than EIP-2929.

1 Like
7

@julianmrodri @frangio As a follow up question to this - imagine we wanted to have a Controller/Manager, almost like a layer on top of the Tokens (maybe to keep a mapping(proxy => Token) for whatever reason).

What would a proper architecture for this be? Would it be better to couple the factory with this manager or as a different contract?

8

I think it would be okay to build that into the factory itself, if you need it in a centralized place.

1 Like
9

Forgive my ignorance, but what is the proper way to actually upgrade the underlying contract?

I imagine something like UUPSUpgradeable(myTokenFactoryUUPS.tokenImplementation).upgradeTo(newImplementation)

Would that be correct?

1 Like
10

The upgradeTo function needs to be called on each proxy, not on the implementation like you did in your snippet.

11

How could one upgrade all implementations at once? That's the use case for a beacon, no? If so, how would we have to change the implementation above for that to work?

12

Based on the code by @julianmrodri, with beacon proxy it would look something like this:

Note: Untested sample code.

// SPDX-License-Identifier: MIT

pragma solidity 0.8.4;

import "@openzeppelin/contracts/proxy/beacon/UpgradeableBeacon.sol";
import "@openzeppelin/contracts/proxy/beacon/BeaconProxy.sol";

import "./MyTokenUpgradeable.sol";

contract MyTokenFactoryBeacon {
    address immutable tokenBeacon;

    event TokenDeployed(address tokenAddress);

    constructor(address upgrader) {
        UpgradeableBeacon _tokenBeacon = new UpgradeableBeacon(address(new MyTokenUpgradeable()));
        _tokenBeacon.transferOwnership(upgrader);
        tokenBeacon = address(tokenBeacon);
    }

    function createToken(string calldata name, string calldata symbol, uint256 initialSupply, address owner) external returns (address) {
        BeaconProxy proxy = new BeaconProxy(
            tokenBeacon,
            abi.encodeWithSelector(MyTokenUpgradeable.initialize.selector, name, symbol, initialSupply, owner)
        );
        emit TokenDeployed(address(proxy));
        return address(proxy);
    }
}
1 Like
13

I had just wrote out the same thing, but you beat me to it :slight_smile:

Only question deals with the abi.encodeWithSelector - the way we have this now, we theoretically couldn't change the initializer of MyTokenUpgradeable.

...unless, MyFactoryTokenBeacon was also upgradeable_, using UUPS or something. Then you could reimplement the the createToken in a MyTokenFactoryBeacon, deploy that, and be on our way, right?

14

Good point. I would do things differently then:

     function createToken(bytes calldata data) external returns (address) {
        BeaconProxy proxy = new BeaconProxy(
            tokenBeacon,
            data
        );
        ...

This way the caller can provide whatever initialization function they want.

If you want to keep something more like the previous version, then yes you could make the factory itself upgradeable (UUPS would be fine).

16

Hi @frangio - I'm also trying to implement a factory for beacon proxies but must be doing something incorrectly.

Here is the problem:
I have a storage variable currentEditionId on the contract being proxied (Artist.sol) that increments each time the createEdition function is called. It starts from 1, and that works when I test Artist.sol by itself. However, when I test the proxied version of it deployed by ArtistCreator.sol, it is starting from zero.

My hunch is I'm making a common mistake? Here are the relevant parts of ArtistCreator.sol:

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

import './Artist.sol';

contract ArtistCreator {
    address immutable beaconAddress;

    mapping(uint32 => address) private artists;

    /// Emitted when an Artist is created reserving the corresponding artist ID.
    /// @param artistId ID of newly created Artist
    event CreatedArtist(uint256 indexed artistId);

    /// Initializes implementation contract
    constructor(address upgrader) {
        UpgradeableBeacon _beacon = new UpgradeableBeacon(address(new Artist()));
        _beacon.transferOwnership(upgrader);
        beaconAddress = address(_beacon);
    }

    /// Creates a new artist proxy
    /// @param _name Name of the artist contract
    /// @param _baseURI Base URI of the artist contract (includes artist's id)
    function createArtist(
        string calldata _name,
        uint32 _artistId,
        string calldata _baseURI
    ) external returns (address) {
        BeaconProxy proxy = new BeaconProxy(
            beaconAddress,
            abi.encodeWithSelector(Artist(address(0)).initialize.selector, tx.origin, _name, _baseURI)
        );

        artists[_artistId] = address(proxy);

        emit CreatedArtist(_artistId);
        return address(proxy);
    }

I also forked the repo and stripped it down to demo the problem in case you want to install & run the tests: https://github.com/SoundCollective/factory-contract-help/blob/master/test/main.test.ts

Any info would be hugely appreciated!

17

@gigamesh, you need to initialize currentEditionId in the initializer function of your Artist.sol contract like this:

    uint256 private currentEditionId;

    // ============ Events ============

    event EditionCreated(
        uint256 quantity,
        uint256 price,
        address fundingRecipient,
        uint256 indexed editionId,
        uint256 royaltyBPS
    );

    event EditionPurchased(
        uint256 indexed editionId,
        uint256 indexed tokenId,
        // `numSold` at time of purchase represents the "serial number" of the NFT.
        uint256 numSold,
        // The account that paid for and received the NFT.
        address indexed buyer
    );

    // ============ Methods ============

    /**
      @param _owner Owner of edition
      @param _name Name of edition, used in the title as "$NAME NUMBER/TOTAL"
      @param _baseURI Base URI of the API that serves the content for each token
     */
    function initialize(
        address _owner,
        string memory _name,
        string memory _baseURI
    ) public initializer {
        baseURI = _baseURI;
        __ERC721_init(_name, 'sound.xyz');
        __Ownable_init();
        // Set ownership to original sender of contract call
        transferOwnership(_owner);
        currentEditionId = 1;
    }

I was able to get your test to pass by making this change.

1 Like
18

Huge thanks @TtheBC01 !

For anyone who comes across this in the future, here is the relevant part of the docs that I missed. :slight_smile:

1 Like
19

This conversation has been great!

I'd like to build on the pattern you laid out above with MyTokenFactoryBeacon, but rather than hard-code MyTokenUpgradeable in the constructor, use simply an implementation address that could point to anything that was upgradeable. Ultimately, this would be a reusable factory, regardless of what the implementation is. One could imagine its implementation to look something like:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.9;

import "@openzeppelin/contracts/proxy/beacon/UpgradeableBeacon.sol";
import "@openzeppelin/contracts/proxy/beacon/BeaconProxy.sol";


 contract ProxyBeaconFactory {
    address public beacon;

    address[] public beacons;

    event ProxyDeployed(address proxyAddress);

    constructor(address implementation_) {
        UpgradeableBeacon _beacon = new UpgradeableBeacon(
            implementation_
        );
        _beacon.transferOwnership(msg.sender);
        beacon = address(_beacon);
    }

    function create(bytes calldata data) external returns (address) {
        address proxyAddress = new BeaconProxy(beacon, data);
        beacons.push(proxyAddress);
        emit ProxyDeployed(proxyAddress);
        return proxyAddress;
    }
}

I do wonder though about the recent announcement regarding potential risks using UUPS. If the implementation_ does indeed implement the UUPS pattern, are there any measures that need to be taken to guard against those risks?

Additionally, I believe the pattern to ultimately use this is something like:

1. Deploy a MyTokenUpgradeable contract normally - without OZ upgrades or anything.
2. Deploy ProxyBeaconFactory(myTokenUpgradeable.address)
3. proxyBeaconFactory.create(dataToInitializeAnInstanceOfMyTokenUpgradeable)

Am I thinking about this the right way?

1 Like
20

The pattern is right and the code looks good. I would just rename the variable beacons to proxies.

Regarding the UUPS vulnerability, please note that it has already been patched. That said, we still recommend that you take the measure of initializing your implementation contracts to make sure they are not usable, as explained in this section of the docs.

2 Likes
21

What is actually the benefit of inheriting UUPSUpgradeable in the implementation contract of your UpgradeableBeacon pattern? After thinking about it for a while, it seems redundant. Or is there some common use case for combining the two upgrade strategies? I guess each deployed beacon proxy could implement its own unique upgrades, but I would think this could cause conflicts when the upgradeableBeacon points to a new implementation.

2 Likes