Getting tx error during update the contract

Furkan_Sezal · August 28, 2023, 9:03pm

Hi, i have transparent proxy contract. I deployed succesfully. But when i try to upgrade the contract i am getting this tx error:

Deployment:
First i deployed proxyAdmin contract then i deployed V1 contract. Then i deployed V2 contract. On V2 contract i have same initialize method.
On upgrade script:

 const proxyAdmin = await ethers.getContract("ProxyAdmin");
  const ContractV2= await ethers.getContract("ContractV2");
 // upgrade the old contract to new contract
  const tx = await proxyAdmin.upgrade(proxyAdmin.address, ContractV2.address);
  await tx.wait(1);

Code to reproduce

On V1 contract

contract ContractV1 is
    Initializable,
    PausableUpgradeable,
    OwnableUpgradeable,
    ReentrancyGuardUpgradeable,
    OrderValidator,
    Errors
{
    /// @custom:oz-upgrades-unsafe-allow constructor
    constructor() {
        _disableInitializers();
    }

    function initialize(
        address[] memory _paymentTokens,
        address treasury,
        address _owner
    ) public initializer {
        __Pausable_init();
        __Ownable_init();
        __ReentrancyGuard_init();
        __EIP712_init("Marketplace", "1");
        transferOwnership(_owner);

     // some logic 
    }
}

TransparentProxy Contract

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

import "@openzeppelin/contracts/access/Ownable.sol";
import "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol";
import "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";

contract TransparentProxy is TransparentUpgradeableProxy, Ownable {
    constructor(
        address _logic,
        address _admin,
        bytes memory _data
    ) payable TransparentUpgradeableProxy(_logic, _admin, _data) {
        transferOwnership(msg.sender);
    }
}

contract ContractV2 is
    Initializable,
    PausableUpgradeable,
    OwnableUpgradeable,
    ReentrancyGuardUpgradeable,
    OrderValidator,
    Errors
{

    function initialize(
        address[] memory _paymentTokens,
        address treasury,
        address _owner
    ) public initializer {
        __Pausable_init();
        __Ownable_init();
        __ReentrancyGuard_init();
        __EIP712_init("Marketplace", "1");
        transferOwnership(_owner);

      // same stuff with contractV1 
    }

Environment

Hardhat

barakman · August 28, 2023, 9:29pm

I personally find the name proxyAdmin to be rather confusing.
There should be a Proxy contract and a Proxy-Admin account, which is typically an externally-owned account (AKA wallet).
Here, you seem to have a "hybrid entity" which refers to the contract but has the name of the wallet.
So I'm not quite sure how to interpret your code.

That said, you can just try proxyAdmin.upgradeTo(ContractV2.address) instead.
But you'll need to make sure that you execute this transaction using the Proxy-Admin wallet.

Furkan_Sezal · August 28, 2023, 9:50pm

Thank you for your reply.
Proxy Admin is accually proxyAdmin contract. Looks like i am doing something wrong here.
I am little bit confused about the first parameter of the proxyAdmin.upgrade function. is first parameter should be the address of proxy contract? When i deploy like this it gave me 2 contract file one contractV1 the other is proxyAdmin. Shouldnt i have 3 with implementation conract? or contractV1 is the proxy one? I am confused.
From your reply what do you mean by proxy-admin wallet? like same deployer?

import { HardhatRuntimeEnvironment } from 'hardhat/types';
import { DeployFunction } from 'hardhat-deploy/types';

const func: DeployFunction = async function (hre: HardhatRuntimeEnvironment) {

    const { deployments } = hre;
    const { deploy } = deployments;

    const [deployer] = await hre.ethers.getSigners();

    await deploy('ProxyAdmin', {
        from: deployer.address,
        log: true,
    });

};
export default func;

barakman · August 29, 2023, 5:16am

No, I mean the value that you pass as _admin in:

contract TransparentProxy is TransparentUpgradeableProxy, Ownable {
    constructor(
        address _logic,
        address _admin,
        bytes memory _data
    ) payable TransparentUpgradeableProxy(_logic, _admin, _data) {
        transferOwnership(msg.sender);
    }
}

This is typically the address of an externally-owned account (AKA wallet).
You can of course pass the address of a smart-contract account instead, if you want.
But you should make sure that you actually need something like this as part of your system.

Furkan_Sezal · August 29, 2023, 8:35am

Do i passing a smart contract account address? Here is my proxyAdmin deployement script

import { HardhatRuntimeEnvironment } from "hardhat/types";
import { DeployFunction } from "hardhat-deploy/types";

const func: DeployFunction = async function (hre: HardhatRuntimeEnvironment) {
  const { deployments } = hre;
  const { deploy } = deployments;

  const [deployer] = await hre.ethers.getSigners();

  const proxyAdmin = await deploy("ProxyAdmin", {
    from: deployer.address,
    log: true,
  });

  console.log("proxyAdmin deployed to : ", proxyAdmin.address);
};
export default func;

barakman · August 29, 2023, 10:07am

Here no:

But in your original question - apparently yes:

So the only account which can tell your Proxy contract to upgrade its pointer to the Logic contract, is the ProxyAdmin contract, which means that you need to have a dedicated function for this purpose, implemented in that contract.

I don't know what this contract looks like, because you haven't shared it, so I am left to assume that function upgrade is the one allowing you to execute this process.

And I can only assume that this function can be executed only by a permitted account, a true wallet in this case (i.e., an externally-owned account, not a smart-contract account).

So you have like 3 levels of executors here:

The wallet permitted to execute the ProxyAdmin.upgrade function
The ProxyAdmin.upgrade function, permitted to execute the Proxy.upgrade function
The Proxy.upgrade function, permitted to update the Proxy contract's pointer to the Logic contract

As I explained before, this seems a bit excessive, and you may as well implement it using 2 levels:

A wallet named ProxyAdmin, permitted to execute the Proxy.upgrade function
The Proxy.upgrade function, permitted to update the Proxy contract's pointer to the Logic contract

Furkan_Sezal · August 29, 2023, 3:33pm

ProxyAdmin contract come from TransparentProxy contract. Its not another contract.
Looks like i shoud do it like this:
const tx = await proxyAdmin.upgrade(ContractV1.address, ContractV2.address);
This worked. Sorry about the confusion.
But i another problem rigth now. I cant transferOwnership of this contract. When i do:

await proxyAdmin.transferOwnership("0x065A32537C53618a64De97bff862b0F4100965C6")

This transfers the ownership of the contract but i can still can call some onlyOwner functions on the implementation contract.
I also changethe admin it didnt help.

I am following this doc : https://docs.openzeppelin.com/contracts/4.x/api/proxy#ProxyAdmin-getProxyAdmin-contract-ITransparentUpgradeableProxy-

barakman · August 29, 2023, 5:51pm

The confusion stems from your naming choice, which is exactly what I have pointed out in my very first comment to your question.

This is a Proxy contract, not a "ProxyAdmin" contract.

Proxy-Admin is an account which controls ("admins") that Proxy contract.

It is typically an externally-owned account (AKA wallet), not a smart-contract account.

Furkan_Sezal · August 29, 2023, 6:13pm

Thank you. It is much more clear now.
How about ownership of the implementation contracts? I have some functions with onlyOwner modifier.
How can i transfer my ownership? just use transferOwnership function?