time.increase should include a fixed timestamp in evm_mine

[simondlr]

Hi!

Sometimes with increasing time, the block.timestamp is off by a second from the specified increase in duration. This was a known issue and was fixed in Ganache by adding a timestamp parameter to evm_mine. trufflesuite/ganache#463.

According to the docs, the expected behaviour is that increases to a precise timestamp:

"Increases the time of the blockchain by duration (in seconds), and mines a new block with that timestamp."

However, due to the discrepancy in sometimes being off by a second, the test helper needs to specify the precise timestamp when doing 'advanceBlock'.

I added a simple function that fixed this.

function advanceBlockFixed (timestamp) {
  return promisify(web3.currentProvider.send.bind(web3.currentProvider))({
    jsonrpc: '2.0',
    method: 'evm_mine',
    params: [timestamp.toNumber()],
    id: new Date().getTime(),
  });
}

Unsure what the simplest fix to this is. Adding a specific new advanceBlock function (eg advaneBlockFixed), or just requiring that all advanceBlock calls require a timestamp.

[frangio]

Thanks for bringing this up!

It seems that evm_mine together with the timestamp parameter is equivalent to our time.increaseTo helper. I think the simplest fix would be to change increaseTo to call evm_mine directly with the parameter, and then change increase so that it delegates to increaseTo.

[simondlr]

Yes. Suspect that's the best solution. You then don't touch advanceBlock, and merely do an evm_mine to a future time. I don't think you need to use evm_timeIncrease then? I don't know if that's still useful or not. Trying to figure out when you'd want to increase the time, but NOT mine a block? I think, regardless, the time test helpers are always expected to mint a new block, so don't think it should matter?

[frangio]

Yeah exactly, since our time helpers already mine a block we can use evm_mine directly.

@simondlr Are you at all interested in contributing this change? 🙂