From bd422ab630d0d801d80a664cc715bbf7754b477b Mon Sep 17 00:00:00 2001 From: jeanmon Date: Thu, 14 Nov 2024 13:10:35 +0000 Subject: [PATCH 1/4] 9770: handle parsing error (squashed original commits) --- .../vm/avm/tests/execution.test.cpp | 100 ++++++++++++------ .../vm/avm/trace/deserialization.cpp | 59 ++++++++--- .../vm/avm/trace/deserialization.hpp | 9 +- .../src/barretenberg/vm/avm/trace/errors.hpp | 5 +- .../barretenberg/vm/avm/trace/execution.cpp | 41 +++---- .../src/barretenberg/vm/avm/trace/helper.cpp | 10 +- .../vm/avm/trace/instructions.hpp | 6 ++ .../src/barretenberg/vm/avm/trace/trace.cpp | 92 ++++++++-------- .../simulator/src/avm/avm_memory_types.ts | 50 ++++----- .../simulator/src/avm/avm_simulator.ts | 5 - yarn-project/simulator/src/avm/errors.ts | 32 ++++++ .../simulator/src/avm/opcodes/memory.ts | 7 +- .../src/avm/serialization/buffer_cursor.ts | 3 - .../serialization/bytecode_serialization.ts | 32 ++++-- 14 files changed, 290 insertions(+), 161 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp index 26242da0bdde..a5d6a718246c 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp @@ -152,7 +152,8 @@ TEST_F(AvmExecutionTests, basicAddReturn) "00FF"; // ret size offset 255 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse_bytecode_statically(bytecode); + auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode); + ASSERT_TRUE(is_ok(error)); // 2 instructions ASSERT_THAT(instructions, SizeIs(5)); @@ -214,7 +215,8 @@ TEST_F(AvmExecutionTests, setAndSubOpcodes) "00FF"; // ret size offset 255 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse_bytecode_statically(bytecode); + auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode); + ASSERT_TRUE(is_ok(error)); ASSERT_THAT(instructions, SizeIs(5)); @@ -295,7 +297,8 @@ TEST_F(AvmExecutionTests, powerWithMulOpcodes) bytecode_hex.append(ret_hex); auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse_bytecode_statically(bytecode); + auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode); + ASSERT_TRUE(is_ok(error)); ASSERT_THAT(instructions, SizeIs(16)); @@ -394,7 +397,8 @@ TEST_F(AvmExecutionTests, simpleInternalCall) ; auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse_bytecode_statically(bytecode); + auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode); + ASSERT_TRUE(is_ok(error)); EXPECT_THAT(instructions, SizeIs(7)); @@ -475,7 +479,8 @@ TEST_F(AvmExecutionTests, nestedInternalCalls) bytecode_f2 + bytecode_f1 + bytecode_g; auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse_bytecode_statically(bytecode); + auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode); + ASSERT_TRUE(is_ok(error)); ASSERT_THAT(instructions, SizeIs(13)); @@ -555,7 +560,8 @@ TEST_F(AvmExecutionTests, jumpAndCalldatacopy) ; auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse_bytecode_statically(bytecode); + auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode); + ASSERT_TRUE(is_ok(error)); ASSERT_THAT(instructions, SizeIs(8)); @@ -657,7 +663,8 @@ TEST_F(AvmExecutionTests, jumpiAndCalldatacopy) ; auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse_bytecode_statically(bytecode); + auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode); + ASSERT_TRUE(is_ok(error)); ASSERT_THAT(instructions, SizeIs(9)); @@ -714,7 +721,8 @@ TEST_F(AvmExecutionTests, movOpcode) "00FF"; // ret size offset 255 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse_bytecode_statically(bytecode); + auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode); + ASSERT_TRUE(is_ok(error)); ASSERT_THAT(instructions, SizeIs(4)); @@ -773,7 +781,8 @@ TEST_F(AvmExecutionTests, indMovOpcode) "00FF"; // ret size offset 255 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse_bytecode_statically(bytecode); + auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode); + ASSERT_TRUE(is_ok(error)); ASSERT_THAT(instructions, SizeIs(6)); @@ -815,7 +824,8 @@ TEST_F(AvmExecutionTests, setAndCastOpcodes) "00FF"; // ret size offset 255 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse_bytecode_statically(bytecode); + auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode); + ASSERT_TRUE(is_ok(error)); ASSERT_THAT(instructions, SizeIs(4)); @@ -884,7 +894,8 @@ TEST_F(AvmExecutionTests, toRadixBeOpcodeBytes) "0200"; // ret size offset 512 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse_bytecode_statically(bytecode); + auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode); + ASSERT_TRUE(is_ok(error)); // Assign a vector that we will mutate internally in gen_trace to store the return values; std::vector returndata; @@ -958,7 +969,8 @@ TEST_F(AvmExecutionTests, toRadixBeOpcodeBitsMode) "0200"; // ret size offset 512 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse_bytecode_statically(bytecode); + auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode); + ASSERT_TRUE(is_ok(error)); // Assign a vector that we will mutate internally in gen_trace to store the return values; std::vector returndata; @@ -1031,7 +1043,8 @@ TEST_F(AvmExecutionTests, sha256CompressionOpcode) "0200"; // ret size offset 512 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse_bytecode_statically(bytecode); + auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode); + ASSERT_TRUE(is_ok(error)); // Assign a vector that we will mutate internally in gen_trace to store the return values; std::vector calldata = std::vector(); @@ -1093,7 +1106,8 @@ TEST_F(AvmExecutionTests, poseidon2PermutationOpCode) "0200"; // ret size offset 512 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse_bytecode_statically(bytecode); + auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode); + ASSERT_TRUE(is_ok(error)); // Assign a vector that we will mutate internally in gen_trace to store the return values; std::vector returndata = std::vector(); @@ -1168,7 +1182,8 @@ TEST_F(AvmExecutionTests, keccakf1600OpCode) "0200"; // ret size offset 512 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse_bytecode_statically(bytecode); + auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode); + ASSERT_TRUE(is_ok(error)); // Assign a vector that we will mutate internally in gen_trace to store the return values; std::vector calldata = std::vector(); @@ -1241,7 +1256,8 @@ TEST_F(AvmExecutionTests, embeddedCurveAddOpCode) "0200"; // ret size offset 512 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse_bytecode_statically(bytecode); + auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode); + ASSERT_TRUE(is_ok(error)); // Assign a vector that we will mutate internally in gen_trace to store the return values; std::vector returndata; @@ -1336,7 +1352,8 @@ TEST_F(AvmExecutionTests, msmOpCode) "0200"; // ret size offset 512 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse_bytecode_statically(bytecode); + auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode); + ASSERT_TRUE(is_ok(error)); // Assign a vector that we will mutate internally in gen_trace to store the return values; std::vector returndata; @@ -1407,7 +1424,8 @@ TEST_F(AvmExecutionTests, getEnvOpcode) "0200"; // ret size offset 512 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse_bytecode_statically(bytecode); + auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode); + ASSERT_TRUE(is_ok(error)); ASSERT_THAT(instructions, SizeIs(13)); @@ -1623,7 +1641,8 @@ TEST_F(AvmExecutionTests, getEnvOpcode) // "0001"; // dst_offset // // auto bytecode = hex_to_bytes(bytecode_hex); -// auto instructions = Deserialization::parse_bytecode_statically(bytecode); +// auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode); +// ASSERT_TRUE(is_ok(error)); // // // Public inputs for the circuit // std::vector calldata; @@ -1660,7 +1679,8 @@ TEST_F(AvmExecutionTests, l2GasLeft) "00FF"; // ret size offset 255 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse_bytecode_statically(bytecode); + auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode); + ASSERT_TRUE(is_ok(error)); ASSERT_THAT(instructions, SizeIs(4)); @@ -1708,7 +1728,8 @@ TEST_F(AvmExecutionTests, daGasLeft) "00FF"; // ret size offset 255 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse_bytecode_statically(bytecode); + auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode); + ASSERT_TRUE(is_ok(error)); ASSERT_THAT(instructions, SizeIs(4)); @@ -1747,7 +1768,8 @@ TEST_F(AvmExecutionTests, ExecutorThrowsWithTooMuchGasAllocated) public_inputs.gas_settings.gas_limits.l2_gas = MAX_L2_GAS_PER_ENQUEUED_CALL; auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse_bytecode_statically(bytecode); + auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode); + ASSERT_TRUE(is_ok(error)); ExecutionHints execution_hints; EXPECT_THROW_WITH_MESSAGE(gen_trace(bytecode, calldata, public_inputs, returndata, execution_hints), @@ -1767,7 +1789,8 @@ TEST_F(AvmExecutionTests, ExecutorThrowsWithTooMuchGasAllocated) // std::vector public_inputs = { 1 }; // // auto bytecode = hex_to_bytes(bytecode_hex); -// auto instructions = Deserialization::parse_bytecode_statically(bytecode); +// auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode); +// ASSERT_TRUE(is_ok(error)); // // ExecutionHints execution_hints; // EXPECT_THROW_WITH_MESSAGE(gen_trace(bytecode, calldata, public_inputs, returndata, execution_hints), @@ -1814,7 +1837,8 @@ TEST_F(AvmExecutionTests, kernelOutputEmitOpcodes) "0000"; // ret size 0 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse_bytecode_statically(bytecode); + auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode); + ASSERT_TRUE(is_ok(error)); ASSERT_THAT(instructions, SizeIs(8)); @@ -1943,7 +1967,8 @@ TEST_F(AvmExecutionTests, kernelOutputStorageLoadOpcodeSimple) "00FF"; // ret size offset 255 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse_bytecode_statically(bytecode); + auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode); + ASSERT_TRUE(is_ok(error)); ASSERT_THAT(instructions, SizeIs(5)); @@ -2010,7 +2035,8 @@ TEST_F(AvmExecutionTests, kernelOutputStorageStoreOpcodeSimple) "0200"; // ret size offset 512 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse_bytecode_statically(bytecode); + auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode); + ASSERT_TRUE(is_ok(error)); std::vector returndata; @@ -2072,7 +2098,8 @@ TEST_F(AvmExecutionTests, kernelOutputStorageOpcodes) "00FF"; // ret size offset 255 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse_bytecode_statically(bytecode); + auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode); + ASSERT_TRUE(is_ok(error)); ASSERT_THAT(instructions, SizeIs(6)); @@ -2159,7 +2186,8 @@ TEST_F(AvmExecutionTests, kernelOutputHashExistsOpcodes) "0200"; // ret size offset 512 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse_bytecode_statically(bytecode); + auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode); + ASSERT_TRUE(is_ok(error)); ASSERT_THAT(instructions, SizeIs(7)); @@ -2313,7 +2341,8 @@ TEST_F(AvmExecutionTests, opCallOpcodes) "0200"; // ret size offset 512 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse_bytecode_statically(bytecode); + auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode); + ASSERT_TRUE(is_ok(error)); std::vector returndata; @@ -2392,7 +2421,8 @@ TEST_F(AvmExecutionTests, opGetContractInstanceOpcode) "0200"; // ret size offset 512 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse_bytecode_statically(bytecode); + auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode); + ASSERT_TRUE(is_ok(error)); ASSERT_THAT(instructions, SizeIs(6)); @@ -2427,7 +2457,9 @@ TEST_F(AvmExecutionTests, opGetContractInstanceOpcodeBadEnum) + to_hex(static_cast(ContractInstanceMember::MAX_MEMBER)); // member enum auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse_bytecode_statically(bytecode); + auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode); + ASSERT_TRUE(is_ok(error)); + ASSERT_THAT(instructions, SizeIs(2)); std::vector calldata; @@ -2457,7 +2489,8 @@ TEST_F(AvmExecutionTests, invalidOpcode) "0000"; // ret size 0 auto bytecode = hex_to_bytes(bytecode_hex); - EXPECT_THROW_WITH_MESSAGE(Deserialization::parse_bytecode_statically(bytecode), "Invalid opcode"); + const auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode); + ASSERT_EQ(error, AvmError::INVALID_OPCODE); } // Negative test detecting an incomplete instruction: instruction tag present but an operand is missing @@ -2474,7 +2507,8 @@ TEST_F(AvmExecutionTests, truncatedInstructionNoOperand) "FF"; // addr b and missing address for c = a-b auto bytecode = hex_to_bytes(bytecode_hex); - EXPECT_THROW_WITH_MESSAGE(Deserialization::parse_bytecode_statically(bytecode), "Operand is missing"); + const auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode); + ASSERT_EQ(error, AvmError::PARSING_ERROR); } } // namespace tests_avm diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp index d3ad4039d157..ba6a60397aed 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp @@ -225,26 +225,38 @@ uint32_t Deserialization::get_pc_increment(OpCode opcode) * @throws runtime_error exception when the bytecode is invalid or pos is out-of-range * @return The instruction */ -Instruction Deserialization::parse(const std::vector& bytecode, size_t pos) +InstructionWithError Deserialization::parse(const std::vector& bytecode, size_t pos) { const auto length = bytecode.size(); if (pos >= length) { - throw_or_abort("Position is out of range. Position: " + std::to_string(pos) + - " Bytecode length: " + std::to_string(length)); + info("Position is out of range. Position: " + std::to_string(pos) + + " Bytecode length: " + std::to_string(length)); + return InstructionWithError{ + .instruction = Instruction(OpCode::LAST_OPCODE_SENTINEL, {}), + .error = AvmError::INVALID_PROGRAM_COUNTER, + }; } const uint8_t opcode_byte = bytecode.at(pos); if (!Bytecode::is_valid(opcode_byte)) { - throw_or_abort("Invalid opcode byte: " + to_hex(opcode_byte) + " at position: " + std::to_string(pos)); + info("Invalid opcode byte: " + to_hex(opcode_byte) + " at position: " + std::to_string(pos)); + return InstructionWithError{ + .instruction = Instruction(OpCode::LAST_OPCODE_SENTINEL, {}), + .error = AvmError::INVALID_OPCODE, + }; } pos++; const auto opcode = static_cast(opcode_byte); const auto iter = OPCODE_WIRE_FORMAT.find(opcode); if (iter == OPCODE_WIRE_FORMAT.end()) { - throw_or_abort("Opcode not found in OPCODE_WIRE_FORMAT: " + to_hex(opcode) + " name " + to_string(opcode)); + info("Opcode not found in OPCODE_WIRE_FORMAT: " + to_hex(opcode) + " name " + to_string(opcode)); + return InstructionWithError{ + .instruction = Instruction(OpCode::LAST_OPCODE_SENTINEL, {}), + .error = AvmError::INVALID_OPCODE, + }; } const std::vector& inst_format = iter->second; @@ -253,16 +265,24 @@ Instruction Deserialization::parse(const std::vector& bytecode, size_t // No underflow as above condition guarantees pos <= length (after pos++) const auto operand_size = OPERAND_TYPE_SIZE.at(op_type); if (length - pos < operand_size) { - throw_or_abort("Operand is missing at position " + std::to_string(pos) + " for opcode " + to_hex(opcode) + - " not enough bytes for operand type " + std::to_string(static_cast(op_type))); + info("Operand is missing at position " + std::to_string(pos) + " for opcode " + to_hex(opcode) + + " not enough bytes for operand type " + std::to_string(static_cast(op_type))); + return InstructionWithError{ + .instruction = Instruction(OpCode::LAST_OPCODE_SENTINEL, {}), + .error = AvmError::PARSING_ERROR, + }; } switch (op_type) { case OperandType::TAG: { uint8_t tag_u8 = bytecode.at(pos); if (tag_u8 > MAX_MEM_TAG) { - throw_or_abort("Instruction tag is invalid at position " + std::to_string(pos) + - " value: " + std::to_string(tag_u8) + " for opcode: " + to_string(opcode)); + info("Instruction tag is invalid at position " + std::to_string(pos) + + " value: " + std::to_string(tag_u8) + " for opcode: " + to_string(opcode)); + return InstructionWithError{ + .instruction = Instruction(OpCode::LAST_OPCODE_SENTINEL, {}), + .error = AvmError::INVALID_TAG_VALUE, + }; } operands.emplace_back(static_cast(tag_u8)); break; @@ -310,8 +330,7 @@ Instruction Deserialization::parse(const std::vector& bytecode, size_t pos += operand_size; } - auto instruction = Instruction(opcode, operands); - return instruction; + return InstructionWithError{ .instruction = Instruction(opcode, operands), .error = AvmError::NO_ERROR }; }; /** @@ -321,18 +340,28 @@ Instruction Deserialization::parse(const std::vector& bytecode, size_t * * @param bytecode The bytecode to be parsed as a vector of bytes/uint8_t * @throws runtime_error exception when the bytecode is invalid or pos is out-of-range - * @return The list of instructions as a vector + * @return The list of instructions as a vector with an error. */ -std::vector Deserialization::parse_bytecode_statically(const std::vector& bytecode) +ParsedBytecode Deserialization::parse_bytecode_statically(const std::vector& bytecode) { uint32_t pc = 0; std::vector instructions; while (pc < bytecode.size()) { - const auto instruction = parse(bytecode, pc); + const auto [instruction, error] = parse(bytecode, pc); + if (!is_ok(error)) { + return ParsedBytecode{ + .instructions = instructions, + .error = error, + }; + } instructions.emplace_back(instruction); pc += get_pc_increment(instruction.op_code); } - return instructions; + + return ParsedBytecode{ + .instructions = instructions, + .error = AvmError::NO_ERROR, + }; } } // namespace bb::avm_trace diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.hpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.hpp index eb58ddc803d6..b71570f1e984 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.hpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.hpp @@ -14,12 +14,17 @@ namespace bb::avm_trace { // INDIRECT is parsed as UINT8 where the bits represent the operands that have indirect mem access. enum class OperandType : uint8_t { INDIRECT8, INDIRECT16, TAG, UINT8, UINT16, UINT32, UINT64, UINT128, FF }; +struct ParsedBytecode { + std::vector instructions; + AvmError error; +}; + class Deserialization { public: Deserialization() = default; - static Instruction parse(const std::vector& bytecode, size_t pos); - static std::vector parse_bytecode_statically(const std::vector& bytecode); + static InstructionWithError parse(const std::vector& bytecode, size_t pos); + static ParsedBytecode parse_bytecode_statically(const std::vector& bytecode); static uint32_t get_pc_increment(OpCode opcode); }; diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/errors.hpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/errors.hpp index 271b4fa56f3f..e31d486e502b 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/errors.hpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/errors.hpp @@ -6,7 +6,10 @@ namespace bb::avm_trace { enum class AvmError : uint32_t { NO_ERROR, - TAG_ERROR, + INVALID_PROGRAM_COUNTER, + INVALID_OPCODE, + INVALID_TAG_VALUE, + CHECK_TAG_ERROR, ADDR_RES_TAG_ERROR, REL_ADDR_OUT_OF_RANGE, DIV_ZERO, diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp index 6d685793eb81..5ded3a39fb8f 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp @@ -316,16 +316,22 @@ std::vector Execution::gen_trace(std::vector const& calldata, // Set this also on nested call - // Copied version of pc maintained in trace builder. The value of pc is evolving based - // on opcode logic and therefore is not maintained here. However, the next opcode in the execution - // is determined by this value which require read access to the code below. - uint32_t pc = 0; - uint32_t counter = 0; - AvmError error = AvmError::NO_ERROR; - while (error == AvmError::NO_ERROR && (pc = trace_builder.get_pc()) < bytecode.size()) { - auto inst = Deserialization::parse(bytecode, pc); - debug("[PC:" + std::to_string(pc) + "] [IC:" + std::to_string(counter++) + "] " + inst.to_string() + - " (gasLeft l2=" + std::to_string(trace_builder.get_l2_gas_left()) + ")"); + // Copied version of pc maintained in trace builder. The value of pc is evolving based + // on opcode logic and therefore is not maintained here. However, the next opcode in the execution + // is determined by this value which require read access to the code below. + uint32_t pc = 0; + uint32_t counter = 0; + AvmError error = AvmError::NO_ERROR; + while (is_ok(error) && (pc = trace_builder.get_pc()) < bytecode.size()) { + auto [inst, parse_error] = Deserialization::parse(bytecode, pc); + error = parse_error; + + if (!is_ok(error)) { + break; + } + + debug("[PC:" + std::to_string(pc) + "] [IC:" + std::to_string(counter++) + "] " + inst.to_string() + + " (gasLeft l2=" + std::to_string(trace_builder.get_l2_gas_left()) + ")"); switch (inst.op_code) { // Compute @@ -818,14 +824,13 @@ std::vector Execution::gen_trace(std::vector const& calldata, } } - if (error != AvmError::NO_ERROR) { - info("AVM stopped due to exceptional halting condition. Error: ", - to_name(error), - " at PC: ", - pc, - " IC: ", - counter - 1); // Need adjustement as counter increment occurs in loop body - } + if (!is_ok(error)) { + info("AVM stopped due to exceptional halting condition. Error: ", + to_name(error), + " at PC: ", + pc, + " IC: ", + counter - 1); // Need adjustement as counter increment occurs in loop body } auto trace = trace_builder.finalize(); diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/helper.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/helper.cpp index 936daf0f0ed1..e40a90129d54 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/helper.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/helper.cpp @@ -105,8 +105,14 @@ std::string to_name(AvmError error) switch (error) { case AvmError::NO_ERROR: return "NO ERROR"; - case AvmError::TAG_ERROR: - return "TAG ERROR"; + case AvmError::INVALID_PROGRAM_COUNTER: + return "INVALID PROGRAM COUNTER"; + case AvmError::INVALID_OPCODE: + return "INVALIE OPCODE"; + case AvmError::INVALID_TAG_VALUE: + return "INVALID TAG VALUE"; + case AvmError::CHECK_TAG_ERROR: + return "TAG CHECKING ERROR"; case AvmError::ADDR_RES_TAG_ERROR: return "ADDRESS RESOLUTION TAG ERROR"; case AvmError::REL_ADDR_OUT_OF_RANGE: diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/instructions.hpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/instructions.hpp index 5c003ece9698..252265ea582e 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/instructions.hpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/instructions.hpp @@ -2,6 +2,7 @@ #include "barretenberg/numeric/uint128/uint128.hpp" #include "barretenberg/vm/avm/trace/common.hpp" +#include "barretenberg/vm/avm/trace/errors.hpp" #include "barretenberg/vm/avm/trace/opcode.hpp" #include @@ -50,4 +51,9 @@ class Instruction { } }; +struct InstructionWithError { + Instruction instruction; + AvmError error; +}; + } // namespace bb::avm_trace diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp index a88951779e28..1694ced6c0d2 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp @@ -353,7 +353,7 @@ AvmError AvmTraceBuilder::op_add( bool tag_match = read_a.tag_match && read_b.tag_match; if (is_ok(error) && !tag_match) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } // a + b = c @@ -436,7 +436,7 @@ AvmError AvmTraceBuilder::op_sub( bool tag_match = read_a.tag_match && read_b.tag_match; if (is_ok(error) && !tag_match) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } // a - b = c @@ -517,7 +517,7 @@ AvmError AvmTraceBuilder::op_mul( bool tag_match = read_a.tag_match && read_b.tag_match; if (is_ok(error) && !tag_match) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } // a * b = c @@ -599,7 +599,7 @@ AvmError AvmTraceBuilder::op_div( // No need to add check_tag_integral(read_b.tag) as this follows from tag matching and that a has integral tag. if (is_ok(error) && !(tag_match && check_tag_integral(read_a.tag))) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } // a / b = c @@ -695,7 +695,7 @@ AvmError AvmTraceBuilder::op_fdiv( bool tag_match = read_a.tag_match && read_b.tag_match; if (is_ok(error) && !tag_match) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } // a * b^(-1) = c @@ -789,7 +789,7 @@ AvmError AvmTraceBuilder::op_eq( bool tag_match = read_a.tag_match && read_b.tag_match; if (is_ok(error) && !tag_match) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } FF a = read_a.val; @@ -860,7 +860,7 @@ AvmError AvmTraceBuilder::op_lt( bool tag_match = read_a.tag_match && read_b.tag_match; if (is_ok(error) && !tag_match) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } FF a = tag_match ? read_a.val : FF(0); @@ -929,7 +929,7 @@ AvmError AvmTraceBuilder::op_lte( bool tag_match = read_a.tag_match && read_b.tag_match; if (is_ok(error) && !tag_match) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } FF a = tag_match ? read_a.val : FF(0); @@ -1003,7 +1003,7 @@ AvmError AvmTraceBuilder::op_and( bool tag_match = read_a.tag_match && read_b.tag_match; // No need to add check_tag_integral(read_b.tag) as this follows from tag matching and that a has integral tag. if (is_ok(error) && !(tag_match && check_tag_integral(read_a.tag))) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } FF a = tag_match ? read_a.val : FF(0); @@ -1073,7 +1073,7 @@ AvmError AvmTraceBuilder::op_or( bool tag_match = read_a.tag_match && read_b.tag_match; // No need to add check_tag_integral(read_b.tag) as this follows from tag matching and that a has integral tag. if (is_ok(error) && !(tag_match && check_tag_integral(read_a.tag))) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } FF a = tag_match ? read_a.val : FF(0); @@ -1143,7 +1143,7 @@ AvmError AvmTraceBuilder::op_xor( bool tag_match = read_a.tag_match && read_b.tag_match; // No need to add check_tag_integral(read_b.tag) as this follows from tag matching and that a has integral tag. if (is_ok(error) && !(tag_match && check_tag_integral(read_a.tag))) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } FF a = tag_match ? read_a.val : FF(0); @@ -1217,7 +1217,7 @@ AvmError AvmTraceBuilder::op_not(uint8_t indirect, uint32_t a_offset, uint32_t d auto read_a = constrained_read_from_memory(call_ptr, clk, resolved_a, in_tag, in_tag, IntermRegister::IA); if (is_ok(error) && !check_tag_integral(read_a.tag)) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } // ~a = c @@ -1284,7 +1284,7 @@ AvmError AvmTraceBuilder::op_shl( auto read_b = unconstrained_read_from_memory(resolved_b); if (is_ok(error) && !(check_tag_integral(read_a.tag) && check_tag(AvmMemoryTag::U8, resolved_b))) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } FF a = is_ok(error) ? read_a.val : FF(0); @@ -1351,7 +1351,7 @@ AvmError AvmTraceBuilder::op_shr( // IntermRegister::IB); bool tag_match = read_a.tag_match && read_b.tag_match; auto read_b = unconstrained_read_from_memory(resolved_b); if (is_ok(error) && !(check_tag_integral(read_a.tag) && check_tag(AvmMemoryTag::U8, resolved_b))) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } FF a = is_ok(error) ? read_a.val : FF(0); @@ -1773,7 +1773,7 @@ AvmError AvmTraceBuilder::op_calldata_copy(uint8_t indirect, bool tag_match = true; if (is_ok(error) && !(check_tag(AvmMemoryTag::U32, cd_offset_resolved) && check_tag(AvmMemoryTag::U32, copy_size_offset_resolved))) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } // TODO: constrain these. @@ -1823,7 +1823,7 @@ AvmError AvmTraceBuilder::op_returndata_size(uint8_t indirect, uint32_t dst_offs error = res_error; if (is_ok(error) && !tag_match) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } FF returndata_size = tag_match ? FF(nested_returndata.size()) : FF(0); @@ -1867,7 +1867,7 @@ AvmError AvmTraceBuilder::op_returndata_copy(uint8_t indirect, bool tag_match = true; if (is_ok(error) && !(check_tag(AvmMemoryTag::U32, rd_offset_resolved) && check_tag(AvmMemoryTag::U32, copy_size_offset_resolved))) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } // TODO: constrain these. @@ -2024,7 +2024,7 @@ AvmError AvmTraceBuilder::op_jumpi(uint8_t indirect, uint32_t cond_offset, uint3 error = res_error; if (is_ok(error) && !tag_match) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } // Specific JUMPI loading of conditional value into intermediate register id without any tag constraint. @@ -2184,7 +2184,7 @@ AvmError AvmTraceBuilder::op_set( call_ptr, clk, resolved_dst_offset, val, AvmMemoryTag::FF, in_tag, IntermRegister::IC); if (is_ok(error) && !write_c.tag_match) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } // Constrain gas cost @@ -2240,7 +2240,7 @@ AvmError AvmTraceBuilder::op_mov(uint8_t indirect, uint32_t src_offset, uint32_t error = res_error; if (is_ok(error) && !tag_match) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } // Reading from memory and loading into ia without tag check. @@ -2305,7 +2305,7 @@ RowWithError AvmTraceBuilder::create_kernel_output_opcode(uint8_t indirect, uint call_ptr, clk, resolved_data, AvmMemoryTag::FF, AvmMemoryTag::FF, IntermRegister::IA); bool tag_match = read_a.tag_match; if (is_ok(error) && !tag_match) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } return RowWithError{ .row = @@ -2360,7 +2360,7 @@ RowWithError AvmTraceBuilder::create_kernel_output_opcode_with_metadata(uint8_t bool tag_match = read_a.tag_match && read_b.tag_match; if (is_ok(error) && !tag_match) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } return RowWithError{ .row = @@ -2507,7 +2507,7 @@ RowWithError AvmTraceBuilder::create_kernel_output_opcode_with_set_value_from_hi bool tag_match = write_a.tag_match && read_b.tag_match; if (is_ok(error) && !tag_match) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } return RowWithError{ .row = @@ -2685,7 +2685,7 @@ AvmError AvmTraceBuilder::op_note_hash_exists(uint8_t indirect, error = res_error; if (is_ok(error) && !check_tag(AvmMemoryTag::FF, resolved_leaf_index)) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } Row row; @@ -2742,7 +2742,7 @@ AvmError AvmTraceBuilder::op_note_hash_exists(uint8_t indirect, row.main_sel_op_note_hash_exists = FF(1); if (is_ok(error) && row.main_tag_err != FF(0)) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } } else { row = Row{ @@ -2814,7 +2814,7 @@ AvmError AvmTraceBuilder::op_nullifier_exists(uint8_t indirect, error = res_error; if (is_ok(error) && !check_tag(AvmMemoryTag::FF, resolved_address)) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } Row row; @@ -2881,7 +2881,7 @@ AvmError AvmTraceBuilder::op_nullifier_exists(uint8_t indirect, // clk, resolved_nullifier_offset, resolved_address, resolved_dest); row.main_sel_op_nullifier_exists = FF(1); if (is_ok(error) && row.main_tag_err != FF(0)) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } } else { row = Row{ @@ -2977,7 +2977,7 @@ AvmError AvmTraceBuilder::op_l1_to_l2_msg_exists(uint8_t indirect, const auto leaf_index = unconstrained_read_from_memory(resolved_leaf_index); if (is_ok(error) && !check_tag(AvmMemoryTag::FF, resolved_leaf_index)) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } Row row; @@ -3035,7 +3035,7 @@ AvmError AvmTraceBuilder::op_l1_to_l2_msg_exists(uint8_t indirect, row.main_sel_op_l1_to_l2_msg_exists = FF(1); if (is_ok(error) && row.main_tag_err != FF(0)) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } } else { row = Row{ @@ -3094,7 +3094,7 @@ AvmError AvmTraceBuilder::op_get_contract_instance( call_ptr, clk, resolved_address_offset, AvmMemoryTag::FF, AvmMemoryTag::FF, IntermRegister::IA); bool tag_match = read_address.tag_match; if (is_ok(error) && !tag_match) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } // Read the contract instance @@ -3196,7 +3196,7 @@ AvmError AvmTraceBuilder::op_emit_unencrypted_log(uint8_t indirect, uint32_t log if (is_ok(error) && !(check_tag(AvmMemoryTag::FF, resolved_log_offset) && check_tag(AvmMemoryTag::U32, resolved_log_size_offset))) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } Row row; @@ -3217,7 +3217,7 @@ AvmError AvmTraceBuilder::op_emit_unencrypted_log(uint8_t indirect, uint32_t log direct_field_addr = AddressWithMode(static_cast(resolved_log_offset)); if (!check_tag_range(AvmMemoryTag::FF, direct_field_addr, log_size)) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; }; } @@ -3337,7 +3337,7 @@ AvmError AvmTraceBuilder::constrain_external_call(OpCode opcode, bool tag_match = read_gas_l2.tag_match && read_gas_da.tag_match && read_addr.tag_match && read_args.tag_match; if (is_ok(error) && !(tag_match && check_tag(AvmMemoryTag::U32, resolved_args_size_offset))) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } // TODO: constrain this @@ -3476,7 +3476,7 @@ ReturnDataError AvmTraceBuilder::op_return(uint8_t indirect, uint32_t ret_offset error = res_error; if (is_ok(error) && !(tag_match && check_tag(AvmMemoryTag::U32, resolved_ret_size_offset))) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } const auto ret_size = static_cast(unconstrained_read_from_memory(resolved_ret_size_offset)); @@ -3550,7 +3550,7 @@ ReturnDataError AvmTraceBuilder::op_revert(uint8_t indirect, uint32_t ret_offset error = res_error; if (is_ok(error) && !(tag_match && check_tag(AvmMemoryTag::U32, ret_size_offset))) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } const auto ret_size = @@ -3631,7 +3631,7 @@ AvmError AvmTraceBuilder::op_debug_log(uint8_t indirect, error = res_error; if (is_ok(error) && !check_tag(AvmMemoryTag::U32, resolved_fields_size_offset)) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } const uint32_t fields_size = @@ -3642,7 +3642,7 @@ AvmError AvmTraceBuilder::op_debug_log(uint8_t indirect, if (is_ok(error) && !(check_tag_range(AvmMemoryTag::U8, resolved_message_offset, message_size) && check_tag_range(AvmMemoryTag::FF, resolved_fields_offset, fields_size))) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } main_trace.push_back(Row{ @@ -3726,7 +3726,7 @@ AvmError AvmTraceBuilder::op_poseidon2_permutation(uint8_t indirect, uint32_t in bool read_tag_valid = read_a.tag_match && read_b.tag_match && read_c.tag_match && read_d.tag_match; if (is_ok(error) && !read_tag_valid) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } if (is_ok(error)) { @@ -3776,7 +3776,7 @@ AvmError AvmTraceBuilder::op_poseidon2_permutation(uint8_t indirect, uint32_t in bool write_tag_valid = write_a.tag_match && write_b.tag_match && write_c.tag_match && write_d.tag_match; if (is_ok(error) && !write_tag_valid) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } } @@ -3836,7 +3836,7 @@ AvmError AvmTraceBuilder::op_sha256_compression(uint8_t indirect, if (is_ok(error) && !(check_tag_range(AvmMemoryTag::U32, resolved_state_offset, STATE_SIZE) && check_tag_range(AvmMemoryTag::U32, resolved_inputs_offset, INPUTS_SIZE))) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } // Constrain gas cost @@ -3935,7 +3935,7 @@ AvmError AvmTraceBuilder::op_keccakf1600(uint8_t indirect, uint32_t output_offse if (is_ok(error) && !(tag_match && check_tag_range(AvmMemoryTag::U64, resolved_input_offset, KECCAKF1600_INPUT_SIZE))) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } // Constrain gas cost @@ -4020,7 +4020,7 @@ AvmError AvmTraceBuilder::op_ec_add(uint16_t indirect, check_tag(AvmMemoryTag::FF, resolved_rhs_y_offset) && check_tag(AvmMemoryTag::U1, resolved_rhs_is_inf_offset); if (is_ok(error) && !tags_valid) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } gas_trace_builder.constrain_gas(clk, OpCode::ECADD); @@ -4092,7 +4092,7 @@ AvmError AvmTraceBuilder::op_variable_msm(uint8_t indirect, error = res_error; if (is_ok(error) && !check_tag(AvmMemoryTag::U32, resolved_point_length_offset)) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } const FF points_length = is_ok(error) ? unconstrained_read_from_memory(resolved_point_length_offset) : 0; @@ -4116,7 +4116,7 @@ AvmError AvmTraceBuilder::op_variable_msm(uint8_t indirect, tags_valid = tags_valid && check_tag_range(AvmMemoryTag::FF, resolved_scalars_offset, scalar_read_length); if (is_ok(error) && !tags_valid) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } // TODO(dbanks12): length needs to fit into u32 here or it will certainly @@ -4245,7 +4245,7 @@ AvmError AvmTraceBuilder::op_to_radix_be(uint8_t indirect, // call_ptr, clk, resolved_radix_offset, AvmMemoryTag::U32, AvmMemoryTag::U32, IntermRegister::IB); if (is_ok(error) && !check_tag(AvmMemoryTag::U32, resolved_radix_offset)) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } auto read_radix = unconstrained_read_from_memory(resolved_radix_offset); @@ -4253,7 +4253,7 @@ AvmError AvmTraceBuilder::op_to_radix_be(uint8_t indirect, FF input = read_src.val; if (is_ok(error) && !read_src.tag_match) { - error = AvmError::TAG_ERROR; + error = AvmError::CHECK_TAG_ERROR; } // TODO(8603): uncomment diff --git a/yarn-project/simulator/src/avm/avm_memory_types.ts b/yarn-project/simulator/src/avm/avm_memory_types.ts index 96420655547d..3acd31600830 100644 --- a/yarn-project/simulator/src/avm/avm_memory_types.ts +++ b/yarn-project/simulator/src/avm/avm_memory_types.ts @@ -15,7 +15,7 @@ import { type FunctionsOf } from '@aztec/foundation/types'; import { strict as assert } from 'assert'; -import { InstructionExecutionError, TagCheckError } from './errors.js'; +import { InstructionExecutionError, InvalidTagValueError, TagCheckError } from './errors.js'; import { Addressing, AddressingMode } from './opcodes/addressing_mode.js'; /** MemoryValue gathers the common operations for all memory types. */ @@ -241,6 +241,10 @@ export class TaggedMemory implements TaggedMemoryInterface { this._mem = []; } + public getMaxMemorySize(): number { + return TaggedMemory.MAX_MEMORY_SIZE; + } + /** Returns a MeteredTaggedMemory instance to track the number of reads and writes if TRACK_MEMORY_ACCESSES is set. */ public track(type: string = 'instruction'): TaggedMemoryInterface { return TaggedMemory.TRACK_MEMORY_ACCESSES ? new MeteredTaggedMemory(this, type) : this; @@ -327,6 +331,22 @@ export class TaggedMemory implements TaggedMemoryInterface { } } + public static checkIsValidTag(tagNumber: number) { + if ( + ![ + TypeTag.UINT1, + TypeTag.UINT8, + TypeTag.UINT16, + TypeTag.UINT32, + TypeTag.UINT64, + TypeTag.UINT128, + TypeTag.FIELD, + ].includes(tagNumber) + ) { + throw new InvalidTagValueError(tagNumber); + } + } + /** * Check tags for memory at all of the specified offsets. */ @@ -400,29 +420,7 @@ export class TaggedMemory implements TaggedMemoryInterface { case TypeTag.UINT128: return new Uint128(v & ((1n << 128n) - 1n)); default: - throw new Error(`${TypeTag[tag]} is not a valid tag.`); - } - } - - // Does not truncate. Type constructor will check that it fits. - public static buildFromTagOrDie(v: bigint | number, tag: TypeTag): MemoryValue { - switch (tag) { - case TypeTag.FIELD: - return new Field(v); - case TypeTag.UINT1: - return new Uint1(v); - case TypeTag.UINT8: - return new Uint8(v); - case TypeTag.UINT16: - return new Uint16(v); - case TypeTag.UINT32: - return new Uint32(v); - case TypeTag.UINT64: - return new Uint64(v); - case TypeTag.UINT128: - return new Uint128(v); - default: - throw new Error(`${TypeTag[tag]} is not a valid integral type.`); + throw new InvalidTagValueError(tag); } } @@ -471,6 +469,10 @@ export class MeteredTaggedMemory implements TaggedMemoryInterface { } } + public getMaxMemorySize(): number { + return this.wrapped.getMaxMemorySize(); + } + public track(type: string = 'instruction'): MeteredTaggedMemory { return new MeteredTaggedMemory(this.wrapped, type); } diff --git a/yarn-project/simulator/src/avm/avm_simulator.ts b/yarn-project/simulator/src/avm/avm_simulator.ts index d6def2e96fcb..643fae72da0c 100644 --- a/yarn-project/simulator/src/avm/avm_simulator.ts +++ b/yarn-project/simulator/src/avm/avm_simulator.ts @@ -141,11 +141,6 @@ export class AvmSimulator { let instrCounter = 0; while (!machineState.getHalted()) { const [instruction, bytesRead] = decodeInstructionFromBytecode(bytecode, machineState.pc, this.instructionSet); - assert( - !!instruction, - 'AVM attempted to execute non-existent instruction. This should never happen (invalid bytecode or AVM simulator bug)!', - ); - const instrStartGas = machineState.gasLeft; // Save gas before executing instruction (for profiling) const instrPc = machineState.pc; // Save PC before executing instruction (for profiling) diff --git a/yarn-project/simulator/src/avm/errors.ts b/yarn-project/simulator/src/avm/errors.ts index 8df7a7d73036..b329d7c86e51 100644 --- a/yarn-project/simulator/src/avm/errors.ts +++ b/yarn-project/simulator/src/avm/errors.ts @@ -3,6 +3,7 @@ import { type AztecAddress, Fr, FunctionSelector, PUBLIC_DISPATCH_SELECTOR } fro import { ExecutionError } from '../common/errors.js'; import { type AvmContext } from './avm_context.js'; +import { Opcode } from './serialization/instruction_serialization.js'; /** * Avm-specific errors should derive from this @@ -39,6 +40,37 @@ export class InvalidProgramCounterError extends AvmExecutionError { } } +/** + * Error is thrown when the program counter points to a byte + * of an invalid opcode. + */ +export class InvalidOpcodeError extends AvmExecutionError { + constructor(opcode: Opcode) { + super(`Opcode ${Opcode[opcode]} (0x${opcode.toString(16)}) not implemented`); + this.name = 'InvalidOpcodeError'; + } +} + +/** + * Error is thrown during parsing. + */ +export class AvmParsingError extends AvmExecutionError { + constructor(str: string) { + super(str); + this.name = 'AvmParsingError'; + } +} + +/** + * Error is thrown when the tag has an invalid value. + */ +export class InvalidTagValueError extends AvmExecutionError { + constructor(tagValue: number) { + super(`Tag value ${tagValue} is invalid.`); + this.name = 'InvalidTagValueError'; + } +} + /** * Error thrown during an instruction's execution (during its execute()). */ diff --git a/yarn-project/simulator/src/avm/opcodes/memory.ts b/yarn-project/simulator/src/avm/opcodes/memory.ts index c79d7b7e1af8..1a0c6c03ca3b 100644 --- a/yarn-project/simulator/src/avm/opcodes/memory.ts +++ b/yarn-project/simulator/src/avm/opcodes/memory.ts @@ -59,16 +59,19 @@ export class Set extends Instruction { private value: bigint | number, ) { super(); + TaggedMemory.checkIsValidTag(inTag); } public async execute(context: AvmContext): Promise { + // Constructor ensured that this.inTag is a valid tag + const res = TaggedMemory.buildFromTagTruncating(this.value, this.inTag); + const memory = context.machineState.memory.track(this.type); context.machineState.consumeGas(this.gasCost()); const operands = [this.dstOffset]; const addressing = Addressing.fromWire(this.indirect, operands.length); const [dstOffset] = addressing.resolve(operands, memory); - const res = TaggedMemory.buildFromTagTruncating(this.value, this.inTag); memory.set(dstOffset, res); memory.assert({ writes: 1, addressing }); @@ -96,6 +99,7 @@ export class Cast extends Instruction { constructor(private indirect: number, private srcOffset: number, private dstOffset: number, private dstTag: number) { super(); + TaggedMemory.checkIsValidTag(dstTag); } public async execute(context: AvmContext): Promise { @@ -107,6 +111,7 @@ export class Cast extends Instruction { const [srcOffset, dstOffset] = addressing.resolve(operands, memory); const a = memory.get(srcOffset); + // Constructor ensured that this.dstTag is a valid tag const casted = TaggedMemory.buildFromTagTruncating(a.toBigInt(), this.dstTag); memory.set(dstOffset, casted); diff --git a/yarn-project/simulator/src/avm/serialization/buffer_cursor.ts b/yarn-project/simulator/src/avm/serialization/buffer_cursor.ts index 005a3d6599fd..5beef8143320 100644 --- a/yarn-project/simulator/src/avm/serialization/buffer_cursor.ts +++ b/yarn-project/simulator/src/avm/serialization/buffer_cursor.ts @@ -1,5 +1,3 @@ -import { strict as assert } from 'assert'; - /* * A Buffer-like class that automatically advances the position. */ @@ -24,7 +22,6 @@ export class BufferCursor { public advance(n: number): void { this._position += n; - assert(n < this._buffer.length); } public peekUint8(): number { diff --git a/yarn-project/simulator/src/avm/serialization/bytecode_serialization.ts b/yarn-project/simulator/src/avm/serialization/bytecode_serialization.ts index 27b603664d82..070b476668fd 100644 --- a/yarn-project/simulator/src/avm/serialization/bytecode_serialization.ts +++ b/yarn-project/simulator/src/avm/serialization/bytecode_serialization.ts @@ -1,3 +1,4 @@ +import { AvmParsingError, InvalidOpcodeError, InvalidProgramCounterError } from '../errors.js'; import { Add, And, @@ -179,18 +180,27 @@ export function decodeInstructionFromBytecode( instructionSet: InstructionSet = INSTRUCTION_SET(), ): [Instruction, number] { if (pc >= bytecode.length) { - throw new Error(`pc ${pc} is out of bounds for bytecode of length ${bytecode.length}`); + throw new InvalidProgramCounterError(pc, bytecode.length); } - const cursor = new BufferCursor(bytecode, pc); - const startingPosition = cursor.position(); - const opcode: Opcode = cursor.bufferAtPosition().readUint8(); // peek. - const instructionDeserializerOrUndef = instructionSet.get(opcode); - if (instructionDeserializerOrUndef === undefined) { - throw new Error(`Opcode ${Opcode[opcode]} (0x${opcode.toString(16)}) not implemented`); + try { + const cursor = new BufferCursor(bytecode, pc); + const startingPosition = cursor.position(); + const opcode: Opcode = cursor.bufferAtPosition().readUint8(); // peek. + + const instructionDeserializerOrUndef = instructionSet.get(opcode); + if (instructionDeserializerOrUndef === undefined) { + throw new InvalidOpcodeError(opcode); + } + + const instructionDeserializer: InstructionDeserializer = instructionDeserializerOrUndef; + const instruction = instructionDeserializer(cursor); + return [instruction, cursor.position() - startingPosition]; + } catch (error) { + if (error instanceof InvalidOpcodeError) { + throw error; + } else { + throw new AvmParsingError(`${error}`); + } } - - const instructionDeserializer: InstructionDeserializer = instructionDeserializerOrUndef; - const instruction = instructionDeserializer(cursor); - return [instruction, cursor.position() - startingPosition]; } From 76c68d0f0a37ae4fbb30d07e5a180366d4b3ffd3 Mon Sep 17 00:00:00 2001 From: jeanmon Date: Tue, 26 Nov 2024 16:25:47 +0000 Subject: [PATCH 2/4] 9770: Addressing review feedback --- yarn-project/simulator/src/avm/errors.ts | 5 +- .../bytecode_serialization.test.ts | 86 ++++++++++++++++++- .../serialization/bytecode_serialization.ts | 16 ++-- .../instruction_serialization.ts | 3 + 4 files changed, 101 insertions(+), 9 deletions(-) diff --git a/yarn-project/simulator/src/avm/errors.ts b/yarn-project/simulator/src/avm/errors.ts index b329d7c86e51..a147aefe922a 100644 --- a/yarn-project/simulator/src/avm/errors.ts +++ b/yarn-project/simulator/src/avm/errors.ts @@ -3,7 +3,6 @@ import { type AztecAddress, Fr, FunctionSelector, PUBLIC_DISPATCH_SELECTOR } fro import { ExecutionError } from '../common/errors.js'; import { type AvmContext } from './avm_context.js'; -import { Opcode } from './serialization/instruction_serialization.js'; /** * Avm-specific errors should derive from this @@ -45,8 +44,8 @@ export class InvalidProgramCounterError extends AvmExecutionError { * of an invalid opcode. */ export class InvalidOpcodeError extends AvmExecutionError { - constructor(opcode: Opcode) { - super(`Opcode ${Opcode[opcode]} (0x${opcode.toString(16)}) not implemented`); + constructor(str: string) { + super(str); this.name = 'InvalidOpcodeError'; } } diff --git a/yarn-project/simulator/src/avm/serialization/bytecode_serialization.test.ts b/yarn-project/simulator/src/avm/serialization/bytecode_serialization.test.ts index ce451ff92b0b..300df1377692 100644 --- a/yarn-project/simulator/src/avm/serialization/bytecode_serialization.test.ts +++ b/yarn-project/simulator/src/avm/serialization/bytecode_serialization.test.ts @@ -1,9 +1,10 @@ import { strict as assert } from 'assert'; +import { AvmParsingError, InvalidOpcodeError, InvalidTagValueError } from '../errors.js'; import { Add, Call, EnvironmentVariable, GetEnvVar, StaticCall, Sub } from '../opcodes/index.js'; import { type BufferCursor } from './buffer_cursor.js'; import { type InstructionSet, decodeFromBytecode, encodeToBytecode } from './bytecode_serialization.js'; -import { Opcode } from './instruction_serialization.js'; +import { MAX_OPCODE_VALUE, Opcode } from './instruction_serialization.js'; class InstA { constructor(private n: number) {} @@ -133,4 +134,87 @@ describe('Bytecode Serialization', () => { const expected = Buffer.concat(instructions.map(i => i.serialize())); expect(actual).toEqual(expected); }); + + it('Should throw an InvalidOpcodeError while deserializing an out-of-range opcode value', () => { + const decodeInvalid = () => { + const wrongOpcode: number = MAX_OPCODE_VALUE + 1; + const buf = Buffer.alloc(1); + buf.writeUint8(wrongOpcode); + decodeFromBytecode(buf); + }; + + expect(decodeInvalid).toThrow(InvalidOpcodeError); + }); + + it('Should throw an InvalidOpcodeError while deserializing an opcode value not in instruction set', () => { + const decodeInvalid = () => { + const instructionSet: InstructionSet = new Map([ + [InstA.opcode, InstA.deserialize], + [InstB.opcode, InstB.deserialize], + ]); + const buf = Buffer.alloc(1); + buf.writeUint8(Opcode.AND_8); // Valid opcode but not in supplied instruction set. + decodeFromBytecode(buf, instructionSet); + }; + + expect(decodeInvalid).toThrow(InvalidOpcodeError); + }); + + it('Should throw an AvmParsingError while deserializing an incomplete instruction', () => { + const decodeIncomplete = (truncated: Buffer) => { + return () => decodeFromBytecode(truncated); + }; + + const instructions = [ + new Call( + /*indirect=*/ 0x01, + /*gasOffset=*/ 0x1234, + /*addrOffset=*/ 0xa234, + /*argsOffset=*/ 0xb234, + /*argsSize=*/ 0xc234, + /*successOffset=*/ 0xf234, + ), + ]; + + const bytecode = encodeToBytecode(instructions); + + for (let i = 1; i < 7; i++) { + const truncated = bytecode.subarray(0, bytecode.length - i); + expect(decodeIncomplete(truncated)).toThrow(AvmParsingError); + } + }); + + it('Should throw an InvalidTagValueError while deserializing a tag value out of range', () => { + const decodeInvalidTag = (buf: Buffer) => { + return () => decodeFromBytecode(buf); + }; + + const bufCast8 = Buffer.from([ + Opcode.CAST_8, // opcode + 0x01, // indirect + 0x10, // aOffset + 0x32, // dstOffset + 0x12, // dstTag (invalid tag) + ]); + + const bufCast16 = Buffer.from([ + Opcode.CAST_16, // opcode + 0x00, // indirect + ...Buffer.from('1234', 'hex'), // aOffset + ...Buffer.from('3456', 'hex'), // dstOffset + 0x65, // dstTag (invalid tag) + ]); + + const bufSet16 = Buffer.from([ + Opcode.SET_16, //opcode + 0x02, // indirect + ...Buffer.from('3456', 'hex'), // dstOffset + 0x21, //tag (invalid) + ...Buffer.from('2397', 'hex'), // value + ]); + + for (const buf of [bufCast8, bufCast16, bufSet16]) { + expect(decodeInvalidTag(buf)).toThrow(InvalidTagValueError); + } + }); }); diff --git a/yarn-project/simulator/src/avm/serialization/bytecode_serialization.ts b/yarn-project/simulator/src/avm/serialization/bytecode_serialization.ts index 070b476668fd..aaade1ead237 100644 --- a/yarn-project/simulator/src/avm/serialization/bytecode_serialization.ts +++ b/yarn-project/simulator/src/avm/serialization/bytecode_serialization.ts @@ -1,4 +1,4 @@ -import { AvmParsingError, InvalidOpcodeError, InvalidProgramCounterError } from '../errors.js'; +import { AvmExecutionError, AvmParsingError, InvalidOpcodeError, InvalidProgramCounterError } from '../errors.js'; import { Add, And, @@ -49,7 +49,7 @@ import { } from '../opcodes/index.js'; import { MultiScalarMul } from '../opcodes/multi_scalar_mul.js'; import { BufferCursor } from './buffer_cursor.js'; -import { Opcode } from './instruction_serialization.js'; +import { MAX_OPCODE_VALUE, Opcode } from './instruction_serialization.js'; export type InstructionDeserializer = (buf: BufferCursor | Buffer) => Instruction; @@ -186,18 +186,24 @@ export function decodeInstructionFromBytecode( try { const cursor = new BufferCursor(bytecode, pc); const startingPosition = cursor.position(); - const opcode: Opcode = cursor.bufferAtPosition().readUint8(); // peek. + const opcode: number = cursor.bufferAtPosition().readUint8(); // peek. + + if (opcode > MAX_OPCODE_VALUE) { + throw new InvalidOpcodeError( + `Opcode ${opcode} (0x${opcode.toString(16)}) value is not in the range of valid opcodes.`, + ); + } const instructionDeserializerOrUndef = instructionSet.get(opcode); if (instructionDeserializerOrUndef === undefined) { - throw new InvalidOpcodeError(opcode); + throw new InvalidOpcodeError(`Opcode ${Opcode[opcode]} (0x${opcode.toString(16)}) is not implemented`); } const instructionDeserializer: InstructionDeserializer = instructionDeserializerOrUndef; const instruction = instructionDeserializer(cursor); return [instruction, cursor.position() - startingPosition]; } catch (error) { - if (error instanceof InvalidOpcodeError) { + if (error instanceof InvalidOpcodeError || error instanceof AvmExecutionError) { throw error; } else { throw new AvmParsingError(`${error}`); diff --git a/yarn-project/simulator/src/avm/serialization/instruction_serialization.ts b/yarn-project/simulator/src/avm/serialization/instruction_serialization.ts index 20c25b2c9994..fdb917a63c34 100644 --- a/yarn-project/simulator/src/avm/serialization/instruction_serialization.ts +++ b/yarn-project/simulator/src/avm/serialization/instruction_serialization.ts @@ -84,8 +84,11 @@ export enum Opcode { MSM, // Conversion TORADIXBE, + // Adapt MAX_OPCODE_VALUE below if TORADIXBE is not the last opcode anymore } +export const MAX_OPCODE_VALUE: number = Opcode.TORADIXBE; + // Possible types for an instruction's operand in its wire format. (Keep in sync with CPP code. // See vm/avm_trace/deserialization.cpp) // Note that cpp code introduced an additional enum value TAG to express the instruction tag. In TS, From e4c226111cade0a7b94ad7def875c1df6370b584 Mon Sep 17 00:00:00 2001 From: jeanmon Date: Wed, 27 Nov 2024 10:46:52 +0000 Subject: [PATCH 3/4] 9770: rebase on master merge conflicts resolution --- .../barretenberg/vm/avm/trace/execution.cpp | 46 +++++++++---------- .../src/barretenberg/vm/avm/trace/trace.cpp | 18 +++++++- 2 files changed, 39 insertions(+), 25 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp index 5ded3a39fb8f..828b46a01e3c 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp @@ -316,22 +316,22 @@ std::vector Execution::gen_trace(std::vector const& calldata, // Set this also on nested call - // Copied version of pc maintained in trace builder. The value of pc is evolving based - // on opcode logic and therefore is not maintained here. However, the next opcode in the execution - // is determined by this value which require read access to the code below. - uint32_t pc = 0; - uint32_t counter = 0; - AvmError error = AvmError::NO_ERROR; - while (is_ok(error) && (pc = trace_builder.get_pc()) < bytecode.size()) { - auto [inst, parse_error] = Deserialization::parse(bytecode, pc); - error = parse_error; - - if (!is_ok(error)) { - break; - } + // Copied version of pc maintained in trace builder. The value of pc is evolving based + // on opcode logic and therefore is not maintained here. However, the next opcode in the execution + // is determined by this value which require read access to the code below. + uint32_t pc = 0; + uint32_t counter = 0; + AvmError error = AvmError::NO_ERROR; + while (is_ok(error) && (pc = trace_builder.get_pc()) < bytecode.size()) { + auto [inst, parse_error] = Deserialization::parse(bytecode, pc); + error = parse_error; + + if (!is_ok(error)) { + break; + } - debug("[PC:" + std::to_string(pc) + "] [IC:" + std::to_string(counter++) + "] " + inst.to_string() + - " (gasLeft l2=" + std::to_string(trace_builder.get_l2_gas_left()) + ")"); + debug("[PC:" + std::to_string(pc) + "] [IC:" + std::to_string(counter++) + "] " + inst.to_string() + + " (gasLeft l2=" + std::to_string(trace_builder.get_l2_gas_left()) + ")"); switch (inst.op_code) { // Compute @@ -824,15 +824,15 @@ std::vector Execution::gen_trace(std::vector const& calldata, } } - if (!is_ok(error)) { - info("AVM stopped due to exceptional halting condition. Error: ", - to_name(error), - " at PC: ", - pc, - " IC: ", - counter - 1); // Need adjustement as counter increment occurs in loop body + if (!is_ok(error)) { + info("AVM stopped due to exceptional halting condition. Error: ", + to_name(error), + " at PC: ", + pc, + " IC: ", + counter - 1); // Need adjustement as counter increment occurs in loop body + } } - auto trace = trace_builder.finalize(); show_trace_info(trace); diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp index 1694ced6c0d2..ecd740ca9bae 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp @@ -2544,9 +2544,12 @@ AvmError AvmTraceBuilder::op_sload(uint8_t indirect, uint32_t slot_offset, uint3 { auto clk = static_cast(main_trace.size()) + 1; + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto [resolved_addrs, res_error] = Addressing<2>::fromWire(indirect, call_ptr).resolve({ slot_offset, dest_offset }, mem_trace_builder); auto [resolved_slot, resolved_dest] = resolved_addrs; + error = res_error; auto read_slot = unconstrained_read_from_memory(resolved_slot); // TODO(https://github.com/AztecProtocol/aztec-packages/issues/7960): Until this is moved @@ -2571,6 +2574,10 @@ AvmError AvmTraceBuilder::op_sload(uint8_t indirect, uint32_t slot_offset, uint3 auto write_a = constrained_write_to_memory( call_ptr, clk, resolved_dest, value, AvmMemoryTag::FF, AvmMemoryTag::FF, IntermRegister::IA); + if (is_ok(error) && !write_a.tag_match) { + error = AvmError::CHECK_TAG_ERROR; + } + // TODO(8945): remove fake rows auto row = Row{ .main_clk = clk, @@ -2600,7 +2607,7 @@ AvmError AvmTraceBuilder::op_sload(uint8_t indirect, uint32_t slot_offset, uint3 clk++; pc += Deserialization::get_pc_increment(OpCode::SLOAD); - return write_a.tag_match ? AvmError::NO_ERROR : AvmError::TAG_ERROR; + return error; } AvmError AvmTraceBuilder::op_sstore(uint8_t indirect, uint32_t src_offset, uint32_t slot_offset) @@ -2608,9 +2615,12 @@ AvmError AvmTraceBuilder::op_sstore(uint8_t indirect, uint32_t src_offset, uint3 // We keep the first encountered error auto clk = static_cast(main_trace.size()) + 1; + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto [resolved_addrs, res_error] = Addressing<2>::fromWire(indirect, call_ptr).resolve({ src_offset, slot_offset }, mem_trace_builder); auto [resolved_src, resolved_slot] = resolved_addrs; + error = res_error; auto read_slot = unconstrained_read_from_memory(resolved_slot); // TODO(https://github.com/AztecProtocol/aztec-packages/issues/7960): Until this is moved @@ -2620,6 +2630,10 @@ AvmError AvmTraceBuilder::op_sstore(uint8_t indirect, uint32_t src_offset, uint3 auto read_a = constrained_read_from_memory( call_ptr, clk, resolved_src, AvmMemoryTag::FF, AvmMemoryTag::FF, IntermRegister::IA); + if (is_ok(error) && !read_a.tag_match) { + error = AvmError::CHECK_TAG_ERROR; + } + // Merkle check for SSTORE // (a) We compute the tree leaf slot of the low nullifier // (b) We check the membership of the low nullifier in the public data tree @@ -2666,7 +2680,7 @@ AvmError AvmTraceBuilder::op_sstore(uint8_t indirect, uint32_t src_offset, uint3 side_effect_counter++; clk++; pc += Deserialization::get_pc_increment(OpCode::SSTORE); - return read_a.tag_match ? AvmError::NO_ERROR : AvmError::TAG_ERROR; + return error; } AvmError AvmTraceBuilder::op_note_hash_exists(uint8_t indirect, From 3d1a27e525edc265358a5c091348a148b95dd3c3 Mon Sep 17 00:00:00 2001 From: jeanmon Date: Wed, 27 Nov 2024 10:59:51 +0000 Subject: [PATCH 4/4] 9770: addressing review comments --- .../src/avm/serialization/bytecode_serialization.test.ts | 2 +- .../src/avm/serialization/instruction_serialization.ts | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/yarn-project/simulator/src/avm/serialization/bytecode_serialization.test.ts b/yarn-project/simulator/src/avm/serialization/bytecode_serialization.test.ts index 300df1377692..ac84cfa19dd6 100644 --- a/yarn-project/simulator/src/avm/serialization/bytecode_serialization.test.ts +++ b/yarn-project/simulator/src/avm/serialization/bytecode_serialization.test.ts @@ -178,7 +178,7 @@ describe('Bytecode Serialization', () => { const bytecode = encodeToBytecode(instructions); - for (let i = 1; i < 7; i++) { + for (let i = 1; i < bytecode.length; i++) { const truncated = bytecode.subarray(0, bytecode.length - i); expect(decodeIncomplete(truncated)).toThrow(AvmParsingError); } diff --git a/yarn-project/simulator/src/avm/serialization/instruction_serialization.ts b/yarn-project/simulator/src/avm/serialization/instruction_serialization.ts index fdb917a63c34..50d6a4e2a56f 100644 --- a/yarn-project/simulator/src/avm/serialization/instruction_serialization.ts +++ b/yarn-project/simulator/src/avm/serialization/instruction_serialization.ts @@ -84,10 +84,13 @@ export enum Opcode { MSM, // Conversion TORADIXBE, - // Adapt MAX_OPCODE_VALUE below if TORADIXBE is not the last opcode anymore } -export const MAX_OPCODE_VALUE: number = Opcode.TORADIXBE; +export const MAX_OPCODE_VALUE = Math.max( + ...Object.values(Opcode) + .map(k => +k) + .filter(k => !isNaN(k)), +); // Possible types for an instruction's operand in its wire format. (Keep in sync with CPP code. // See vm/avm_trace/deserialization.cpp)