-
Notifications
You must be signed in to change notification settings - Fork 332
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cheats: fix assume not precompile #594
Conversation
After the cancun hardfork, the point evaluation precompile was added at `address(10)` which the `assumeNotPrecompile` implementation did not take into account. Pectra is scheduled to include BLS precompiles and likely more precompiles will be introduced in other future hardforks. We might as well reserve the least significant byte for Ethereum precompiles, as the address space is large and there is no need to make calls from these addresses, and in return code will not break on new EVM specs. This was causing test failures in the Optimism test suite when we updated the EVM version to cancun.
src/StdCheats.sol
Outdated
// These should be present on all EVM-compatible chains. | ||
vm.assume(addr < address(0x1) || addr > address(0x9)); | ||
// These are reserved by Ethereum and may be on all EVM-compatible chains. | ||
vm.assume(addr < address(0x1) || addr > address(0x100)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with the larger upper bound so this is more future-proof. Do you have a source for the 0x100
value? I've only seen https://eips.ethereum.org/EIPS/eip-1352 which reserves through 0xffff
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The genesis for testnets includes 1 wei for the precompiles in the range address(0)
to address(0xff)
. See https://github.com/eth-clients/holesky/blob/main/metadata/genesis.json
I meant to follow this standard, will update the PR to use address(0xff)
rather than address(0x100)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in dab1435
Use the address range of precomiples that testnets use, see https://github.com/eth-clients/holesky/blob/main/metadata/genesis.json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
After the cancun hardfork, the point evaluation precompile was added at
address(10)
which theassumeNotPrecompile
implementation did not take into account. Pectra is scheduled to include BLS precompiles and likely more precompiles will be introduced in other future hardforks. We might as well reserve the least significant byte for Ethereum precompiles, as the address space is large and there is no need to make calls from these addresses, and in return code will not break on new EVM specs.This was causing test failures in the Optimism test suite when we updated the EVM version to cancun.