diff --git a/.github/workflows/arbitrator-ci.yml b/.github/workflows/arbitrator-ci.yml index a315bff9d1e..4b4d7ad9ed7 100644 --- a/.github/workflows/arbitrator-ci.yml +++ b/.github/workflows/arbitrator-ci.yml @@ -139,7 +139,7 @@ jobs: uses: actions-rs/cargo@v1 with: command: clippy - args: --all --manifest-path arbitrator/Cargo.toml + args: --all --manifest-path arbitrator/Cargo.toml -- -D warnings - name: Run rust tests uses: actions-rs/cargo@v1 diff --git a/arbitrator/prover/src/lib.rs b/arbitrator/prover/src/lib.rs index 0d5ba902239..be6f117b5c8 100644 --- a/arbitrator/prover/src/lib.rs +++ b/arbitrator/prover/src/lib.rs @@ -1,7 +1,7 @@ // Copyright 2021-2022, Offchain Labs, Inc. // For license information, see https://github.com/nitro/blob/master/LICENSE -#![allow(clippy::missing_safety_doc)] // We have a lot of unsafe ABI +#![allow(clippy::missing_safety_doc, clippy::too_many_arguments)] pub mod binary; /// cbindgen:ignore diff --git a/arbitrator/prover/src/machine.rs b/arbitrator/prover/src/machine.rs index 4ccf122610f..82f529a7417 100644 --- a/arbitrator/prover/src/machine.rs +++ b/arbitrator/prover/src/machine.rs @@ -347,7 +347,7 @@ impl Module { ) }, func_ty.clone(), - &types, + types, )?); host_call_hooks.push(None); } @@ -1061,7 +1061,7 @@ impl Machine { expected_type.inputs.push(ArbValueType::I32); // argc expected_type.inputs.push(ArbValueType::I32); // argv ensure!( - &main_module.func_types[f as usize] == &expected_type, + main_module.func_types[f as usize] == expected_type, "Run function doesn't match expected signature of [argc, argv]", ); // Go's flags library panics if the argument list is empty. @@ -1345,7 +1345,9 @@ impl Machine { let module = self.modules.last().expect("no module"); let export = module.exports.iter().find(|x| x.0 == func); - let export = export.expect(&format!("func {} not found", func)).1; + let export = export + .unwrap_or_else(|| panic!("func {} not found", func)) + .1; self.frame_stack.clear(); self.block_stack.clear(); @@ -1361,7 +1363,7 @@ impl Machine { } pub fn get_final_result(&self) -> Result> { - if self.frame_stack.len() != 0 { + if !self.frame_stack.is_empty() { bail!( "machine has not successfully computed a final result {:?}", self.status @@ -1452,20 +1454,6 @@ impl Machine { match inst.opcode { Opcode::Unreachable => error!(), Opcode::Nop => {} - Opcode::Block => { - let idx = inst.argument_data as usize; - self.block_stack.push(idx); - debug_assert!(func.code.len() > idx); - } - Opcode::EndBlock => { - self.block_stack.pop(); - } - Opcode::EndBlockIf => { - let x = self.value_stack.last().unwrap(); - if !x.is_i32_zero() { - self.block_stack.pop().unwrap(); - } - } Opcode::InitFrame => { let caller_module_internals = self.value_stack.pop().unwrap().assume_u32(); let caller_module = self.value_stack.pop().unwrap().assume_u32(); @@ -1493,17 +1481,6 @@ impl Machine { Machine::test_next_instruction(func, &self.pc); } } - Opcode::Branch => { - self.pc.inst = self.block_stack.pop().unwrap(); - Machine::test_next_instruction(func, &self.pc); - } - Opcode::BranchIf => { - let x = self.value_stack.pop().unwrap(); - if !x.is_i32_zero() { - self.pc.inst = self.block_stack.pop().unwrap(); - Machine::test_next_instruction(func, &self.pc); - } - } Opcode::Return => { let frame = self.frame_stack.pop().unwrap(); match frame.return_ref { @@ -1868,20 +1845,12 @@ impl Machine { } self.value_stack.push(Value::I64(x)); } - Opcode::PushStackBoundary => { - self.value_stack.push(Value::StackBoundary); - } Opcode::MoveFromStackToInternal => { self.internal_stack.push(self.value_stack.pop().unwrap()); } Opcode::MoveFromInternalToStack => { self.value_stack.push(self.internal_stack.pop().unwrap()); } - Opcode::IsStackBoundary => { - let val = self.value_stack.pop().unwrap(); - self.value_stack - .push(Value::I32((val == Value::StackBoundary) as u32)); - } Opcode::Dup => { let val = self.value_stack.last().cloned().unwrap(); self.value_stack.push(val); diff --git a/arbitrator/prover/src/value.rs b/arbitrator/prover/src/value.rs index 0b79518fbbc..9267a21782c 100644 --- a/arbitrator/prover/src/value.rs +++ b/arbitrator/prover/src/value.rs @@ -20,7 +20,6 @@ pub enum ArbValueType { RefNull, FuncRef, InternalRef, - StackBoundary, } impl ArbValueType { @@ -96,7 +95,6 @@ pub enum Value { RefNull, FuncRef(u32), InternalRef(ProgramCounter), - StackBoundary, } impl Value { @@ -109,7 +107,6 @@ impl Value { Value::RefNull => ArbValueType::RefNull, Value::FuncRef(_) => ArbValueType::FuncRef, Value::InternalRef(_) => ArbValueType::InternalRef, - Value::StackBoundary => ArbValueType::StackBoundary, } } @@ -122,7 +119,6 @@ impl Value { Value::RefNull => Bytes32::default(), Value::FuncRef(x) => x.into(), Value::InternalRef(pc) => pc.serialize(), - Value::StackBoundary => Bytes32::default(), } } @@ -186,9 +182,6 @@ impl Value { ArbValueType::RefNull | ArbValueType::FuncRef | ArbValueType::InternalRef => { Value::RefNull } - ArbValueType::StackBoundary => { - panic!("Attempted to make default of StackBoundary type") - } } } @@ -232,10 +225,9 @@ impl Value { } Value::F32(value) => single!("f32", *value), Value::F64(value) => single!("f64", *value), - Value::RefNull => format!("null"), + Value::RefNull => "null".into(), Value::FuncRef(func) => format!("func {}", func), Value::InternalRef(pc) => format!("inst {} in {}-{}", pc.inst, pc.module, pc.func), - Value::StackBoundary => format!("stack boundary"), } } } diff --git a/arbitrator/prover/src/wavm.rs b/arbitrator/prover/src/wavm.rs index a4ef905c079..e8f7fa25573 100644 --- a/arbitrator/prover/src/wavm.rs +++ b/arbitrator/prover/src/wavm.rs @@ -78,10 +78,6 @@ impl IBinOpType { pub enum Opcode { Unreachable, Nop, - Block, - // Loop and If are wrapped into Block - Branch, - BranchIf, Return, Call, @@ -137,24 +133,16 @@ pub enum Opcode { IBinOp(IntegerValType, IBinOpType), // Custom opcodes not in WASM. Documented more in "Custom opcodes.md". - /// Branch is partially split up into these. - EndBlock, - /// Custom opcode not in wasm. - /// Like "EndBlock" but conditional. - /// Keeps its condition on the stack. - EndBlockIf, /// Custom opcode not in wasm. InitFrame, + /// Unconditional jump to an arbitrary point in code. + ArbitraryJump, /// Conditional jump to an arbitrary point in code. ArbitraryJumpIf, - /// Push a Value::StackBoundary to the stack - PushStackBoundary, /// Pop a value from the value stack and push it to the internal stack MoveFromStackToInternal, /// Pop a value from the internal stack and push it to the value stack MoveFromInternalToStack, - /// Pop a value from the value stack, then push an I32 1 if it's a stack boundary, I32 0 otherwise. - IsStackBoundary, /// Duplicate the top value on the stack Dup, /// Call a function in a different module @@ -175,8 +163,6 @@ pub enum Opcode { ReadInboxMessage, /// Stop exexcuting the machine and move to the finished status HaltAndSetFinished, - /// Unconditional jump to an arbitrary point in code. - ArbitraryJump, } impl Opcode { @@ -184,9 +170,6 @@ impl Opcode { match self { Opcode::Unreachable => 0x00, Opcode::Nop => 0x01, - Opcode::Block => 0x02, - Opcode::Branch => 0x0C, - Opcode::BranchIf => 0x0D, Opcode::Return => 0x0F, Opcode::Call => 0x10, Opcode::CallIndirect => 0x11, @@ -275,14 +258,11 @@ impl Opcode { _ => panic!("Unsupported {:?}", self), }, // Internal instructions: - Opcode::EndBlock => 0x8000, - Opcode::EndBlockIf => 0x8001, Opcode::InitFrame => 0x8002, - Opcode::ArbitraryJumpIf => 0x8003, - Opcode::PushStackBoundary => 0x8004, + Opcode::ArbitraryJump => 0x8003, + Opcode::ArbitraryJumpIf => 0x8004, Opcode::MoveFromStackToInternal => 0x8005, Opcode::MoveFromInternalToStack => 0x8006, - Opcode::IsStackBoundary => 0x8007, Opcode::Dup => 0x8008, Opcode::CrossModuleCall => 0x8009, Opcode::CallerModuleInternalCall => 0x800A, @@ -293,7 +273,6 @@ impl Opcode { Opcode::ReadPreImage => 0x8020, Opcode::ReadInboxMessage => 0x8021, Opcode::HaltAndSetFinished => 0x8022, - Opcode::ArbitraryJump => 0x8023, } } diff --git a/contracts/src/osp/OneStepProver0.sol b/contracts/src/osp/OneStepProver0.sol index 4b50313cdbd..c364b324697 100644 --- a/contracts/src/osp/OneStepProver0.sol +++ b/contracts/src/osp/OneStepProver0.sol @@ -51,8 +51,6 @@ contract OneStepProver0 is IOneStepProver { ty = ValueType.F32; } else if (opcode == Instructions.F64_CONST) { ty = ValueType.F64; - } else if (opcode == Instructions.PUSH_STACK_BOUNDARY) { - ty = ValueType.STACK_BOUNDARY; } else { revert("CONST_PUSH_INVALID_OPCODE"); } @@ -86,39 +84,6 @@ contract OneStepProver0 is IOneStepProver { } } - function executeBlock( - Machine memory mach, - Module memory, - Instruction calldata inst, - bytes calldata - ) internal pure { - uint32 targetPc = uint32(inst.argumentData); - require(targetPc == inst.argumentData, "BAD_BLOCK_PC"); - mach.blockStack.push(targetPc); - } - - function executeBranch( - Machine memory mach, - Module memory, - Instruction calldata, - bytes calldata - ) internal pure { - mach.functionPc = mach.blockStack.pop(); - } - - function executeBranchIf( - Machine memory mach, - Module memory, - Instruction calldata, - bytes calldata - ) internal pure { - uint32 cond = mach.valueStack.pop().assumeI32(); - if (cond != 0) { - // Jump to target - mach.functionPc = mach.blockStack.pop(); - } - } - function executeReturn( Machine memory mach, Module memory, @@ -419,27 +384,6 @@ contract OneStepProver0 is IOneStepProver { ); } - function executeEndBlock( - Machine memory mach, - Module memory, - Instruction calldata, - bytes calldata - ) internal pure { - mach.blockStack.pop(); - } - - function executeEndBlockIf( - Machine memory mach, - Module memory, - Instruction calldata, - bytes calldata - ) internal pure { - uint32 cond = mach.valueStack.peek().assumeI32(); - if (cond != 0) { - mach.blockStack.pop(); - } - } - function executeInitFrame( Machine memory mach, Module memory, @@ -476,20 +420,6 @@ contract OneStepProver0 is IOneStepProver { } } - function executeIsStackBoundary( - Machine memory mach, - Module memory, - Instruction calldata, - bytes calldata - ) internal pure { - Value memory val = mach.valueStack.pop(); - uint32 newContents = 0; - if (val.valueType == ValueType.STACK_BOUNDARY) { - newContents = 1; - } - mach.valueStack.push(ValueLib.newI32(newContents)); - } - function executeDup( Machine memory mach, Module memory, @@ -519,12 +449,6 @@ contract OneStepProver0 is IOneStepProver { impl = executeUnreachable; } else if (opcode == Instructions.NOP) { impl = executeNop; - } else if (opcode == Instructions.BLOCK) { - impl = executeBlock; - } else if (opcode == Instructions.BRANCH) { - impl = executeBranch; - } else if (opcode == Instructions.BRANCH_IF) { - impl = executeBranchIf; } else if (opcode == Instructions.RETURN) { impl = executeReturn; } else if (opcode == Instructions.CALL) { @@ -535,10 +459,6 @@ contract OneStepProver0 is IOneStepProver { impl = executeCallerModuleInternalCall; } else if (opcode == Instructions.CALL_INDIRECT) { impl = executeCallIndirect; - } else if (opcode == Instructions.END_BLOCK) { - impl = executeEndBlock; - } else if (opcode == Instructions.END_BLOCK_IF) { - impl = executeEndBlockIf; } else if (opcode == Instructions.ARBITRARY_JUMP) { impl = executeArbitraryJump; } else if (opcode == Instructions.ARBITRARY_JUMP_IF) { @@ -557,18 +477,13 @@ contract OneStepProver0 is IOneStepProver { impl = executeDrop; } else if (opcode == Instructions.SELECT) { impl = executeSelect; - } else if ( - (opcode >= Instructions.I32_CONST && opcode <= Instructions.F64_CONST) || - opcode == Instructions.PUSH_STACK_BOUNDARY - ) { + } else if (opcode >= Instructions.I32_CONST && opcode <= Instructions.F64_CONST) { impl = executeConstPush; } else if ( opcode == Instructions.MOVE_FROM_STACK_TO_INTERNAL || opcode == Instructions.MOVE_FROM_INTERNAL_TO_STACK ) { impl = executeMoveInternal; - } else if (opcode == Instructions.IS_STACK_BOUNDARY) { - impl = executeIsStackBoundary; } else if (opcode == Instructions.DUP) { impl = executeDup; } else { diff --git a/contracts/src/state/Instructions.sol b/contracts/src/state/Instructions.sol index 625da095c69..196899c93ff 100644 --- a/contracts/src/state/Instructions.sol +++ b/contracts/src/state/Instructions.sol @@ -12,9 +12,6 @@ struct Instruction { library Instructions { uint16 internal constant UNREACHABLE = 0x00; uint16 internal constant NOP = 0x01; - uint16 internal constant BLOCK = 0x02; - uint16 internal constant BRANCH = 0x0C; - uint16 internal constant BRANCH_IF = 0x0D; uint16 internal constant RETURN = 0x0F; uint16 internal constant CALL = 0x10; uint16 internal constant CALL_INDIRECT = 0x11; @@ -129,14 +126,11 @@ library Instructions { uint16 internal constant I64_EXTEND_16S = 0xC3; uint16 internal constant I64_EXTEND_32S = 0xC4; - uint16 internal constant END_BLOCK = 0x8000; - uint16 internal constant END_BLOCK_IF = 0x8001; uint16 internal constant INIT_FRAME = 0x8002; - uint16 internal constant ARBITRARY_JUMP_IF = 0x8003; - uint16 internal constant PUSH_STACK_BOUNDARY = 0x8004; + uint16 internal constant ARBITRARY_JUMP = 0x8003; + uint16 internal constant ARBITRARY_JUMP_IF = 0x8004; uint16 internal constant MOVE_FROM_STACK_TO_INTERNAL = 0x8005; uint16 internal constant MOVE_FROM_INTERNAL_TO_STACK = 0x8006; - uint16 internal constant IS_STACK_BOUNDARY = 0x8007; uint16 internal constant DUP = 0x8008; uint16 internal constant CROSS_MODULE_CALL = 0x8009; uint16 internal constant CALLER_MODULE_INTERNAL_CALL = 0x800A; @@ -150,8 +144,6 @@ library Instructions { uint16 internal constant READ_INBOX_MESSAGE = 0x8021; uint16 internal constant HALT_AND_SET_FINISHED = 0x8022; - uint16 internal constant ARBITRARY_JUMP = 0x8023; - uint256 internal constant INBOX_INDEX_SEQUENCER = 0; uint256 internal constant INBOX_INDEX_DELAYED = 1; diff --git a/contracts/src/state/Value.sol b/contracts/src/state/Value.sol index 3784eab6699..6e0a837b2b3 100644 --- a/contracts/src/state/Value.sol +++ b/contracts/src/state/Value.sol @@ -11,8 +11,7 @@ enum ValueType { F64, REF_NULL, FUNC_REF, - INTERNAL_REF, - STACK_BOUNDARY + INTERNAL_REF } struct Value { @@ -26,7 +25,7 @@ library ValueLib { } function maxValueType() internal pure returns (ValueType) { - return ValueType.STACK_BOUNDARY; + return ValueType.INTERNAL_REF; } function assumeI32(Value memory val) internal pure returns (uint32) { diff --git a/go.mod b/go.mod index 00c1117de02..565f907439d 100644 --- a/go.mod +++ b/go.mod @@ -106,7 +106,7 @@ require ( golang.org/x/crypto v0.0.0-20210322153248-0c34fe9e7dc2 golang.org/x/net v0.0.0-20220127200216-cd36cc0744dd // indirect golang.org/x/sync v0.0.0-20210220032951-036812b2e83c // indirect - golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e // indirect + golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e golang.org/x/text v0.3.7 // indirect golang.org/x/time v0.0.0-20210220033141-f8bda1e9f3ba // indirect gopkg.in/natefinch/npipe.v2 v2.0.0-20160621034901-c1b8fa8bdcce // indirect