Trouble testing upgradeable contract & ERC721 token

emobe · December 29, 2019, 11:42pm

I am having trouble testing this smart contract. When I call isAdmin, I get the role error.

Is msg.sender different every time? Is there a way to specify a “from” address? I’ve tried a few different ways of testing but not really getting anywhere. My code is below.

EstateAgent.sol

pragma solidity ^0.5.0;
import "@openzeppelin/contracts-ethereum-package/contracts/access/Roles.sol";
import "@openzeppelin/upgrades/contracts/Initializable.sol";


contract EstateAgent is Initializable  {
  using Roles for Roles.Role;

    Roles.Role private _admins;
     function initialize() public initializer {
       _admins.add(msg.sender);
    }
    function isAdmin() public view returns (bool){
      require(_admins.has(msg.sender), "NOT_ADMIN__ROLE");
      return true;
    }
}

EstateAgent.test.js

const { accounts, contract, web3 } = require('@openzeppelin/test-environment');
const { BN, balance, ether } = require('@openzeppelin/test-helpers');
const { expect } = require('chai');

const EstateAgent = contract.fromArtifact('EstateAgent');

describe('EstateAgent', function () {

  beforeEach(async function () {
    this.token = await EstateAgent.new();
  });

  describe('Is admin', function () {
    it('Should be admin', async function () {
      expect(await this.token.isAdmin()).to.be.true;
    });
  });
});

kostysh · December 30, 2019, 12:48am

await EstateAgent.new();

this way of deployment not calling initialize() method, so your code _admins.add(msg.sender); has not been reached at all.
If you want to create a proxy with initializer - please read the docs on how to do it. Here you can find a sample of code: https://docs.openzeppelin.com/sdk/2.6/zos-lib

Also, always clearly define who calls the method or creating deployment with “{from: }”

abcoathup · December 30, 2019, 2:12am

Hi @emobe,

Welcome to the community

Thanks to @kostysh for answering. As I was mid reply, will post to add to their excellent solution.

As per @kostysh, when we test upgradeable contracts we need to initialize them, as MyContract.new() doesn’t call initialize. (Documentation on initializers: https://docs.openzeppelin.com/sdk/2.6/writing-contracts#initializers)

We should explicitly provide an address rather than use msg.sender as this depends on who actually calls the initializer.

The upgradeable version of MinterRole is an example of this:

github.com

OpenZeppelin/openzeppelin-contracts-ethereum-package/blob/master/contracts/access/roles/MinterRole.sol#L16


import "../Roles.sol";


contract MinterRole is Initializable, Context {
    using Roles for Roles.Role;


    event MinterAdded(address indexed account);
    event MinterRemoved(address indexed account);


    Roles.Role private _minters;


    function initialize(address sender) public initializer {
        if (!isMinter(sender)) {
            _addMinter(sender);
        }
    }


    modifier onlyMinter() {
        require(isMinter(_msgSender()), "MinterRole: caller does not have the Minter role");
        _;
    }

Also, currently the test is testing only the logic version of the contract (which is fine to do). To test the proxy we can use the following documentation (this is the updated version using OpenZeppelin Test Environment): https://github.com/OpenZeppelin/openzeppelin-sdk/blob/master/packages/docs/modules/ROOT/pages/testing.adoc

The EstateAgent contract and test could be changed as follows:

EstateAgent.sol

pragma solidity ^0.5.0;

import "@openzeppelin/contracts-ethereum-package/contracts/access/Roles.sol";
import "@openzeppelin/upgrades/contracts/Initializable.sol";

contract EstateAgent is Initializable  {
  using Roles for Roles.Role;

    Roles.Role private _admins;

    function initialize(address sender) public initializer {
       _admins.add(sender);
    }

    function isAdmin(address account) public view returns (bool){
      return _admins.has(account);
    }
}

EstateAgent.test.js

const { accounts, provider } = require('@openzeppelin/test-environment');
const { TestHelper } = require('@openzeppelin/cli');
const { Contracts, ZWeb3 } = require('@openzeppelin/upgrades');

const { BN, balance, ether } = require('@openzeppelin/test-helpers');
const { expect } = require('chai');

ZWeb3.initialize(provider);

const EstateAgent = Contracts.getFromLocal('EstateAgent');

describe('EstateAgent', function () {
  const [ admin, nonAdmin, ...otherAccounts ] = accounts;

  beforeEach(async function () {
    this.project = await TestHelper();
    this.proxy = await this.project.createProxy(EstateAgent, { initMethod: 'initialize', initArgs: [admin] });
  });

  describe('Is admin', function () {
    it('Should be admin', async function () {
      expect(await this.proxy.methods.isAdmin(admin).call()).to.be.true;
    });

    it('Should not be admin', async function () {
      expect(await this.proxy.methods.isAdmin(nonAdmin).call()).to.be.false;
    });
  });
});

emobe · December 30, 2019, 10:28am

Thanks for the help! @kostysh & @abcoathup

Also, always clearly define who calls the method or creating deployment with “{from: }”

How can I do this deployment with from? I cannot find it as this is why I used msg.sender.

emobe · December 30, 2019, 11:43am

I am now having another issue with testing an ERC721 token. @abcoathup

I am trying to allow the initial EstateAgent admin to mine Deed tokens but when I call balanceOf in the test, it returns 0. Is there something wrong with how they’re interacting with each other?

Or should I be using the test helpers to check events?

Also I read I should be using something like await this.deed.methods['initialize(address)'](admin); for initialising the token but I’m not sure entirely where to place it as I can’t get it to work I think

Deed.sol

pragma solidity ^0.5.0;

import '@openzeppelin/upgrades/contracts/Initializable.sol';
import '@openzeppelin/contracts-ethereum-package/contracts/token/ERC721/ERC721Full.sol';
import '@openzeppelin/contracts-ethereum-package/contracts/token/ERC721/ERC721Mintable.sol';
import "@openzeppelin/contracts-ethereum-package/contracts/drafts/Counters.sol";

contract Deed is Initializable, ERC721Full, ERC721Mintable {
  using Counters for Counters.Counter;
  Counters.Counter private _tokenIds;

  function initialize(address agent) public initializer {
    ERC721.initialize();
    ERC721Enumerable.initialize();
    ERC721Metadata.initialize("Deed", "DEED");
    ERC721Mintable.initialize(agent);
  }

  function createDeed(address to, string memory tokenURI) public returns (uint256){
      _tokenIds.increment();

        uint256 newItemId = _tokenIds.current();
        _mint(to, newItemId);
        _setTokenURI(newItemId, tokenURI);

        return newItemId;
  }
}

EstateAgent.test.js

const { accounts, provider } = require('@openzeppelin/test-environment');
const { TestHelper } = require('@openzeppelin/cli');
const { Contracts, ZWeb3 } = require('@openzeppelin/upgrades');

const { BN, balance, ether } = require('@openzeppelin/test-helpers');
const { expect } = require('chai');

ZWeb3.initialize(provider);

const EstateAgent = Contracts.getFromLocal('EstateAgent');
const Deed = Contracts.getFromLocal('Deed');

describe('EstateAgent', function () {
  const [admin, nonAdmin, ...otherAccounts] = accounts;

  beforeEach(async function () {
    this.project = await TestHelper();
    this.agent = await this.project.createProxy(EstateAgent, { initMethod: 'initialize', initArgs: [admin] });
    this.deed = await this.project.createProxy(Deed, { initMethod: 'initialize', initArgs: [admin] })
  });

  describe('Is admin', function () {
    it('Should be admin', async function () {
      expect(await this.agent.methods.isAdmin(admin).call()).to.be.true;
    });

    it('Should not be admin', async function () {
      expect(await this.agent.methods.isAdmin(nonAdmin).call()).to.be.false;
    });

    it('should create deed', async function () {
      await this.deed.methods.createDeed(admin, 'testurl').call()
      expect(await this.deed.methods.balanceOf(admin).call()).to.be.true;
    });
  });
});

kostysh · December 30, 2019, 4:04pm

How can I do this deployment with from ? I cannot find it as this is why I used msg.sender.

await this.proxy.methods.isAdmin(nonAdmin).call({
   from: [sender_address]
})

emobe · December 31, 2019, 9:45am

Thanks. Any docs for this? Tried looking through the source too

kostysh · December 31, 2019, 11:19am

methods calls and transaction options are documented in the web3 docs. With these options you also can set, for example, amount of gas that will be sent with transaction, or amount of ether

emobe · December 31, 2019, 11:38am

Ah yes. Thank you!

Do you have any idea why my ERC721 test doesn’t work? I’ve ran it with oz create/send-tx etc and it works but not in the test

Edit: I’ve also found it works when I don’t use the upgradeable package.

abcoathup · January 2, 2020, 4:22am

Hi @emobe,

The issue with the test was using call rather than send for sending the createDeed transaction. We also need to specify who the transaction is from and the gas limit.

    it('should create deed', async function () {
      await this.deed.methods.createDeed(admin, 'testurl').send({from:admin, gas: 5e6});
      expect(await this.deed.methods.balanceOf(admin).call()).bignumber.equal('1');
    });

I suggest for new questions that you create a new, standalone topic (unless they are tightly related). This makes it easier for other community members to potentially answer.

As an aside, I suggest having a test file per contract, as in this case there were tests for two contracts in the one file.

emobe · January 2, 2020, 9:35am

I posted it in here as I assumed the problem was to do with it being upgradeable but now I understand, it’s like oz cli where you have to do call or send-tx.

Yep, I will definitely be splitting it up, this is more or less code that will be changed anyway as I was trying to get a minimal token test.

Thanks a lot for your help. Definitely helped me figure out whats going on in the testing.

Topic		Replies	Views
Upgradeable NFT Smart Contract Upgrades erc721 , proxies	32	1790	November 29, 2022
How to write a successful test.js file for my ERC721 contract's proxy Smart Contracts erc721 , test-environment	2	653	December 14, 2022
Using setUpRole on AccessControlUpgradeable in a test for a smart contract the admin address is always 0x000..0 Support proxies , upgrades , upgrades-plugins , erc1155	2	1187	April 13, 2022
Testing upgradeable contracts deployed by a factory Upgrades	1	714	December 16, 2021
Need help with Tutorial on upgradeProxy Smart Contracts upgrades	5	1188	September 30, 2022

Trouble testing upgradeable contract & ERC721 token

EstateAgent.sol

EstateAgent.test.js

Related topics