Correct setup to test openzeppelin-contracts-upgradeable

I checked out openzeppelin-contracts-upgradeable from GitHub and docs as well.

I created my own Token contract from preset: https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable/blob/v3.0.0/contracts/presets/ERC20PresetMinterPauser.sol

I changed and added some needed logic to it. I deployed and manually tested it using the local hardhat network.

It is time to write tests for my contract to be sure everything is ok. Before doing it I decided to copy preset contract and test for it to my local project to be sure I have the correct setup and copy few tests to my own contract.

I copypasted the ERC20PresetMinterPauser.sol and test file (https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable/blob/master/test/presets/ERC20PresetMinterPauser.test.js) for it to my project, and tests failed. I saw, that test uses ERC20PresetMinterPauser, but the real smart contact is ERC20PresetMinterPauserUpgradeable. I also mentioned few scripts in ‘scripts’ folder that add suffix ‘Upgradeable’ in the repo. I manually change the name from:

const ERC20PresetMinterPauser = artifacts.require(‘ERC20PresetMinterPauser’);

to

const ERC20PresetMinterPauser = artifacts.require(‘ERC20PresetMinterPauserUpgradeable’);

and still tests failed. I tried different ways to init a contract (using ‘initialize’ method, ‘deploy’ and ‘deployProxy’) for this test but was not able to get tests passed.

Can you please advise me on how to init ERC20PresetMinterPauser contract correctly to pass the existed tests? Also, I tried to copy tests for ERC20 contract, AccessControll but had the same issue.

:computer: Environment
“dependencies”: {
“@openzeppelin/cli”: “^2.8.2”,
“@openzeppelin/contracts”: “^3.3.0”,
“@openzeppelin/contracts-upgradeable”: “^3.3.0”,
“@openzeppelin/upgrades”: “^2.8.0”
},
“devDependencies”: {
“@nomiclabs/hardhat-ethers”: “^2.0.1”,
“@nomiclabs/hardhat-truffle5”: “^2.0.0”,
“@nomiclabs/hardhat-waffle”: “^2.0.1”,
“@nomiclabs/hardhat-web3”: “^2.0.0”,
“@openzeppelin/gsn-helpers”: “^0.2.4”,
“@openzeppelin/hardhat-upgrades”: “^1.5.0”,
“@openzeppelin/test-environment”: “^0.1.9”,
“@openzeppelin/test-helpers”: “^0.5.10”,
“chai”: “^4.2.0”,
“ethereum-waffle”: “^3.2.2”,
“ethers”: “^5.0.26”,
“hardhat”: “^2.0.8”,
“mocha”: “^8.2.1”,
“web3”: “^1.3.3”
}

:1234: Code to reproduce

const ERC20PresetMinterPauser = artifacts.require('ERC20PresetMinterPauser'); //ERC20PresetMinterPauserUpgradeable

contract('ERC20PresetMinterPauser', function (accounts) {
  const [ deployer, other ] = accounts;

  const name = 'MinterPauserToken';
  const symbol = 'DRT';

  const amount = new BN('5000');

  const DEFAULT_ADMIN_ROLE = '0x0000000000000000000000000000000000000000000000000000000000000000';
  const MINTER_ROLE = web3.utils.soliditySha3('MINTER_ROLE');
  const PAUSER_ROLE = web3.utils.soliditySha3('PAUSER_ROLE');

  beforeEach(async function () {
    // what is the correct way to init token to get tests passed?
    this.token = await ERC20PresetMinterPauser.new(name, symbol, { from: deployer });
  });
1 Like

Hi @Hantok,

Welcome to the community :wave:

We can unit test our implementation contracts and create high level tests to test upgrades (https://docs.openzeppelin.com/upgrades-plugins/1.x/truffle-upgrades#test-usage)

For an example of these tests see: OpenZeppelin Upgrades: Step by Step Tutorial for Truffle

I will have to come back to you on the tests in the OpenZeppelin Contracts Upgradeable.

1 Like

Hello @abcoathup,

Thanks for your quick reply! Following your advice let me share my results:

Using ‘truffle-upgrades’ dependency I got the following error:

:1234: Test file snippet

const { BN, constants, expectEvent, expectRevert } = require('@openzeppelin/test-helpers');
const { ZERO_ADDRESS } = constants;

const { expect } = require('chai');
const { deployProxy } = require('@openzeppelin/truffle-upgrades');

const ERC20PresetMinterPauser = artifacts.require('ERC20PresetMinterPauserUpgradeable');

contract('ERC20PresetMinterPauser', function (accounts) {
    const [ deployer, other ] = accounts;

const name = 'MinterPauserToken';
const symbol = 'DRT';

const amount = new BN('5000');

const DEFAULT_ADMIN_ROLE = '0x0000000000000000000000000000000000000000000000000000000000000000';
const MINTER_ROLE = web3.utils.soliditySha3('MINTER_ROLE');
const PAUSER_ROLE = web3.utils.soliditySha3('PAUSER_ROLE');

beforeEach(async function () {
    this.token = await deployProxy(ERC20PresetMinterPauser, [name, symbol], { initializer: 'initialize', unsafeAllowCustomTypes: true, from: deployer });
});

it('deployer has the default admin role', async function () {
    expect(await this.token.getRoleMemberCount(DEFAULT_ADMIN_ROLE)).to.be.bignumber.equal('1');
    expect(await this.token.getRoleMember(DEFAULT_ADMIN_ROLE, 0)).to.equal(deployer);
});
.....

:spider_web: Error

Contract: ERC20PresetMinterPauser
1) "before each" hook for "deployer has the default admin role"

  0 passing (10ms)
  1 failing

  1) Contract: ERC20PresetMinterPauser
       "before each" hook for "deployer has the default admin role":
     TypeError: Cannot read property 'send' of undefined
      at Object.wrapProvider (node_modules/@openzeppelin/truffle-upgrades/src/wrap-provider.ts:9:39)
      at deployProxy (node_modules/@openzeppelin/truffle-upgrades/src/deploy-proxy.ts:42:20)
      at Context.<anonymous> (test/TokenUpgradable/ERC20PresetMinterPauserUpgradeable.test.js:25:28)
      at processImmediate (internal/timers.js:461:21)

In another case, I used to work with ‘hardhat’ ‘upgrades’, I get no exceptions, but tests do not pass as well:

:1234: Code snippet:

const { ethers, upgrades } = require("hardhat");

contract('ERC20PresetMinterPauser', function (accounts) {
    const [ deployer, other ] = accounts;

    const name = 'MinterPauserToken';
    const symbol = 'DRT';

    const amount = new BN('5000');

    const DEFAULT_ADMIN_ROLE = '0x0000000000000000000000000000000000000000000000000000000000000000';
    const MINTER_ROLE = web3.utils.soliditySha3('MINTER_ROLE');
    const PAUSER_ROLE = web3.utils.soliditySha3('PAUSER_ROLE');

    beforeEach(async function () {
        const ERC20PresetMinterPauser = await ethers.getContractFactory("ERC20PresetMinterPauserUpgradeable");
        this.token = await upgrades.deployProxy(ERC20PresetMinterPauser, [name, symbol], { initializer: 'initialize', unsafeAllowCustomTypes: true, from: deployer });
    });

So, my key reason is to be sure, that current tests pass, then I will write my own ones to test for new contract features.

Thanks! :pray:

1 Like

Hi @Hantok,

If you’re using the package @openzeppelin/contracts-upgradeable you should be looking at versions v3.2.0 onwards, so for example: https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable/blob/v3.2.0/contracts/presets/ERC20PresetMinterPauserUpgradeable.sol

Our test suite uses a few tricks to be able to run the test suite from non-upgradeable OpenZeppelin Contracts without any changes. This makes things easier for us because it means there are less differences between the two repos.

There are two keys to this:

  1. We have a custom artifacts.require function that automatically adds the Upgradeable prefix (sort of, see below). See env-artifacts.js.
  2. We have a set of contracts called _____UpgradeableWithInit that call the initializer as part of the constructor. This again allows us to keep the test suite the same because we don’t need to add calls to initializers. For example:

This is why our test suite hasn’t worked for you.

You don’t need to do this in your own tests, it’s all quite specific to our setup for publishing the upgradeable contracts more easily.

In your own tests, you should make sure to use the proper contract name for artifacts.require, and you should make sure to somehow call your initializer function, either in a separate transaction via JavaScript or using a wrapper contract that calls the initializer in the constructor.

2 Likes

Hello @frangio,

Thanks for your reply. Really helpful!

I knew there is a trick with scripts, but did not understand correctly how it works.

At this moment I found the solution for me to make ‘constructor’ contract using solidity for tests like in the ‘contracts-upgradeable’ repo.

contract ERC20PresetMinterPauserUpgradeableWithInit is ERC20PresetMinterPauserUpgradeable {
    constructor(string memory name, string memory symbol) public payable {
        initialize(name, symbol);
    }
}

The same behavior I used for my contract. I am not so familiar with JavaScript, no idea how to repeat such a constructor on JS. :slight_smile:

Anyway, the problem resolved. I wrote couple tests for new contract functionality.

Also, I would like to be my contract audited. Is there any place where can submit the request for this?

Thanks!

1 Like

I’m glad that worked for you! Repeating it in JS would be something like:

const erc20 = await ERC20PresetMinterPauserUpgradeable.new();
await erc20.initialize(name, symbol);

So there is no need to create an extra *WithInit contract.

For audit requests you need to submit this form on our site: https://openzeppelin.com/request/

2 Likes