Code Monkey home page Code Monkey logo

Comments (11)

pcaversaccio avatar pcaversaccio commented on August 15, 2024 3

Hey guys, I personally feel this issue convo is better suited in the @openzeppelin/hardhat-upgrades repository. @charlie-kim may I ask you to open an issue in their repo and linking to this issue for the record. I will close this issue as "not planned" accordingly. Furthermore, as stated above, it's out-of-scope for xdeployer to offer tooling around upgradeable contracts, but I would like to nudge @ericglau & @cairoeth to consider incorporating CreateX into the Defender.

from xdeployer.

charlie-kim avatar charlie-kim commented on August 15, 2024 2

Using defender.deployImplementation worked fine. Thanks for jumping in, all! @pcaversaccio @ericglau @cairoeth

from xdeployer.

pcaversaccio avatar pcaversaccio commented on August 15, 2024

Hey @charlie-kim, thanks for sharing this feature request. Deploying and managing upgradeable contracts on Ethereum is something very complex and that's why OpenZeppelin has built their plugin. It's out-of-scope for xdeployer to offer this tooling and thus the best way to integrate the xdeployer functionalities into the @openzeppelin/hardhat-upgrades plugin is to enhance their CREATE2 support in the existing API:

image

So, to start with they could add the option to supply a CREATE2 factory like the one used in xdeployer: CreateX with address 0xba5Ed099633D3B313e4D5F7bdc1305d3c28ba5Ed. Furthermore, they could have the attributes networks and rpcUrls similar to xdeployer. What I'm saying is that it would be best to have a deterministic, cross-chain deployment solution for upgradeable contracts natively built into @openzeppelin/hardhat-upgrades. Hence, what I would recommend doing is to open in their repo an issue with this feature request.

cc: @ericglau

from xdeployer.

charlie-kim avatar charlie-kim commented on August 15, 2024

Thanks for your response, @pcaversaccio.

Unfortunately, Openzeppelin only supports CREATE2 for defender users which is $$$$. And it comes with way more features that I need. I guess it's too late to complain when I am already using their library...!

from xdeployer.

cairoeth avatar cairoeth commented on August 15, 2024

Thanks for your response, @pcaversaccio.

Unfortunately, Openzeppelin only supports CREATE2 for defender users which is $$$$. And it comes with way more features that I need. I guess it's too late to complain when I am already using their library...!

hey @charlie-kim! you should be able to use CREATE2 with the Deploy module using Defender free tier. are you getting any specific errors?

from xdeployer.

charlie-kim avatar charlie-kim commented on August 15, 2024

Hey @cairoeth, thanks for jumping in! It's great to hear defender free tier can work.

I tried defender option with hardhat-upgrades 2.5.1, then I got this error.

Error: Deployment response with id 21ce1ca1-6dca-4583-9771-d15518ebce7f does not include a contract address

The Hardhat Upgrades plugin is not currently compatible with this type of deployment. Use a relayer for your default deploy approval process in Defender.

Here's the beginning of the contract file.

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

import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
import {BeaconProxy} from "@openzeppelin/contracts/proxy/beacon/BeaconProxy.sol";

Also, is there a way to deploy regular contract with defender CREATE2?

from xdeployer.

ericglau avatar ericglau commented on August 15, 2024

Hi @charlie-kim, the error that you see is resolved a newer version of hardhat-upgrades. The latest version is currently 3.0.5.

Note that hardhat-upgrades versions 3.x is a new major version so there are some breaking changes compared to 2.x. See https://github.com/OpenZeppelin/openzeppelin-upgrades/releases/tag/%40openzeppelin%2Fhardhat-upgrades%403.0.0 for a summary -- mainly the proxy contracts are from OpenZeppelin Contracts 5.0.

You can also deploy non-upgradeable contracts using CREATE2, by using defender.deployContract and passing in a salt to the options of that function.

from xdeployer.

charlie-kim avatar charlie-kim commented on August 15, 2024

Hi @ericglau, the latest version is using an hardhat version that has a bug but I can still try deployment with it.

It seems like defender.deployContract has a more strict rule or bug with beacon proxy contract from @openzeppelin/contracts 4.9.5. I get this error.

Error: The contract contracts/Collateral.sol:Collateral looks like an upgradeable contract.

When I try to use defender.deployProxy, I get this error.

Error: Contract `contracts/Collateral.sol:Collateral` is not upgrade safe

contracts/Collateral.sol:109: Contract `Collateral` has a constructor
    Define an initializer instead
    https://zpl.in/upgrades/error-001

@openzeppelin/contracts/access/Ownable.sol:28: Contract `Ownable` has a constructor
    Define an initializer instead
    https://zpl.in/upgrades/error-001

I got rid of constructor of

constructor() {
        _disableInitializers();
    }

But I still get

Error: Contract `contracts/Collateral.sol:Collateral` is not upgrade safe

@openzeppelin/contracts/access/Ownable.sol:28: Contract `Ownable` has a constructor
    Define an initializer instead
    https://zpl.in/upgrades/error-00

The contract already has a initialize function.

function initialize(
        string memory _name,
        address _owner
    ) public validAddress(_owner) initializer {
        _transferOwnership(_owner);

        name = _name;
        factory = msg.sender;
        admins.push(_owner);
    }

Beacon contract uses these libs

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

import {UpgradeableBeacon} from "@openzeppelin/contracts/proxy/beacon/UpgradeableBeacon.sol";
import {IBeacon} from "@openzeppelin/contracts/proxy/beacon/IBeacon.sol";
import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";

Implementation contract uses these libs

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

import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

Using openzeppelin libs as below

"@openzeppelin/contracts": "^4.9.5",
"@openzeppelin/contracts-upgradeable": "^4.9.5"

from xdeployer.

ericglau avatar ericglau commented on August 15, 2024

@charlie-kim It seems there might be a misunderstanding of how hardhat-upgrades should be used, so I'll clarify:

  • defender.deployContract is only for deploying non-upgradeable contracts. Can you share what your Collateral.sol looks like?
  • For your implementation contract, you should inherit from @openzeppelin/contracts-upgradeable, not @openzeppelin/contracts because those use constructors which are generally not upgrade safe. So for example, use OwnableUpgradeable instead of Ownable.
  • For your implementation contract, the recommendation is to have your initializer, and also the following code (including the annotation):
    /// @custom:oz-upgrades-unsafe-allow constructor
    constructor() {
        _disableInitializers();
    }
  • Generally, there is no need to write your own code for the beacon or beacon proxy contracts. You can just use the deployBeacon and deployBeaconProxy functions.

from xdeployer.

charlie-kim avatar charlie-kim commented on August 15, 2024

@ericglau The contracts look like this.

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

import {UpgradeableBeacon} from "@openzeppelin/contracts/proxy/beacon/UpgradeableBeacon.sol";
import {IBeacon} from "@openzeppelin/contracts/proxy/beacon/IBeacon.sol";
import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";

/**
 *  @title Beacon contract
 *  @notice Wrapper of beacon contract to Collateral implementation contract
 *  @dev Update the implementation of all Collateral contracts
 *       generated by CollateralFactory in a single transaction.
 *       Using beacon proxy pattern that Collateral contracts(proxies) points to this contract,
 *       while this contract points to the current implementation of Collateral.
 *       https://docs.openzeppelin.com/contracts/4.x/api/proxy#BeaconProxy
 */
contract Beacon is IBeacon, Ownable {
    /// @notice Holds reference to beacon contract
    UpgradeableBeacon public immutable BEACON;

    /**
     * @notice Initialize
     * @param _initialImplementation initial Collateral implementation contract address
     * @param _owner owner of this contract
     */
    constructor(address _initialImplementation, address _owner) {
        BEACON = new UpgradeableBeacon(_initialImplementation);
        transferOwnership(_owner);
    }

    /**
     * @notice update Collateral implementation
     * @dev will update implementation of multiple Collateral contracts
     */
    function updateImplementation(address _newImplementation) public onlyOwner {
        BEACON.upgradeTo(_newImplementation);
    }

    /**
     * @notice get RainCollateral implementation contract address
     * @return address of implementation contract saved in beacon
     */
    function implementation() public view returns (address) {
        return BEACON.implementation();
    }
}
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;

import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
import {IFactory} from "./interfaces/IFactory.sol";

using SafeERC20 for IERC20;

contract Collateral is Initializable, Ownable {
    /// @notice name of collateral
    string public name;

    /// @notice Factory Address
    ///         It is used to get controller address held in factory
    address public factory;


    /// @notice Prevent the implementation contract from being used
    constructor() {
        _disableInitializers();
    }

    /**
     * @notice Initialize this contract
     * @param _name name of collateral
     * @param _owner address of collateral owner
     *               also becomes the first admin
     */
    function initialize(
        string memory _name,
        address _owner
    ) public initializer {
        _transferOwnership(_owner);

        name = _name;
        factory = msg.sender;
    }
    
    /**
     * @notice Block renounceOwnership
     * @dev Prevent assets to be stuck in the contract without an owner
     */
    function renounceOwnership() public virtual override onlyOwner {
        revert DisabledOwnershipRenouncement();
    }
}

from xdeployer.

charlie-kim avatar charlie-kim commented on August 15, 2024

I made a small progress by using OwnableUpgradeable and putting annotation above constructor.

Now I am getting this error.

Error: missing arguemnt: types/values length mismatch (count=0, expectedCount=2, code=MISSING_ARGUMENT, version=6.8.1)

When deploying with

const collateralFactory = await ethers.getContractFactory("Collateral", owner);
    const collateral = await defender.deployProxy(
      collateralFactory,
      [],
      {
        initializer: "initialize",
        salt: SALT
      }
    );

I am not sure what parameter I am supposed to use for initialize function because I am deploying an implementation contract.

Should I use defender.deployImplementation instead?

from xdeployer.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.