From 54f614d9e838b22afd99ce9fbe289d0efc0aa899 Mon Sep 17 00:00:00 2001 From: inphi Date: Thu, 9 May 2024 12:03:42 -0400 Subject: [PATCH 1/2] cannon: Handle div by zero in MIPS.sol --- packages/contracts-bedrock/semver-lock.json | 4 ++-- .../contracts-bedrock/src/cannon/MIPS.sol | 10 ++++++++-- .../contracts-bedrock/test/cannon/MIPS.t.sol | 20 +++++++++++++++++++ 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/packages/contracts-bedrock/semver-lock.json b/packages/contracts-bedrock/semver-lock.json index 3a757520659b..91bebecb04ee 100644 --- a/packages/contracts-bedrock/semver-lock.json +++ b/packages/contracts-bedrock/semver-lock.json @@ -112,8 +112,8 @@ "sourceCodeHash": "0x73aa5934e56ba2a45f368806c5db1d442bf5713d51b2184749f4638eaceb832e" }, "src/cannon/MIPS.sol": { - "initCodeHash": "0xd447743c7a3c1babe141050603dd643467ada00bd90465a52ef3093445ee1b02", - "sourceCodeHash": "0xe37efbb893e0a7499fcd565f8495f6ba911611ba36795ac131738474f116fce5" + "initCodeHash": "0xb9b9fee06015fdfb10a684d3f3de78071663284eb58fdffd0654843fd98dae76", + "sourceCodeHash": "0xa5b516e738cb1f9e92e9d2f8c05be59295444c9b8101ff430193814b8e355e57" }, "src/cannon/PreimageOracle.sol": { "initCodeHash": "0xe5db668fe41436f53995e910488c7c140766ba8745e19743773ebab508efd090", diff --git a/packages/contracts-bedrock/src/cannon/MIPS.sol b/packages/contracts-bedrock/src/cannon/MIPS.sol index 0736f32c7656..013dcb6df1a5 100644 --- a/packages/contracts-bedrock/src/cannon/MIPS.sol +++ b/packages/contracts-bedrock/src/cannon/MIPS.sol @@ -43,8 +43,8 @@ contract MIPS is ISemver { uint32 public constant BRK_START = 0x40000000; /// @notice The semantic version of the MIPS contract. - /// @custom:semver 1.0.0 - string public constant version = "1.0.0"; + /// @custom:semver 1.0.1 + string public constant version = "1.0.1"; uint32 internal constant FD_STDIN = 0; uint32 internal constant FD_STDOUT = 1; @@ -422,6 +422,9 @@ contract MIPS is ISemver { // Stores the quotient in LO // And the remainder in HI else if (_func == 0x1a) { + if (int32(_rt) == 0) { + revert("division by zero"); + } state.hi = uint32(int32(_rs) % int32(_rt)); state.lo = uint32(int32(_rs) / int32(_rt)); } @@ -429,6 +432,9 @@ contract MIPS is ISemver { // Stores the quotient in LO // And the remainder in HI else if (_func == 0x1b) { + if (_rt == 0) { + revert("division by zero"); + } state.hi = _rs % _rt; state.lo = _rs / _rt; } diff --git a/packages/contracts-bedrock/test/cannon/MIPS.t.sol b/packages/contracts-bedrock/test/cannon/MIPS.t.sol index 823cb81daee6..00d82cfd5c83 100644 --- a/packages/contracts-bedrock/test/cannon/MIPS.t.sol +++ b/packages/contracts-bedrock/test/cannon/MIPS.t.sol @@ -892,6 +892,26 @@ contract MIPS_Test is CommonTest { assertEq(postState, outputState(expect), "unexpected post state"); } + function test_div_byZero_fails() external { + uint32 insn = encodespec(0x9, 0xa, 0x0, 0x1a); // div t1, t2 + (MIPS.State memory state, bytes memory proof) = constructMIPSState(0, insn, 0x4, 0); + state.registers[9] = 5; // t1 + state.registers[10] = 0; // t2 + + vm.expectRevert("division by zero"); + mips.step(encodeState(state), proof, 0); + } + + function test_divu_byZero_fails() external { + uint32 insn = encodespec(0x9, 0xa, 0x0, 0x1b); // divu t1, t2 + (MIPS.State memory state, bytes memory proof) = constructMIPSState(0, insn, 0x4, 0); + state.registers[9] = 5; // t1 + state.registers[10] = 0; // t2 + + vm.expectRevert("division by zero"); + mips.step(encodeState(state), proof, 0); + } + function test_beq_succeeds() external { uint16 boff = 0x10; uint32 insn = encodeitype(0x4, 0x9, 0x8, boff); // beq $t0, $t1, 16 From 9e1bd1601b031939dee8f8ce0dc50ce22ca1b0e9 Mon Sep 17 00:00:00 2001 From: inphi Date: Thu, 9 May 2024 12:10:12 -0400 Subject: [PATCH 2/2] fix revert stmt style --- packages/contracts-bedrock/semver-lock.json | 4 ++-- packages/contracts-bedrock/src/cannon/MIPS.sol | 4 ++-- packages/contracts-bedrock/test/cannon/MIPS.t.sol | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/contracts-bedrock/semver-lock.json b/packages/contracts-bedrock/semver-lock.json index 91bebecb04ee..347bc36f3e75 100644 --- a/packages/contracts-bedrock/semver-lock.json +++ b/packages/contracts-bedrock/semver-lock.json @@ -112,8 +112,8 @@ "sourceCodeHash": "0x73aa5934e56ba2a45f368806c5db1d442bf5713d51b2184749f4638eaceb832e" }, "src/cannon/MIPS.sol": { - "initCodeHash": "0xb9b9fee06015fdfb10a684d3f3de78071663284eb58fdffd0654843fd98dae76", - "sourceCodeHash": "0xa5b516e738cb1f9e92e9d2f8c05be59295444c9b8101ff430193814b8e355e57" + "initCodeHash": "0xa5d36fc67170ad87322f358f612695f642757bbf5280800d5d878da21402579a", + "sourceCodeHash": "0x75701f3efb7a9c16079ba0a4ed2867999aab7d95bfa0fe5ebb131cfc278593aa" }, "src/cannon/PreimageOracle.sol": { "initCodeHash": "0xe5db668fe41436f53995e910488c7c140766ba8745e19743773ebab508efd090", diff --git a/packages/contracts-bedrock/src/cannon/MIPS.sol b/packages/contracts-bedrock/src/cannon/MIPS.sol index 013dcb6df1a5..b47fb9313f7e 100644 --- a/packages/contracts-bedrock/src/cannon/MIPS.sol +++ b/packages/contracts-bedrock/src/cannon/MIPS.sol @@ -423,7 +423,7 @@ contract MIPS is ISemver { // And the remainder in HI else if (_func == 0x1a) { if (int32(_rt) == 0) { - revert("division by zero"); + revert("MIPS: division by zero"); } state.hi = uint32(int32(_rs) % int32(_rt)); state.lo = uint32(int32(_rs) / int32(_rt)); @@ -433,7 +433,7 @@ contract MIPS is ISemver { // And the remainder in HI else if (_func == 0x1b) { if (_rt == 0) { - revert("division by zero"); + revert("MIPS: division by zero"); } state.hi = _rs % _rt; state.lo = _rs / _rt; diff --git a/packages/contracts-bedrock/test/cannon/MIPS.t.sol b/packages/contracts-bedrock/test/cannon/MIPS.t.sol index 00d82cfd5c83..f62aaceac4f5 100644 --- a/packages/contracts-bedrock/test/cannon/MIPS.t.sol +++ b/packages/contracts-bedrock/test/cannon/MIPS.t.sol @@ -898,7 +898,7 @@ contract MIPS_Test is CommonTest { state.registers[9] = 5; // t1 state.registers[10] = 0; // t2 - vm.expectRevert("division by zero"); + vm.expectRevert("MIPS: division by zero"); mips.step(encodeState(state), proof, 0); } @@ -908,7 +908,7 @@ contract MIPS_Test is CommonTest { state.registers[9] = 5; // t1 state.registers[10] = 0; // t2 - vm.expectRevert("division by zero"); + vm.expectRevert("MIPS: division by zero"); mips.step(encodeState(state), proof, 0); }