ERC777 operatorBurn breaks

[glowkeeper]

I have an ERC777 derived token, Enervator, which, aside from operatorBurn, behaves correctly. For example, all the tests below pass, except “Burns correctly”, which causes the VM to break:

Error: Returned error: VM Exception while processing transaction: revert
at Object.ErrorResponse (/usr/local/lib/node_modules/truffle/build/webpack:/~/web3-core-requestmanager/~/web3-core-helpers/src/errors.js:29:1)
      at /usr/local/lib/node_modules/truffle/build/webpack:/~/web3-core-requestmanager/src/index.js:140:1
      at /usr/local/lib/node_modules/truffle/build/webpack:/packages/truffle-provider/wrapper.js:112:1
      at XMLHttpRequest.request.onreadystatechange (/usr/local/lib/node_modules/truffle/build/webpack:/~/web3-providers-http/src/index.js:96:1)
      at XMLHttpRequestEventTarget.dispatchEvent (/usr/local/lib/node_modules/truffle/build/webpack:/~/xhr2-cookies/dist/xml-http-request-event-target.js:34:1)
      at XMLHttpRequest._setReadyState (/usr/local/lib/node_modules/truffle/build/webpack:/~/xhr2-cookies/dist/xml-http-request.js:208:1)
      at XMLHttpRequest._onHttpResponseEnd (/usr/local/lib/node_modules/truffle/build/webpack:/~/xhr2-cookies/dist/xml-http-request.js:318:1)
      at IncomingMessage.<anonymous> (/usr/local/lib/node_modules/truffle/build/webpack:/~/xhr2-cookies/dist/xml-http-request.js:289:47)
      at endReadableNT (_stream_readable.js:1178:12)
      at processTicksAndRejections (internal/process/task_queues.js:80:21)

Here are my tests:

it('has the correct name', async function () {

    const name = await this.manager.getTokenName()
    assert.equal( name, 'Enervator' )

  });

  it('has the correct symbol', async function () {

    const symbol = await this.manager.getTokenSymbol()
    assert.equal( symbol, 'EOR' )

  });

  it('Sets supply correctly', async function () {

    const newSupply = new BN('7727623693', 10)
    const shiftedSupply = this.decimilisation.mul( newSupply )
    await this.manager.addTokens(shiftedSupply)
    const supply = await this.manager.getTotalSupply()
    const retrievedNewSupply = supply.div(this.decimilisation)
    const thisRetrievedNewSupply = parseInt(retrievedNewSupply.toString())
    assert.equal( thisRetrievedNewSupply, '7727623693' )

  });

  it('Burns correctly', async function () {

    const burnAmount = new BN('1000000000', 10)
    const shiftedBurn = this.decimilisation.mul( burnAmount )
    await this.manager.burnTokens(shiftedBurn)
    const supply = await this.manager.getTotalSupply()
    const retrievedNewSupply = supply.div(this.decimilisation)
    const thisRetrievedNewSupply = parseInt(retrievedNewSupply.toString())
    const supplyShouldEqual = 7727623693 - 1000000000
    assert.equal( thisRetrievedNewSupply, supplyShouldEqual )

  });

  it('has the correct current TPES', async function () {

    const TPES = await this.manager.getCurrentTPES()
    const retrievedTPES = TPES.div(this.multiplier)
    const currentTPES = retrievedTPES.toString()
    assert.equal( currentTPES, '162494360000' )

  });

  it('has the correct old TPES', async function () {

    const TPES = new BN('162494360000', 10)
    const thisTPES = this.multiplier.mul(TPES)
    await this.manager.setNewTPES(thisTPES)
    const currentTPES = await this.manager.getCurrentTPES()
    const oldTPES = await this.manager.getOldTPES()
    const retrievedCurrentTPES = currentTPES.div(this.multiplier)
    const thisCurrentTPES = retrievedCurrentTPES.toString()
    const retrievedOldTPES = oldTPES.div(this.multiplier)
    const thisOldTPES = retrievedOldTPES.toString()
    assert.equal( thisOldTPES, '162494360000' )
    assert.equal( thisCurrentTPES, '162494360000' )

  });

  it('has the correct per capita energy', async function () {

    const perCapita = await this.manager.getPerCapitaEnergy()
    const retrievedPerCapita = perCapita.div(this.multiplier)
    const thisPerCapita = retrievedPerCapita.toString()
    assert.equal( thisPerCapita, '22' )

  });

  it('has the correct unit value', async function () {
    
    const pricePerMWh = await this.manager.getPricePerMWh()
    const currentTPES = await this.manager.getCurrentTPES()
    const oldTPES = await this.manager.getOldTPES()
    const perCapita = await this.manager.getPerCapitaEnergy()
    const derivedUnitValue = parseFloat( pricePerMWh.toString() ) * ( parseFloat( oldTPES.toString() ) / parseFloat( currentTPES.toString() ) ) / parseFloat( perCapita.toString() )
    const thisDerivedUnitValue = ( derivedUnitValue ).toFixed( 2 )
    const unitValue = await this.manager.getUnitValue()
    const retrievedUnitValue = parseFloat(unitValue.toString())
    const thisUnitValue = ( retrievedUnitValue / 2**64 ).toFixed( 2 )
    assert.equal( thisDerivedUnitValue, thisUnitValue )

  });

  it('has the correct rate', async function () {

    const code = ethers.utils.formatBytes32String( "USD" )
    const rate = new DECIMAL(0.07)
    const thisTwo = new DECIMAL(2)
    const thisSixtyFour = new DECIMAL(64)
    const thisMultiplier = thisTwo.pow(thisSixtyFour)
    const thisNewBigRate = Math.round(thisMultiplier.mul(rate).toString())
    await this.exchanger.setRate( code, thisNewBigRate.toString() )
    const savedRate = BIG(await this.forex.getRate( code ))
    const retrievedRate = savedRate.div(thisMultiplier)
    const thisRetrievedRate = retrievedRate.toFixed(2)
    assert.equal( thisRetrievedRate, rate.toString() )

  });

  it('deposits correctly', async function () {

    const code = ethers.utils.formatBytes32String( "USD" )
    const depositRef = ethers.utils.formatBytes32String( "USDDEP" )
    const amount = '1000'
    const bigAmount = new BN( amount, 10 )
    const thisAmount = this.multiplier.mul(bigAmount)
    await this.exchanger.deposit( '0xc220728701829A7351Fa3e16b11Aaf223543AAc3', depositRef, code, thisAmount )
    const savedAmount = await this.deposit.getDepositedAmount( depositRef )
    const retrievedAmount = savedAmount.div( this.multiplier )
    const thisRetrievedAmount = retrievedAmount.toString()
    assert.equal( thisRetrievedAmount, amount )

  });

  it('Buys correctly', async function () {

    const buyRef = ethers.utils.formatBytes32String( "USDBUY" )
    const depositRef = ethers.utils.formatBytes32String( "USDDEP" )
    const amount = await this.deposit.getDepositedAmount( depositRef )
    await this.exchanger.buy( '0xc220728701829A7351Fa3e16b11Aaf223543AAc3', buyRef, depositRef, amount )
    const newDepositedAmount = await this.deposit.getDepositedAmount( depositRef )
    const canWithdraw = await this.deposit.getCanWithdraw( depositRef )
    assert.equal( newDepositedAmount, 0 )
    assert.equal( canWithdraw, false )

  });

Er, is this a bug in operatorBurn, maybe? I’m not certain, so I thought I’d post here before logging an issue on GitHub.

[abcoathup]

Hi @glowkeeper,

I don’t think it is an issue with operatorBurn. I created a test of the operatorBurn functionality in Enervator where an operator burns an amount of a holders tokens.

Enervator.test.js

const { singletons, constants } = require('openzeppelin-test-helpers');

const Enervator = artifacts.require('Enervator');

contract('Enervator', function ([_, registryFunder, creator, operator, holder]) {
  beforeEach(async function () {
    this.erc1820 = await singletons.ERC1820Registry(registryFunder);
    this.token = await Enervator.new([operator], { from: creator });
  });

  it('allows operator burn', async function () {
    const amount = 1;
    const data = web3.utils.sha3('777Data');
    const operatorData = web3.utils.sha3('777OperatorData');
    
    await this.token.addSupply(amount, {from: operator});
    await this.token.send(holder, amount, data, {from: operator})
    assert.equal(amount, await this.token.balanceOf(holder))

    await this.token.operatorBurn(holder, amount, data, operatorData, {from: operator});

    assert.equal(0, await this.token.balanceOf(holder))
  });
});

You may want to look at having tests per contract, as it appears you have one set of tests covering all the functionality. Also tests appear to share state, my preference is to have a new contract created per test, so that one test doesn’t impact another.

I haven’t dug into why your test is failing, though hopefully the above can help track it down.

[glowkeeper]

Hi @abcoathup,

That’s awesome feedback - thanks - you’re a star! I’m really pleased you got that to work and the problem’s my end…

There are some subtle differences between your tests and mine; my operator is also the holder, so in effect, my code does this:

await this.token.operatorBurn(operator, amount, data, operatorData, {from: operator});

Also, I don’t send data or operatorData (I just send empty strings).

So I’ll run this test, later:

await this.token.operatorBurn(operator, amount, "", "", {from: operator});

That’s also a great point about test coverage. I’ve written my tests that way because it mimics the architecture of my react frontend apps’. However, you’ve demonstrated that my current set of tests is insufficient, so I’ll add to them. However, I’ve got to demo’ those apps on Friday and I still have to finish my fiat-to-enervator exchanger. Since the inability to burn tokens is not really a show stopper, I’m going to have to postpone investigating for a few days.

Thanks again for the feedback!

@glowkeeper

[abcoathup]

Hi @glowkeeper,

Good luck with the demo on Friday!

[abcoathup]

Hi @glowkeeper,

I hope the demo went well. Feel free to ask more questions.

[glowkeeper]

Hi @abcoathup!

It went really well! Here’s a picture from the presentation I gave:

Later this week, I will be uploading the two apps I demonstrated to dat - so you can have a play, too

Something interesting may come of it all - present was someone who runs energy services for a local government organisation, and afterwards, we had a chat about using a version of Enervator to tokenise an energy efficiency drive for their many services. That’s really exciting - I hope something comes of it!

[glowkeeper]

Also, I don’t send data or operatorData (I just send empty strings).

Indeed - as your test hinted, that was the issue - unlike _mint, it appears data and operatorData are necessary for operatorBurn. So I now do this:

bytes memory data =  abi.encodePacked( uint ( TokenSend.BURN ) );
bytes memory operatorData =  abi.encodePacked( uint ( TokenSend.BURN ) );
token.operatorBurn( address(this), _amount, data, operatorData );

Which means, later, in the tokensToSend hook, I can do something like this:

uint thisUserdata = abi.decode( userData, ( uint ) );
if ( thisUserdata == uint( TokenSend.BURN ) )
{
    //do something...
} else if ( thisUserdata == uint( TokenSend.MINT ) )
{
        //do something else
}

....which is pretty cool

Rather than just breaking, if operatorBurn needs those data and operatorData variables (I don't have time to investigate exactly why), then it might be nice if it could return a revert string telling me so.

Still - I have operatorBurn working well, so all good

Thanks so much for your help!

[abcoathup]

Hi @glowkeeper,

Thanks for sharing the photo from your presentation. Glad it went well.

Also good to hear about your use of data and operatorData. I will need to do some more investigation on this.