Skip to content

Commit

Permalink
fix: make state.exited be zero or one (ethereum-optimism#246)
Browse files Browse the repository at this point in the history
Modifies the MIPS contracts to enforce that state.exited is either
exactly zero or one and cannot have any other value.
  • Loading branch information
smartcontracts authored and samlaf committed Aug 20, 2024
1 parent 9196b9f commit c920184
Show file tree
Hide file tree
Showing 9 changed files with 63 additions and 18 deletions.
4 changes: 2 additions & 2 deletions packages/contracts-bedrock/semver-lock.json
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,8 @@
"sourceCodeHash": "0x3ff4a3f21202478935412d47fd5ef7f94a170402ddc50e5c062013ce5544c83f"
},
"src/cannon/MIPS.sol": {
"initCodeHash": "0x0f7ca5db42a3829a6d20d8fe81e3462922c1d0e335be1394fbf79287c289c3f3",
"sourceCodeHash": "0x5f5d13be282305b2947079cbf8f01e750e5c88a2b7a7a7267e79cba3ee4616fa"
"initCodeHash": "0xb001e9a981943f3711836b8b7ce8f40122e9e124249eee64737cf2b4e5d009d6",
"sourceCodeHash": "0x185a37ed58c94527410cd35e35cae0f9372e364a4f28140c3e443349676a0a28"
},
"src/cannon/MIPS2.sol": {
"initCodeHash": "0xf3ff16b352e256534761844d49e159c267dd09e79bc9ddec78d0c90f8746ea20",
Expand Down

Large diffs are not rendered by default.

17 changes: 17 additions & 0 deletions packages/contracts-bedrock/src/cannon/MIPS.sol
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,14 @@ contract MIPS is ISemver {
from, to := copyMem(from, to, 8) // step
from := add(from, 32) // offset to registers

// Verify that the value of exited is valid (0 or 1)
if gt(exited, 1) {
// revert InvalidExitedValue();
let ptr := mload(0x40)
mstore(ptr, shl(224, 0x0136cc76))
revert(ptr, 0x04)
}

// Copy registers
for { let i := 0 } lt(i, 32) { i := add(i, 1) } { from, to := copyMem(from, to, 4) }

Expand Down Expand Up @@ -243,8 +251,17 @@ contract MIPS is ISemver {
c, m := putField(c, m, 4) // heap
c, m := putField(c, m, 1) // exitCode
c, m := putField(c, m, 1) // exited
let exited := mload(sub(m, 32))
c, m := putField(c, m, 8) // step

// Verify that the value of exited is valid (0 or 1)
if gt(exited, 1) {
// revert InvalidExitedValue();
let ptr := mload(0x40)
mstore(ptr, shl(224, 0x0136cc76))
revert(ptr, 0x04)
}

// Unpack register calldata into memory
mstore(m, add(m, 32)) // offset to registers
m := add(m, 32)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,6 @@ error InsufficientBond();

/// @notice Thrown when a bond transfer fails.
error BondTransferFailed();

/// @notice Thrown when the value of the exited boolean is not 0 or 1.
error InvalidExitedValue();
25 changes: 25 additions & 0 deletions packages/contracts-bedrock/test/cannon/MIPS.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { CommonTest } from "test/setup/CommonTest.sol";
import { MIPS } from "src/cannon/MIPS.sol";
import { PreimageOracle } from "src/cannon/PreimageOracle.sol";
import { MIPSInstructions } from "src/cannon/libraries/MIPSInstructions.sol";
import { InvalidExitedValue } from "src/cannon/libraries/CannonErrors.sol";
import "src/dispute/lib/Types.sol";

contract MIPS_Test is CommonTest {
Expand Down Expand Up @@ -59,6 +60,30 @@ contract MIPS_Test is CommonTest {
assertNotEq(postState, bytes32(0));
}

/// @notice Tests that the mips step function fails when the value of the exited field is
/// invalid (anything greater than 1).
/// @param _exited Value to set the exited field to.
function testFuzz_step_invalidExitedValue_fails(uint8 _exited) external {
// Assume
// Make sure the value of _exited is invalid.
_exited = uint8(bound(uint256(_exited), 2, type(uint8).max));

// Rest of this stuff doesn't matter very much, just setting up some state to edit.
// Here just using the parameters for the ADD test below.
uint32 insn = encodespec(17, 18, 8, 0x20);
(MIPS.State memory state, bytes memory proof) = constructMIPSState(0, insn, 0x4, 0);

// Compute the encoded state and manipulate it.
bytes memory enc = encodeState(state);
assembly {
mstore8(add(add(enc, 0x20), 89), _exited)
}

// Call the step function and expect a revert.
vm.expectRevert(InvalidExitedValue.selector);
mips.step(enc, proof, 0);
}

function test_add_succeeds() external {
uint32 insn = encodespec(17, 18, 8, 0x20); // add t0, s1, s2
(MIPS.State memory state, bytes memory proof) = constructMIPSState(0, insn, 0x4, 0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ contract DeploymentSummary is DeploymentSummaryCode {
address internal constant l1StandardBridgeProxyAddress = 0x20A42a5a785622c6Ba2576B2D6e924aA82BFA11D;
address internal constant l2OutputOracleAddress = 0x19652082F846171168Daf378C4fD3ee85a0D4A60;
address internal constant l2OutputOracleProxyAddress = 0x39Af23E00F1e662025aA01b0cEdA19542B78DF99;
address internal constant mipsAddress = 0xaB87a32314674174f62160a7Ce648b24e6D9D36e;
address internal constant mipsAddress = 0xc8b67F21b0773A778e1Ca113CbFa9327fef75750;
address internal constant optimismMintableERC20FactoryAddress = 0x39Aea2Dd53f2d01c15877aCc2791af6BDD7aD567;
address internal constant optimismMintableERC20FactoryProxyAddress = 0xc7B87b2b892EA5C3CfF47168881FE168C00377FB;
address internal constant optimismPortalAddress = 0xbdD90485FCbcac869D5b5752179815a3103d8131;
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ contract DeploymentSummaryFaultProofs is DeploymentSummaryFaultProofsCode {
address internal constant l1StandardBridgeProxyAddress = 0x20A42a5a785622c6Ba2576B2D6e924aA82BFA11D;
address internal constant l2OutputOracleAddress = 0x19652082F846171168Daf378C4fD3ee85a0D4A60;
address internal constant l2OutputOracleProxyAddress = 0x39Af23E00F1e662025aA01b0cEdA19542B78DF99;
address internal constant mipsAddress = 0xaB87a32314674174f62160a7Ce648b24e6D9D36e;
address internal constant mipsAddress = 0xc8b67F21b0773A778e1Ca113CbFa9327fef75750;
address internal constant optimismMintableERC20FactoryAddress = 0x39Aea2Dd53f2d01c15877aCc2791af6BDD7aD567;
address internal constant optimismMintableERC20FactoryProxyAddress = 0xc7B87b2b892EA5C3CfF47168881FE168C00377FB;
address internal constant optimismPortalAddress = 0xbdD90485FCbcac869D5b5752179815a3103d8131;
Expand Down

Large diffs are not rendered by default.

0 comments on commit c920184

Please sign in to comment.