Refund Reset

[qbzzt]

Check if the refund amount (for sstore(<cell>, 0)) resets when various things happen.

[jochem-brouwer]

Here is an edge case with EthereumJS found which is not covered by tests (and also not by this PR). It can only happen on Frontier.

1. on contract A, a nonzero storage slot is set to zero (so this charges a refund)
2. contract A creates a new contract B
3. The code in contract B is ran, however, contract B cannot pay the final code deposit fee (i.e. the gas to pay to deposit the code, per byte, which is 200 gas)
4. In Frontier/Chainstart this means that: the nonce/balance/storage of contract B is all committed, except that now the code deposited is the empty code (it thus does not change)
5. contract A succesfully ends the transaction

Our bug happens because in (4) we clear out the storage refunds and the selfdestruct, but this should not happen

[jochem-brouwer]

This transaction caused the consensus bug:

https://etherscan.io/tx/0xbb59b675291237985bc558b6f66f8c701890cc84aea6604500c50f1a23e58605

This PR fixed the bug;

ethereumjs/ethereumjs-monorepo#2439

[qbzzt]

Here is an edge case with EthereumJS found which is not covered by tests (and also not by this PR). It can only happen on Frontier.

1. on contract A, a nonzero storage slot is set to zero (so this charges a refund)
2. contract A creates a new contract B
3. The code in contract B is ran, however, contract B cannot pay the final code deposit fee (i.e. the gas to pay to deposit the code, per byte, which is 200 gas)
4. In Frontier/Chainstart this means that: the nonce/balance/storage of contract B is all committed, except that now the code deposited is the empty code (it thus does not change)
5. contract A succesfully ends the transaction

Our bug happens because in (4) we clear out the storage refunds and the selfdestruct, but this should not happen

So it matters exactly where in the CREATE process the OOG happens, just having OOG in the constructor is useless. Let me see how I can get that. Since it's the final stage, I assume that if n is the minimum gas that results in a successful CREATE, n-1 would fail in the right location, right?

[qbzzt]

@jochem-brouwer , can you check if the latest test (https://github.com/ethereum/tests/blob/aa7a5b466ad73919995c63e01c80dc34ebe99af6/src/GeneralStateTestsFiller/stRefundTest/refundResetFrontierFiller.yml) covers this corner case? Also, does did it happen only on Frontier, or also on other versions?

[jochem-brouwer]

I just checked (and might have missed this before, if that is the case - sorry!) - test refundResetFrontier_d2g0v0_Frontier indeed triggers our problem.

It only happens on Frontier. This bug is caused by: there is enough gas to pay for execution for creating a contract, but not enough to pay the final deposit fee. Per Homestead (first hardfork, right after Frontier): https://eips.ethereum.org/EIPS/eip-2, quote; If contract creation does not have enough gas to pay for the final gas fee for adding the contract code to the state, the contract creation fails (i.e. goes out-of-gas) rather than leaving an empty contract. (point 3). This means that the situation which caused this bug cannot happen at Homestead or after (since it does not trigger the "not enough gas for code deposit error")

Thanks for writing these tests 😄

[winsvega]

Yay! ready to merge?

[qbzzt]

It depends. Do we want to generalize it to other similar (not identical) problems? Or is this the kind of rare fluke we don't expect to ever happen again, so we shouldn't look for variants on the theme of refund counter after a failed call.

[winsvega]

Lets merge this one, the rest can be smth to think about for the future testccases.
Can open an issue

[qbzzt]

Sure, it's ready to merge.

[winsvega]

make an issue in case you would want to return to this topic to write more scenarios. about refund reset in different context