Withdrawal account balance overflow

siladu commented 1 year ago

From @shemnon :

t8n work is almost able to run the execution-spec-tests, but I found one corner case not covered by reference tests. How do we handle a deposit overflow in the account? this case https://github.com/hyperledger/besu/pull/5030/files#diff-623780fd83396397db75bc1878527f771719aac1c529f75f53fbcc502b62a581R25-R52 - account balance is 2^256-1 and we deposit 1 gwei. I wind up with this:

java.lang.ArithmeticException: UInt256 overflow
    at org.apache.tuweni.units.bigints.UInt256Value.addExact(UInt256Value.java:92)
    at org.hyperledger.besu.evm.account.MutableAccount.incrementBalance(MutableAccount.java:53)
    at org.hyperledger.besu.ethereum.mainnet.WithdrawalsProcessor.processWithdrawals(WithdrawalsProcessor.java:29)
    at org.hyperledger.besu.evmtool.T8nSubCommand.lambda$run$0(T8nSubCommand.java:380)
    at java.base/java.util.Optional.ifPresent(Optional.java:178)
    at org.hyperledger.besu.evmtool.T8nSubCommand.run(T8nSubCommand.java:380)

Is this covered by hive tests?

cc @jframe

jframe commented 1 year ago

The hive tests don't cover the overflow case.

We handle the account balance overflow by catching the ArtithmeticException in the AbstractBlockProcessor. This change was made in https://github.com/hyperledger/besu/commit/6b9c1ce40d1b575c1ffb823d4468e3e493b64d99 so that the executionSpec tests could be run and we would handle this case.

jframe commented 1 year ago

Clarified the test case further for execution spec tests when there could be multiple withdrawals and one of the withdrawals overflows. In this case, we consider the block invalid so all changes should be rolled back.

The current implementation throws an exception in this case so we will roll back and return an error.

hyperledger / besu

Withdrawal account balance overflow #5079