Bug when using --hardfork istanbul

[varasev]

Expected Behavior

The code

array.push(variable);
array.push(variable);

should work for Istanbul.

Current Behavior

We have smart contracts with complex unit tests and use ganache-cli along with truffle. Everything works fine if we don't use --hardfork istanbul even for 6.8.0-istanbul.0, but when we activate the istanbul, there is a pretty weird bug.

Steps to Reproduce

$ git clone -b ganache-istanbul-bug https://github.com/poanetwork/posdao-contracts
$ cd posdao-contracts
$ npm i
$ npm run test

After that, the unit tests are run. On the step staking epoch trufflesuite/ganache-cli#3 finished you will see revert failure:

This is the line in test/BlockRewardAuRa.js which calls BlockRewardAuRa.reward function which in turn calls ValidatorSetAuRa.newValidatorSet function.

The ValidatorSetAuRa.newValidatorSet function calls the internal _setPendingValidators function. And that _setPendingValidators reverts here:

// This reverts for some reason:
// _stakingAddresses.length == 4 here and gas is enough
for (uint256 i = 0; i < _stakingAddresses.length; i++) {
    _pendingValidators.push(_stakingAddresses[0]);
}

If we remove the --hardfork istanbul flag from our cli parameters, the test works fine as it should.

Please, take a look at these lines (and the comments there): https://github.com/poanetwork/posdao-contracts/blob/b0abf0cfd50bb78894b744f2f5fd6734054a476c/contracts/ValidatorSetAuRa.sol#L787-L808

It seems that the issue somehow relates to these lines which modificate the _pendingValidators array. The bug is only reproduced after those lines are executed in the same block.

Context

This issue doesn't let us test our contracts for Istanbul.

Your Environment

• Version used: 6.8.0-istanbul.0
• Version of Truffle used: 5.0.34
• NodeJS Version: v8.16.0
• Operating System and version: macOS High Sierra 10.13.6
• Link to your project or repro gist: https://github.com/poanetwork/posdao-contracts/tree/ganache-istanbul-bug
• Commit hash to use with above link for reproduction: b0abf0cfd50bb78894b744f2f5fd6734054a476c

[davidmurdoch]

@holgerd77, can you or someone from the ethereumjs-vm team take a look quick look at this to determine if this is a new issue with the istanbul hardfork changes in the VM?

[varasev]

I thought the reason is in gas consumption and tried to increase the gas limit here, but that didn't help.

[holgerd77]

From reading the error message from the screenshot above I wonder if @alcuadrado worked on something eventually touching relevant behavior (don't remember the context though) and can eventually help here.

[alcuadrado]

I haven't seen anything like this so far. I could reproduce the error with Ganache, but not with Buidler EVM, so I suspect the problem is in ganache-core's code, and not in ethereumjs-vm's.

Maybe @s1na knows if this can be related to an instanbul change in the VM.

[s1na]

Hm, if it's related to ethereumjs-vm my guess would be EIP-2200 (PR) since the _pendingValidators storage item is being modified and they were cleared before delete _pendingValidators;. But not sure why Builder EVM is not failing in that case?

[alcuadrado]

But EIP-2200 could only lead to an OOG error, right? That would have a different error message.

[s1na]

But EIP-2200 could only lead to an OOG error, right? That would have a different error message.

Unless there's a bug. I thought EIP-2200 because that seems to be the only istanbul-relevant code that this contract snippet touches.

I recently started to make the VM pass the istanbul state tests in ethereumjs/ethereumjs-monorepo#607. If there's a bug in the VM it'll most likely surface there.

[haltman-at]

Hi, do you know whether this is still an issue on latest Ganache? Thank you!

[gnidan]

Closing for issue maintenance. Let us know if you're still running into this, and we'll be happy to re-open. Thanks!