Uninitialized class won't initialize on upgrade

Archive SDK
1

to recreate…
Lets say I’ve got a token.sol it is is Initializable, ERC20, ERC20Detailed, ERC20Burnable, ERC20Pausable

Did a full upgradable deploy… But after this deploy I noticed that the contract’s initializer only initialized ERC20Detailed and ERC20Burnable ERC20Detailed.initialize(name, symbol, decimals); ERC20Mintable.initialize(msg.sender);

Modifying the token contract with a new method function initPausable() public { ERC20Pausable.initialize(msg.sender); } I expect to be able to init the Pausable class…

No such luck. No mater what I do, I get :

ALERT: [ethjs-rpc] rpc error with payload {"id":8737563104927,"jsonrpc":"2.0","params":["0xf86f0a8504a817c800832dc6c09483752ecafebf4707258dedffbd9c7443148169db8084e2b646238602d6d5a8bcf7a0c2f6bfdcfe0c75fbdd3b0173a78049a669558696a5e3b75c5ce98bfb680bc955a01bcff7758e610aac16f0833c06e847b3b99c2745f1543156d488c35ea2337e04"],"method":"eth_sendRawTransaction"} Error: VM Exception while processing transaction: revert Contract instance has already been initialized

2 Likes
2

Hi @genecyber thanks for posting in the forum.

Reproducing the issue:
Create a contract that inherits from ERC20Pausable but doesn’t call ERC20Pausable.initialize in the initialize function.

pragma solidity ^0.5.0;

import "zos-lib/contracts/Initializable.sol";
import "openzeppelin-eth/contracts/token/ERC20/ERC20Detailed.sol";
import "openzeppelin-eth/contracts/token/ERC20/ERC20Mintable.sol";
import "openzeppelin-eth/contracts/token/ERC20/ERC20Burnable.sol";
import "openzeppelin-eth/contracts/token/ERC20/ERC20Pausable.sol";

contract MyToken is Initializable, ERC20Detailed, ERC20Burnable, ERC20Mintable, ERC20Pausable {
    function initialize(address sender, string memory name, string memory symbol, uint8 decimals) public initializer {
        ERC20Detailed.initialize(name, symbol, decimals);
        ERC20Mintable.initialize(sender);
    }
}

Add an initializePausable function to the contract.

    function initializePausable(address sender) public {
        ERC20Pausable.initialize(sender);
    }

Then upgrade the contract, which fails with a VM revert.
npx zos update MyToken --init initializePausable --args 0x90da32D93d03cA2cC5693e64E0d00c64F53E3FD6

I assume that when you inherit from ERC20Pausable but don’t explicitly initialize, it is implicitly initialized but without assigning the PauserRole to any address. Meaning that you can’t then Pause the token.

1 Like
3

That makes sense… is there any remedying this? Can I add to the pauser role another way?

2 Likes
4

Is this in production on mainnet?

5

Yes it is. I’d like to understand how to fix it.

2 Likes
6

So… Is it possible?

2 Likes
7

Hi @genecyber

The ERC20Pausable contract initializer was not run, but the entire contract is marked as initialized. Which means you can’t initialize it again.

You could upgrade the contract and add a Pauser, though you would need to protect this initialization so that it is only called once, otherwise anyone could call this function and add a Pauser.

I recommend testing any upgrades, including testing prior to upgrading in production:

The following is an example upgrade:

MyToken Version 1

pragma solidity ^0.5.0;

import "zos-lib/contracts/Initializable.sol";
import "openzeppelin-eth/contracts/token/ERC20/ERC20Detailed.sol";
import "openzeppelin-eth/contracts/token/ERC20/ERC20Mintable.sol";
import "openzeppelin-eth/contracts/token/ERC20/ERC20Burnable.sol";
import "openzeppelin-eth/contracts/token/ERC20/ERC20Pausable.sol";

contract MyToken is Initializable, ERC20Detailed, ERC20Burnable, ERC20Mintable, ERC20Pausable {

    function initialize(address sender, string memory name, string memory symbol, uint8 decimals) public initializer {
        ERC20Detailed.initialize(name, symbol, decimals);
        ERC20Mintable.initialize(sender);
        // Missing: ERC20Pausable.initialize(sender)
    }
}

MyToken Version 2

Added initializePausable with protection to prevent it being run again
Depending on how you wish to perform the upgrade will depend if you update the existing MyToken contract or have a version 2.

pragma solidity ^0.5.0;

import "zos-lib/contracts/Initializable.sol";
import "openzeppelin-eth/contracts/token/ERC20/ERC20Detailed.sol";
import "openzeppelin-eth/contracts/token/ERC20/ERC20Mintable.sol";
import "openzeppelin-eth/contracts/token/ERC20/ERC20Burnable.sol";
import "openzeppelin-eth/contracts/token/ERC20/ERC20Pausable.sol";

contract MyToken2 is Initializable, ERC20Detailed, ERC20Burnable, ERC20Mintable, ERC20Pausable {

    bool private _pausableInitialized;

    function initialize(address sender, string memory name, string memory symbol, uint8 decimals) public initializer {
        ERC20Detailed.initialize(name, symbol, decimals);
        ERC20Mintable.initialize(sender);
        // Missing: ERC20Pausable.initialize(sender)
    }

    function initializePausable(address sender) public {
        require(!_pausableInitialized, "MyToken2: pausable has already been initialized");
        _pausableInitialized = true;
        _addPauser(sender);
    }
}

MyToken.test.js

Testing Guide: https://docs.zeppelinos.org/docs/testing.html
Test based on upgrade script in https://docs.zeppelinos.org/docs/zos_lib.html#running-the-script

const { TestHelper } = require('zos');
const { Contracts, ZWeb3 } = require('zos-lib');

ZWeb3.initialize(web3.currentProvider);

// Import preferred chai flavor: both expect and should are supported
const { expect } = require('chai');

const MyToken = Contracts.getFromLocal('MyToken');  // build manually using npx truffle compile
const MyToken2 = Contracts.getFromLocal('MyToken2');

contract('MyToken', function ([_, admin, minter, pauser, anotherAddress]) {

  beforeEach(async function () {
    this.project = await TestHelper({from: admin});
  })

  it('should create a proxy', async function () {
    const proxy = await this.project.createProxy(MyToken, {
        initMethod: "initialize",
        initArgs: [minter, "Test Token", "TST", 18]
    });
    expect(await proxy.methods.name().call()).to.be.equal("Test Token");
    // ERC20Pausable not initialised
    expect(await proxy.methods.isPauser(pauser).call()).to.be.equal(false);
    
  });

  it('should upgrade', async function () {
    const proxy = await this.project.createProxy(MyToken, {
        initMethod: "initialize",
        initArgs: [minter, "Test Token", "TST", 18]
    });

    const instance = await this.project.upgradeProxy(proxy.address, MyToken2,
        { initMethod: 'initializePausable', initArgs: [pauser], initFrom: admin });
        expect(await proxy.methods.isPauser(pauser).call()).to.be.equal(true);
  });

  it('should fail to add Pauser for another address once initialized', async function () {
    const proxy = await this.project.createProxy(MyToken, {
        initMethod: "initialize",
        initArgs: [minter, "Test Token", "TST", 18]
    });

    const instance = await this.project.upgradeProxy(proxy.address, MyToken2,
        { initMethod: 'initializePausable', initArgs: [pauser], initFrom: admin });

    // try to add Pauser once initialized
    await instance.methods.initializePausable(anotherAddress);

    expect(await instance.methods.isPauser(pauser).call()).to.be.equal(true);
    expect(await instance.methods.isPauser(anotherAddress).call()).to.be.equal(false);
  });
})

Upgrade

(Update, added for completeness)

To create the contract initially I followed the instructions on Deploying

npx zos push --deploy-dependencies
npx zos create MyToken --init initialize --args 0x90da32D93d03cA2cC5693e64E0d00c64F53E3FD6,MyToken,TKN,18

I updated the source code of the contract to add initializePausable with the check if it had already been run and followed the instructions on Upgrading

npx zos push
npx zos update MyToken --init initializePausable --args 0x90da32D93d03cA2cC5693e64E0d00c64F53E3FD6

As well as automated tests for the upgrade. I manually tested in the console to check that the Pauser had been added and that calling initializePausable reverted when called again to prevent anyone from adding a new pauser using the initializePausable function again.

truffle(ganache)> m = await MyToken.deployed()
truffle(ganache)> m.isPauser("0x7BbD3B8c606ffA0fCC54F5852719eF0C00f929b0")
false
truffle(ganache)> m.isPauser("0x90da32D93d03cA2cC5693e64E0d00c64F53E3FD6")
true
truffle(ganache)> m.initializePausable("0x7BbD3B8c606ffA0fCC54F5852719eF0C00f929b0")
Thrown:
{ Error: Returned error: VM Exception while processing transaction: revert MyToken2: pausable has already been initialized -- Reason given: MyToken2: pausable has already been initialized.
3 Likes
8

Thank you so very much… Your test gave me a clue as well as your initializePausable method…

So I was trying to do ERC20Pausable.initialize(sender) inside my initializePausable method and it was failing… but using _addPauser(sender) worked

Also your test helped me realize I should be initializing when using zos update

so zod update MyToken --init initializePausable --args “MyAddress” worked perfectly (It utilizes the “Upgrade and Call” feature of the admin/upgrade contracts

Thanks Thanks Thanks

1 Like
9

Awesome. Glad I could help. Thanks for being part of the community @genecyber.

The main thing is to make sure that the initializePausable is protected so that it can only be called once, otherwise everyone would make themselves a Pauser :smile:

I should have put my zos commands in my reply. (I have done an update to add these in for completeness).

A note on house keeping, I moved the topic to the #support:zeppelinos category and marked my reply as the solution and removed Solved from the title, as we get a little check box instead. :smile: