From eb103127c94efa3042844e6ebf9761c87f87a8ac Mon Sep 17 00:00:00 2001 From: fcarreiro Date: Wed, 11 Sep 2024 10:40:13 +0000 Subject: [PATCH 1/3] feat(avm)!: variants for REVERT opcode --- avm-transpiler/src/opcodes.rs | 6 ++++-- avm-transpiler/src/transpile.rs | 16 +++++++++++++--- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/avm-transpiler/src/opcodes.rs b/avm-transpiler/src/opcodes.rs index f06cece6b7d..0ac140b2d3c 100644 --- a/avm-transpiler/src/opcodes.rs +++ b/avm-transpiler/src/opcodes.rs @@ -79,7 +79,8 @@ pub enum AvmOpcode { STATICCALL, DELEGATECALL, RETURN, - REVERT, + REVERT_8, + REVERT_16, // Misc DEBUGLOG, // Gadgets @@ -189,7 +190,8 @@ impl AvmOpcode { AvmOpcode::STATICCALL => "STATICCALL", AvmOpcode::DELEGATECALL => "DELEGATECALL", AvmOpcode::RETURN => "RETURN", - AvmOpcode::REVERT => "REVERT", + AvmOpcode::REVERT_8 => "REVERT_8", + AvmOpcode::REVERT_16 => "REVERT_16", // Misc AvmOpcode::DEBUGLOG => "DEBUGLOG", diff --git a/avm-transpiler/src/transpile.rs b/avm-transpiler/src/transpile.rs index 23c3e9fbc94..9cf34dd66c9 100644 --- a/avm-transpiler/src/transpile.rs +++ b/avm-transpiler/src/transpile.rs @@ -274,12 +274,22 @@ pub fn brillig_to_avm( }); } BrilligOpcode::Trap { revert_data } => { + let bits_needed = [revert_data.pointer.0, revert_data.size] + .iter() + .map(bits_needed_for) + .max() + .unwrap(); + let avm_opcode = match bits_needed { + 8 => AvmOpcode::REVERT_8, + 16 => AvmOpcode::REVERT_16, + _ => panic!("REVERT only support 8 or 16 bit encodings, got: {}", bits_needed), + }; avm_instrs.push(AvmInstruction { - opcode: AvmOpcode::REVERT, + opcode: avm_opcode, indirect: Some(ZEROTH_OPERAND_INDIRECT), operands: vec![ - AvmOperand::U32 { value: revert_data.pointer.0 as u32 }, - AvmOperand::U32 { value: revert_data.size as u32 }, + make_operand(bits_needed, &revert_data.pointer.0), + make_operand(bits_needed, &revert_data.size), ], ..Default::default() }); From b6fe874d2036fa1d402ccd81d0546bc5b385c1df Mon Sep 17 00:00:00 2001 From: fcarreiro Date: Wed, 11 Sep 2024 11:08:58 +0000 Subject: [PATCH 2/3] simulator changes --- yarn-project/simulator/src/avm/avm_gas.ts | 6 ++++-- .../src/avm/opcodes/external_calls.test.ts | 13 ++++++++----- .../simulator/src/avm/opcodes/external_calls.ts | 16 +++++++++++----- .../avm/serialization/bytecode_serialization.ts | 3 ++- .../serialization/instruction_serialization.ts | 3 ++- 5 files changed, 27 insertions(+), 14 deletions(-) diff --git a/yarn-project/simulator/src/avm/avm_gas.ts b/yarn-project/simulator/src/avm/avm_gas.ts index 43f5d48d782..d325aa5a327 100644 --- a/yarn-project/simulator/src/avm/avm_gas.ts +++ b/yarn-project/simulator/src/avm/avm_gas.ts @@ -126,7 +126,8 @@ const BaseGasCosts: Record = { [Opcode.STATICCALL]: makeCost(c.AVM_STATICCALL_BASE_L2_GAS, 0), [Opcode.DELEGATECALL]: makeCost(c.AVM_DELEGATECALL_BASE_L2_GAS, 0), [Opcode.RETURN]: makeCost(c.AVM_RETURN_BASE_L2_GAS, 0), - [Opcode.REVERT]: makeCost(c.AVM_REVERT_BASE_L2_GAS, 0), + [Opcode.REVERT_8]: makeCost(c.AVM_REVERT_BASE_L2_GAS, 0), + [Opcode.REVERT_16]: makeCost(c.AVM_REVERT_BASE_L2_GAS, 0), [Opcode.DEBUGLOG]: makeCost(c.AVM_DEBUGLOG_BASE_L2_GAS, 0), [Opcode.KECCAK]: makeCost(c.AVM_KECCAK_BASE_L2_GAS, 0), [Opcode.POSEIDON2]: makeCost(c.AVM_POSEIDON2_BASE_L2_GAS, 0), @@ -210,7 +211,8 @@ const DynamicGasCosts: Record = { [Opcode.STATICCALL]: makeCost(c.AVM_STATICCALL_DYN_L2_GAS, 0), [Opcode.DELEGATECALL]: makeCost(c.AVM_DELEGATECALL_DYN_L2_GAS, 0), [Opcode.RETURN]: makeCost(c.AVM_RETURN_DYN_L2_GAS, 0), - [Opcode.REVERT]: makeCost(c.AVM_REVERT_DYN_L2_GAS, 0), + [Opcode.REVERT_8]: makeCost(c.AVM_REVERT_DYN_L2_GAS, 0), + [Opcode.REVERT_16]: makeCost(c.AVM_REVERT_DYN_L2_GAS, 0), [Opcode.DEBUGLOG]: makeCost(c.AVM_DEBUGLOG_DYN_L2_GAS, 0), [Opcode.KECCAK]: makeCost(c.AVM_KECCAK_DYN_L2_GAS, 0), [Opcode.POSEIDON2]: makeCost(c.AVM_POSEIDON2_DYN_L2_GAS, 0), diff --git a/yarn-project/simulator/src/avm/opcodes/external_calls.test.ts b/yarn-project/simulator/src/avm/opcodes/external_calls.test.ts index 8cde0f61cb4..ce6bbe4a7f7 100644 --- a/yarn-project/simulator/src/avm/opcodes/external_calls.test.ts +++ b/yarn-project/simulator/src/avm/opcodes/external_calls.test.ts @@ -287,14 +287,17 @@ describe('External Calls', () => { describe('REVERT', () => { it('Should (de)serialize correctly', () => { const buf = Buffer.from([ - Revert.opcode, // opcode + Opcode.REVERT_16, // opcode 0x01, // indirect - ...Buffer.from('12345678', 'hex'), // returnOffset - ...Buffer.from('a2345678', 'hex'), // retSize + ...Buffer.from('1234', 'hex'), // returnOffset + ...Buffer.from('a234', 'hex'), // retSize ]); - const inst = new Revert(/*indirect=*/ 0x01, /*returnOffset=*/ 0x12345678, /*retSize=*/ 0xa2345678); + const inst = new Revert(/*indirect=*/ 0x01, /*returnOffset=*/ 0x1234, /*retSize=*/ 0xa234).as( + Opcode.REVERT_16, + Revert.wireFormat16, + ); - expect(Revert.deserialize(buf)).toEqual(inst); + expect(Revert.as(Revert.wireFormat16).deserialize(buf)).toEqual(inst); expect(inst.serialize()).toEqual(buf); }); diff --git a/yarn-project/simulator/src/avm/opcodes/external_calls.ts b/yarn-project/simulator/src/avm/opcodes/external_calls.ts index cf58f60189e..afc6d967ff5 100644 --- a/yarn-project/simulator/src/avm/opcodes/external_calls.ts +++ b/yarn-project/simulator/src/avm/opcodes/external_calls.ts @@ -183,13 +183,19 @@ export class Return extends Instruction { export class Revert extends Instruction { static type: string = 'REVERT'; - static readonly opcode: Opcode = Opcode.REVERT; - // Informs (de)serialization. See Instruction.deserialize. - static readonly wireFormat: OperandType[] = [ + static readonly opcode: Opcode = Opcode.REVERT_8; + + static readonly wireFormat8: OperandType[] = [ OperandType.UINT8, OperandType.UINT8, - OperandType.UINT32, - OperandType.UINT32, + OperandType.UINT8, + OperandType.UINT8, + ]; + static readonly wireFormat16: OperandType[] = [ + OperandType.UINT8, + OperandType.UINT8, + OperandType.UINT16, + OperandType.UINT16, ]; constructor(private indirect: number, private returnOffset: number, private retSize: number) { diff --git a/yarn-project/simulator/src/avm/serialization/bytecode_serialization.ts b/yarn-project/simulator/src/avm/serialization/bytecode_serialization.ts index 3b435b0386a..54071c8104e 100644 --- a/yarn-project/simulator/src/avm/serialization/bytecode_serialization.ts +++ b/yarn-project/simulator/src/avm/serialization/bytecode_serialization.ts @@ -155,7 +155,8 @@ const INSTRUCTION_SET = () => [StaticCall.opcode, Instruction.deserialize.bind(StaticCall)], //[DelegateCall.opcode, Instruction.deserialize.bind(DelegateCall)], [Return.opcode, Instruction.deserialize.bind(Return)], - [Revert.opcode, Instruction.deserialize.bind(Revert)], + [Opcode.REVERT_8, Revert.as(Revert.wireFormat8).deserialize], + [Opcode.REVERT_16, Revert.as(Revert.wireFormat16).deserialize], // Misc [DebugLog.opcode, Instruction.deserialize.bind(DebugLog)], diff --git a/yarn-project/simulator/src/avm/serialization/instruction_serialization.ts b/yarn-project/simulator/src/avm/serialization/instruction_serialization.ts index 904c9001118..277b9d797a6 100644 --- a/yarn-project/simulator/src/avm/serialization/instruction_serialization.ts +++ b/yarn-project/simulator/src/avm/serialization/instruction_serialization.ts @@ -83,7 +83,8 @@ export enum Opcode { STATICCALL, DELEGATECALL, RETURN, - REVERT, + REVERT_8, + REVERT_16, // Misc DEBUGLOG, // Gadgets From f43c39bb5a2ceb60d6f191ba7e5d70f883c51b62 Mon Sep 17 00:00:00 2001 From: fcarreiro Date: Wed, 11 Sep 2024 11:14:36 +0000 Subject: [PATCH 3/3] cpp part --- .../barretenberg/vm/avm/trace/deserialization.cpp | 3 ++- .../src/barretenberg/vm/avm/trace/execution.cpp | 14 +++++++++++--- .../src/barretenberg/vm/avm/trace/fixed_gas.cpp | 3 ++- .../cpp/src/barretenberg/vm/avm/trace/opcode.cpp | 6 ++++-- .../cpp/src/barretenberg/vm/avm/trace/opcode.hpp | 3 ++- 5 files changed, 21 insertions(+), 8 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp index b7854957ac4..d5793f7d9a5 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp @@ -160,7 +160,8 @@ const std::unordered_map> OPCODE_WIRE_FORMAT = // DELEGATECALL, -- not in simulator { OpCode::RETURN, { OperandType::INDIRECT, OperandType::UINT32, OperandType::UINT32 } }, // REVERT, - { OpCode::REVERT, { OperandType::INDIRECT, OperandType::UINT32, OperandType::UINT32 } }, + { OpCode::REVERT_8, { OperandType::INDIRECT, OperandType::UINT8, OperandType::UINT8 } }, + { OpCode::REVERT_16, { OperandType::INDIRECT, OperandType::UINT16, OperandType::UINT16 } }, // Misc { OpCode::DEBUGLOG, diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp index 6c6904fcac9..eabc14ae073 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp @@ -815,10 +815,18 @@ std::vector Execution::gen_trace(std::vector const& instructio break; } - case OpCode::REVERT: { + case OpCode::REVERT_8: { auto ret = trace_builder.op_revert(std::get(inst.operands.at(0)), - std::get(inst.operands.at(1)), - std::get(inst.operands.at(2))); + std::get(inst.operands.at(1)), + std::get(inst.operands.at(2))); + returndata.insert(returndata.end(), ret.begin(), ret.end()); + + break; + } + case OpCode::REVERT_16: { + auto ret = trace_builder.op_revert(std::get(inst.operands.at(0)), + std::get(inst.operands.at(1)), + std::get(inst.operands.at(2))); returndata.insert(returndata.end(), ret.begin(), ret.end()); break; diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/fixed_gas.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/fixed_gas.cpp index 43005b68564..ba61d8efc49 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/fixed_gas.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/fixed_gas.cpp @@ -89,7 +89,8 @@ const std::unordered_map GAS_COST_TABLE = { { OpCode::STATICCALL, make_cost(AVM_STATICCALL_BASE_L2_GAS, 0, AVM_STATICCALL_DYN_L2_GAS, 0) }, { OpCode::DELEGATECALL, make_cost(AVM_DELEGATECALL_BASE_L2_GAS, 0, AVM_DELEGATECALL_DYN_L2_GAS, 0) }, { OpCode::RETURN, make_cost(AVM_RETURN_BASE_L2_GAS, 0, AVM_RETURN_DYN_L2_GAS, 0) }, - { OpCode::REVERT, make_cost(AVM_REVERT_BASE_L2_GAS, 0, AVM_REVERT_DYN_L2_GAS, 0) }, + { OpCode::REVERT_8, make_cost(AVM_REVERT_BASE_L2_GAS, 0, AVM_REVERT_DYN_L2_GAS, 0) }, + { OpCode::REVERT_16, make_cost(AVM_REVERT_BASE_L2_GAS, 0, AVM_REVERT_DYN_L2_GAS, 0) }, { OpCode::DEBUGLOG, make_cost(AVM_DEBUGLOG_BASE_L2_GAS, 0, AVM_DEBUGLOG_DYN_L2_GAS, 0) }, { OpCode::KECCAK, make_cost(AVM_KECCAK_BASE_L2_GAS, 0, AVM_KECCAK_DYN_L2_GAS, 0) }, { OpCode::POSEIDON2, make_cost(AVM_POSEIDON2_BASE_L2_GAS, 0, AVM_POSEIDON2_DYN_L2_GAS, 0) }, diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/opcode.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/opcode.cpp index cbee33a6ddd..4e71de41b39 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/opcode.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/opcode.cpp @@ -174,8 +174,10 @@ std::string to_string(OpCode opcode) return "DELEGATECALL"; case OpCode::RETURN: return "RETURN"; - case OpCode::REVERT: - return "REVERT"; + case OpCode::REVERT_8: + return "REVERT_8"; + case OpCode::REVERT_16: + return "REVERT_16"; // Misc case OpCode::DEBUGLOG: return "DEBUGLOG"; diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/opcode.hpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/opcode.hpp index e27931cebf4..5237a6db170 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/opcode.hpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/opcode.hpp @@ -105,7 +105,8 @@ enum class OpCode : uint8_t { STATICCALL, DELEGATECALL, RETURN, - REVERT, + REVERT_8, + REVERT_16, // Misc DEBUGLOG,