fix(levm): fix gas refunds #1410
Conversation
maximopalopoli
added
ef-tests
| sender, | ||
| U256::from(report.gas_refunded) | ||
| .checked_mul(self.env.gas_price) | ||
| .ok_or(VMError::GasLimitPriceProductOverflow)?, |
There was a problem hiding this comment.
Maybe this error name is not the most appropriate but it's ok though.
There was a problem hiding this comment.
Shouldn't it be OutOfGas or BalanceUnderflow?
There was a problem hiding this comment.
The overflow would be the result of multiplying the refunded gas by the gas price, so it should not be an issue in the balance sheet itself. Furthermore, since it is added to the balance sheet, it would be an overflow in any case.
|
Wait, I remembered that, according to the Yellowpaper, the gas refunds have to be capped at 1/5th of the total gas used. let refunded_gas = report.gas_refunded.min(
consumed_gas
.checked_div(5)
.ok_or(VMError::Internal(InternalError::UndefinedState(-1)))?,
);This PR is a good fit for that I believe, but we have to ensure that at least the same amount of tests pass |
|
One extra thing, today (or at the latest tomorrow) I'll probably change the logic behind gas consumption and the update of the user's balance. So we can merge this and then change that. |
Motivation
The gas refunds had a bug in the sstore calculation. Fixes all the tests from
stShiftfolder.Description
Also, this PR sums the refunds to the sender's balance.