OpenZeppelin / openzeppelin-upgrades

Plugins for Hardhat and Foundry to deploy and manage upgradeable contracts on Ethereum.
MIT License
618 stars 266 forks source link

Issue with deployProxy when using ContractFactory instantiated directly with ABI and Bytecode #916

Open mshakeg opened 11 months ago

mshakeg commented 11 months ago

Summary:

I encountered an error while using the @openzeppelin/hardhat-upgrades@2.3.3 plugin's deployProxy function. The error arises when I use a ContractFactory created directly with the ABI and Bytecode, whereas using getContractFactory works correctly.

Details:

  1. Initial Problem:

    • When trying to use the deployProxy function from @openzeppelin/hardhat-upgrades, I received the following error:

      Error: factory runner does not support sending transactions (operation="sendTransaction", code=UNSUPPORTED_OPERATION, version=6.4.0)

  2. Setup:

    • Ethers.js version initially was 6.4.0.
    • Hardhat version used: 2.19.0
    • The problematic contract factory was created as follows:
      const RebalancerFactory = new ethers.ContractFactory(REBALANCER_FACTORY_ABI, REBALANCER_FACTORY_BYTECODE, deployer);
  3. Working Alternative:

    • When I use hardhat ethers@6.7.0 getContractFactory, the error doesn't appear and everything works as expected:
      const RebalancerFactory = await ethers.getContractFactory("RebalancerFactory", deployer);
    • However, on ethers 6.4.0(which I had installed originally) I faced an issue related to the absence of the attach method on the ContractFactory:

      Property 'attach' does not exist on type 'RebalancerFactory__factory'.

  4. Current Status:

    • With ethers@6.7.0, using getContractFactory works flawlessly.
    • However, creating the ContractFactory directly with ABI and Bytecode (as shown above) still results in the aforementioned deployProxy error.

Steps to Reproduce:

  1. Set up a Hardhat project with ethers@6.7.0 and hardhat@2.19.0.
  2. Use the @openzeppelin/hardhat-upgrades plugin.
  3. Attempt to create a contract factory using the direct ABI and Bytecode method:
    const RebalancerFactory = new ethers.ContractFactory(REBALANCER_FACTORY_ABI, REBALANCER_FACTORY_BYTECODE, deployer);
  4. Use this contract factory with the deployProxy function.
  5. Observe the error mentioned above.

Expected Behavior:

Both methods of creating a ContractFactory (direct ABI/Bytecode instantiation and using getContractFactory) should work seamlessly with the deployProxy function without any errors.

mshakeg commented 11 months ago

My issue is potentially caused by having an importer file that specifically imports a contract from an installed dependency which I'm doing to avoid the issue described in #86

ericglau commented 11 months ago

Can you share what is your use case for using a ContractFactory with ABI+bytecode?

The plugin normally expects getContractFactory("ContractName"...) because that provides access to the source code of the implementation contract, which we use for extracting storage layout information for comparison as part of the upgrade safety checks.

mshakeg commented 11 months ago

@ericglau in a hardhat project I'm installing a package that has upgradeable contracts, those packages are private but for the purposes of this discussion let's say this package is an openzeppelin upgradeable version of Uniswap/v3-core. Now in my hardhat project that installs this upgradeable @uniswap/v3-core-upgradeable package I would like to import the upgradeable UniswapV3Factory abi and bytecode from the relevant @uniswap/v3-core json artifact as shown here. However, as explained in the issue description attempting this fails:

const UniswapV3FactoryContractFactory = new ethers.ContractFactory(UPGRADEABLE_FACTORY_ABI, UPGRADEABLE_FACTORY_BYTECODE, deployerSigner);

const upgradeableUniswapV3Factory = await upgrades.deployProxy(UniswapV3FactoryContractFactory, [], {
  initializer: "initialize",
});

With the error:

Error: factory runner does not support sending transactions (operation="sendTransaction", code=UNSUPPORTED_OPERATION, version=6.4.0)

mshakeg commented 11 months ago

@ericglau it would be great to at least have the option(say called forceDeploy) to disable/ignore those safety checks.

ericglau commented 11 months ago

it would be great to at least have the option(say called forceDeploy) to disable/ignore those safety checks.

We can consider adding an option similar to this.

However, I think your scenario is failing at an earlier step because the deployer that you are passing in does not seem to be a signer. I notice the same error if I use ABI+bytecode according to your steps but do not pass in a signer when instantiating the contract factory.

If I pass in an actual signer (for example in tests, (await ethers.getSigners())[0]), I get a different error (which is the one I expect because source code is not found): The requested contract was not found. Make sure the source code is available for compilation

But aside from the signer issue, for now you may need to deploy the implementation and proxy contracts directly (using ERC1967Proxy.sol or TransparentUpgradeableProxy.sol) until we add the proposed option.

mshakeg commented 11 months ago

@ericglau thanks for investigating, yeah the deployer issue was a bug on my end.