Uninitialized class won't initialize on upgrade

Archive SDK
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