ethereum-optimism / cannon

On chain interactive fault prover for Ethereum
MIT License
584 stars 135 forks source link

Integer overflow risks, update Solidity version pragma to 0.8.x #78

Open shazow opened 2 years ago

shazow commented 2 years ago

The Challenge.sol contract uses uint256 without overflow checks, and there is some arithmetic in places that would benefit from overflow checks.

Updating the pragma to 0.8.x would include built-in solc overflow checks, easy fix that's worth doing.

norswap commented 2 years ago

Thanks for the issue 🙏 We will definitely do this.

protolambda commented 1 year ago

We will soon be looking at an upgrade to the latest solidity version. But note that some overflows are a feature, not a bug, to emulate uint32 behavior of registers, where overflows are expected to happen. We'll need to be careful with introducing safe-math-by-default behavior of 0.8.x to not break expected MIPS behavior.

shazow commented 1 year ago

IIRC the specific overflow issue was in the challenge game (maybe something about being able to wrap around the binary search cursor?).

Could make sense to disable overflow protections in the VM implementation as needed, while still have it in the higher-level contracts.

But yea, worth considering these choices carefully when the time comes. :)