ethereum-optimism / ethereum-optimism/optimism

Tests for branch in delay slot are incomplete (and not needed)

Open
#13,441 0 comments 0 reactions 0 assignees View on GitHub
A-pkg-contracts-bedrock C-good first issue MT cannon - audit findings
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.