Help required with writing extensive test suite for ZeppelinOS project

Archive SDK
1

Hi,

We have been working away writing our test suite for our smart contracts which are using ZeppelinOS for upgradeability.

We have mostly been following a lot of the patterns that are shown by example in the Centre Token repo:

While we now have more than 100 tests correctly running I am still having some issues getting a few of the test cases working due to the difference between ZOS and the previous method of testing using the older Zeppelin framework.

Specifically I have the following questions I hope someone can help with. Specifically I am trying to understand how to convert the line in bold to something that is compatible with the current version of ZeppelinOS:

  1. How can I capture the ‘Upgraded’ event in a test? The centre repo has the following code:

    it(‘et008 should check Upgraded event’, async function () {
    var upgradedToken = await UpgradedFiatTokenNewFields.new();
    const initializeData = encodeCall(‘initV2’, [‘bool’, ‘address’, ‘uint256’], [true, pauserAccount, 12]);
    let upgrade = await proxy.upgradeToAndCall(upgradedToken.address, initializeData, { from: proxyOwnerAccount });
    checkUpgradeEvent(upgrade, upgradedToken.address);
    });

  2. How do I capture the AdminChanged event? Again, centre code:

    it(‘et009 should check AdminChanged event’, async function () {
    let adminChanged = await proxy.changeAdmin(arbitraryAccount, {from: proxyOwnerAccount});
    checkAdminChangedEvent(adminChanged, proxyOwnerAccount, arbitraryAccount);
    });

Hope someone can help!

Cheers
Rick

1 Like
2

Suggest looking at the ZeppelinOS Tests as examples.

https://github.com/zeppelinos/zos/blob/master/packages/lib/test/contracts/upgradeability/AdminUpgradeabilityProxy.behaviour.js

        it('emits an event', async function() {
          const { events } = await this.proxy.methods
            .upgradeTo(this.implementation_v1)
            .send({ from });
          events.should.have.key('Upgraded');
          events['Upgraded'].returnValues.implementation.should.be.equal(
            this.implementation_v1,
          );
        });

https://github.com/zeppelinos/zos/blob/master/packages/lib/test/contracts/upgradeability/AdminUpgradeabilityProxy.behaviour.js

        it('emits an event', function() {
          this.events.should.have.key('AdminChanged');
          this.events[
            'AdminChanged'
          ].returnValues.previousAdmin.should.be.equalIgnoreCase(
            proxyAdminAddress,
          );
          this.events[
            'AdminChanged'
          ].returnValues.newAdmin.should.be.equalIgnoreCase(newAdmin);
        });

Though I assume you could use openzeppelin-test-helpers expectEvent to check that the event is emitted.

1 Like
3

Housekeeping: moved to #support:zeppelinos category

1 Like
4

@roy.batty were you able to test for the two events being emitted?

1 Like
5

@abcoathup - not quite got it working… so currently all my test files have been using the TestHelper to run contract functions and check events as below::

  beforeEach(async () => {
    myProject = await TestHelper();
    myTokenProxy = await myProject.createProxy(MyToken, {
      initMethod: 'initialize',
      initArgs: [myTokenName, myTokenSymbol, myTokenCurrency, myTokenDecimals],
    });

when I want to fun a contract function and check an event in a test I have been doing the following successfully:

  it('ET-001 should check Transfer events', async () => {
    const transferEvent = (await myTokenProxy.methods
      .transfer(arbitraryAccount2, testingAmount).send({ from: arbitraryAccount })).events.Transfer;

    assert.equal(transferEvent.event, 'Transfer');
    assert.equal(transferEvent.returnValues.from, from);
    assert.equal(transferEvent.returnValues.to, to);
    assert.equal(transferEvent.returnValues.value, value);
  });

So the above approach is fine when testing and checking for events on my own contracts but I think I cannot use the TestHelper to help me with checking events at the Proxy level? ie. ‘upgrading’, ‘changing proxy admin’, etc?

Cheers
Rick

1 Like
6

OK, I managed to get a test written for the ‘Upgraded’ event using the following code:

const initializeData = Buffer.from('');
this.implementation_v1 = (await MyContract.new()).address;

aup = await AdminUpgradeabilityProxy
  .new(this.implementation_v1, proxyAdminAddress, initializeData, { from: proxyAdminOwner });



  it('PET-008 (et008) should check Upgraded event', async () => {
    this.implementation_v2 = (await MyContractV2.new()).address;
    [_, proxyAdminAddress, proxyAdminOwner] = _accounts;

    const { events } = await aup.methods
      .upgradeTo(this.implementation_v2)
      .send({ from: proxyAdminAddress });

    events.should.have.key('Upgraded');
    events.Upgraded.returnValues.implementation.should.be.equal(
      this.implementation_v2,
    );
  });

However, when I try a similar pattern for running the ‘changeAdmin’ function on the Proxy then I get a transaction revert but without any error message? The code I have is:

  it('PET-009 (et009) should check AdminChanged event', async () => {
    [_, anotherAccount, proxyAdminAddress, proxyAdminOwner] = _accounts;

    let newProxyAdmin = await aup.methods
      .admin()
      .call({ from: proxyAdminAddress });

    const newAdmin = anotherAccount;
    const { events } = await aup.methods
      .changeAdmin(newAdmin)
      .send({ from: proxyAdminAddress });

    newProxyAdmin = await aup.methods
      .admin()
      .call({ from: newAdmin });

    newProxyAdmin.should.be.equalIgnoreCase(anotherAccount);
    events.should.have.key('AdminChanged');
    events.AdminChanged
      .returnValues.previousAdmin.should.be.equalIgnoreCase(
        proxyAdminAddress,
      );
    this.events.AdminChanged
      .returnValues.newAdmin.should.be.equalIgnoreCase(
        newAdmin,
      );
  });

In fact even just checking who is the admin in this function causes a revert:

  let newProxyAdmin = await aup.methods
      .admin()
      .call({ from: proxyAdminAddress });

I am just wondering if this is anything to do with point 2 from @spalladino here: Best Practice for transfer of Proxy Admin or Proxy Owner?

Any help appreciated! @spalladino @ylv-io @abcoathup

Cheers
Rick

2 Likes
7

Hi @roy.batty

Well done on being able to test for Upgraded.

Regards testing changing the admin, my suggestion is to have a look at the tests to see how it works there. I was searching for changeAdmin in the repository.

1 Like
8

Hi @roy.batty

TestHelper (zos-lib Project under the covers) returns the object already setup and doesn't keep the transaction that created it. So it doesn't look to be possible to use TestHelper to test for these type of events unfortunately.

1 Like
9

Thanks @abcoathup. So I guess I am heading in the right path by using a reference to the AdminUpgradeabilityProxy directly to test the functions and events at the Proxy level (eg, Upgraded event or changeAdmin function).

I’ll try and come back to this today as I left the problem alone for a couple of days to focus on some other tasks and also hope that my background brain processes might figure something out :slight_smile:

Cheers
Rick

1 Like