ethereum-optimism / ethereum-optimism/optimism
Tests for branch in delay slot are incomplete (and not needed)
- Dominant language
- Go
- Stars
- 6.5k
- Forks
- 4k
- Avg merge
- 2d 15h
- Merged PRs (30d)
- 145
Description
**Description**
The test if (_cpu.nextPC != _cpu.pc + 4), used to detect if there is a jump in the delay slot, has 2 problems:
Will not detect that there is a jump in the delay slot if the previous instruction was a jump to pc + 8;
Is not strictly needed, given the assumption that the guest code is trusted (i.e. that it will not try to exploit differences between this and other MIPS implementations to cause the execution to diverge).\
**Recommendation**:
Remove the tests.
[packages/contracts-bedrock/src/cannon/libraries/MIPS64Instructions.sol](https://cantina.xyz/code/4ea41142-4359-4f62-bfaa-a542172cab5d/packages/contracts-bedrock/src/cannon/libraries/MIPS64Instructions.sol#L622)
if (_cpu.nextPC != _cpu.pc + 4) {
[packages/contracts-bedrock/src/cannon/libraries/MIPS64Instructions.sol](https://cantina.xyz/code/4ea41142-4359-4f62-bfaa-a542172cab5d/packages/contracts-bedrock/src/cannon/libraries/MIPS64Instructions.sol#L809)
if (_cpu.nextPC != _cpu.pc + 4) {
Contributor guide
Research direction
Read packages/contracts-bedrock/src/cannon/libraries/MIPS64Instructions.sol around the two conditions at lines 622 and 809. Confirm how each check handles a jump in a delay slot, remove the unnecessary checks as recommended, and run the relevant existing test suite to verify Cannon's MIPS execution behavior remains correct.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- solidity
- Domain
- backend, testing
- Issue type
- Refactor
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 52/100