From 4c23ef81121d98729e32a62c7293f23925d83669 Mon Sep 17 00:00:00 2001 From: jeanmon Date: Thu, 14 Nov 2024 13:10:35 +0000 Subject: [PATCH 1/5] 9131: Add integral tag checking for DIV opcode --- barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp index 58bd15bd44f7..49c279629256 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp @@ -563,6 +563,8 @@ AvmError AvmTraceBuilder::op_div( auto read_b = constrained_read_from_memory(call_ptr, clk, resolved_b, in_tag, in_tag, IntermRegister::IB); bool tag_match = read_a.tag_match && read_b.tag_match; + bool op_valid = tag_match && check_tag_integral(read_a.tag); + // a / b = c FF a = read_a.val; FF b = read_b.val; @@ -577,7 +579,7 @@ AvmError AvmTraceBuilder::op_div( if (!b.is_zero()) { // If b is not zero, we prove it is not by providing its inverse as well inv = b.invert(); - c = tag_match ? alu_trace_builder.op_div(a, b, in_tag, clk) : FF(0); + c = op_valid ? alu_trace_builder.op_div(a, b, in_tag, clk) : FF(0); } else { inv = 1; c = 0; @@ -605,7 +607,7 @@ AvmError AvmTraceBuilder::op_div( .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_dst.direct_address), - .main_op_err = tag_match ? FF(static_cast(div_error)) : FF(1), + .main_op_err = op_valid ? FF(static_cast(div_error)) : FF(1), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), @@ -622,7 +624,7 @@ AvmError AvmTraceBuilder::op_div( ASSERT(op_code == OpCode::DIV_8 || op_code == OpCode::DIV_16); pc += Deserialization::get_pc_increment(op_code); - return !tag_match ? AvmError::TAG_ERROR : div_error ? AvmError::DIV_ZERO : AvmError::NO_ERROR; + return !op_valid ? AvmError::TAG_ERROR : div_error ? AvmError::DIV_ZERO : AvmError::NO_ERROR; } /** From 1234f667a0cc4a098742f17add16b342d773b7f0 Mon Sep 17 00:00:00 2001 From: jeanmon Date: Fri, 15 Nov 2024 13:37:23 +0000 Subject: [PATCH 2/5] 9131: Add error handling for address resolution --- .../vm/avm/trace/addressing_mode.hpp | 47 +- .../src/barretenberg/vm/avm/trace/common.hpp | 6 +- .../src/barretenberg/vm/avm/trace/helper.cpp | 11 +- .../src/barretenberg/vm/avm/trace/helper.hpp | 1 + .../src/barretenberg/vm/avm/trace/trace.cpp | 1035 +++++++++++------ .../src/barretenberg/vm/avm/trace/trace.hpp | 57 +- yarn-project/simulator/src/avm/errors.ts | 11 + .../src/avm/opcodes/addressing_mode.ts | 8 +- 8 files changed, 798 insertions(+), 378 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/addressing_mode.hpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/addressing_mode.hpp index 876f37c4b3c7..d6f044986da3 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/addressing_mode.hpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/addressing_mode.hpp @@ -1,5 +1,6 @@ #pragma once +#include "barretenberg/vm/avm/trace/common.hpp" #include "barretenberg/vm/avm/trace/mem_trace.hpp" #include @@ -30,6 +31,11 @@ struct AddressWithMode { AddressWithMode operator+(uint val) const noexcept { return { mode, offset + val }; } }; +template struct AddressResolution { + std::array addresses; + AvmError error; +}; + template class Addressing { public: Addressing(const std::array& mode_per_operand, uint8_t space_id) @@ -47,26 +53,45 @@ template class Addressing { return Addressing(modes, space_id); } - std::array resolve(const std::array& offsets, AvmMemTraceBuilder& mem_builder) const + AddressResolution resolve(const std::array& offsets, AvmMemTraceBuilder& mem_builder) const { - std::array resolved; + std::array resolved_addresses; + for (size_t i = 0; i < N; i++) { - resolved[i] = offsets[i]; + auto& res_addr = resolved_addresses[i]; + res_addr = offsets[i]; const auto mode = mode_per_operand[i]; if ((static_cast(mode) & static_cast(AddressingMode::RELATIVE)) != 0) { const auto mem_tag = mem_builder.unconstrained_get_memory_tag(space_id, 0); - // TODO(#9131): Error handling needs to be done - ASSERT(mem_tag == AvmMemoryTag::U32); - resolved[i] += static_cast(mem_builder.unconstrained_read(space_id, 0)); + + if (mem_tag != AvmMemoryTag::U32) { + return AddressResolution{ .addresses = resolved_addresses, + .error = AvmError::ADDR_RES_TAG_ERROR }; + } + + const auto base_addr = static_cast(mem_builder.unconstrained_read(space_id, 0)); + + // Test if we overflow over uint32_t + if (res_addr + base_addr < base_addr) { + return AddressResolution{ .addresses = resolved_addresses, + .error = AvmError::REL_ADDR_OUT_OF_RANGE }; + } + + res_addr += base_addr; } + if ((static_cast(mode) & static_cast(AddressingMode::INDIRECT)) != 0) { - const auto mem_tag = mem_builder.unconstrained_get_memory_tag(space_id, resolved[i]); - // TODO(#9131): Error handling needs to be done - ASSERT(mem_tag == AvmMemoryTag::U32); - resolved[i] = static_cast(mem_builder.unconstrained_read(space_id, resolved[i])); + const auto mem_tag = mem_builder.unconstrained_get_memory_tag(space_id, res_addr); + + if (mem_tag != AvmMemoryTag::U32) { + return AddressResolution{ .addresses = resolved_addresses, + .error = AvmError::ADDR_RES_TAG_ERROR }; + } + + res_addr = static_cast(mem_builder.unconstrained_read(space_id, res_addr)); } } - return resolved; + return AddressResolution{ .addresses = resolved_addresses, .error = AvmError::NO_ERROR }; } private: diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/common.hpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/common.hpp index c8983191ec4b..0d8bc15bbc82 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/common.hpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/common.hpp @@ -52,11 +52,13 @@ static const uint32_t MAX_MEM_TAG = MEM_TAG_U128; enum class AvmError : uint32_t { NO_ERROR, TAG_ERROR, - ADDR_RES_ERROR, + ADDR_RES_TAG_ERROR, + REL_ADDR_OUT_OF_RANGE, DIV_ZERO, PARSING_ERROR, ENV_VAR_UNKNOWN, - CONTRACT_INST_MEM_UNKNOWN + CONTRACT_INST_MEM_UNKNOWN, + RADIX_OUT_OF_BOUNDS, }; static const size_t NUM_MEM_SPACES = 256; diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/helper.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/helper.cpp index 1960cac37b36..196bff2317d5 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/helper.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/helper.cpp @@ -106,8 +106,10 @@ std::string to_name(AvmError error) return "NO ERROR"; case AvmError::TAG_ERROR: return "TAG ERROR"; - case AvmError::ADDR_RES_ERROR: - return "ADDRESS RESOLUTION ERROR"; + case AvmError::ADDR_RES_TAG_ERROR: + return "ADDRESS RESOLUTION TAG ERROR"; + case AvmError::REL_ADDR_OUT_OF_RANGE: + return "RELATIVE ADDRESS IS OUT OF RANGE"; case AvmError::DIV_ZERO: return "DIVISION BY ZERO"; case AvmError::PARSING_ERROR: @@ -122,6 +124,11 @@ std::string to_name(AvmError error) } } +bool is_valid(AvmError error) +{ + return error == AvmError::NO_ERROR; +} + /** * * ONLY FOR TESTS - Required by dsl module and therefore cannot be moved to test/helpers.test.cpp diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/helper.hpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/helper.hpp index cc1976dc5011..1f92386dc660 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/helper.hpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/helper.hpp @@ -233,6 +233,7 @@ std::string to_hex(bb::avm_trace::AvmMemoryTag tag); std::string to_name(bb::avm_trace::AvmMemoryTag tag); std::string to_name(AvmError error); +bool is_valid(AvmError error); // Mutate the inputs void inject_end_gas_values(VmPublicInputs& public_inputs, std::vector& trace); diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp index 49c279629256..10585405acca 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp @@ -329,11 +329,15 @@ AvmTraceBuilder::AvmTraceBuilder(VmPublicInputs public_inputs, AvmError AvmTraceBuilder::op_add( uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code) { + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto clk = static_cast(main_trace.size()) + 1; // Resolve any potential indirects in the order they are encoded in the indirect byte. - auto [resolved_a, resolved_b, resolved_c] = + auto [resolved_addrs, res_error] = Addressing<3>::fromWire(indirect, call_ptr).resolve({ a_offset, b_offset, dst_offset }, mem_trace_builder); + auto [resolved_a, resolved_b, resolved_c] = resolved_addrs; + error = res_error; // We get our representative memory tag from the resolved_a memory address. AvmMemoryTag in_tag = unconstrained_get_memory_tag(resolved_a); @@ -342,6 +346,9 @@ AvmError AvmTraceBuilder::op_add( auto read_b = constrained_read_from_memory(call_ptr, clk, resolved_b, in_tag, in_tag, IntermRegister::IB); bool tag_match = read_a.tag_match && read_b.tag_match; + if (is_valid(error) && !tag_match) { + error = AvmError::TAG_ERROR; + } // a + b = c FF a = read_a.val; @@ -350,7 +357,7 @@ AvmError AvmTraceBuilder::op_add( // In case of a memory tag error, we do not perform the computation. // Therefore, we do not create any entry in ALU table and store the value 0 as // output (c) in memory. - FF c = tag_match ? alu_trace_builder.op_add(a, b, in_tag, clk) : FF(0); + FF c = is_valid(error) ? alu_trace_builder.op_add(a, b, in_tag, clk) : FF(0); // Write into memory value c from intermediate register ic. auto write_c = constrained_write_to_memory(call_ptr, clk, resolved_c, c, in_tag, in_tag, IntermRegister::IC); @@ -372,6 +379,7 @@ AvmError AvmTraceBuilder::op_add( .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), + .main_op_err = FF(static_cast(!is_valid(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), @@ -388,7 +396,7 @@ AvmError AvmTraceBuilder::op_add( ASSERT(op_code == OpCode::ADD_8 || op_code == OpCode::ADD_16); pc += Deserialization::get_pc_increment(op_code); - return tag_match ? AvmError::NO_ERROR : AvmError::TAG_ERROR; + return error; } /** @@ -403,11 +411,16 @@ AvmError AvmTraceBuilder::op_add( AvmError AvmTraceBuilder::op_sub( uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code) { + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto clk = static_cast(main_trace.size()) + 1; // Resolve any potential indirects in the order they are encoded in the indirect byte. - auto [resolved_a, resolved_b, resolved_c] = + + auto [resolved_addrs, res_error] = Addressing<3>::fromWire(indirect, call_ptr).resolve({ a_offset, b_offset, dst_offset }, mem_trace_builder); + auto [resolved_a, resolved_b, resolved_c] = resolved_addrs; + error = res_error; // We get our representative memory tag from the resolved_a memory address. AvmMemoryTag in_tag = unconstrained_get_memory_tag(resolved_a); @@ -416,6 +429,9 @@ AvmError AvmTraceBuilder::op_sub( auto read_b = constrained_read_from_memory(call_ptr, clk, resolved_b, in_tag, in_tag, IntermRegister::IB); bool tag_match = read_a.tag_match && read_b.tag_match; + if (is_valid(error) && !tag_match) { + error = AvmError::TAG_ERROR; + } // a - b = c FF a = read_a.val; @@ -424,7 +440,7 @@ AvmError AvmTraceBuilder::op_sub( // In case of a memory tag error, we do not perform the computation. // Therefore, we do not create any entry in ALU table and store the value 0 as // output (c) in memory. - FF c = tag_match ? alu_trace_builder.op_sub(a, b, in_tag, clk) : FF(0); + FF c = is_valid(error) ? alu_trace_builder.op_sub(a, b, in_tag, clk) : FF(0); // Write into memory value c from intermediate register ic. auto write_c = constrained_write_to_memory(call_ptr, clk, resolved_c, c, in_tag, in_tag, IntermRegister::IC); @@ -446,6 +462,7 @@ AvmError AvmTraceBuilder::op_sub( .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), + .main_op_err = FF(static_cast(!is_valid(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), @@ -462,9 +479,8 @@ AvmError AvmTraceBuilder::op_sub( ASSERT(op_code == OpCode::SUB_8 || op_code == OpCode::SUB_16); pc += Deserialization::get_pc_increment(op_code); - return tag_match ? AvmError::NO_ERROR : AvmError::TAG_ERROR; + return error; } - /** * @brief Multiplication with direct or indirect memory access. * @@ -477,11 +493,15 @@ AvmError AvmTraceBuilder::op_sub( AvmError AvmTraceBuilder::op_mul( uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code) { + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto clk = static_cast(main_trace.size()) + 1; // Resolve any potential indirects in the order they are encoded in the indirect byte. - auto [resolved_a, resolved_b, resolved_c] = + auto [resolved_addrs, res_error] = Addressing<3>::fromWire(indirect, call_ptr).resolve({ a_offset, b_offset, dst_offset }, mem_trace_builder); + auto [resolved_a, resolved_b, resolved_c] = resolved_addrs; + error = res_error; // We get our representative memory tag from the resolved_a memory address. AvmMemoryTag in_tag = unconstrained_get_memory_tag(resolved_a); @@ -490,6 +510,9 @@ AvmError AvmTraceBuilder::op_mul( auto read_b = constrained_read_from_memory(call_ptr, clk, resolved_b, in_tag, in_tag, IntermRegister::IB); bool tag_match = read_a.tag_match && read_b.tag_match; + if (is_valid(error) && !tag_match) { + error = AvmError::TAG_ERROR; + } // a * b = c FF a = read_a.val; @@ -498,7 +521,7 @@ AvmError AvmTraceBuilder::op_mul( // In case of a memory tag error, we do not perform the computation. // Therefore, we do not create any entry in ALU table and store the value 0 as // output (c) in memory. - FF c = tag_match ? alu_trace_builder.op_mul(a, b, in_tag, clk) : FF(0); + FF c = is_valid(error) ? alu_trace_builder.op_mul(a, b, in_tag, clk) : FF(0); // Write into memory value c from intermediate register ic. auto write_c = constrained_write_to_memory(call_ptr, clk, resolved_c, c, in_tag, in_tag, IntermRegister::IC); @@ -520,6 +543,7 @@ AvmError AvmTraceBuilder::op_mul( .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), + .main_op_err = FF(static_cast(!is_valid(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), @@ -536,7 +560,7 @@ AvmError AvmTraceBuilder::op_mul( ASSERT(op_code == OpCode::MUL_8 || op_code == OpCode::MUL_16); pc += Deserialization::get_pc_increment(op_code); - return tag_match ? AvmError::NO_ERROR : AvmError::TAG_ERROR; + return error; } /** @@ -551,10 +575,14 @@ AvmError AvmTraceBuilder::op_mul( AvmError AvmTraceBuilder::op_div( uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code) { + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto clk = static_cast(main_trace.size()) + 1; - auto [resolved_a, resolved_b, resolved_dst] = + auto [resolved_addrs, res_error] = Addressing<3>::fromWire(indirect, call_ptr).resolve({ a_offset, b_offset, dst_offset }, mem_trace_builder); + auto [resolved_a, resolved_b, resolved_dst] = resolved_addrs; + error = res_error; // We get our representative memory tag from the resolved_a memory address. AvmMemoryTag in_tag = unconstrained_get_memory_tag(resolved_a); @@ -563,7 +591,10 @@ AvmError AvmTraceBuilder::op_div( auto read_b = constrained_read_from_memory(call_ptr, clk, resolved_b, in_tag, in_tag, IntermRegister::IB); bool tag_match = read_a.tag_match && read_b.tag_match; - bool op_valid = tag_match && check_tag_integral(read_a.tag); + // No need to add check_tag_integral(read_b.tag) as this follows from tag matching and that a has integral tag. + if (is_valid(error) && !(tag_match && check_tag_integral(read_a.tag))) { + error = AvmError::TAG_ERROR; + } // a / b = c FF a = read_a.val; @@ -574,16 +605,17 @@ AvmError AvmTraceBuilder::op_div( // output (c) in memory. FF c; FF inv; - bool div_error = false; if (!b.is_zero()) { // If b is not zero, we prove it is not by providing its inverse as well inv = b.invert(); - c = op_valid ? alu_trace_builder.op_div(a, b, in_tag, clk) : FF(0); + c = is_valid(error) ? alu_trace_builder.op_div(a, b, in_tag, clk) : FF(0); } else { inv = 1; c = 0; - div_error = true; + if (is_valid(error)) { + error = AvmError::DIV_ZERO; + } } // Write into memory value c from intermediate register ic. @@ -607,7 +639,7 @@ AvmError AvmTraceBuilder::op_div( .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_dst.direct_address), - .main_op_err = op_valid ? FF(static_cast(div_error)) : FF(1), + .main_op_err = FF(static_cast(!is_valid(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), @@ -624,7 +656,7 @@ AvmError AvmTraceBuilder::op_div( ASSERT(op_code == OpCode::DIV_8 || op_code == OpCode::DIV_16); pc += Deserialization::get_pc_increment(op_code); - return !op_valid ? AvmError::TAG_ERROR : div_error ? AvmError::DIV_ZERO : AvmError::NO_ERROR; + return error; } /** @@ -639,11 +671,15 @@ AvmError AvmTraceBuilder::op_div( AvmError AvmTraceBuilder::op_fdiv( uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code) { + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto clk = static_cast(main_trace.size()) + 1; // Resolve any potential indirects in the order they are encoded in the indirect byte. - auto [resolved_a, resolved_b, resolved_c] = + auto [resolved_addrs, res_error] = Addressing<3>::fromWire(indirect, call_ptr).resolve({ a_offset, b_offset, dst_offset }, mem_trace_builder); + auto [resolved_a, resolved_b, resolved_c] = resolved_addrs; + error = res_error; // Reading from memory and loading into ia resp. ib. auto read_a = @@ -652,13 +688,15 @@ AvmError AvmTraceBuilder::op_fdiv( constrained_read_from_memory(call_ptr, clk, resolved_b, AvmMemoryTag::FF, AvmMemoryTag::FF, IntermRegister::IB); bool tag_match = read_a.tag_match && read_b.tag_match; + if (is_valid(error) && !tag_match) { + error = AvmError::TAG_ERROR; + } // a * b^(-1) = c FF a = read_a.val; FF b = read_b.val; FF c; FF inv; - bool div_error = false; if (!b.is_zero()) { inv = b.invert(); @@ -666,7 +704,9 @@ AvmError AvmTraceBuilder::op_fdiv( } else { inv = 1; c = 0; - div_error = true; + if (is_valid(error)) { + error = AvmError::DIV_ZERO; + } } // Write into memory value c from intermediate register ic. @@ -690,7 +730,7 @@ AvmError AvmTraceBuilder::op_fdiv( .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), - .main_op_err = tag_match ? FF(static_cast(div_error)) : FF(1), + .main_op_err = FF(static_cast(!is_valid(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(AvmMemoryTag::FF)), .main_rwc = FF(1), @@ -707,7 +747,7 @@ AvmError AvmTraceBuilder::op_fdiv( ASSERT(op_code == OpCode::FDIV_8 || op_code == OpCode::FDIV_16); pc += Deserialization::get_pc_increment(op_code); - return !tag_match ? AvmError::TAG_ERROR : div_error ? AvmError::DIV_ZERO : AvmError::NO_ERROR; + return error; } /************************************************************************************************** @@ -726,10 +766,14 @@ AvmError AvmTraceBuilder::op_fdiv( AvmError AvmTraceBuilder::op_eq( uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code) { + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto clk = static_cast(main_trace.size()) + 1; - auto [resolved_a, resolved_b, resolved_c] = + auto [resolved_addrs, res_error] = Addressing<3>::fromWire(indirect, call_ptr).resolve({ a_offset, b_offset, dst_offset }, mem_trace_builder); + auto [resolved_a, resolved_b, resolved_c] = resolved_addrs; + error = res_error; // We get our representative memory tag from the resolved_a memory address. AvmMemoryTag in_tag = unconstrained_get_memory_tag(resolved_a); @@ -738,13 +782,17 @@ AvmError AvmTraceBuilder::op_eq( auto read_b = constrained_read_from_memory(call_ptr, clk, resolved_b, in_tag, AvmMemoryTag::U1, IntermRegister::IB); bool tag_match = read_a.tag_match && read_b.tag_match; + if (is_valid(error) && !tag_match) { + error = AvmError::TAG_ERROR; + } + FF a = read_a.val; FF b = read_b.val; // In case of a memory tag error, we do not perform the computation. // Therefore, we do not create any entry in ALU table and store the value 0 as // output (c) in memory. - FF c = tag_match ? alu_trace_builder.op_eq(a, b, in_tag, clk) : FF(0); + FF c = is_valid(error) ? alu_trace_builder.op_eq(a, b, in_tag, clk) : FF(0); // Write into memory value c from intermediate register ic. auto write_c = @@ -767,6 +815,7 @@ AvmError AvmTraceBuilder::op_eq( .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), + .main_op_err = FF(static_cast(!is_valid(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), @@ -783,16 +832,20 @@ AvmError AvmTraceBuilder::op_eq( ASSERT(op_code == OpCode::EQ_8 || op_code == OpCode::EQ_16); pc += Deserialization::get_pc_increment(op_code); - return tag_match ? AvmError::NO_ERROR : AvmError::TAG_ERROR; + return error; } AvmError AvmTraceBuilder::op_lt( uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code) { + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto clk = static_cast(main_trace.size()) + 1; - auto [resolved_a, resolved_b, resolved_c] = + auto [resolved_addrs, res_error] = Addressing<3>::fromWire(indirect, call_ptr).resolve({ a_offset, b_offset, dst_offset }, mem_trace_builder); + auto [resolved_a, resolved_b, resolved_c] = resolved_addrs; + error = res_error; // We get our representative memory tag from the resolved_a memory address. AvmMemoryTag in_tag = unconstrained_get_memory_tag(resolved_a); @@ -800,10 +853,14 @@ AvmError AvmTraceBuilder::op_lt( auto read_b = constrained_read_from_memory(call_ptr, clk, resolved_b, in_tag, AvmMemoryTag::U1, IntermRegister::IB); bool tag_match = read_a.tag_match && read_b.tag_match; + if (is_valid(error) && !tag_match) { + error = AvmError::TAG_ERROR; + } + FF a = tag_match ? read_a.val : FF(0); FF b = tag_match ? read_b.val : FF(0); - FF c = tag_match ? alu_trace_builder.op_lt(a, b, in_tag, clk) : FF(0); + FF c = is_valid(error) ? alu_trace_builder.op_lt(a, b, in_tag, clk) : FF(0); // Write into memory value c from intermediate register ic. auto write_c = @@ -826,6 +883,7 @@ AvmError AvmTraceBuilder::op_lt( .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), + .main_op_err = FF(static_cast(!is_valid(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), @@ -842,16 +900,20 @@ AvmError AvmTraceBuilder::op_lt( ASSERT(op_code == OpCode::LT_8 || op_code == OpCode::LT_16); pc += Deserialization::get_pc_increment(op_code); - return tag_match ? AvmError::NO_ERROR : AvmError::TAG_ERROR; + return error; } AvmError AvmTraceBuilder::op_lte( uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code) { + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto clk = static_cast(main_trace.size()) + 1; - auto [resolved_a, resolved_b, resolved_c] = + auto [resolved_addrs, res_error] = Addressing<3>::fromWire(indirect, call_ptr).resolve({ a_offset, b_offset, dst_offset }, mem_trace_builder); + auto [resolved_a, resolved_b, resolved_c] = resolved_addrs; + error = res_error; // We get our representative memory tag from the resolved_a memory address. AvmMemoryTag in_tag = unconstrained_get_memory_tag(resolved_a); @@ -860,10 +922,14 @@ AvmError AvmTraceBuilder::op_lte( auto read_b = constrained_read_from_memory(call_ptr, clk, resolved_b, in_tag, AvmMemoryTag::U1, IntermRegister::IB); bool tag_match = read_a.tag_match && read_b.tag_match; + if (is_valid(error) && !tag_match) { + error = AvmError::TAG_ERROR; + } + FF a = tag_match ? read_a.val : FF(0); FF b = tag_match ? read_b.val : FF(0); - FF c = tag_match ? alu_trace_builder.op_lte(a, b, in_tag, clk) : FF(0); + FF c = is_valid(error) ? alu_trace_builder.op_lte(a, b, in_tag, clk) : FF(0); // Write into memory value c from intermediate register ic. auto write_c = @@ -886,6 +952,7 @@ AvmError AvmTraceBuilder::op_lte( .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), + .main_op_err = FF(static_cast(!is_valid(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), @@ -902,7 +969,7 @@ AvmError AvmTraceBuilder::op_lte( ASSERT(op_code == OpCode::LTE_8 || op_code == OpCode::LTE_16); pc += Deserialization::get_pc_increment(op_code); - return tag_match ? AvmError::NO_ERROR : AvmError::TAG_ERROR; + return error; } /************************************************************************************************** @@ -912,10 +979,14 @@ AvmError AvmTraceBuilder::op_lte( AvmError AvmTraceBuilder::op_and( uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code) { + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto clk = static_cast(main_trace.size()) + 1; - auto [resolved_a, resolved_b, resolved_c] = + auto [resolved_addrs, res_error] = Addressing<3>::fromWire(indirect, call_ptr).resolve({ a_offset, b_offset, dst_offset }, mem_trace_builder); + auto [resolved_a, resolved_b, resolved_c] = resolved_addrs; + error = res_error; // We get our representative memory tag from the resolved_a memory address. AvmMemoryTag in_tag = unconstrained_get_memory_tag(resolved_a); @@ -925,12 +996,14 @@ 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. - bool op_valid = tag_match && check_tag_integral(read_a.tag); + if (is_valid(error) && !(tag_match && check_tag_integral(read_a.tag))) { + error = AvmError::TAG_ERROR; + } FF a = tag_match ? read_a.val : FF(0); FF b = tag_match ? read_b.val : FF(0); - FF c = op_valid ? bin_trace_builder.op_and(a, b, in_tag, clk) : FF(0); + FF c = is_valid(error) ? bin_trace_builder.op_and(a, b, in_tag, clk) : FF(0); // Write into memory value c from intermediate register ic. auto write_c = constrained_write_to_memory(call_ptr, clk, resolved_c, c, in_tag, in_tag, IntermRegister::IC); @@ -952,11 +1025,11 @@ AvmError AvmTraceBuilder::op_and( .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), - .main_op_err = FF(static_cast(!op_valid)), + .main_op_err = FF(static_cast(!is_valid(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), - .main_sel_bin = FF(static_cast(op_valid)), + .main_sel_bin = FF(static_cast(is_valid(error))), .main_sel_mem_op_a = FF(1), .main_sel_mem_op_b = FF(1), .main_sel_mem_op_c = FF(1), @@ -970,15 +1043,20 @@ AvmError AvmTraceBuilder::op_and( ASSERT(op_code == OpCode::AND_8 || op_code == OpCode::AND_16); pc += Deserialization::get_pc_increment(op_code); - return op_valid ? AvmError::NO_ERROR : AvmError::TAG_ERROR; + return error; } AvmError AvmTraceBuilder::op_or( uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code) { + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto clk = static_cast(main_trace.size()) + 1; - auto [resolved_a, resolved_b, resolved_c] = + + auto [resolved_addrs, res_error] = Addressing<3>::fromWire(indirect, call_ptr).resolve({ a_offset, b_offset, dst_offset }, mem_trace_builder); + auto [resolved_a, resolved_b, resolved_c] = resolved_addrs; + error = res_error; // We get our representative memory tag from the resolved_a memory address. AvmMemoryTag in_tag = unconstrained_get_memory_tag(resolved_a); @@ -988,12 +1066,14 @@ 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. - bool op_valid = tag_match && check_tag_integral(read_a.tag); + if (is_valid(error) && !(tag_match && check_tag_integral(read_a.tag))) { + error = AvmError::TAG_ERROR; + } FF a = tag_match ? read_a.val : FF(0); FF b = tag_match ? read_b.val : FF(0); - FF c = op_valid ? bin_trace_builder.op_or(a, b, in_tag, clk) : FF(0); + FF c = is_valid(error) ? bin_trace_builder.op_or(a, b, in_tag, clk) : FF(0); // Write into memory value c from intermediate register ic. auto write_c = constrained_write_to_memory(call_ptr, clk, resolved_c, c, in_tag, in_tag, IntermRegister::IC); @@ -1015,11 +1095,11 @@ AvmError AvmTraceBuilder::op_or( .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), - .main_op_err = FF(static_cast(!op_valid)), + .main_op_err = FF(static_cast(!is_valid(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), - .main_sel_bin = FF(static_cast(op_valid)), + .main_sel_bin = FF(static_cast(is_valid(error))), .main_sel_mem_op_a = FF(1), .main_sel_mem_op_b = FF(1), .main_sel_mem_op_c = FF(1), @@ -1033,16 +1113,20 @@ AvmError AvmTraceBuilder::op_or( ASSERT(op_code == OpCode::OR_8 || op_code == OpCode::OR_16); pc += Deserialization::get_pc_increment(op_code); - return op_valid ? AvmError::NO_ERROR : AvmError::TAG_ERROR; + return error; } AvmError AvmTraceBuilder::op_xor( uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code) { + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto clk = static_cast(main_trace.size()) + 1; - auto [resolved_a, resolved_b, resolved_c] = + auto [resolved_addrs, res_error] = Addressing<3>::fromWire(indirect, call_ptr).resolve({ a_offset, b_offset, dst_offset }, mem_trace_builder); + auto [resolved_a, resolved_b, resolved_c] = resolved_addrs; + error = res_error; // We get our representative memory tag from the resolved_a memory address. AvmMemoryTag in_tag = unconstrained_get_memory_tag(resolved_a); @@ -1052,12 +1136,14 @@ 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. - bool op_valid = tag_match && check_tag_integral(read_a.tag); + if (is_valid(error) && !(tag_match && check_tag_integral(read_a.tag))) { + error = AvmError::TAG_ERROR; + } FF a = tag_match ? read_a.val : FF(0); FF b = tag_match ? read_b.val : FF(0); - FF c = op_valid ? bin_trace_builder.op_xor(a, b, in_tag, clk) : FF(0); + FF c = is_valid(error) ? bin_trace_builder.op_xor(a, b, in_tag, clk) : FF(0); // Write into memory value c from intermediate register ic. auto write_c = constrained_write_to_memory(call_ptr, clk, resolved_c, c, in_tag, in_tag, IntermRegister::IC); @@ -1079,11 +1165,11 @@ AvmError AvmTraceBuilder::op_xor( .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), - .main_op_err = FF(static_cast(!op_valid)), + .main_op_err = FF(static_cast(!is_valid(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), - .main_sel_bin = FF(static_cast(op_valid)), + .main_sel_bin = FF(static_cast(is_valid(error))), .main_sel_mem_op_a = FF(1), .main_sel_mem_op_b = FF(1), .main_sel_mem_op_c = FF(1), @@ -1097,7 +1183,7 @@ AvmError AvmTraceBuilder::op_xor( ASSERT(op_code == OpCode::XOR_8 || op_code == OpCode::XOR_16); pc += Deserialization::get_pc_increment(op_code); - return op_valid ? AvmError::NO_ERROR : AvmError::TAG_ERROR; + return error; } /** @@ -1109,25 +1195,32 @@ AvmError AvmTraceBuilder::op_xor( */ AvmError AvmTraceBuilder::op_not(uint8_t indirect, uint32_t a_offset, uint32_t dst_offset, OpCode op_code) { + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto clk = static_cast(main_trace.size()) + 1; // Resolve any potential indirects in the order they are encoded in the indirect byte. - auto [resolved_a, resolved_c] = + auto [resolved_addrs, res_error] = Addressing<2>::fromWire(indirect, call_ptr).resolve({ a_offset, dst_offset }, mem_trace_builder); + auto [resolved_a, resolved_c] = resolved_addrs; + error = res_error; // We get our representative memory tag from the resolved_a memory address. AvmMemoryTag in_tag = unconstrained_get_memory_tag(resolved_a); // Reading from memory and loading into ia auto read_a = constrained_read_from_memory(call_ptr, clk, resolved_a, in_tag, in_tag, IntermRegister::IA); - bool op_valid = check_tag_integral(read_a.tag); + if (is_valid(error) && !check_tag_integral(read_a.tag)) { + error = AvmError::TAG_ERROR; + } + // ~a = c FF a = read_a.val; // In case of an error (tag of type FF), we do not perform the computation. // Therefore, we do not create any entry in ALU table and store the value 0 as // output (c) in memory. - FF c = op_valid ? alu_trace_builder.op_not(a, in_tag, clk) : FF(0); + FF c = is_valid(error) ? alu_trace_builder.op_not(a, in_tag, clk) : FF(0); // Write into memory value c from intermediate register ic. auto write_c = constrained_write_to_memory(call_ptr, clk, resolved_c, c, in_tag, in_tag, IntermRegister::IC); @@ -1146,7 +1239,7 @@ AvmError AvmTraceBuilder::op_not(uint8_t indirect, uint32_t a_offset, uint32_t d .main_internal_return_ptr = FF(internal_return_ptr), .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_c = FF(write_c.direct_address), - .main_op_err = FF(static_cast(!op_valid)), + .main_op_err = FF(static_cast(!is_valid(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), @@ -1160,16 +1253,20 @@ AvmError AvmTraceBuilder::op_not(uint8_t indirect, uint32_t a_offset, uint32_t d ASSERT(op_code == OpCode::NOT_8 || op_code == OpCode::NOT_16); pc += Deserialization::get_pc_increment(op_code); - return op_valid ? AvmError::NO_ERROR : AvmError::TAG_ERROR; + return error; } AvmError AvmTraceBuilder::op_shl( uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code) { + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto clk = static_cast(main_trace.size()) + 1; - auto [resolved_a, resolved_b, resolved_c] = + auto [resolved_addrs, res_error] = Addressing<3>::fromWire(indirect, call_ptr).resolve({ a_offset, b_offset, dst_offset }, mem_trace_builder); + auto [resolved_a, resolved_b, resolved_c] = resolved_addrs; + error = res_error; // We get our representative memory tag from the resolved_a memory address. AvmMemoryTag in_tag = unconstrained_get_memory_tag(resolved_a); @@ -1179,12 +1276,15 @@ AvmError AvmTraceBuilder::op_shl( // auto read_b = constrained_read_from_memory(call_ptr, clk, resolved_b, AvmMemoryTag::U8, AvmMemoryTag::U8, // IntermRegister::IB); bool tag_match = read_a.tag_match && read_b.tag_match; auto read_b = unconstrained_read_from_memory(resolved_b); - bool op_valid = check_tag_integral(read_a.tag) && check_tag(AvmMemoryTag::U8, resolved_b); - FF a = op_valid ? read_a.val : FF(0); - FF b = op_valid ? read_b : FF(0); + if (is_valid(error) && !(check_tag_integral(read_a.tag) && check_tag(AvmMemoryTag::U8, resolved_b))) { + error = AvmError::TAG_ERROR; + } - FF c = op_valid ? alu_trace_builder.op_shl(a, b, in_tag, clk) : FF(0); + FF a = is_valid(error) ? read_a.val : FF(0); + FF b = is_valid(error) ? read_b : FF(0); + + FF c = is_valid(error) ? alu_trace_builder.op_shl(a, b, in_tag, clk) : FF(0); // Write into memory value c from intermediate register ic. auto write_c = constrained_write_to_memory(call_ptr, clk, resolved_c, c, in_tag, in_tag, IntermRegister::IC); @@ -1205,7 +1305,7 @@ AvmError AvmTraceBuilder::op_shl( .main_mem_addr_a = FF(read_a.direct_address), //.main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), - .main_op_err = FF(static_cast(!op_valid)), + .main_op_err = FF(static_cast(!is_valid(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), @@ -1221,16 +1321,20 @@ AvmError AvmTraceBuilder::op_shl( ASSERT(op_code == OpCode::SHL_8 || op_code == OpCode::SHL_16); pc += Deserialization::get_pc_increment(op_code); - return op_valid ? AvmError::NO_ERROR : AvmError::TAG_ERROR; + return error; } AvmError AvmTraceBuilder::op_shr( uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code) { + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto clk = static_cast(main_trace.size()) + 1; - auto [resolved_a, resolved_b, resolved_c] = + auto [resolved_addrs, res_error] = Addressing<3>::fromWire(indirect, call_ptr).resolve({ a_offset, b_offset, dst_offset }, mem_trace_builder); + auto [resolved_a, resolved_b, resolved_c] = resolved_addrs; + error = res_error; // We get our representative memory tag from the resolved_a memory address. AvmMemoryTag in_tag = unconstrained_get_memory_tag(resolved_a); @@ -1240,12 +1344,14 @@ AvmError AvmTraceBuilder::op_shr( // auto read_b = constrained_read_from_memory(call_ptr, clk, resolved_b, AvmMemoryTag::U8, AvmMemoryTag::U8, // IntermRegister::IB); bool tag_match = read_a.tag_match && read_b.tag_match; auto read_b = unconstrained_read_from_memory(resolved_b); - bool op_valid = check_tag_integral(read_a.tag) && check_tag(AvmMemoryTag::U8, resolved_b); + if (is_valid(error) && !(check_tag_integral(read_a.tag) && check_tag(AvmMemoryTag::U8, resolved_b))) { + error = AvmError::TAG_ERROR; + } - FF a = op_valid ? read_a.val : FF(0); - FF b = op_valid ? read_b : FF(0); + FF a = is_valid(error) ? read_a.val : FF(0); + FF b = is_valid(error) ? read_b : FF(0); - FF c = op_valid ? alu_trace_builder.op_shr(a, b, in_tag, clk) : FF(0); + FF c = is_valid(error) ? alu_trace_builder.op_shr(a, b, in_tag, clk) : FF(0); // Write into memory value c from intermediate register ic. auto write_c = constrained_write_to_memory(call_ptr, clk, resolved_c, c, in_tag, in_tag, IntermRegister::IC); @@ -1268,7 +1374,7 @@ AvmError AvmTraceBuilder::op_shr( // TODO(8603): uncomment //.main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), - .main_op_err = FF(static_cast(!op_valid)), + .main_op_err = FF(static_cast(!is_valid(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), @@ -1286,7 +1392,7 @@ AvmError AvmTraceBuilder::op_shr( ASSERT(op_code == OpCode::SHR_8 || op_code == OpCode::SHR_16); pc += Deserialization::get_pc_increment(op_code); - return op_valid ? AvmError::NO_ERROR : AvmError::TAG_ERROR; + return error; } /************************************************************************************************** @@ -1307,8 +1413,9 @@ AvmError AvmTraceBuilder::op_cast( { auto const clk = static_cast(main_trace.size()) + 1; - auto [resolved_a, resolved_c] = + auto [resolved_addrs, res_error] = Addressing<2>::fromWire(indirect, call_ptr).resolve({ a_offset, dst_offset }, mem_trace_builder); + auto [resolved_a, resolved_c] = resolved_addrs; // Reading from memory and loading into ia // There cannot be any tag error in this case. @@ -1334,6 +1441,7 @@ AvmError AvmTraceBuilder::op_cast( .main_internal_return_ptr = FF(internal_return_ptr), .main_mem_addr_a = FF(resolved_a), .main_mem_addr_c = FF(resolved_c), + .main_op_err = FF(static_cast(!is_valid(res_error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(memEntry.tag)), .main_rwc = FF(1), @@ -1345,7 +1453,7 @@ AvmError AvmTraceBuilder::op_cast( ASSERT(op_code == OpCode::CAST_8 || op_code == OpCode::CAST_16); pc += Deserialization::get_pc_increment(op_code); - return AvmError::NO_ERROR; + return res_error; } /************************************************************************************************** @@ -1363,31 +1471,39 @@ AvmError AvmTraceBuilder::op_cast( * @param dst_offset - Memory address to write the lookup result to * @param value - The value read from the memory address * @param w_tag - The memory tag of the value read - * @return Row + * @return RowWithError */ -Row AvmTraceBuilder::create_kernel_lookup_opcode(uint8_t indirect, uint32_t dst_offset, FF value, AvmMemoryTag w_tag) +RowWithError AvmTraceBuilder::create_kernel_lookup_opcode(uint8_t indirect, + uint32_t dst_offset, + FF value, + AvmMemoryTag w_tag) { auto const clk = static_cast(main_trace.size()) + 1; - auto [resolved_dst] = Addressing<1>::fromWire(indirect, call_ptr).resolve({ dst_offset }, mem_trace_builder); + auto [resolved_addrs, res_error] = + Addressing<1>::fromWire(indirect, call_ptr).resolve({ dst_offset }, mem_trace_builder); + auto [resolved_dst] = resolved_addrs; auto write_dst = constrained_write_to_memory(call_ptr, clk, resolved_dst, value, AvmMemoryTag::FF, w_tag, IntermRegister::IA); - return Row{ - .main_clk = clk, - .main_call_ptr = call_ptr, - .main_ia = value, - .main_ind_addr_a = FF(write_dst.indirect_address), - .main_internal_return_ptr = internal_return_ptr, - .main_mem_addr_a = FF(write_dst.direct_address), - .main_pc = pc, - .main_rwa = 1, - .main_sel_mem_op_a = 1, - .main_sel_resolve_ind_addr_a = FF(static_cast(write_dst.is_indirect)), - .main_tag_err = FF(static_cast(!write_dst.tag_match)), - .main_w_in_tag = static_cast(w_tag), - }; + return RowWithError{ .row = + Row{ + .main_clk = clk, + .main_call_ptr = call_ptr, + .main_ia = value, + .main_ind_addr_a = FF(write_dst.indirect_address), + .main_internal_return_ptr = internal_return_ptr, + .main_mem_addr_a = FF(write_dst.direct_address), + .main_op_err = FF(static_cast(!is_valid(res_error))), + .main_pc = pc, + .main_rwa = 1, + .main_sel_mem_op_a = 1, + .main_sel_resolve_ind_addr_a = FF(static_cast(write_dst.is_indirect)), + .main_tag_err = FF(static_cast(!write_dst.tag_match)), + .main_w_in_tag = static_cast(w_tag), + }, + .error = res_error }; } AvmError AvmTraceBuilder::op_get_env_var(uint8_t indirect, uint8_t env_var, uint32_t dst_offset) @@ -1411,46 +1527,47 @@ AvmError AvmTraceBuilder::op_get_env_var(uint8_t indirect, uint8_t env_var, uint return AvmError::ENV_VAR_UNKNOWN; } else { EnvironmentVariable var = static_cast(env_var); + AvmError error = AvmError::NO_ERROR; switch (var) { case EnvironmentVariable::ADDRESS: - op_address(indirect, dst_offset); + error = op_address(indirect, dst_offset); break; case EnvironmentVariable::SENDER: - op_sender(indirect, dst_offset); + error = op_sender(indirect, dst_offset); break; case EnvironmentVariable::FUNCTIONSELECTOR: - op_function_selector(indirect, dst_offset); + error = op_function_selector(indirect, dst_offset); break; case EnvironmentVariable::TRANSACTIONFEE: - op_transaction_fee(indirect, dst_offset); + error = op_transaction_fee(indirect, dst_offset); break; case EnvironmentVariable::CHAINID: - op_chain_id(indirect, dst_offset); + error = op_chain_id(indirect, dst_offset); break; case EnvironmentVariable::VERSION: - op_version(indirect, dst_offset); + error = op_version(indirect, dst_offset); break; case EnvironmentVariable::BLOCKNUMBER: - op_block_number(indirect, dst_offset); + error = op_block_number(indirect, dst_offset); break; case EnvironmentVariable::TIMESTAMP: - op_timestamp(indirect, dst_offset); + error = op_timestamp(indirect, dst_offset); break; case EnvironmentVariable::FEEPERL2GAS: - op_fee_per_l2_gas(indirect, dst_offset); + error = op_fee_per_l2_gas(indirect, dst_offset); break; case EnvironmentVariable::FEEPERDAGAS: - op_fee_per_da_gas(indirect, dst_offset); + error = op_fee_per_da_gas(indirect, dst_offset); break; case EnvironmentVariable::ISSTATICCALL: - op_is_static_call(indirect, dst_offset); + error = op_is_static_call(indirect, dst_offset); break; case EnvironmentVariable::L2GASLEFT: - op_l2gasleft(indirect, dst_offset); + error = op_l2gasleft(indirect, dst_offset); break; case EnvironmentVariable::DAGASLEFT: - op_dagasleft(indirect, dst_offset); + error = op_dagasleft(indirect, dst_offset); break; default: // Cannot happen thanks to the first if clause. This is to make the compiler happy. @@ -1458,155 +1575,166 @@ AvmError AvmTraceBuilder::op_get_env_var(uint8_t indirect, uint8_t env_var, uint break; } pc += Deserialization::get_pc_increment(OpCode::GETENVVAR_16); - return AvmError::NO_ERROR; + return error; } } -void AvmTraceBuilder::op_address(uint8_t indirect, uint32_t dst_offset) +AvmError AvmTraceBuilder::op_address(uint8_t indirect, uint32_t dst_offset) { auto const clk = static_cast(main_trace.size()) + 1; FF ia_value = kernel_trace_builder.op_address(clk); - Row row = create_kernel_lookup_opcode(indirect, dst_offset, ia_value, AvmMemoryTag::FF); + auto [row, error] = create_kernel_lookup_opcode(indirect, dst_offset, ia_value, AvmMemoryTag::FF); row.main_sel_op_address = FF(1); // Constrain gas cost gas_trace_builder.constrain_gas(static_cast(row.main_clk), OpCode::GETENVVAR_16); main_trace.push_back(row); + return error; } -void AvmTraceBuilder::op_sender(uint8_t indirect, uint32_t dst_offset) +AvmError AvmTraceBuilder::op_sender(uint8_t indirect, uint32_t dst_offset) { auto const clk = static_cast(main_trace.size()) + 1; FF ia_value = kernel_trace_builder.op_sender(clk); - Row row = create_kernel_lookup_opcode(indirect, dst_offset, ia_value, AvmMemoryTag::FF); + auto [row, error] = create_kernel_lookup_opcode(indirect, dst_offset, ia_value, AvmMemoryTag::FF); row.main_sel_op_sender = FF(1); // Constrain gas cost gas_trace_builder.constrain_gas(static_cast(row.main_clk), OpCode::GETENVVAR_16); main_trace.push_back(row); + return error; } -void AvmTraceBuilder::op_function_selector(uint8_t indirect, uint32_t dst_offset) +AvmError AvmTraceBuilder::op_function_selector(uint8_t indirect, uint32_t dst_offset) { auto const clk = static_cast(main_trace.size()) + 1; FF ia_value = kernel_trace_builder.op_function_selector(clk); - Row row = create_kernel_lookup_opcode(indirect, dst_offset, ia_value, AvmMemoryTag::U32); + auto [row, error] = create_kernel_lookup_opcode(indirect, dst_offset, ia_value, AvmMemoryTag::U32); row.main_sel_op_function_selector = FF(1); // Constrain gas cost gas_trace_builder.constrain_gas(static_cast(row.main_clk), OpCode::GETENVVAR_16); main_trace.push_back(row); + return error; } -void AvmTraceBuilder::op_transaction_fee(uint8_t indirect, uint32_t dst_offset) +AvmError AvmTraceBuilder::op_transaction_fee(uint8_t indirect, uint32_t dst_offset) { auto const clk = static_cast(main_trace.size()) + 1; FF ia_value = kernel_trace_builder.op_transaction_fee(clk); - Row row = create_kernel_lookup_opcode(indirect, dst_offset, ia_value, AvmMemoryTag::FF); + auto [row, error] = create_kernel_lookup_opcode(indirect, dst_offset, ia_value, AvmMemoryTag::FF); row.main_sel_op_transaction_fee = FF(1); // Constrain gas cost gas_trace_builder.constrain_gas(static_cast(row.main_clk), OpCode::GETENVVAR_16); main_trace.push_back(row); + return error; } -void AvmTraceBuilder::op_is_static_call(uint8_t indirect, uint32_t dst_offset) +AvmError AvmTraceBuilder::op_is_static_call(uint8_t indirect, uint32_t dst_offset) { auto const clk = static_cast(main_trace.size()) + 1; FF ia_value = kernel_trace_builder.op_is_static_call(clk); - Row row = create_kernel_lookup_opcode(indirect, dst_offset, ia_value, AvmMemoryTag::FF); + auto [row, error] = create_kernel_lookup_opcode(indirect, dst_offset, ia_value, AvmMemoryTag::FF); row.main_sel_op_is_static_call = FF(1); // Constrain gas cost gas_trace_builder.constrain_gas(static_cast(row.main_clk), OpCode::GETENVVAR_16); main_trace.push_back(row); + return error; } /************************************************************************************************** * EXECUTION ENVIRONMENT - GLOBALS **************************************************************************************************/ -void AvmTraceBuilder::op_chain_id(uint8_t indirect, uint32_t dst_offset) +AvmError AvmTraceBuilder::op_chain_id(uint8_t indirect, uint32_t dst_offset) { auto const clk = static_cast(main_trace.size()) + 1; FF ia_value = kernel_trace_builder.op_chain_id(clk); - Row row = create_kernel_lookup_opcode(indirect, dst_offset, ia_value, AvmMemoryTag::FF); + auto [row, error] = create_kernel_lookup_opcode(indirect, dst_offset, ia_value, AvmMemoryTag::FF); row.main_sel_op_chain_id = FF(1); // Constrain gas cost gas_trace_builder.constrain_gas(static_cast(row.main_clk), OpCode::GETENVVAR_16); main_trace.push_back(row); + return error; } -void AvmTraceBuilder::op_version(uint8_t indirect, uint32_t dst_offset) +AvmError AvmTraceBuilder::op_version(uint8_t indirect, uint32_t dst_offset) { auto const clk = static_cast(main_trace.size()) + 1; FF ia_value = kernel_trace_builder.op_version(clk); - Row row = create_kernel_lookup_opcode(indirect, dst_offset, ia_value, AvmMemoryTag::FF); + auto [row, error] = create_kernel_lookup_opcode(indirect, dst_offset, ia_value, AvmMemoryTag::FF); row.main_sel_op_version = FF(1); // Constrain gas cost gas_trace_builder.constrain_gas(static_cast(row.main_clk), OpCode::GETENVVAR_16); main_trace.push_back(row); + return error; } -void AvmTraceBuilder::op_block_number(uint8_t indirect, uint32_t dst_offset) +AvmError AvmTraceBuilder::op_block_number(uint8_t indirect, uint32_t dst_offset) { auto const clk = static_cast(main_trace.size()) + 1; FF ia_value = kernel_trace_builder.op_block_number(clk); - Row row = create_kernel_lookup_opcode(indirect, dst_offset, ia_value, AvmMemoryTag::FF); + auto [row, error] = create_kernel_lookup_opcode(indirect, dst_offset, ia_value, AvmMemoryTag::FF); row.main_sel_op_block_number = FF(1); // Constrain gas cost gas_trace_builder.constrain_gas(static_cast(row.main_clk), OpCode::GETENVVAR_16); main_trace.push_back(row); + return error; } -void AvmTraceBuilder::op_timestamp(uint8_t indirect, uint32_t dst_offset) +AvmError AvmTraceBuilder::op_timestamp(uint8_t indirect, uint32_t dst_offset) { auto const clk = static_cast(main_trace.size()) + 1; FF ia_value = kernel_trace_builder.op_timestamp(clk); - Row row = create_kernel_lookup_opcode(indirect, dst_offset, ia_value, AvmMemoryTag::U64); + auto [row, error] = create_kernel_lookup_opcode(indirect, dst_offset, ia_value, AvmMemoryTag::U64); row.main_sel_op_timestamp = FF(1); // Constrain gas cost gas_trace_builder.constrain_gas(static_cast(row.main_clk), OpCode::GETENVVAR_16); main_trace.push_back(row); + return error; } -void AvmTraceBuilder::op_fee_per_l2_gas(uint8_t indirect, uint32_t dst_offset) +AvmError AvmTraceBuilder::op_fee_per_l2_gas(uint8_t indirect, uint32_t dst_offset) { auto const clk = static_cast(main_trace.size()) + 1; FF ia_value = kernel_trace_builder.op_fee_per_l2_gas(clk); - Row row = create_kernel_lookup_opcode(indirect, dst_offset, ia_value, AvmMemoryTag::FF); + auto [row, error] = create_kernel_lookup_opcode(indirect, dst_offset, ia_value, AvmMemoryTag::FF); row.main_sel_op_fee_per_l2_gas = FF(1); // Constrain gas cost gas_trace_builder.constrain_gas(static_cast(row.main_clk), OpCode::GETENVVAR_16); main_trace.push_back(row); + return error; } -void AvmTraceBuilder::op_fee_per_da_gas(uint8_t indirect, uint32_t dst_offset) +AvmError AvmTraceBuilder::op_fee_per_da_gas(uint8_t indirect, uint32_t dst_offset) { auto const clk = static_cast(main_trace.size()) + 1; FF ia_value = kernel_trace_builder.op_fee_per_da_gas(clk); - Row row = create_kernel_lookup_opcode(indirect, dst_offset, ia_value, AvmMemoryTag::FF); + auto [row, error] = create_kernel_lookup_opcode(indirect, dst_offset, ia_value, AvmMemoryTag::FF); row.main_sel_op_fee_per_da_gas = FF(1); // Constrain gas cost gas_trace_builder.constrain_gas(static_cast(row.main_clk), OpCode::GETENVVAR_16); main_trace.push_back(row); + return error; } /************************************************************************************************** @@ -1636,22 +1764,28 @@ AvmError AvmTraceBuilder::op_calldata_copy(uint8_t indirect, uint32_t copy_size_address, uint32_t dst_offset) { + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto clk = static_cast(main_trace.size()) + 1; - auto [cd_offset_resolved, copy_size_offset_resolved, dst_offset_resolved] = + auto [resolved_addrs, res_error] = Addressing<3>::fromWire(indirect, call_ptr) .resolve({ cd_offset_address, copy_size_address, dst_offset }, mem_trace_builder); + auto [cd_offset_resolved, copy_size_offset_resolved, dst_offset_resolved] = resolved_addrs; + error = res_error; // This boolean will not be a trivial constant anymore once we constrain address resolution. bool tag_match = true; - bool op_valid = tag_match && check_tag(AvmMemoryTag::U32, cd_offset_resolved) && - check_tag(AvmMemoryTag::U32, copy_size_offset_resolved); + if (is_valid(error) && !(check_tag(AvmMemoryTag::U32, cd_offset_resolved) && + check_tag(AvmMemoryTag::U32, copy_size_offset_resolved))) { + error = AvmError::TAG_ERROR; + } // TODO: constrain these. const uint32_t cd_offset = static_cast(unconstrained_read_from_memory(cd_offset_resolved)); const uint32_t copy_size = static_cast(unconstrained_read_from_memory(copy_size_offset_resolved)); - if (op_valid) { + if (is_valid(error)) { slice_trace_builder.create_calldata_copy_slice( calldata, clk, call_ptr, cd_offset, copy_size, dst_offset_resolved); mem_trace_builder.write_calldata_copy(calldata, clk, call_ptr, cd_offset, copy_size, dst_offset_resolved); @@ -1667,26 +1801,35 @@ AvmError AvmTraceBuilder::op_calldata_copy(uint8_t indirect, .main_ib = copy_size, .main_internal_return_ptr = FF(internal_return_ptr), .main_mem_addr_c = dst_offset_resolved, - .main_op_err = static_cast(!op_valid), + .main_op_err = static_cast(!is_valid(error)), .main_pc = pc, .main_r_in_tag = static_cast(AvmMemoryTag::FF), .main_sel_op_calldata_copy = 1, - .main_sel_slice_gadget = static_cast(op_valid), + .main_sel_slice_gadget = static_cast(is_valid(error)), .main_tag_err = static_cast(!tag_match), .main_w_in_tag = static_cast(AvmMemoryTag::FF), }); pc += Deserialization::get_pc_increment(OpCode::CALLDATACOPY); - return op_valid ? AvmError::NO_ERROR : AvmError::TAG_ERROR; + return error; } AvmError AvmTraceBuilder::op_returndata_size(uint8_t indirect, uint32_t dst_offset) { + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto const clk = static_cast(main_trace.size()) + 1; // This boolean will not be a trivial constant anymore once we constrain address resolution. bool tag_match = true; - auto [resolved_dst_offset] = Addressing<1>::fromWire(indirect, call_ptr).resolve({ dst_offset }, mem_trace_builder); + auto [resolved_addrs, res_error] = + Addressing<1>::fromWire(indirect, call_ptr).resolve({ dst_offset }, mem_trace_builder); + auto [resolved_dst_offset] = resolved_addrs; + error = res_error; + + if (is_valid(error) && !tag_match) { + error = AvmError::TAG_ERROR; + } FF returndata_size = tag_match ? FF(nested_returndata.size()) : FF(0); // TODO: constrain @@ -1699,6 +1842,7 @@ AvmError AvmTraceBuilder::op_returndata_size(uint8_t indirect, uint32_t dst_offs .main_clk = clk, .main_call_ptr = call_ptr, .main_internal_return_ptr = FF(internal_return_ptr), + .main_op_err = FF(static_cast(!is_valid(error))), .main_pc = FF(pc), .main_sel_op_returndata_size = FF(1), .main_tag_err = FF(static_cast(!tag_match)), @@ -1706,7 +1850,7 @@ AvmError AvmTraceBuilder::op_returndata_size(uint8_t indirect, uint32_t dst_offs }); pc += Deserialization::get_pc_increment(OpCode::RETURNDATASIZE); - return tag_match ? AvmError::NO_ERROR : AvmError::TAG_ERROR; + return error; } AvmError AvmTraceBuilder::op_returndata_copy(uint8_t indirect, @@ -1714,16 +1858,22 @@ AvmError AvmTraceBuilder::op_returndata_copy(uint8_t indirect, uint32_t copy_size_offset, uint32_t dst_offset) { + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto clk = static_cast(main_trace.size()) + 1; - auto [rd_offset_resolved, copy_size_offset_resolved, dst_offset_resolved] = + auto [resolved_addrs, res_error] = Addressing<3>::fromWire(indirect, call_ptr) .resolve({ rd_offset_address, copy_size_offset, dst_offset }, mem_trace_builder); + auto [rd_offset_resolved, copy_size_offset_resolved, dst_offset_resolved] = resolved_addrs; + error = res_error; // This boolean will not be a trivial constant anymore once we constrain address resolution. bool tag_match = true; - bool op_valid = tag_match && check_tag(AvmMemoryTag::U32, rd_offset_address) && - check_tag(AvmMemoryTag::U32, copy_size_offset_resolved); + if (is_valid(error) && !(check_tag(AvmMemoryTag::U32, rd_offset_resolved) && + check_tag(AvmMemoryTag::U32, copy_size_offset_resolved))) { + error = AvmError::TAG_ERROR; + } // TODO: constrain these. const uint32_t rd_offset = static_cast(unconstrained_read_from_memory(rd_offset_resolved)); @@ -1736,13 +1886,13 @@ AvmError AvmTraceBuilder::op_returndata_copy(uint8_t indirect, main_trace.push_back(Row{ .main_clk = clk, .main_internal_return_ptr = FF(internal_return_ptr), - .main_op_err = static_cast(!op_valid), + .main_op_err = static_cast(!is_valid(error)), .main_pc = FF(pc), .main_sel_op_returndata_copy = FF(1), .main_tag_err = FF(static_cast(!tag_match)), }); - if (op_valid) { + if (is_valid(error)) { // Write the return data to memory // TODO: validate bounds auto returndata_slice = @@ -1754,7 +1904,7 @@ AvmError AvmTraceBuilder::op_returndata_copy(uint8_t indirect, // is implemented with opcodes (SET and JUMP). write_slice_to_memory(dst_offset_resolved, AvmMemoryTag::FF, returndata_slice); } - return op_valid ? AvmError::NO_ERROR : AvmError::TAG_ERROR; + return error; } /************************************************************************************************** @@ -1762,13 +1912,15 @@ AvmError AvmTraceBuilder::op_returndata_copy(uint8_t indirect, **************************************************************************************************/ // Helper for "gas left" related opcodes -void AvmTraceBuilder::execute_gasleft(EnvironmentVariable var, uint8_t indirect, uint32_t dst_offset) +AvmError AvmTraceBuilder::execute_gasleft(EnvironmentVariable var, uint8_t indirect, uint32_t dst_offset) { ASSERT(var == EnvironmentVariable::L2GASLEFT || var == EnvironmentVariable::DAGASLEFT); auto clk = static_cast(main_trace.size()) + 1; - auto [resolved_dst] = Addressing<1>::fromWire(indirect, call_ptr).resolve({ dst_offset }, mem_trace_builder); + auto [resolved_addrs, res_error] = + Addressing<1>::fromWire(indirect, call_ptr).resolve({ dst_offset }, mem_trace_builder); + auto [resolved_dst] = resolved_addrs; // Constrain gas cost gas_trace_builder.constrain_gas(clk, OpCode::GETENVVAR_16); @@ -1793,6 +1945,7 @@ void AvmTraceBuilder::execute_gasleft(EnvironmentVariable var, uint8_t indirect, .main_ind_addr_a = FF(write_dst.indirect_address), .main_internal_return_ptr = FF(internal_return_ptr), .main_mem_addr_a = FF(write_dst.direct_address), + .main_op_err = FF(static_cast(!is_valid(res_error))), .main_pc = FF(pc), .main_rwa = FF(1), .main_sel_mem_op_a = FF(1), @@ -1803,16 +1956,17 @@ void AvmTraceBuilder::execute_gasleft(EnvironmentVariable var, uint8_t indirect, .main_w_in_tag = FF(static_cast(AvmMemoryTag::FF)), // TODO: probably will be U32 in final version // Should the circuit (pil) constrain U32? }); + return res_error; } -void AvmTraceBuilder::op_l2gasleft(uint8_t indirect, uint32_t dst_offset) +AvmError AvmTraceBuilder::op_l2gasleft(uint8_t indirect, uint32_t dst_offset) { - execute_gasleft(EnvironmentVariable::L2GASLEFT, indirect, dst_offset); + return execute_gasleft(EnvironmentVariable::L2GASLEFT, indirect, dst_offset); } -void AvmTraceBuilder::op_dagasleft(uint8_t indirect, uint32_t dst_offset) +AvmError AvmTraceBuilder::op_dagasleft(uint8_t indirect, uint32_t dst_offset) { - execute_gasleft(EnvironmentVariable::DAGASLEFT, indirect, dst_offset); + return execute_gasleft(EnvironmentVariable::DAGASLEFT, indirect, dst_offset); } /************************************************************************************************** @@ -1862,13 +2016,21 @@ AvmError AvmTraceBuilder::op_jump(uint32_t jmp_dest, bool skip_gas) */ AvmError AvmTraceBuilder::op_jumpi(uint8_t indirect, uint32_t jmp_dest, uint32_t cond_offset) { + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto clk = static_cast(main_trace.size()) + 1; // Will be a non-trivial constant once we constrain address resolution bool tag_match = true; - auto [resolved_cond_offset] = + auto [resolved_addrs, res_error] = Addressing<1>::fromWire(indirect, call_ptr).resolve({ cond_offset }, mem_trace_builder); + auto [resolved_cond_offset] = resolved_addrs; + error = res_error; + + if (is_valid(error) && !tag_match) { + error = AvmError::TAG_ERROR; + } // Specific JUMPI loading of conditional value into intermediate register id without any tag constraint. auto read_d = mem_trace_builder.read_and_load_jumpi_opcode(call_ptr, clk, resolved_cond_offset); @@ -1889,6 +2051,7 @@ AvmError AvmTraceBuilder::op_jumpi(uint8_t indirect, uint32_t jmp_dest, uint32_t .main_internal_return_ptr = FF(internal_return_ptr), .main_inv = inv, .main_mem_addr_d = resolved_cond_offset, + .main_op_err = static_cast(!is_valid(error)), .main_pc = FF(pc), .main_r_in_tag = static_cast(read_d.tag), .main_sel_mem_op_d = 1, @@ -1899,7 +2062,7 @@ AvmError AvmTraceBuilder::op_jumpi(uint8_t indirect, uint32_t jmp_dest, uint32_t // Adjust parameters for the next row pc = next_pc; - return tag_match ? AvmError::NO_ERROR : AvmError::TAG_ERROR; + return error; } /** @@ -2014,12 +2177,22 @@ AvmError AvmTraceBuilder::op_internal_return() AvmError AvmTraceBuilder::op_set( uint8_t indirect, FF val_ff, uint32_t dst_offset, AvmMemoryTag in_tag, OpCode op_code, bool skip_gas) { - auto const clk = static_cast(main_trace.size()) + 1; - auto [resolved_dst_offset] = Addressing<1>::fromWire(indirect, call_ptr).resolve({ dst_offset }, mem_trace_builder); + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; + const auto clk = static_cast(main_trace.size()) + 1; + + auto [resolved_addrs, res_error] = + Addressing<1>::fromWire(indirect, call_ptr).resolve({ dst_offset }, mem_trace_builder); + auto [resolved_dst_offset] = resolved_addrs; + error = res_error; auto write_c = constrained_write_to_memory( call_ptr, clk, resolved_dst_offset, val_ff, AvmMemoryTag::FF, in_tag, IntermRegister::IC); + if (is_valid(error) && !write_c.tag_match) { + error = AvmError::TAG_ERROR; + } + // Constrain gas cost // FIXME: not great that we are having to choose one specific opcode here! if (!skip_gas) { @@ -2033,6 +2206,7 @@ AvmError AvmTraceBuilder::op_set( .main_ind_addr_c = FF(write_c.indirect_address), .main_internal_return_ptr = internal_return_ptr, .main_mem_addr_c = FF(write_c.direct_address), + .main_op_err = static_cast(!is_valid(error)), .main_pc = pc, .main_rwc = 1, .main_sel_mem_op_c = 1, @@ -2046,7 +2220,7 @@ AvmError AvmTraceBuilder::op_set( OpCode::SET_64, OpCode::SET_128, OpCode::SET_FF }; ASSERT(set_family.contains(op_code)); pc += Deserialization::get_pc_increment(op_code); - return write_c.tag_match ? AvmError::NO_ERROR : AvmError::TAG_ERROR; + return error; } /** @@ -2059,16 +2233,24 @@ AvmError AvmTraceBuilder::op_set( */ AvmError AvmTraceBuilder::op_mov(uint8_t indirect, uint32_t src_offset, uint32_t dst_offset, OpCode op_code) { - auto const clk = static_cast(main_trace.size()) + 1; + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; + const auto clk = static_cast(main_trace.size()) + 1; // Will be a non-trivial constant once we constrain address resolution bool tag_match = true; - auto [resolved_src_offset, resolved_dst_offset] = + auto [resolved_addrs, res_error] = Addressing<2>::fromWire(indirect, call_ptr).resolve({ src_offset, dst_offset }, mem_trace_builder); + auto [resolved_src_offset, resolved_dst_offset] = resolved_addrs; + error = res_error; + + if (is_valid(error) && !tag_match) { + error = AvmError::TAG_ERROR; + } // Reading from memory and loading into ia without tag check. - auto const [val, tag] = mem_trace_builder.read_and_load_mov_opcode(call_ptr, clk, resolved_src_offset); + const auto [val, tag] = mem_trace_builder.read_and_load_mov_opcode(call_ptr, clk, resolved_src_offset); // Write into memory from intermediate register ic. mem_trace_builder.write_into_memory(call_ptr, clk, IntermRegister::IC, resolved_dst_offset, val, tag, tag); @@ -2085,6 +2267,7 @@ AvmError AvmTraceBuilder::op_mov(uint8_t indirect, uint32_t src_offset, uint32_t .main_internal_return_ptr = internal_return_ptr, .main_mem_addr_a = resolved_src_offset, .main_mem_addr_c = resolved_dst_offset, + .main_op_err = static_cast(!is_valid(error)), .main_pc = pc, .main_r_in_tag = static_cast(tag), .main_rwc = 1, @@ -2098,7 +2281,7 @@ AvmError AvmTraceBuilder::op_mov(uint8_t indirect, uint32_t src_offset, uint32_t ASSERT(op_code == OpCode::MOV_8 || op_code == OpCode::MOV_16); pc += Deserialization::get_pc_increment(op_code); - return tag_match ? AvmError::NO_ERROR : AvmError::TAG_ERROR; + return error; } /************************************************************************************************** @@ -2115,27 +2298,38 @@ AvmError AvmTraceBuilder::op_mov(uint8_t indirect, uint32_t src_offset, uint32_t * @param data_offset - The memory address to read the output from * @return Row */ -Row AvmTraceBuilder::create_kernel_output_opcode(uint8_t indirect, uint32_t clk, uint32_t data_offset) +RowWithError AvmTraceBuilder::create_kernel_output_opcode(uint8_t indirect, uint32_t clk, uint32_t data_offset) { - auto [resolved_data] = Addressing<1>::fromWire(indirect, call_ptr).resolve({ data_offset }, mem_trace_builder); + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; + auto [resolved_addrs, res_error] = + Addressing<1>::fromWire(indirect, call_ptr).resolve({ data_offset }, mem_trace_builder); + auto [resolved_data] = resolved_addrs; + error = res_error; auto read_a = constrained_read_from_memory( call_ptr, clk, resolved_data, AvmMemoryTag::FF, AvmMemoryTag::FF, IntermRegister::IA); bool tag_match = read_a.tag_match; + if (is_valid(error) && !tag_match) { + error = AvmError::TAG_ERROR; + } - return Row{ - .main_clk = clk, - .main_ia = read_a.val, - .main_ind_addr_a = FF(read_a.indirect_address), - .main_internal_return_ptr = internal_return_ptr, - .main_mem_addr_a = FF(read_a.direct_address), - .main_pc = pc, - .main_r_in_tag = static_cast(AvmMemoryTag::FF), - .main_rwa = 0, - .main_sel_mem_op_a = 1, - .main_sel_resolve_ind_addr_a = FF(static_cast(read_a.is_indirect)), - .main_tag_err = FF(static_cast(!tag_match)), - }; + return RowWithError{ .row = + Row{ + .main_clk = clk, + .main_ia = read_a.val, + .main_ind_addr_a = FF(read_a.indirect_address), + .main_internal_return_ptr = internal_return_ptr, + .main_mem_addr_a = FF(read_a.direct_address), + .main_op_err = FF(static_cast(!is_valid(error))), + .main_pc = pc, + .main_r_in_tag = static_cast(AvmMemoryTag::FF), + .main_rwa = 0, + .main_sel_mem_op_a = 1, + .main_sel_resolve_ind_addr_a = FF(static_cast(read_a.is_indirect)), + .main_tag_err = FF(static_cast(!tag_match)), + }, + .error = error }; } /** @@ -2151,15 +2345,19 @@ Row AvmTraceBuilder::create_kernel_output_opcode(uint8_t indirect, uint32_t clk, * @param metadata_r_tag - The data type of the metadata * @return Row */ -Row AvmTraceBuilder::create_kernel_output_opcode_with_metadata(uint8_t indirect, - uint32_t clk, - uint32_t data_offset, - AvmMemoryTag data_r_tag, - uint32_t metadata_offset, - AvmMemoryTag metadata_r_tag) +RowWithError AvmTraceBuilder::create_kernel_output_opcode_with_metadata(uint8_t indirect, + uint32_t clk, + uint32_t data_offset, + AvmMemoryTag data_r_tag, + uint32_t metadata_offset, + AvmMemoryTag metadata_r_tag) { - auto [resolved_data, resolved_metadata] = + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; + auto [resolved_addrs, res_error] = Addressing<2>::fromWire(indirect, call_ptr).resolve({ data_offset, metadata_offset }, mem_trace_builder); + auto [resolved_data, resolved_metadata] = resolved_addrs; + error = res_error; auto read_a = constrained_read_from_memory(call_ptr, clk, resolved_data, data_r_tag, AvmMemoryTag::FF, IntermRegister::IA); @@ -2167,25 +2365,32 @@ Row AvmTraceBuilder::create_kernel_output_opcode_with_metadata(uint8_t indirect, call_ptr, clk, resolved_metadata, metadata_r_tag, AvmMemoryTag::FF, IntermRegister::IB); bool tag_match = read_a.tag_match && read_b.tag_match; - return Row{ - .main_clk = clk, - .main_ia = read_a.val, - .main_ib = read_b.val, - .main_ind_addr_a = FF(read_a.indirect_address), - .main_ind_addr_b = FF(read_b.indirect_address), - .main_internal_return_ptr = internal_return_ptr, - .main_mem_addr_a = FF(read_a.direct_address), - .main_mem_addr_b = FF(read_b.direct_address), - .main_pc = pc, - .main_r_in_tag = static_cast(data_r_tag), - .main_rwa = 0, - .main_rwb = 0, - .main_sel_mem_op_a = 1, - .main_sel_mem_op_b = 1, - .main_sel_resolve_ind_addr_a = FF(static_cast(read_a.is_indirect)), - .main_sel_resolve_ind_addr_b = FF(static_cast(read_b.is_indirect)), - .main_tag_err = FF(static_cast(!tag_match)), - }; + if (is_valid(error) && !tag_match) { + error = AvmError::TAG_ERROR; + } + + return RowWithError{ .row = + Row{ + .main_clk = clk, + .main_ia = read_a.val, + .main_ib = read_b.val, + .main_ind_addr_a = FF(read_a.indirect_address), + .main_ind_addr_b = FF(read_b.indirect_address), + .main_internal_return_ptr = internal_return_ptr, + .main_mem_addr_a = FF(read_a.direct_address), + .main_mem_addr_b = FF(read_b.direct_address), + .main_op_err = FF(static_cast(!is_valid(error))), + .main_pc = pc, + .main_r_in_tag = static_cast(data_r_tag), + .main_rwa = 0, + .main_rwb = 0, + .main_sel_mem_op_a = 1, + .main_sel_mem_op_b = 1, + .main_sel_resolve_ind_addr_a = FF(static_cast(read_a.is_indirect)), + .main_sel_resolve_ind_addr_b = FF(static_cast(read_b.is_indirect)), + .main_tag_err = FF(static_cast(!tag_match)), + }, + .error = error }; } /** @@ -2286,16 +2491,20 @@ Row AvmTraceBuilder::create_kernel_output_opcode_for_leaf_index(uint32_t clk, * @param metadata_offset - The offset of the metadata (slot in the sload example) * @return Row */ -Row AvmTraceBuilder::create_kernel_output_opcode_with_set_value_from_hint(uint8_t indirect, - uint32_t clk, - uint32_t data_offset, - uint32_t metadata_offset) +RowWithError AvmTraceBuilder::create_kernel_output_opcode_with_set_value_from_hint(uint8_t indirect, + uint32_t clk, + uint32_t data_offset, + uint32_t metadata_offset) { FF value = execution_hints.get_side_effect_hints().at(side_effect_counter); // TODO: throw error if incorrect - auto [resolved_data, resolved_metadata] = + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; + auto [resolved_addrs, res_error] = Addressing<2>::fromWire(indirect, call_ptr).resolve({ data_offset, metadata_offset }, mem_trace_builder); + auto [resolved_data, resolved_metadata] = resolved_addrs; + error = res_error; auto write_a = constrained_write_to_memory( call_ptr, clk, resolved_data, value, AvmMemoryTag::FF, AvmMemoryTag::FF, IntermRegister::IA); @@ -2303,27 +2512,34 @@ Row AvmTraceBuilder::create_kernel_output_opcode_with_set_value_from_hint(uint8_ call_ptr, clk, resolved_metadata, AvmMemoryTag::FF, AvmMemoryTag::FF, IntermRegister::IB); bool tag_match = write_a.tag_match && read_b.tag_match; - return Row{ - .main_clk = clk, - .main_ia = write_a.val, - .main_ib = read_b.val, - .main_ind_addr_a = FF(write_a.indirect_address), - .main_ind_addr_b = FF(read_b.indirect_address), - .main_internal_return_ptr = internal_return_ptr, - .main_mem_addr_a = FF(write_a.direct_address), - .main_mem_addr_b = FF(read_b.direct_address), - .main_pc = pc, // No PC increment here since we do it in the specific ops - .main_r_in_tag = static_cast(AvmMemoryTag::FF), - .main_rwa = 1, - .main_rwb = 0, - .main_sel_mem_op_a = 1, - .main_sel_mem_op_b = 1, - .main_sel_q_kernel_output_lookup = 1, - .main_sel_resolve_ind_addr_a = FF(static_cast(write_a.is_indirect)), - .main_sel_resolve_ind_addr_b = FF(static_cast(read_b.is_indirect)), - .main_tag_err = static_cast(!tag_match), - .main_w_in_tag = static_cast(AvmMemoryTag::FF), - }; + if (is_valid(error) && !tag_match) { + error = AvmError::TAG_ERROR; + } + + return RowWithError{ .row = + Row{ + .main_clk = clk, + .main_ia = write_a.val, + .main_ib = read_b.val, + .main_ind_addr_a = FF(write_a.indirect_address), + .main_ind_addr_b = FF(read_b.indirect_address), + .main_internal_return_ptr = internal_return_ptr, + .main_mem_addr_a = FF(write_a.direct_address), + .main_mem_addr_b = FF(read_b.direct_address), + .main_op_err = FF(static_cast(!is_valid(error))), + .main_pc = pc, // No PC increment here since we do it in the specific ops + .main_r_in_tag = static_cast(AvmMemoryTag::FF), + .main_rwa = 1, + .main_rwb = 0, + .main_sel_mem_op_a = 1, + .main_sel_mem_op_b = 1, + .main_sel_q_kernel_output_lookup = 1, + .main_sel_resolve_ind_addr_a = FF(static_cast(write_a.is_indirect)), + .main_sel_resolve_ind_addr_b = FF(static_cast(read_b.is_indirect)), + .main_tag_err = static_cast(!tag_match), + .main_w_in_tag = static_cast(AvmMemoryTag::FF), + }, + .error = error }; } /************************************************************************************************** @@ -2332,10 +2548,14 @@ Row AvmTraceBuilder::create_kernel_output_opcode_with_set_value_from_hint(uint8_ AvmError AvmTraceBuilder::op_sload(uint8_t indirect, uint32_t slot_offset, uint32_t size, uint32_t dest_offset) { + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto clk = static_cast(main_trace.size()) + 1; - auto [resolved_slot, resolved_dest] = + 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 @@ -2406,16 +2626,25 @@ AvmError AvmTraceBuilder::op_sload(uint8_t indirect, uint32_t slot_offset, uint3 // After the first loop, all future write destinations are direct, increment the direct address write_dst = AddressWithMode{ AddressingMode::DIRECT, write_a.direct_address + 1 }; } + + if (is_valid(error) && !accumulated_tag_match) { + error = AvmError::TAG_ERROR; + } + pc += Deserialization::get_pc_increment(OpCode::SLOAD); - return accumulated_tag_match ? AvmError::NO_ERROR : AvmError::TAG_ERROR; + return error; } AvmError AvmTraceBuilder::op_sstore(uint8_t indirect, uint32_t src_offset, uint32_t size, uint32_t slot_offset) { + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto clk = static_cast(main_trace.size()) + 1; - auto [resolved_src, resolved_slot] = + 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 @@ -2483,8 +2712,12 @@ AvmError AvmTraceBuilder::op_sstore(uint8_t indirect, uint32_t src_offset, uint3 read_src = AddressWithMode{ AddressingMode::DIRECT, read_a.direct_address + 1 }; } + if (is_valid(error) && !accumulated_tag_match) { + error = AvmError::TAG_ERROR; + } + pc += Deserialization::get_pc_increment(OpCode::SSTORE); - return accumulated_tag_match ? AvmError::NO_ERROR : AvmError::TAG_ERROR; + return error; } AvmError AvmTraceBuilder::op_note_hash_exists(uint8_t indirect, @@ -2492,17 +2725,24 @@ AvmError AvmTraceBuilder::op_note_hash_exists(uint8_t indirect, uint32_t leaf_index_offset, uint32_t dest_offset) { + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto const clk = static_cast(main_trace.size()) + 1; - auto [resolved_note_hash, resolved_leaf_index, resolved_dest] = + auto [resolved_addrs, res_error] = Addressing<3>::fromWire(indirect, call_ptr) .resolve({ note_hash_offset, leaf_index_offset, dest_offset }, mem_trace_builder); + auto [resolved_note_hash, resolved_leaf_index, resolved_dest] = resolved_addrs; + error = res_error; const auto leaf_index = unconstrained_read_from_memory(resolved_leaf_index); - bool op_valid = check_tag(AvmMemoryTag::FF, resolved_leaf_index); + if (is_valid(error) && !check_tag(AvmMemoryTag::FF, resolved_leaf_index)) { + error = AvmError::TAG_ERROR; + } + Row row; - if (op_valid) { + if (is_valid(error)) { row = create_kernel_output_opcode_for_leaf_index( clk, resolved_note_hash, static_cast(leaf_index), resolved_dest); @@ -2511,7 +2751,9 @@ AvmError AvmTraceBuilder::op_note_hash_exists(uint8_t indirect, row.main_ia, /*safe*/ static_cast(row.main_ib)); row.main_sel_op_note_hash_exists = FF(1); - op_valid = op_valid && row.main_tag_err == FF(0); + if (is_valid(error) && row.main_tag_err != FF(0)) { + error = AvmError::TAG_ERROR; + } } else { row = Row{ .main_clk = clk, @@ -2529,14 +2771,14 @@ AvmError AvmTraceBuilder::op_note_hash_exists(uint8_t indirect, debug("note_hash_exists side-effect cnt: ", side_effect_counter); pc += Deserialization::get_pc_increment(OpCode::NOTEHASHEXISTS); - return op_valid ? AvmError::NO_ERROR : AvmError::TAG_ERROR; + return error; } AvmError AvmTraceBuilder::op_emit_note_hash(uint8_t indirect, uint32_t note_hash_offset) { auto const clk = static_cast(main_trace.size()) + 1; - Row row = create_kernel_output_opcode(indirect, clk, note_hash_offset); + auto [row, error] = create_kernel_output_opcode(indirect, clk, note_hash_offset); kernel_trace_builder.op_emit_note_hash(clk, side_effect_counter, row.main_ia); row.main_sel_op_emit_note_hash = FF(1); @@ -2549,7 +2791,7 @@ AvmError AvmTraceBuilder::op_emit_note_hash(uint8_t indirect, uint32_t note_hash side_effect_counter++; pc += Deserialization::get_pc_increment(OpCode::EMITNOTEHASH); - return row.main_tag_err == FF(0) ? AvmError::NO_ERROR : AvmError::TAG_ERROR; + return error; } AvmError AvmTraceBuilder::op_nullifier_exists(uint8_t indirect, @@ -2557,23 +2799,31 @@ AvmError AvmTraceBuilder::op_nullifier_exists(uint8_t indirect, uint32_t address_offset, uint32_t dest_offset) { + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto const clk = static_cast(main_trace.size()) + 1; - auto [resolved_nullifier_offset, resolved_address, resolved_dest] = + auto [resolved_addrs, res_error] = Addressing<3>::fromWire(indirect, call_ptr) .resolve({ nullifier_offset, address_offset, dest_offset }, mem_trace_builder); + auto [resolved_nullifier_offset, resolved_address, resolved_dest] = resolved_addrs; + error = res_error; - bool op_valid = check_tag(AvmMemoryTag::FF, resolved_address); + if (is_valid(error) && !check_tag(AvmMemoryTag::FF, resolved_address)) { + error = AvmError::TAG_ERROR; + } Row row; - if (op_valid) { + if (is_valid(error)) { row = create_kernel_output_opcode_with_set_metadata_output_from_hint( clk, resolved_nullifier_offset, resolved_address, resolved_dest); kernel_trace_builder.op_nullifier_exists( clk, side_effect_counter, row.main_ia, /*safe*/ static_cast(row.main_ib)); row.main_sel_op_nullifier_exists = FF(1); - op_valid = op_valid && row.main_tag_err == FF(0); + if (is_valid(error) && row.main_tag_err != FF(0)) { + error = AvmError::TAG_ERROR; + } } else { row = Row{ .main_clk = clk, @@ -2593,14 +2843,14 @@ AvmError AvmTraceBuilder::op_nullifier_exists(uint8_t indirect, side_effect_counter++; pc += Deserialization::get_pc_increment(OpCode::NULLIFIEREXISTS); - return op_valid ? AvmError::NO_ERROR : AvmError::TAG_ERROR; + return error; } AvmError AvmTraceBuilder::op_emit_nullifier(uint8_t indirect, uint32_t nullifier_offset) { auto const clk = static_cast(main_trace.size()) + 1; - Row row = create_kernel_output_opcode(indirect, clk, nullifier_offset); + auto [row, error] = create_kernel_output_opcode(indirect, clk, nullifier_offset); kernel_trace_builder.op_emit_nullifier(clk, side_effect_counter, row.main_ia); row.main_sel_op_emit_nullifier = FF(1); @@ -2613,7 +2863,7 @@ AvmError AvmTraceBuilder::op_emit_nullifier(uint8_t indirect, uint32_t nullifier side_effect_counter++; pc += Deserialization::get_pc_increment(OpCode::EMITNULLIFIER); - return row.main_tag_err == FF(0) ? AvmError::NO_ERROR : AvmError::TAG_ERROR; + return error; } AvmError AvmTraceBuilder::op_l1_to_l2_msg_exists(uint8_t indirect, @@ -2621,17 +2871,23 @@ AvmError AvmTraceBuilder::op_l1_to_l2_msg_exists(uint8_t indirect, uint32_t leaf_index_offset, uint32_t dest_offset) { + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto const clk = static_cast(main_trace.size()) + 1; - auto [resolved_log, resolved_leaf_index, resolved_dest] = - Addressing<3>::fromWire(indirect, call_ptr) - .resolve({ log_offset, leaf_index_offset, dest_offset }, mem_trace_builder); + auto [resolved_addrs, res_error] = Addressing<3>::fromWire(indirect, call_ptr) + .resolve({ log_offset, leaf_index_offset, dest_offset }, mem_trace_builder); + auto [resolved_log, resolved_leaf_index, resolved_dest] = resolved_addrs; + error = res_error; const auto leaf_index = unconstrained_read_from_memory(resolved_leaf_index); - bool op_valid = check_tag(AvmMemoryTag::FF, resolved_leaf_index); + if (is_valid(error) && !check_tag(AvmMemoryTag::FF, resolved_leaf_index)) { + error = AvmError::TAG_ERROR; + } + Row row; - if (op_valid) { + if (is_valid(error)) { row = create_kernel_output_opcode_for_leaf_index( clk, resolved_log, static_cast(leaf_index), resolved_dest); kernel_trace_builder.op_l1_to_l2_msg_exists(clk, @@ -2639,7 +2895,9 @@ AvmError AvmTraceBuilder::op_l1_to_l2_msg_exists(uint8_t indirect, row.main_ia, /*safe*/ static_cast(row.main_ib)); row.main_sel_op_l1_to_l2_msg_exists = FF(1); - op_valid = op_valid && row.main_tag_err == FF(0); + if (is_valid(error) && row.main_tag_err != FF(0)) { + error = AvmError::TAG_ERROR; + } } else { row = Row{ .main_clk = clk, @@ -2658,12 +2916,14 @@ AvmError AvmTraceBuilder::op_l1_to_l2_msg_exists(uint8_t indirect, debug("l1_to_l2_msg_exists side-effect cnt: ", side_effect_counter); pc += Deserialization::get_pc_increment(OpCode::L1TOL2MSGEXISTS); - return op_valid ? AvmError::NO_ERROR : AvmError::TAG_ERROR; + return error; } AvmError AvmTraceBuilder::op_get_contract_instance( uint8_t indirect, uint8_t member_enum, uint16_t address_offset, uint16_t dst_offset, uint16_t exists_offset) { + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto clk = static_cast(main_trace.size()) + 1; // Constrain gas cost gas_trace_builder.constrain_gas(clk, OpCode::GETCONTRACTINSTANCE); @@ -2686,13 +2946,17 @@ AvmError AvmTraceBuilder::op_get_contract_instance( ContractInstanceMember chosen_member = static_cast(member_enum); - auto [resolved_address_offset, resolved_dst_offset, resolved_exists_offset] = - Addressing<3>::fromWire(indirect, call_ptr) - .resolve({ address_offset, dst_offset, exists_offset }, mem_trace_builder); + auto [resolved_addrs, res_error] = Addressing<3>::fromWire(indirect, call_ptr) + .resolve({ address_offset, dst_offset, exists_offset }, mem_trace_builder); + auto [resolved_address_offset, resolved_dst_offset, resolved_exists_offset] = resolved_addrs; + error = res_error; auto read_address = constrained_read_from_memory( call_ptr, clk, resolved_address_offset, AvmMemoryTag::FF, AvmMemoryTag::FF, IntermRegister::IA); bool tag_match = read_address.tag_match; + if (is_valid(error) && !tag_match) { + error = AvmError::TAG_ERROR; + } // Read the contract instance ContractInstanceHint instance = execution_hints.contract_instance_hints.at(read_address.val); @@ -2732,6 +2996,7 @@ AvmError AvmTraceBuilder::op_get_contract_instance( //.main_ind_addr_d = FF(write_exists.indirect_address), .main_internal_return_ptr = FF(internal_return_ptr), .main_mem_addr_a = FF(read_address.direct_address), + .main_op_err = FF(static_cast(!is_valid(error))), //.main_mem_addr_c = FF(write_dst.direct_address), //.main_mem_addr_d = FF(write_exists.direct_address), .main_pc = FF(pc), @@ -2759,7 +3024,7 @@ AvmError AvmTraceBuilder::op_get_contract_instance( debug("contract_instance cnt: ", side_effect_counter); side_effect_counter++; - return tag_match ? AvmError::NO_ERROR : AvmError::TAG_ERROR; + return error; } /************************************************************************************************** @@ -2770,11 +3035,15 @@ AvmError AvmTraceBuilder::op_emit_unencrypted_log(uint8_t indirect, uint32_t log { std::vector bytes_to_hash; + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto const clk = static_cast(main_trace.size()) + 1; // FIXME: read (and constrain) log_size_offset - auto [resolved_log_offset, resolved_log_size_offset] = + auto [resolved_addrs, res_error] = Addressing<2>::fromWire(indirect, call_ptr).resolve({ log_offset, log_size_offset }, mem_trace_builder); + auto [resolved_log_offset, resolved_log_size_offset] = resolved_addrs; + error = res_error; // This is a hack to get the contract address from the first contract instance // Once we have 1-enqueued call and proper nested contexts, this should use that address of the current context @@ -2786,15 +3055,17 @@ AvmError AvmTraceBuilder::op_emit_unencrypted_log(uint8_t indirect, uint32_t log std::make_move_iterator(contract_address_bytes.begin()), std::make_move_iterator(contract_address_bytes.end())); - bool op_valid = - check_tag(AvmMemoryTag::FF, resolved_log_offset) && check_tag(AvmMemoryTag::U32, resolved_log_size_offset); + if (is_valid(error) && + !(check_tag(AvmMemoryTag::FF, resolved_log_offset) && check_tag(AvmMemoryTag::U32, resolved_log_size_offset))) { + error = AvmError::TAG_ERROR; + } Row row; uint32_t log_size = 0; AddressWithMode direct_field_addr; uint32_t num_bytes = 0; - if (op_valid) { + if (is_valid(error)) { log_size = static_cast(unconstrained_read_from_memory(resolved_log_size_offset)); // The size is in fields of 32 bytes, the length used for the hash is in terms of bytes @@ -2806,10 +3077,12 @@ AvmError AvmTraceBuilder::op_emit_unencrypted_log(uint8_t indirect, uint32_t log std::make_move_iterator(log_size_bytes.end())); direct_field_addr = AddressWithMode(static_cast(resolved_log_offset)); - op_valid = op_valid && check_tag_range(AvmMemoryTag::FF, direct_field_addr, log_size); + if (!check_tag_range(AvmMemoryTag::FF, direct_field_addr, log_size)) { + error = AvmError::TAG_ERROR; + }; } - if (op_valid) { + if (is_valid(error)) { // We need to read the rest of the log_size number of elements for (uint32_t i = 0; i < log_size; i++) { FF log_value = unconstrained_read_from_memory(direct_field_addr + i); @@ -2857,7 +3130,7 @@ AvmError AvmTraceBuilder::op_emit_unencrypted_log(uint8_t indirect, uint32_t log debug("emit_unencrypted_log side-effect cnt: ", side_effect_counter); side_effect_counter++; pc += Deserialization::get_pc_increment(OpCode::EMITUNENCRYPTEDLOG); - return op_valid ? AvmError::NO_ERROR : AvmError::TAG_ERROR; + return error; } AvmError AvmTraceBuilder::op_emit_l2_to_l1_msg(uint8_t indirect, uint32_t recipient_offset, uint32_t content_offset) @@ -2865,7 +3138,7 @@ AvmError AvmTraceBuilder::op_emit_l2_to_l1_msg(uint8_t indirect, uint32_t recipi auto const clk = static_cast(main_trace.size()) + 1; // Note: unorthodox order - as seen in L2ToL1Message struct in TS - Row row = create_kernel_output_opcode_with_metadata( + auto [row, error] = create_kernel_output_opcode_with_metadata( indirect, clk, content_offset, AvmMemoryTag::FF, recipient_offset, AvmMemoryTag::FF); kernel_trace_builder.op_emit_l2_to_l1_msg(clk, side_effect_counter, row.main_ia, row.main_ib); row.main_sel_op_emit_l2_to_l1_msg = FF(1); @@ -2879,7 +3152,7 @@ AvmError AvmTraceBuilder::op_emit_l2_to_l1_msg(uint8_t indirect, uint32_t recipi side_effect_counter++; pc += Deserialization::get_pc_increment(OpCode::SENDL2TOL1MSG); - return row.main_tag_err == FF(0) ? AvmError::NO_ERROR : AvmError::TAG_ERROR; + return error; } /************************************************************************************************** @@ -2896,16 +3169,20 @@ AvmError AvmTraceBuilder::constrain_external_call(OpCode opcode, uint32_t success_offset) { ASSERT(opcode == OpCode::CALL || opcode == OpCode::STATICCALL); + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto clk = static_cast(main_trace.size()) + 1; const ExternalCallHint& hint = execution_hints.externalcall_hints.at(external_call_counter); + auto [resolved_addrs, res_error] = + Addressing<5>::fromWire(indirect, call_ptr) + .resolve({ gas_offset, addr_offset, args_offset, args_size_offset, success_offset }, mem_trace_builder); auto [resolved_gas_offset, resolved_addr_offset, resolved_args_offset, resolved_args_size_offset, - resolved_success_offset] = - Addressing<5>::fromWire(indirect, call_ptr) - .resolve({ gas_offset, addr_offset, args_offset, args_size_offset, success_offset }, mem_trace_builder); + resolved_success_offset] = resolved_addrs; + error = res_error; // Should read the address next to read_gas as well (tuple of gas values (l2Gas, daGas)) auto read_gas_l2 = constrained_read_from_memory( @@ -2918,10 +3195,13 @@ AvmError AvmTraceBuilder::constrain_external_call(OpCode opcode, call_ptr, clk, resolved_args_offset, AvmMemoryTag::FF, AvmMemoryTag::FF, IntermRegister::ID); bool tag_match = read_gas_l2.tag_match && read_gas_da.tag_match && read_addr.tag_match && read_args.tag_match; - bool op_valid = check_tag(AvmMemoryTag::U32, resolved_args_size_offset); + if (is_valid(error) && !(tag_match && check_tag(AvmMemoryTag::U32, resolved_args_size_offset))) { + error = AvmError::TAG_ERROR; + } // TODO: constrain this - auto args_size = op_valid ? static_cast(unconstrained_read_from_memory(resolved_args_size_offset)) : 0; + auto args_size = + is_valid(error) ? static_cast(unconstrained_read_from_memory(resolved_args_size_offset)) : 0; gas_trace_builder.constrain_gas(clk, opcode, @@ -2943,7 +3223,7 @@ AvmError AvmTraceBuilder::constrain_external_call(OpCode opcode, .main_mem_addr_b = FF(read_gas_l2.direct_address + 1), .main_mem_addr_c = FF(read_addr.direct_address), .main_mem_addr_d = FF(read_args.direct_address), - .main_op_err = FF(static_cast(!op_valid)), + .main_op_err = FF(static_cast(!is_valid(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(AvmMemoryTag::FF)), .main_sel_mem_op_a = FF(1), @@ -2973,7 +3253,7 @@ AvmError AvmTraceBuilder::constrain_external_call(OpCode opcode, if (opcode == OpCode::CALL) { side_effect_counter = static_cast(hint.end_side_effect_counter); } - return op_valid ? AvmError::NO_ERROR : AvmError::TAG_ERROR; + return error; } /** @@ -3041,16 +3321,23 @@ AvmError AvmTraceBuilder::op_static_call(uint16_t indirect, */ ReturnDataError AvmTraceBuilder::op_return(uint8_t indirect, uint32_t ret_offset, uint32_t ret_size_offset) { + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto clk = static_cast(main_trace.size()) + 1; // This boolean will not be a trivial constant once we re-enable constraining address resolution bool tag_match = true; // Resolve operands - auto [resolved_ret_offset, resolved_ret_size_offset] = + auto [resolved_addrs, res_error] = Addressing<2>::fromWire(indirect, call_ptr).resolve({ ret_offset, ret_size_offset }, mem_trace_builder); + auto [resolved_ret_offset, resolved_ret_size_offset] = resolved_addrs; + error = res_error; + + if (is_valid(error) && !(tag_match && check_tag(AvmMemoryTag::U32, resolved_ret_size_offset))) { + error = AvmError::TAG_ERROR; + } - bool op_valid = tag_match && check_tag(AvmMemoryTag::U32, resolved_ret_size_offset); const auto ret_size = static_cast(unconstrained_read_from_memory(resolved_ret_size_offset)); gas_trace_builder.constrain_gas(clk, OpCode::RETURN, ret_size); @@ -3061,7 +3348,7 @@ ReturnDataError AvmTraceBuilder::op_return(uint8_t indirect, uint32_t ret_offset .main_call_ptr = call_ptr, .main_ib = ret_size, .main_internal_return_ptr = FF(internal_return_ptr), - .main_op_err = static_cast(!op_valid), + .main_op_err = static_cast(!is_valid(error)), .main_pc = pc, .main_sel_op_external_return = 1, }); @@ -3070,7 +3357,7 @@ ReturnDataError AvmTraceBuilder::op_return(uint8_t indirect, uint32_t ret_offset return ReturnDataError{ .return_data = {}, - .error = op_valid ? AvmError::NO_ERROR : AvmError::TAG_ERROR, + .error = error, }; } @@ -3089,7 +3376,7 @@ ReturnDataError AvmTraceBuilder::op_return(uint8_t indirect, uint32_t ret_offset .main_ib = ret_size, .main_internal_return_ptr = FF(internal_return_ptr), .main_mem_addr_c = resolved_ret_offset, - .main_op_err = static_cast(!op_valid), + .main_op_err = static_cast(!is_valid(error)), .main_pc = pc, .main_r_in_tag = static_cast(AvmMemoryTag::FF), .main_sel_op_external_return = 1, @@ -3102,24 +3389,31 @@ ReturnDataError AvmTraceBuilder::op_return(uint8_t indirect, uint32_t ret_offset return ReturnDataError{ .return_data = returndata, - .error = op_valid ? AvmError::NO_ERROR : AvmError::TAG_ERROR, + .error = error, }; } ReturnDataError AvmTraceBuilder::op_revert(uint8_t indirect, uint32_t ret_offset, uint32_t ret_size_offset) { // TODO: This opcode is still masquerading as RETURN. + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto clk = static_cast(main_trace.size()) + 1; // This boolean will not be a trivial constant once we re-enable constraining address resolution bool tag_match = true; - auto [resolved_ret_offset, resolved_ret_size_offset] = + auto [resolved_addrs, res_error] = Addressing<2>::fromWire(indirect, call_ptr).resolve({ ret_offset, ret_size_offset }, mem_trace_builder); + auto [resolved_ret_offset, resolved_ret_size_offset] = resolved_addrs; + error = res_error; + + if (is_valid(error) && !(tag_match && check_tag(AvmMemoryTag::U32, ret_size_offset))) { + error = AvmError::TAG_ERROR; + } - bool op_valid = check_tag(AvmMemoryTag::U32, ret_size_offset); const auto ret_size = - op_valid ? static_cast(unconstrained_read_from_memory(resolved_ret_size_offset)) : 0; + is_valid(error) ? static_cast(unconstrained_read_from_memory(resolved_ret_size_offset)) : 0; gas_trace_builder.constrain_gas(clk, OpCode::REVERT_8, ret_size); @@ -3130,7 +3424,7 @@ ReturnDataError AvmTraceBuilder::op_revert(uint8_t indirect, uint32_t ret_offset .main_call_ptr = call_ptr, .main_ib = ret_size, .main_internal_return_ptr = FF(internal_return_ptr), - .main_op_err = FF(static_cast(!op_valid)), + .main_op_err = FF(static_cast(!is_valid(error))), .main_pc = pc, .main_sel_op_external_return = 1, }); @@ -3138,7 +3432,7 @@ ReturnDataError AvmTraceBuilder::op_revert(uint8_t indirect, uint32_t ret_offset pc = UINT32_MAX; // This ensures that no subsequent opcode will be executed. return ReturnDataError{ .return_data = {}, - .error = op_valid ? AvmError::NO_ERROR : AvmError::TAG_ERROR, + .error = error, }; } @@ -3157,6 +3451,7 @@ ReturnDataError AvmTraceBuilder::op_revert(uint8_t indirect, uint32_t ret_offset .main_ib = ret_size, .main_internal_return_ptr = FF(internal_return_ptr), .main_mem_addr_c = resolved_ret_offset, + .main_op_err = static_cast(!is_valid(error)), .main_pc = pc, .main_r_in_tag = static_cast(AvmMemoryTag::FF), .main_sel_op_external_return = 1, @@ -3170,7 +3465,7 @@ ReturnDataError AvmTraceBuilder::op_revert(uint8_t indirect, uint32_t ret_offset // op_valid == true otherwise, ret_size == 0 and we would have returned above. return ReturnDataError{ .return_data = returndata, - .error = tag_match ? AvmError::NO_ERROR : AvmError::TAG_ERROR, + .error = error, }; } @@ -3184,37 +3479,42 @@ AvmError AvmTraceBuilder::op_debug_log(uint8_t indirect, uint32_t fields_offset, uint32_t fields_size_offset) { + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto clk = static_cast(main_trace.size()) + 1; - auto [resolved_message_offset, resolved_fields_offset, resolved_fields_size_offset] = + auto [resolved_addrs, res_error] = Addressing<3>::fromWire(indirect, call_ptr) .resolve({ message_offset, fields_offset, fields_size_offset }, mem_trace_builder); + auto [resolved_message_offset, resolved_fields_offset, resolved_fields_size_offset] = resolved_addrs; + error = res_error; - // Tags checking - bool op_valid = check_tag(AvmMemoryTag::U32, resolved_fields_size_offset); + if (is_valid(error) && !check_tag(AvmMemoryTag::U32, resolved_fields_size_offset)) { + error = AvmError::TAG_ERROR; + } const uint32_t fields_size = - op_valid ? static_cast(unconstrained_read_from_memory(resolved_fields_size_offset)) : 0; + is_valid(error) ? static_cast(unconstrained_read_from_memory(resolved_fields_size_offset)) : 0; // Constrain gas cost gas_trace_builder.constrain_gas(clk, OpCode::DEBUGLOG, message_size + fields_size); - if (op_valid) { - op_valid = op_valid && check_tag_range(AvmMemoryTag::U8, resolved_message_offset, message_size) && - check_tag_range(AvmMemoryTag::FF, resolved_fields_offset, fields_size); + if (is_valid(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; } main_trace.push_back(Row{ .main_clk = clk, .main_call_ptr = call_ptr, .main_internal_return_ptr = FF(internal_return_ptr), - .main_op_err = FF(static_cast(!op_valid)), + .main_op_err = FF(static_cast(!is_valid(error))), .main_pc = FF(pc), .main_sel_op_debug_log = FF(1), }); pc += Deserialization::get_pc_increment(OpCode::DEBUGLOG); - return op_valid ? AvmError::NO_ERROR : AvmError::TAG_ERROR; + return error; } /************************************************************************************************** @@ -3232,13 +3532,17 @@ AvmError AvmTraceBuilder::op_debug_log(uint8_t indirect, */ AvmError AvmTraceBuilder::op_poseidon2_permutation(uint8_t indirect, uint32_t input_offset, uint32_t output_offset) { + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto clk = static_cast(main_trace.size()) + 1; // Resolve the indirect flags, the results of this function are used to determine the memory offsets // that point to the starting memory addresses for the input, output and h_init values // Note::This function will add memory reads at clk in the mem_trace_builder - auto [resolved_input_offset, resolved_output_offset] = + auto [resolved_addrs, res_error] = Addressing<2>::fromWire(indirect, call_ptr).resolve({ input_offset, output_offset }, mem_trace_builder); + auto [resolved_input_offset, resolved_output_offset] = resolved_addrs; + error = res_error; // Resolve indirects in the main trace. Do not resolve the value stored in direct addresses. @@ -3278,9 +3582,13 @@ AvmError AvmTraceBuilder::op_poseidon2_permutation(uint8_t indirect, uint32_t in IntermRegister::ID, AvmMemTraceBuilder::POSEIDON2); - bool op_valid = read_a.tag_match && read_b.tag_match && read_c.tag_match && read_d.tag_match; + bool read_tag_valid = read_a.tag_match && read_b.tag_match && read_c.tag_match && read_d.tag_match; + + if (is_valid(error) && !read_tag_valid) { + error = AvmError::TAG_ERROR; + } - if (op_valid) { + if (is_valid(error)) { std::array input = { read_a.val, read_b.val, read_c.val, read_d.val }; std::array result = poseidon2_trace_builder.poseidon2_permutation( input, call_ptr, clk, resolved_input_offset, resolved_output_offset); @@ -3325,7 +3633,10 @@ AvmError AvmTraceBuilder::op_poseidon2_permutation(uint8_t indirect, uint32_t in IntermRegister::ID, AvmMemTraceBuilder::POSEIDON2); - op_valid = write_a.tag_match && write_b.tag_match && write_c.tag_match && write_d.tag_match; + bool write_tag_valid = write_a.tag_match && write_b.tag_match && write_c.tag_match && write_d.tag_match; + if (is_valid(error) && !write_tag_valid) { + error = AvmError::TAG_ERROR; + } } // Main trace contains on operand values from the bytecode and resolved indirects @@ -3334,14 +3645,14 @@ AvmError AvmTraceBuilder::op_poseidon2_permutation(uint8_t indirect, uint32_t in .main_internal_return_ptr = FF(internal_return_ptr), .main_mem_addr_a = resolved_input_offset, .main_mem_addr_b = resolved_output_offset, - .main_op_err = FF(static_cast(!op_valid)), + .main_op_err = FF(static_cast(!is_valid(error))), .main_pc = FF(pc), .main_sel_op_poseidon2 = FF(1), }); pc += Deserialization::get_pc_increment(OpCode::POSEIDON2PERM); - return op_valid ? AvmError::NO_ERROR : AvmError::TAG_ERROR; + return error; } /** @@ -3366,11 +3677,15 @@ AvmError AvmTraceBuilder::op_sha256_compression(uint8_t indirect, // The clk plays a crucial role in this function as we attempt to write across multiple lines in the main trace. auto clk = static_cast(main_trace.size()) + 1; + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; + // Resolve the indirect flags, the results of this function are used to determine the memory offsets // that point to the starting memory addresses for the input and output values. - auto [resolved_output_offset, resolved_state_offset, resolved_inputs_offset] = - Addressing<3>::fromWire(indirect, call_ptr) - .resolve({ output_offset, state_offset, inputs_offset }, mem_trace_builder); + auto [resolved_addrs, res_error] = Addressing<3>::fromWire(indirect, call_ptr) + .resolve({ output_offset, state_offset, inputs_offset }, mem_trace_builder); + auto [resolved_output_offset, resolved_state_offset, resolved_inputs_offset] = resolved_addrs; + error = res_error; auto read_a = constrained_read_from_memory( call_ptr, clk, resolved_state_offset, AvmMemoryTag::U32, AvmMemoryTag::FF, IntermRegister::IA); @@ -3378,8 +3693,10 @@ AvmError AvmTraceBuilder::op_sha256_compression(uint8_t indirect, call_ptr, clk, resolved_inputs_offset, AvmMemoryTag::U32, AvmMemoryTag::FF, IntermRegister::IB); bool tag_match = read_a.tag_match && read_b.tag_match; - bool op_valid = tag_match && check_tag_range(AvmMemoryTag::U32, resolved_state_offset, STATE_SIZE) && - check_tag_range(AvmMemoryTag::U32, resolved_inputs_offset, INPUTS_SIZE); + if (is_valid(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; + } // Constrain gas cost gas_trace_builder.constrain_gas(clk, OpCode::SHA256COMPRESSION); @@ -3401,7 +3718,7 @@ AvmError AvmTraceBuilder::op_sha256_compression(uint8_t indirect, .main_internal_return_ptr = FF(internal_return_ptr), .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), - .main_op_err = FF(static_cast(!op_valid)), + .main_op_err = FF(static_cast(!is_valid(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(AvmMemoryTag::U32)), .main_sel_mem_op_a = FF(1), @@ -3412,8 +3729,8 @@ AvmError AvmTraceBuilder::op_sha256_compression(uint8_t indirect, .main_tag_err = FF(static_cast(!tag_match)), }); - if (!op_valid) { - return AvmError::TAG_ERROR; + if (!is_valid(error)) { + return error; } // We store the current clk this main trace row occurred so that we can line up the sha256 gadget operation at @@ -3462,14 +3779,23 @@ AvmError AvmTraceBuilder::op_sha256_compression(uint8_t indirect, */ AvmError AvmTraceBuilder::op_keccakf1600(uint8_t indirect, uint32_t output_offset, uint32_t input_offset) { + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto clk = static_cast(main_trace.size()) + 1; - auto [resolved_output_offset, resolved_input_offset] = + + auto [resolved_addrs, res_error] = Addressing<2>::fromWire(indirect, call_ptr).resolve({ output_offset, input_offset }, mem_trace_builder); + auto [resolved_output_offset, resolved_input_offset] = resolved_addrs; + error = res_error; + auto input_read = constrained_read_from_memory( call_ptr, clk, resolved_input_offset, AvmMemoryTag::U64, AvmMemoryTag::FF, IntermRegister::IA); bool tag_match = input_read.tag_match; - bool op_valid = tag_match && check_tag_range(AvmMemoryTag::U64, resolved_input_offset, KECCAKF1600_INPUT_SIZE); + if (is_valid(error) && + !(tag_match && check_tag_range(AvmMemoryTag::U64, resolved_input_offset, KECCAKF1600_INPUT_SIZE))) { + error = AvmError::TAG_ERROR; + } // Constrain gas cost gas_trace_builder.constrain_gas(clk, OpCode::KECCAKF1600); @@ -3480,7 +3806,7 @@ AvmError AvmTraceBuilder::op_keccakf1600(uint8_t indirect, uint32_t output_offse .main_ind_addr_a = FF(input_read.indirect_address), .main_internal_return_ptr = FF(internal_return_ptr), .main_mem_addr_a = FF(input_read.direct_address), - .main_op_err = FF(static_cast(!op_valid)), + .main_op_err = FF(static_cast(!is_valid(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(AvmMemoryTag::U64)), .main_sel_mem_op_a = FF(1), @@ -3489,8 +3815,8 @@ AvmError AvmTraceBuilder::op_keccakf1600(uint8_t indirect, uint32_t output_offse .main_tag_err = FF(static_cast(!tag_match)), }); - if (!op_valid) { - return AvmError::TAG_ERROR; + if (!is_valid(error)) { + return error; } // Array input is fixed to 1600 bits @@ -3522,32 +3848,43 @@ AvmError AvmTraceBuilder::op_ec_add(uint16_t indirect, uint32_t rhs_is_inf_offset, uint32_t output_offset) { + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto clk = static_cast(main_trace.size()) + 1; + + auto [resolved_addrs, res_error] = Addressing<7>::fromWire(indirect, call_ptr) + .resolve({ lhs_x_offset, + lhs_y_offset, + lhs_is_inf_offset, + rhs_x_offset, + rhs_y_offset, + rhs_is_inf_offset, + output_offset }, + mem_trace_builder); + auto [resolved_lhs_x_offset, resolved_lhs_y_offset, resolved_lhs_is_inf_offset, resolved_rhs_x_offset, resolved_rhs_y_offset, resolved_rhs_is_inf_offset, - resolved_output_offset] = Addressing<7>::fromWire(indirect, call_ptr) - .resolve({ lhs_x_offset, - lhs_y_offset, - lhs_is_inf_offset, - rhs_x_offset, - rhs_y_offset, - rhs_is_inf_offset, - output_offset }, - mem_trace_builder); + resolved_output_offset] = resolved_addrs; + + error = res_error; // Tag checking - bool op_valid = + bool tags_valid = check_tag(AvmMemoryTag::FF, resolved_lhs_x_offset) && check_tag(AvmMemoryTag::FF, resolved_lhs_y_offset) && check_tag(AvmMemoryTag::U1, resolved_lhs_is_inf_offset) && check_tag(AvmMemoryTag::FF, resolved_rhs_x_offset) && check_tag(AvmMemoryTag::FF, resolved_rhs_y_offset) && check_tag(AvmMemoryTag::U1, resolved_rhs_is_inf_offset); + if (is_valid(error) && !tags_valid) { + error = AvmError::TAG_ERROR; + } + gas_trace_builder.constrain_gas(clk, OpCode::ECADD); - if (!op_valid) { + if (!is_valid(error)) { main_trace.push_back(Row{ .main_clk = clk, .main_internal_return_ptr = FF(internal_return_ptr), @@ -3555,7 +3892,7 @@ AvmError AvmTraceBuilder::op_ec_add(uint16_t indirect, .main_pc = FF(pc), .main_sel_op_ecadd = 1, }); - return AvmError::TAG_ERROR; + return error; } // Load lhs point @@ -3602,13 +3939,22 @@ AvmError AvmTraceBuilder::op_variable_msm(uint8_t indirect, uint32_t output_offset, uint32_t point_length_offset) { + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; + auto clk = static_cast(main_trace.size()) + 1; - auto [resolved_points_offset, resolved_scalars_offset, resolved_output_offset, resolved_point_length_offset] = + auto [resolved_addrs, res_error] = Addressing<4>::fromWire(indirect, call_ptr) .resolve({ points_offset, scalars_offset, output_offset, point_length_offset }, mem_trace_builder); + auto [resolved_points_offset, resolved_scalars_offset, resolved_output_offset, resolved_point_length_offset] = + resolved_addrs; + error = res_error; + + if (is_valid(error) && !check_tag(AvmMemoryTag::U32, resolved_point_length_offset)) { + error = AvmError::TAG_ERROR; + } - bool op_valid = check_tag(AvmMemoryTag::U32, resolved_point_length_offset); - const FF points_length = op_valid ? unconstrained_read_from_memory(resolved_point_length_offset) : 0; + const FF points_length = is_valid(error) ? unconstrained_read_from_memory(resolved_point_length_offset) : 0; // Points are stored as [x1, y1, inf1, x2, y2, inf2, ...] with the types [FF, FF, U8, FF, FF, U8, ...] const uint32_t num_points = uint32_t(points_length) / 3; // 3 elements per point @@ -3617,30 +3963,35 @@ AvmError AvmTraceBuilder::op_variable_msm(uint8_t indirect, std::vector points_inf_vec; std::vector scalars_vec; + bool tags_valid = true; for (uint32_t i = 0; i < num_points; i++) { - op_valid = op_valid && check_tag_range(AvmMemoryTag::FF, resolved_points_offset + 3 * i, 2) && - check_tag(AvmMemoryTag::U1, resolved_points_offset + 3 * i + 2); + tags_valid = tags_valid && check_tag_range(AvmMemoryTag::FF, resolved_points_offset + 3 * i, 2) && + check_tag(AvmMemoryTag::U1, resolved_points_offset + 3 * i + 2); } // Scalar read length is num_points* 2 since scalars are stored as lo and hi limbs uint32_t scalar_read_length = num_points * 2; - op_valid = op_valid && check_tag_range(AvmMemoryTag::FF, resolved_scalars_offset, scalar_read_length); + tags_valid = tags_valid && check_tag_range(AvmMemoryTag::FF, resolved_scalars_offset, scalar_read_length); + + if (is_valid(error) && !tags_valid) { + error = AvmError::TAG_ERROR; + } // TODO(dbanks12): length needs to fit into u32 here or it will certainly // run out of gas. Casting/truncating here is not secure. gas_trace_builder.constrain_gas(clk, OpCode::MSM, static_cast(points_length)); - if (!op_valid) { + if (!is_valid(error)) { main_trace.push_back(Row{ .main_clk = clk, .main_internal_return_ptr = FF(internal_return_ptr), - .main_op_err = FF(static_cast(!op_valid)), + .main_op_err = FF(1), .main_pc = FF(pc), .main_sel_op_msm = 1, }); - return AvmError::TAG_ERROR; + return error; } // Loading the points is a bit more complex since we need to read the coordinates and the infinity flags @@ -3729,15 +4080,18 @@ AvmError AvmTraceBuilder::op_to_radix_be(uint8_t indirect, uint32_t num_limbs, uint8_t output_bits) { + // We keep the first encountered error + AvmError error = AvmError::NO_ERROR; auto clk = static_cast(main_trace.size()) + 1; // write output as bits or bytes AvmMemoryTag w_in_tag = output_bits > 0 ? AvmMemoryTag::U1 // bits mode : AvmMemoryTag::U8; - auto [resolved_src_offset, resolved_dst_offset, resolved_radix_offset] = - Addressing<3>::fromWire(indirect, call_ptr) - .resolve({ src_offset, dst_offset, radix_offset }, mem_trace_builder); + auto [resolved_addrs, res_error] = Addressing<3>::fromWire(indirect, call_ptr) + .resolve({ src_offset, dst_offset, radix_offset }, mem_trace_builder); + auto [resolved_src_offset, resolved_dst_offset, resolved_radix_offset] = resolved_addrs; + error = res_error; // Constrain gas cost gas_trace_builder.constrain_gas(clk, OpCode::TORADIXBE, num_limbs); @@ -3749,23 +4103,32 @@ AvmError AvmTraceBuilder::op_to_radix_be(uint8_t indirect, // auto read_radix = constrained_read_from_memory( // call_ptr, clk, resolved_radix_offset, AvmMemoryTag::U32, AvmMemoryTag::U32, IntermRegister::IB); - bool op_valid = check_tag(AvmMemoryTag::U32, resolved_radix_offset); + if (is_valid(error) && !check_tag(AvmMemoryTag::U32, resolved_radix_offset)) { + error = AvmError::TAG_ERROR; + } auto read_radix = unconstrained_read_from_memory(resolved_radix_offset); FF input = read_src.val; + + if (is_valid(error) && !read_src.tag_match) { + error = AvmError::TAG_ERROR; + } + // TODO(8603): uncomment // uint32_t radix = static_cast(read_radix.val); uint32_t radix = static_cast(read_radix); bool radix_out_of_bounds = radix > 256; - bool error = !op_valid || radix_out_of_bounds || !read_src.tag_match; // || !read_radix.tag_match; + if (is_valid(error) && radix_out_of_bounds) { + error = AvmError::RADIX_OUT_OF_BOUNDS; + } // In case of an error, we do not perform the computation. // Therefore, we do not create any entry in gadget table and we return a vector of 0. - std::vector res = error - ? std::vector(num_limbs, 0) - : conversion_trace_builder.op_to_radix_be(input, radix, num_limbs, output_bits, clk); + std::vector res = is_valid(error) + ? conversion_trace_builder.op_to_radix_be(input, radix, num_limbs, output_bits, clk) + : std::vector(num_limbs, 0); // This is the row that contains the selector to trigger the sel_op_radix_be // In this row, we read the input value and the destination address into register A and B respectively @@ -3783,7 +4146,7 @@ AvmError AvmTraceBuilder::op_to_radix_be(uint8_t indirect, .main_mem_addr_a = read_src.direct_address, // TODO(8603): uncomment //.main_mem_addr_b = read_radix.direct_address, - .main_op_err = FF(static_cast(error)), + .main_op_err = FF(static_cast(!is_valid(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(AvmMemoryTag::FF)), .main_sel_mem_op_a = FF(1), @@ -3801,7 +4164,7 @@ AvmError AvmTraceBuilder::op_to_radix_be(uint8_t indirect, // Crucial to perform this operation after having incremented pc because write_slice_to_memory // is implemented with opcodes (SET and JUMP). write_slice_to_memory(resolved_dst_offset, w_in_tag, res); - return error ? AvmError::TAG_ERROR : AvmError::NO_ERROR; + return error; } /************************************************************************************************** diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.hpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.hpp index cfe330881df9..b001aee1f26b 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.hpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.hpp @@ -28,6 +28,11 @@ struct ReturnDataError { AvmError error; }; +struct RowWithError { + Row row; + AvmError error; +}; + // This is the internal context that we keep along the lifecycle of bytecode execution // to iteratively build the whole trace. This is effectively performing witness generation. // At the end of circuit building, mainTrace can be moved to AvmCircuitBuilder by calling @@ -86,19 +91,19 @@ class AvmTraceBuilder { // Execution Environment AvmError op_get_env_var(uint8_t indirect, uint8_t env_var, uint32_t dst_offset); - void op_address(uint8_t indirect, uint32_t dst_offset); - void op_sender(uint8_t indirect, uint32_t dst_offset); - void op_function_selector(uint8_t indirect, uint32_t dst_offset); - void op_transaction_fee(uint8_t indirect, uint32_t dst_offset); - void op_is_static_call(uint8_t indirect, uint32_t dst_offset); + AvmError op_address(uint8_t indirect, uint32_t dst_offset); + AvmError op_sender(uint8_t indirect, uint32_t dst_offset); + AvmError op_function_selector(uint8_t indirect, uint32_t dst_offset); + AvmError op_transaction_fee(uint8_t indirect, uint32_t dst_offset); + AvmError op_is_static_call(uint8_t indirect, uint32_t dst_offset); // Execution Environment - Globals - void op_chain_id(uint8_t indirect, uint32_t dst_offset); - void op_version(uint8_t indirect, uint32_t dst_offset); - void op_block_number(uint8_t indirect, uint32_t dst_offset); - void op_timestamp(uint8_t indirect, uint32_t dst_offset); - void op_fee_per_l2_gas(uint8_t indirect, uint32_t dst_offset); - void op_fee_per_da_gas(uint8_t indirect, uint32_t dst_offset); + AvmError op_chain_id(uint8_t indirect, uint32_t dst_offset); + AvmError op_version(uint8_t indirect, uint32_t dst_offset); + AvmError op_block_number(uint8_t indirect, uint32_t dst_offset); + AvmError op_timestamp(uint8_t indirect, uint32_t dst_offset); + AvmError op_fee_per_l2_gas(uint8_t indirect, uint32_t dst_offset); + AvmError op_fee_per_da_gas(uint8_t indirect, uint32_t dst_offset); // Execution Environment - Calldata AvmError op_calldata_copy(uint8_t indirect, @@ -112,8 +117,8 @@ class AvmTraceBuilder { uint32_t dst_offset); // Machine State - Gas - void op_l2gasleft(uint8_t indirect, uint32_t dst_offset); - void op_dagasleft(uint8_t indirect, uint32_t dst_offset); + AvmError op_l2gasleft(uint8_t indirect, uint32_t dst_offset); + AvmError op_dagasleft(uint8_t indirect, uint32_t dst_offset); // Machine State - Internal Control Flow // TODO(8945): skip_gas boolean is temporary and should be removed once all fake rows are removed @@ -265,16 +270,16 @@ class AvmTraceBuilder { AvmRangeCheckBuilder range_check_builder; AvmBytecodeTraceBuilder bytecode_trace_builder; - Row create_kernel_lookup_opcode(uint8_t indirect, uint32_t dst_offset, FF value, AvmMemoryTag w_tag); + RowWithError create_kernel_lookup_opcode(uint8_t indirect, uint32_t dst_offset, FF value, AvmMemoryTag w_tag); - Row create_kernel_output_opcode(uint8_t indirect, uint32_t clk, uint32_t data_offset); + RowWithError create_kernel_output_opcode(uint8_t indirect, uint32_t clk, uint32_t data_offset); - Row create_kernel_output_opcode_with_metadata(uint8_t indirect, - uint32_t clk, - uint32_t data_offset, - AvmMemoryTag data_r_tag, - uint32_t metadata_offset, - AvmMemoryTag metadata_r_tag); + RowWithError create_kernel_output_opcode_with_metadata(uint8_t indirect, + uint32_t clk, + uint32_t data_offset, + AvmMemoryTag data_r_tag, + uint32_t metadata_offset, + AvmMemoryTag metadata_r_tag); Row create_kernel_output_opcode_with_set_metadata_output_from_hint(uint32_t clk, uint32_t data_offset, @@ -286,10 +291,10 @@ class AvmTraceBuilder { uint32_t leaf_index, uint32_t metadata_offset); - Row create_kernel_output_opcode_with_set_value_from_hint(uint8_t indirect, - uint32_t clk, - uint32_t data_offset, - uint32_t metadata_offset); + RowWithError create_kernel_output_opcode_with_set_value_from_hint(uint8_t indirect, + uint32_t clk, + uint32_t data_offset, + uint32_t metadata_offset); AvmError constrain_external_call(OpCode opcode, uint16_t indirect, @@ -299,7 +304,7 @@ class AvmTraceBuilder { uint32_t args_size_offset, uint32_t success_offset); - void execute_gasleft(EnvironmentVariable var, uint8_t indirect, uint32_t dst_offset); + AvmError execute_gasleft(EnvironmentVariable var, uint8_t indirect, uint32_t dst_offset); void finalise_mem_trace_lookup_counts(); diff --git a/yarn-project/simulator/src/avm/errors.ts b/yarn-project/simulator/src/avm/errors.ts index f9c5d819a770..8df7a7d73036 100644 --- a/yarn-project/simulator/src/avm/errors.ts +++ b/yarn-project/simulator/src/avm/errors.ts @@ -67,6 +67,17 @@ export class TagCheckError extends AvmExecutionError { } } +/** + * Error is thrown when a relative memory address resolved to an offset which + * is out of range, i.e, greater than maxUint32. + */ +export class AddressOutOfRangeError extends AvmExecutionError { + constructor(baseAddr: number, relOffset: number) { + super(`Address out of range. Base address ${baseAddr}, relative offset ${relOffset}`); + this.name = 'AddressOutOfRangeError'; + } +} + /** Error thrown when out of gas. */ export class OutOfGasError extends AvmExecutionError { constructor(dimensions: string[]) { diff --git a/yarn-project/simulator/src/avm/opcodes/addressing_mode.ts b/yarn-project/simulator/src/avm/opcodes/addressing_mode.ts index 2d054f7a2996..b7dd6163fff4 100644 --- a/yarn-project/simulator/src/avm/opcodes/addressing_mode.ts +++ b/yarn-project/simulator/src/avm/opcodes/addressing_mode.ts @@ -1,6 +1,8 @@ import { strict as assert } from 'assert'; +import { maxUint32 } from 'viem'; import { type TaggedMemoryInterface } from '../avm_memory_types.js'; +import { AddressOutOfRangeError } from '../errors.js'; export enum AddressingMode { DIRECT = 0, @@ -63,7 +65,11 @@ export class Addressing { resolved[i] = offset; if (mode & AddressingMode.RELATIVE) { mem.checkIsValidMemoryOffsetTag(0); - resolved[i] += Number(mem.get(0).toBigInt()); + const baseAddr = Number(mem.get(0).toBigInt()); + resolved[i] += baseAddr; + if (resolved[i] > maxUint32) { + throw new AddressOutOfRangeError(baseAddr, offset); + } } if (mode & AddressingMode.INDIRECT) { mem.checkIsValidMemoryOffsetTag(resolved[i]); From 8f69116ec7967e1a49bf114a78733d41fafeae1b Mon Sep 17 00:00:00 2001 From: jeanmon Date: Fri, 15 Nov 2024 13:47:46 +0000 Subject: [PATCH 3/5] Update some comments --- .../cpp/src/barretenberg/vm/avm/tests/cast.test.cpp | 2 +- .../src/barretenberg/vm/avm/tests/mem_opcodes.test.cpp | 8 ++++---- .../cpp/src/barretenberg/vm/avm/tests/slice.test.cpp | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/tests/cast.test.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/tests/cast.test.cpp index 1dd11e5bf6c6..425fa18f89cb 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/tests/cast.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/tests/cast.test.cpp @@ -242,7 +242,7 @@ TEST_F(AvmCastTests, indirectAddrTruncationU64ToU8) TEST_F(AvmCastTests, indirectAddrWrongResolutionU64ToU8) { - // TODO(#9131): Re-enable as part of #9131 + // TODO(#9995): Re-enable as part of #9995 GTEST_SKIP(); // Indirect addresses. src:5 dst:6 // Direct addresses. src:10 dst:11 diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/tests/mem_opcodes.test.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/tests/mem_opcodes.test.cpp index 40a0d822d24d..50fb1ef79343 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/tests/mem_opcodes.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/tests/mem_opcodes.test.cpp @@ -228,7 +228,7 @@ TEST_F(AvmMemOpcodeTests, uninitializedValueMov) TEST_F(AvmMemOpcodeTests, indUninitializedValueMov) { - // TODO(#9131): Re-enable once we have error handling on wrong address resolution + // TODO(#9995): Re-enable once we have error handling on wrong address resolution GTEST_SKIP(); trace_builder.op_set(0, 1, 3, AvmMemoryTag::U32); @@ -244,7 +244,7 @@ TEST_F(AvmMemOpcodeTests, indUninitializedValueMov) TEST_F(AvmMemOpcodeTests, indUninitializedAddrMov) { - // TODO(#9131): Re-enable once we have error handling on wrong address resolution + // TODO(#9995): Re-enable once we have error handling on wrong address resolution GTEST_SKIP(); trace_builder.op_set(0, 1, 3, AvmMemoryTag::U32); @@ -268,7 +268,7 @@ TEST_F(AvmMemOpcodeTests, indirectMov) TEST_F(AvmMemOpcodeTests, indirectMovInvalidAddressTag) { - // TODO(#9131): Re-enable once we have error handling on wrong address resolution + // TODO(#9995): Re-enable once we have error handling on wrong address resolution GTEST_SKIP(); trace_builder.op_set(0, 15, 100, AvmMemoryTag::U32); @@ -369,7 +369,7 @@ TEST_F(AvmMemOpcodeTests, indirectSet) TEST_F(AvmMemOpcodeTests, indirectSetWrongTag) { - // TODO(#9131): Re-enable once we have error handling on wrong address resolution + // TODO(#9995): Re-enable once we have error handling on wrong address resolution GTEST_SKIP(); trace_builder.op_set(0, 100, 10, AvmMemoryTag::U8); // The address 100 has incorrect tag U8. diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/tests/slice.test.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/tests/slice.test.cpp index d3a3c8883ec8..dcd21e6799b5 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/tests/slice.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/tests/slice.test.cpp @@ -245,7 +245,7 @@ TEST_F(AvmSliceTests, indirectTwoCallsOverlap) TEST_F(AvmSliceTests, indirectFailedResolution) { - // TODO(#9131): Re-enable as part of #9131 + // TODO(#9995): Re-enable as part of #9995 GTEST_SKIP(); gen_trace_builder({ 2, 3, 4, 5, 6 }); From 7452e0d1cf6dbb41953851b6a61d0b3522488f49 Mon Sep 17 00:00:00 2001 From: jeanmon Date: Fri, 15 Nov 2024 16:05:05 +0000 Subject: [PATCH 4/5] 9131: Addressing review feedback --- .../vm/avm/trace/addressing_mode.hpp | 2 +- .../src/barretenberg/vm/avm/trace/common.hpp | 12 - .../src/barretenberg/vm/avm/trace/errors.hpp | 19 ++ .../src/barretenberg/vm/avm/trace/helper.cpp | 4 +- .../src/barretenberg/vm/avm/trace/helper.hpp | 2 +- .../src/barretenberg/vm/avm/trace/trace.cpp | 258 +++++++++--------- .../simulator/src/avm/avm_memory_types.ts | 24 +- .../src/avm/opcodes/addressing_mode.ts | 3 +- 8 files changed, 168 insertions(+), 156 deletions(-) create mode 100644 barretenberg/cpp/src/barretenberg/vm/avm/trace/errors.hpp diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/addressing_mode.hpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/addressing_mode.hpp index d6f044986da3..0218372b5217 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/addressing_mode.hpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/addressing_mode.hpp @@ -1,6 +1,6 @@ #pragma once -#include "barretenberg/vm/avm/trace/common.hpp" +#include "barretenberg/vm/avm/trace/errors.hpp" #include "barretenberg/vm/avm/trace/mem_trace.hpp" #include diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/common.hpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/common.hpp index 0d8bc15bbc82..fd5de470550e 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/common.hpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/common.hpp @@ -49,18 +49,6 @@ enum class AvmMemoryTag : uint32_t { static const uint32_t MAX_MEM_TAG = MEM_TAG_U128; -enum class AvmError : uint32_t { - NO_ERROR, - TAG_ERROR, - ADDR_RES_TAG_ERROR, - REL_ADDR_OUT_OF_RANGE, - DIV_ZERO, - PARSING_ERROR, - ENV_VAR_UNKNOWN, - CONTRACT_INST_MEM_UNKNOWN, - RADIX_OUT_OF_BOUNDS, -}; - static const size_t NUM_MEM_SPACES = 256; static const uint8_t INTERNAL_CALL_SPACE_ID = 255; static const uint32_t MAX_SIZE_INTERNAL_STACK = 1 << 16; diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/errors.hpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/errors.hpp new file mode 100644 index 000000000000..157f0952ab05 --- /dev/null +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/errors.hpp @@ -0,0 +1,19 @@ +#pragma once + +#include + +namespace bb::avm_trace { + +enum class AvmError : uint32_t { + NO_ERROR, + TAG_ERROR, + ADDR_RES_TAG_ERROR, + REL_ADDR_OUT_OF_RANGE, + DIV_ZERO, + PARSING_ERROR, + ENV_VAR_UNKNOWN, + CONTRACT_INST_MEM_UNKNOWN, + RADIX_OUT_OF_BOUNDS, +}; + +} // namespace bb::avm_trace \ No newline at end of file diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/helper.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/helper.cpp index 196bff2317d5..63a6662c2d39 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/helper.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/helper.cpp @@ -118,13 +118,15 @@ std::string to_name(AvmError error) return "ENVIRONMENT VARIABLE UNKNOWN"; case AvmError::CONTRACT_INST_MEM_UNKNOWN: return "CONTRACT INSTANCE MEMBER UNKNOWN"; + case AvmError::RADIX_OUT_OF_BOUNDS: + return "RADIX OUT OF BOUNDS"; default: throw std::runtime_error("Invalid error type"); break; } } -bool is_valid(AvmError error) +bool is_ok(AvmError error) { return error == AvmError::NO_ERROR; } diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/helper.hpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/helper.hpp index 1f92386dc660..5f4fdaea409f 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/helper.hpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/helper.hpp @@ -233,7 +233,7 @@ std::string to_hex(bb::avm_trace::AvmMemoryTag tag); std::string to_name(bb::avm_trace::AvmMemoryTag tag); std::string to_name(AvmError error); -bool is_valid(AvmError error); +bool is_ok(AvmError error); // Mutate the inputs void inject_end_gas_values(VmPublicInputs& public_inputs, std::vector& trace); diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp index 10585405acca..21c17268bb48 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp @@ -346,7 +346,7 @@ AvmError AvmTraceBuilder::op_add( auto read_b = constrained_read_from_memory(call_ptr, clk, resolved_b, in_tag, in_tag, IntermRegister::IB); bool tag_match = read_a.tag_match && read_b.tag_match; - if (is_valid(error) && !tag_match) { + if (is_ok(error) && !tag_match) { error = AvmError::TAG_ERROR; } @@ -357,7 +357,7 @@ AvmError AvmTraceBuilder::op_add( // In case of a memory tag error, we do not perform the computation. // Therefore, we do not create any entry in ALU table and store the value 0 as // output (c) in memory. - FF c = is_valid(error) ? alu_trace_builder.op_add(a, b, in_tag, clk) : FF(0); + FF c = is_ok(error) ? alu_trace_builder.op_add(a, b, in_tag, clk) : FF(0); // Write into memory value c from intermediate register ic. auto write_c = constrained_write_to_memory(call_ptr, clk, resolved_c, c, in_tag, in_tag, IntermRegister::IC); @@ -379,7 +379,7 @@ AvmError AvmTraceBuilder::op_add( .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), - .main_op_err = FF(static_cast(!is_valid(error))), + .main_op_err = FF(static_cast(!is_ok(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), @@ -429,7 +429,7 @@ AvmError AvmTraceBuilder::op_sub( auto read_b = constrained_read_from_memory(call_ptr, clk, resolved_b, in_tag, in_tag, IntermRegister::IB); bool tag_match = read_a.tag_match && read_b.tag_match; - if (is_valid(error) && !tag_match) { + if (is_ok(error) && !tag_match) { error = AvmError::TAG_ERROR; } @@ -440,7 +440,7 @@ AvmError AvmTraceBuilder::op_sub( // In case of a memory tag error, we do not perform the computation. // Therefore, we do not create any entry in ALU table and store the value 0 as // output (c) in memory. - FF c = is_valid(error) ? alu_trace_builder.op_sub(a, b, in_tag, clk) : FF(0); + FF c = is_ok(error) ? alu_trace_builder.op_sub(a, b, in_tag, clk) : FF(0); // Write into memory value c from intermediate register ic. auto write_c = constrained_write_to_memory(call_ptr, clk, resolved_c, c, in_tag, in_tag, IntermRegister::IC); @@ -462,7 +462,7 @@ AvmError AvmTraceBuilder::op_sub( .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), - .main_op_err = FF(static_cast(!is_valid(error))), + .main_op_err = FF(static_cast(!is_ok(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), @@ -510,7 +510,7 @@ AvmError AvmTraceBuilder::op_mul( auto read_b = constrained_read_from_memory(call_ptr, clk, resolved_b, in_tag, in_tag, IntermRegister::IB); bool tag_match = read_a.tag_match && read_b.tag_match; - if (is_valid(error) && !tag_match) { + if (is_ok(error) && !tag_match) { error = AvmError::TAG_ERROR; } @@ -521,7 +521,7 @@ AvmError AvmTraceBuilder::op_mul( // In case of a memory tag error, we do not perform the computation. // Therefore, we do not create any entry in ALU table and store the value 0 as // output (c) in memory. - FF c = is_valid(error) ? alu_trace_builder.op_mul(a, b, in_tag, clk) : FF(0); + FF c = is_ok(error) ? alu_trace_builder.op_mul(a, b, in_tag, clk) : FF(0); // Write into memory value c from intermediate register ic. auto write_c = constrained_write_to_memory(call_ptr, clk, resolved_c, c, in_tag, in_tag, IntermRegister::IC); @@ -543,7 +543,7 @@ AvmError AvmTraceBuilder::op_mul( .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), - .main_op_err = FF(static_cast(!is_valid(error))), + .main_op_err = FF(static_cast(!is_ok(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), @@ -592,7 +592,7 @@ AvmError AvmTraceBuilder::op_div( 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_valid(error) && !(tag_match && check_tag_integral(read_a.tag))) { + if (is_ok(error) && !(tag_match && check_tag_integral(read_a.tag))) { error = AvmError::TAG_ERROR; } @@ -609,11 +609,11 @@ AvmError AvmTraceBuilder::op_div( if (!b.is_zero()) { // If b is not zero, we prove it is not by providing its inverse as well inv = b.invert(); - c = is_valid(error) ? alu_trace_builder.op_div(a, b, in_tag, clk) : FF(0); + c = is_ok(error) ? alu_trace_builder.op_div(a, b, in_tag, clk) : FF(0); } else { inv = 1; c = 0; - if (is_valid(error)) { + if (is_ok(error)) { error = AvmError::DIV_ZERO; } } @@ -639,7 +639,7 @@ AvmError AvmTraceBuilder::op_div( .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_dst.direct_address), - .main_op_err = FF(static_cast(!is_valid(error))), + .main_op_err = FF(static_cast(!is_ok(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), @@ -688,7 +688,7 @@ AvmError AvmTraceBuilder::op_fdiv( constrained_read_from_memory(call_ptr, clk, resolved_b, AvmMemoryTag::FF, AvmMemoryTag::FF, IntermRegister::IB); bool tag_match = read_a.tag_match && read_b.tag_match; - if (is_valid(error) && !tag_match) { + if (is_ok(error) && !tag_match) { error = AvmError::TAG_ERROR; } @@ -704,7 +704,7 @@ AvmError AvmTraceBuilder::op_fdiv( } else { inv = 1; c = 0; - if (is_valid(error)) { + if (is_ok(error)) { error = AvmError::DIV_ZERO; } } @@ -730,7 +730,7 @@ AvmError AvmTraceBuilder::op_fdiv( .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), - .main_op_err = FF(static_cast(!is_valid(error))), + .main_op_err = FF(static_cast(!is_ok(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(AvmMemoryTag::FF)), .main_rwc = FF(1), @@ -782,7 +782,7 @@ AvmError AvmTraceBuilder::op_eq( auto read_b = constrained_read_from_memory(call_ptr, clk, resolved_b, in_tag, AvmMemoryTag::U1, IntermRegister::IB); bool tag_match = read_a.tag_match && read_b.tag_match; - if (is_valid(error) && !tag_match) { + if (is_ok(error) && !tag_match) { error = AvmError::TAG_ERROR; } @@ -792,7 +792,7 @@ AvmError AvmTraceBuilder::op_eq( // In case of a memory tag error, we do not perform the computation. // Therefore, we do not create any entry in ALU table and store the value 0 as // output (c) in memory. - FF c = is_valid(error) ? alu_trace_builder.op_eq(a, b, in_tag, clk) : FF(0); + FF c = is_ok(error) ? alu_trace_builder.op_eq(a, b, in_tag, clk) : FF(0); // Write into memory value c from intermediate register ic. auto write_c = @@ -815,7 +815,7 @@ AvmError AvmTraceBuilder::op_eq( .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), - .main_op_err = FF(static_cast(!is_valid(error))), + .main_op_err = FF(static_cast(!is_ok(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), @@ -853,14 +853,14 @@ AvmError AvmTraceBuilder::op_lt( auto read_b = constrained_read_from_memory(call_ptr, clk, resolved_b, in_tag, AvmMemoryTag::U1, IntermRegister::IB); bool tag_match = read_a.tag_match && read_b.tag_match; - if (is_valid(error) && !tag_match) { + if (is_ok(error) && !tag_match) { error = AvmError::TAG_ERROR; } FF a = tag_match ? read_a.val : FF(0); FF b = tag_match ? read_b.val : FF(0); - FF c = is_valid(error) ? alu_trace_builder.op_lt(a, b, in_tag, clk) : FF(0); + FF c = is_ok(error) ? alu_trace_builder.op_lt(a, b, in_tag, clk) : FF(0); // Write into memory value c from intermediate register ic. auto write_c = @@ -883,7 +883,7 @@ AvmError AvmTraceBuilder::op_lt( .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), - .main_op_err = FF(static_cast(!is_valid(error))), + .main_op_err = FF(static_cast(!is_ok(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), @@ -922,14 +922,14 @@ AvmError AvmTraceBuilder::op_lte( auto read_b = constrained_read_from_memory(call_ptr, clk, resolved_b, in_tag, AvmMemoryTag::U1, IntermRegister::IB); bool tag_match = read_a.tag_match && read_b.tag_match; - if (is_valid(error) && !tag_match) { + if (is_ok(error) && !tag_match) { error = AvmError::TAG_ERROR; } FF a = tag_match ? read_a.val : FF(0); FF b = tag_match ? read_b.val : FF(0); - FF c = is_valid(error) ? alu_trace_builder.op_lte(a, b, in_tag, clk) : FF(0); + FF c = is_ok(error) ? alu_trace_builder.op_lte(a, b, in_tag, clk) : FF(0); // Write into memory value c from intermediate register ic. auto write_c = @@ -952,7 +952,7 @@ AvmError AvmTraceBuilder::op_lte( .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), - .main_op_err = FF(static_cast(!is_valid(error))), + .main_op_err = FF(static_cast(!is_ok(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), @@ -996,14 +996,14 @@ 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_valid(error) && !(tag_match && check_tag_integral(read_a.tag))) { + if (is_ok(error) && !(tag_match && check_tag_integral(read_a.tag))) { error = AvmError::TAG_ERROR; } FF a = tag_match ? read_a.val : FF(0); FF b = tag_match ? read_b.val : FF(0); - FF c = is_valid(error) ? bin_trace_builder.op_and(a, b, in_tag, clk) : FF(0); + FF c = is_ok(error) ? bin_trace_builder.op_and(a, b, in_tag, clk) : FF(0); // Write into memory value c from intermediate register ic. auto write_c = constrained_write_to_memory(call_ptr, clk, resolved_c, c, in_tag, in_tag, IntermRegister::IC); @@ -1025,11 +1025,11 @@ AvmError AvmTraceBuilder::op_and( .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), - .main_op_err = FF(static_cast(!is_valid(error))), + .main_op_err = FF(static_cast(!is_ok(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), - .main_sel_bin = FF(static_cast(is_valid(error))), + .main_sel_bin = FF(static_cast(is_ok(error))), .main_sel_mem_op_a = FF(1), .main_sel_mem_op_b = FF(1), .main_sel_mem_op_c = FF(1), @@ -1066,14 +1066,14 @@ 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_valid(error) && !(tag_match && check_tag_integral(read_a.tag))) { + if (is_ok(error) && !(tag_match && check_tag_integral(read_a.tag))) { error = AvmError::TAG_ERROR; } FF a = tag_match ? read_a.val : FF(0); FF b = tag_match ? read_b.val : FF(0); - FF c = is_valid(error) ? bin_trace_builder.op_or(a, b, in_tag, clk) : FF(0); + FF c = is_ok(error) ? bin_trace_builder.op_or(a, b, in_tag, clk) : FF(0); // Write into memory value c from intermediate register ic. auto write_c = constrained_write_to_memory(call_ptr, clk, resolved_c, c, in_tag, in_tag, IntermRegister::IC); @@ -1095,11 +1095,11 @@ AvmError AvmTraceBuilder::op_or( .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), - .main_op_err = FF(static_cast(!is_valid(error))), + .main_op_err = FF(static_cast(!is_ok(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), - .main_sel_bin = FF(static_cast(is_valid(error))), + .main_sel_bin = FF(static_cast(is_ok(error))), .main_sel_mem_op_a = FF(1), .main_sel_mem_op_b = FF(1), .main_sel_mem_op_c = FF(1), @@ -1136,14 +1136,14 @@ 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_valid(error) && !(tag_match && check_tag_integral(read_a.tag))) { + if (is_ok(error) && !(tag_match && check_tag_integral(read_a.tag))) { error = AvmError::TAG_ERROR; } FF a = tag_match ? read_a.val : FF(0); FF b = tag_match ? read_b.val : FF(0); - FF c = is_valid(error) ? bin_trace_builder.op_xor(a, b, in_tag, clk) : FF(0); + FF c = is_ok(error) ? bin_trace_builder.op_xor(a, b, in_tag, clk) : FF(0); // Write into memory value c from intermediate register ic. auto write_c = constrained_write_to_memory(call_ptr, clk, resolved_c, c, in_tag, in_tag, IntermRegister::IC); @@ -1165,11 +1165,11 @@ AvmError AvmTraceBuilder::op_xor( .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), - .main_op_err = FF(static_cast(!is_valid(error))), + .main_op_err = FF(static_cast(!is_ok(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), - .main_sel_bin = FF(static_cast(is_valid(error))), + .main_sel_bin = FF(static_cast(is_ok(error))), .main_sel_mem_op_a = FF(1), .main_sel_mem_op_b = FF(1), .main_sel_mem_op_c = FF(1), @@ -1210,7 +1210,7 @@ AvmError AvmTraceBuilder::op_not(uint8_t indirect, uint32_t a_offset, uint32_t d // Reading from memory and loading into ia auto read_a = constrained_read_from_memory(call_ptr, clk, resolved_a, in_tag, in_tag, IntermRegister::IA); - if (is_valid(error) && !check_tag_integral(read_a.tag)) { + if (is_ok(error) && !check_tag_integral(read_a.tag)) { error = AvmError::TAG_ERROR; } @@ -1220,7 +1220,7 @@ AvmError AvmTraceBuilder::op_not(uint8_t indirect, uint32_t a_offset, uint32_t d // In case of an error (tag of type FF), we do not perform the computation. // Therefore, we do not create any entry in ALU table and store the value 0 as // output (c) in memory. - FF c = is_valid(error) ? alu_trace_builder.op_not(a, in_tag, clk) : FF(0); + FF c = is_ok(error) ? alu_trace_builder.op_not(a, in_tag, clk) : FF(0); // Write into memory value c from intermediate register ic. auto write_c = constrained_write_to_memory(call_ptr, clk, resolved_c, c, in_tag, in_tag, IntermRegister::IC); @@ -1239,7 +1239,7 @@ AvmError AvmTraceBuilder::op_not(uint8_t indirect, uint32_t a_offset, uint32_t d .main_internal_return_ptr = FF(internal_return_ptr), .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_c = FF(write_c.direct_address), - .main_op_err = FF(static_cast(!is_valid(error))), + .main_op_err = FF(static_cast(!is_ok(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), @@ -1277,14 +1277,14 @@ AvmError AvmTraceBuilder::op_shl( // IntermRegister::IB); bool tag_match = read_a.tag_match && read_b.tag_match; auto read_b = unconstrained_read_from_memory(resolved_b); - if (is_valid(error) && !(check_tag_integral(read_a.tag) && check_tag(AvmMemoryTag::U8, resolved_b))) { + if (is_ok(error) && !(check_tag_integral(read_a.tag) && check_tag(AvmMemoryTag::U8, resolved_b))) { error = AvmError::TAG_ERROR; } - FF a = is_valid(error) ? read_a.val : FF(0); - FF b = is_valid(error) ? read_b : FF(0); + FF a = is_ok(error) ? read_a.val : FF(0); + FF b = is_ok(error) ? read_b : FF(0); - FF c = is_valid(error) ? alu_trace_builder.op_shl(a, b, in_tag, clk) : FF(0); + FF c = is_ok(error) ? alu_trace_builder.op_shl(a, b, in_tag, clk) : FF(0); // Write into memory value c from intermediate register ic. auto write_c = constrained_write_to_memory(call_ptr, clk, resolved_c, c, in_tag, in_tag, IntermRegister::IC); @@ -1305,7 +1305,7 @@ AvmError AvmTraceBuilder::op_shl( .main_mem_addr_a = FF(read_a.direct_address), //.main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), - .main_op_err = FF(static_cast(!is_valid(error))), + .main_op_err = FF(static_cast(!is_ok(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), @@ -1344,14 +1344,14 @@ AvmError AvmTraceBuilder::op_shr( // auto read_b = constrained_read_from_memory(call_ptr, clk, resolved_b, AvmMemoryTag::U8, AvmMemoryTag::U8, // IntermRegister::IB); bool tag_match = read_a.tag_match && read_b.tag_match; auto read_b = unconstrained_read_from_memory(resolved_b); - if (is_valid(error) && !(check_tag_integral(read_a.tag) && check_tag(AvmMemoryTag::U8, resolved_b))) { + if (is_ok(error) && !(check_tag_integral(read_a.tag) && check_tag(AvmMemoryTag::U8, resolved_b))) { error = AvmError::TAG_ERROR; } - FF a = is_valid(error) ? read_a.val : FF(0); - FF b = is_valid(error) ? read_b : FF(0); + FF a = is_ok(error) ? read_a.val : FF(0); + FF b = is_ok(error) ? read_b : FF(0); - FF c = is_valid(error) ? alu_trace_builder.op_shr(a, b, in_tag, clk) : FF(0); + FF c = is_ok(error) ? alu_trace_builder.op_shr(a, b, in_tag, clk) : FF(0); // Write into memory value c from intermediate register ic. auto write_c = constrained_write_to_memory(call_ptr, clk, resolved_c, c, in_tag, in_tag, IntermRegister::IC); @@ -1374,7 +1374,7 @@ AvmError AvmTraceBuilder::op_shr( // TODO(8603): uncomment //.main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), - .main_op_err = FF(static_cast(!is_valid(error))), + .main_op_err = FF(static_cast(!is_ok(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), @@ -1441,7 +1441,7 @@ AvmError AvmTraceBuilder::op_cast( .main_internal_return_ptr = FF(internal_return_ptr), .main_mem_addr_a = FF(resolved_a), .main_mem_addr_c = FF(resolved_c), - .main_op_err = FF(static_cast(!is_valid(res_error))), + .main_op_err = FF(static_cast(!is_ok(res_error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(memEntry.tag)), .main_rwc = FF(1), @@ -1495,7 +1495,7 @@ RowWithError AvmTraceBuilder::create_kernel_lookup_opcode(uint8_t indirect, .main_ind_addr_a = FF(write_dst.indirect_address), .main_internal_return_ptr = internal_return_ptr, .main_mem_addr_a = FF(write_dst.direct_address), - .main_op_err = FF(static_cast(!is_valid(res_error))), + .main_op_err = FF(static_cast(!is_ok(res_error))), .main_pc = pc, .main_rwa = 1, .main_sel_mem_op_a = 1, @@ -1776,8 +1776,8 @@ AvmError AvmTraceBuilder::op_calldata_copy(uint8_t indirect, // This boolean will not be a trivial constant anymore once we constrain address resolution. bool tag_match = true; - if (is_valid(error) && !(check_tag(AvmMemoryTag::U32, cd_offset_resolved) && - check_tag(AvmMemoryTag::U32, copy_size_offset_resolved))) { + if (is_ok(error) && !(check_tag(AvmMemoryTag::U32, cd_offset_resolved) && + check_tag(AvmMemoryTag::U32, copy_size_offset_resolved))) { error = AvmError::TAG_ERROR; } @@ -1785,7 +1785,7 @@ AvmError AvmTraceBuilder::op_calldata_copy(uint8_t indirect, const uint32_t cd_offset = static_cast(unconstrained_read_from_memory(cd_offset_resolved)); const uint32_t copy_size = static_cast(unconstrained_read_from_memory(copy_size_offset_resolved)); - if (is_valid(error)) { + if (is_ok(error)) { slice_trace_builder.create_calldata_copy_slice( calldata, clk, call_ptr, cd_offset, copy_size, dst_offset_resolved); mem_trace_builder.write_calldata_copy(calldata, clk, call_ptr, cd_offset, copy_size, dst_offset_resolved); @@ -1801,11 +1801,11 @@ AvmError AvmTraceBuilder::op_calldata_copy(uint8_t indirect, .main_ib = copy_size, .main_internal_return_ptr = FF(internal_return_ptr), .main_mem_addr_c = dst_offset_resolved, - .main_op_err = static_cast(!is_valid(error)), + .main_op_err = static_cast(!is_ok(error)), .main_pc = pc, .main_r_in_tag = static_cast(AvmMemoryTag::FF), .main_sel_op_calldata_copy = 1, - .main_sel_slice_gadget = static_cast(is_valid(error)), + .main_sel_slice_gadget = static_cast(is_ok(error)), .main_tag_err = static_cast(!tag_match), .main_w_in_tag = static_cast(AvmMemoryTag::FF), }); @@ -1827,7 +1827,7 @@ AvmError AvmTraceBuilder::op_returndata_size(uint8_t indirect, uint32_t dst_offs auto [resolved_dst_offset] = resolved_addrs; error = res_error; - if (is_valid(error) && !tag_match) { + if (is_ok(error) && !tag_match) { error = AvmError::TAG_ERROR; } @@ -1842,7 +1842,7 @@ AvmError AvmTraceBuilder::op_returndata_size(uint8_t indirect, uint32_t dst_offs .main_clk = clk, .main_call_ptr = call_ptr, .main_internal_return_ptr = FF(internal_return_ptr), - .main_op_err = FF(static_cast(!is_valid(error))), + .main_op_err = FF(static_cast(!is_ok(error))), .main_pc = FF(pc), .main_sel_op_returndata_size = FF(1), .main_tag_err = FF(static_cast(!tag_match)), @@ -1870,8 +1870,8 @@ AvmError AvmTraceBuilder::op_returndata_copy(uint8_t indirect, // This boolean will not be a trivial constant anymore once we constrain address resolution. bool tag_match = true; - if (is_valid(error) && !(check_tag(AvmMemoryTag::U32, rd_offset_resolved) && - check_tag(AvmMemoryTag::U32, copy_size_offset_resolved))) { + if (is_ok(error) && !(check_tag(AvmMemoryTag::U32, rd_offset_resolved) && + check_tag(AvmMemoryTag::U32, copy_size_offset_resolved))) { error = AvmError::TAG_ERROR; } @@ -1886,13 +1886,13 @@ AvmError AvmTraceBuilder::op_returndata_copy(uint8_t indirect, main_trace.push_back(Row{ .main_clk = clk, .main_internal_return_ptr = FF(internal_return_ptr), - .main_op_err = static_cast(!is_valid(error)), + .main_op_err = static_cast(!is_ok(error)), .main_pc = FF(pc), .main_sel_op_returndata_copy = FF(1), .main_tag_err = FF(static_cast(!tag_match)), }); - if (is_valid(error)) { + if (is_ok(error)) { // Write the return data to memory // TODO: validate bounds auto returndata_slice = @@ -1945,7 +1945,7 @@ AvmError AvmTraceBuilder::execute_gasleft(EnvironmentVariable var, uint8_t indir .main_ind_addr_a = FF(write_dst.indirect_address), .main_internal_return_ptr = FF(internal_return_ptr), .main_mem_addr_a = FF(write_dst.direct_address), - .main_op_err = FF(static_cast(!is_valid(res_error))), + .main_op_err = FF(static_cast(!is_ok(res_error))), .main_pc = FF(pc), .main_rwa = FF(1), .main_sel_mem_op_a = FF(1), @@ -2028,7 +2028,7 @@ AvmError AvmTraceBuilder::op_jumpi(uint8_t indirect, uint32_t jmp_dest, uint32_t auto [resolved_cond_offset] = resolved_addrs; error = res_error; - if (is_valid(error) && !tag_match) { + if (is_ok(error) && !tag_match) { error = AvmError::TAG_ERROR; } @@ -2051,7 +2051,7 @@ AvmError AvmTraceBuilder::op_jumpi(uint8_t indirect, uint32_t jmp_dest, uint32_t .main_internal_return_ptr = FF(internal_return_ptr), .main_inv = inv, .main_mem_addr_d = resolved_cond_offset, - .main_op_err = static_cast(!is_valid(error)), + .main_op_err = static_cast(!is_ok(error)), .main_pc = FF(pc), .main_r_in_tag = static_cast(read_d.tag), .main_sel_mem_op_d = 1, @@ -2189,7 +2189,7 @@ AvmError AvmTraceBuilder::op_set( auto write_c = constrained_write_to_memory( call_ptr, clk, resolved_dst_offset, val_ff, AvmMemoryTag::FF, in_tag, IntermRegister::IC); - if (is_valid(error) && !write_c.tag_match) { + if (is_ok(error) && !write_c.tag_match) { error = AvmError::TAG_ERROR; } @@ -2206,7 +2206,7 @@ AvmError AvmTraceBuilder::op_set( .main_ind_addr_c = FF(write_c.indirect_address), .main_internal_return_ptr = internal_return_ptr, .main_mem_addr_c = FF(write_c.direct_address), - .main_op_err = static_cast(!is_valid(error)), + .main_op_err = static_cast(!is_ok(error)), .main_pc = pc, .main_rwc = 1, .main_sel_mem_op_c = 1, @@ -2245,7 +2245,7 @@ AvmError AvmTraceBuilder::op_mov(uint8_t indirect, uint32_t src_offset, uint32_t auto [resolved_src_offset, resolved_dst_offset] = resolved_addrs; error = res_error; - if (is_valid(error) && !tag_match) { + if (is_ok(error) && !tag_match) { error = AvmError::TAG_ERROR; } @@ -2267,7 +2267,7 @@ AvmError AvmTraceBuilder::op_mov(uint8_t indirect, uint32_t src_offset, uint32_t .main_internal_return_ptr = internal_return_ptr, .main_mem_addr_a = resolved_src_offset, .main_mem_addr_c = resolved_dst_offset, - .main_op_err = static_cast(!is_valid(error)), + .main_op_err = static_cast(!is_ok(error)), .main_pc = pc, .main_r_in_tag = static_cast(tag), .main_rwc = 1, @@ -2310,7 +2310,7 @@ RowWithError AvmTraceBuilder::create_kernel_output_opcode(uint8_t indirect, uint auto read_a = constrained_read_from_memory( call_ptr, clk, resolved_data, AvmMemoryTag::FF, AvmMemoryTag::FF, IntermRegister::IA); bool tag_match = read_a.tag_match; - if (is_valid(error) && !tag_match) { + if (is_ok(error) && !tag_match) { error = AvmError::TAG_ERROR; } @@ -2321,7 +2321,7 @@ RowWithError AvmTraceBuilder::create_kernel_output_opcode(uint8_t indirect, uint .main_ind_addr_a = FF(read_a.indirect_address), .main_internal_return_ptr = internal_return_ptr, .main_mem_addr_a = FF(read_a.direct_address), - .main_op_err = FF(static_cast(!is_valid(error))), + .main_op_err = FF(static_cast(!is_ok(error))), .main_pc = pc, .main_r_in_tag = static_cast(AvmMemoryTag::FF), .main_rwa = 0, @@ -2365,7 +2365,7 @@ RowWithError AvmTraceBuilder::create_kernel_output_opcode_with_metadata(uint8_t call_ptr, clk, resolved_metadata, metadata_r_tag, AvmMemoryTag::FF, IntermRegister::IB); bool tag_match = read_a.tag_match && read_b.tag_match; - if (is_valid(error) && !tag_match) { + if (is_ok(error) && !tag_match) { error = AvmError::TAG_ERROR; } @@ -2379,7 +2379,7 @@ RowWithError AvmTraceBuilder::create_kernel_output_opcode_with_metadata(uint8_t .main_internal_return_ptr = internal_return_ptr, .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), - .main_op_err = FF(static_cast(!is_valid(error))), + .main_op_err = FF(static_cast(!is_ok(error))), .main_pc = pc, .main_r_in_tag = static_cast(data_r_tag), .main_rwa = 0, @@ -2512,7 +2512,7 @@ RowWithError AvmTraceBuilder::create_kernel_output_opcode_with_set_value_from_hi call_ptr, clk, resolved_metadata, AvmMemoryTag::FF, AvmMemoryTag::FF, IntermRegister::IB); bool tag_match = write_a.tag_match && read_b.tag_match; - if (is_valid(error) && !tag_match) { + if (is_ok(error) && !tag_match) { error = AvmError::TAG_ERROR; } @@ -2526,7 +2526,7 @@ RowWithError AvmTraceBuilder::create_kernel_output_opcode_with_set_value_from_hi .main_internal_return_ptr = internal_return_ptr, .main_mem_addr_a = FF(write_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), - .main_op_err = FF(static_cast(!is_valid(error))), + .main_op_err = FF(static_cast(!is_ok(error))), .main_pc = pc, // No PC increment here since we do it in the specific ops .main_r_in_tag = static_cast(AvmMemoryTag::FF), .main_rwa = 1, @@ -2627,7 +2627,7 @@ AvmError AvmTraceBuilder::op_sload(uint8_t indirect, uint32_t slot_offset, uint3 write_dst = AddressWithMode{ AddressingMode::DIRECT, write_a.direct_address + 1 }; } - if (is_valid(error) && !accumulated_tag_match) { + if (is_ok(error) && !accumulated_tag_match) { error = AvmError::TAG_ERROR; } @@ -2712,7 +2712,7 @@ AvmError AvmTraceBuilder::op_sstore(uint8_t indirect, uint32_t src_offset, uint3 read_src = AddressWithMode{ AddressingMode::DIRECT, read_a.direct_address + 1 }; } - if (is_valid(error) && !accumulated_tag_match) { + if (is_ok(error) && !accumulated_tag_match) { error = AvmError::TAG_ERROR; } @@ -2736,13 +2736,13 @@ AvmError AvmTraceBuilder::op_note_hash_exists(uint8_t indirect, error = res_error; const auto leaf_index = unconstrained_read_from_memory(resolved_leaf_index); - if (is_valid(error) && !check_tag(AvmMemoryTag::FF, resolved_leaf_index)) { + if (is_ok(error) && !check_tag(AvmMemoryTag::FF, resolved_leaf_index)) { error = AvmError::TAG_ERROR; } Row row; - if (is_valid(error)) { + if (is_ok(error)) { row = create_kernel_output_opcode_for_leaf_index( clk, resolved_note_hash, static_cast(leaf_index), resolved_dest); @@ -2751,7 +2751,7 @@ AvmError AvmTraceBuilder::op_note_hash_exists(uint8_t indirect, row.main_ia, /*safe*/ static_cast(row.main_ib)); row.main_sel_op_note_hash_exists = FF(1); - if (is_valid(error) && row.main_tag_err != FF(0)) { + if (is_ok(error) && row.main_tag_err != FF(0)) { error = AvmError::TAG_ERROR; } } else { @@ -2809,19 +2809,19 @@ AvmError AvmTraceBuilder::op_nullifier_exists(uint8_t indirect, auto [resolved_nullifier_offset, resolved_address, resolved_dest] = resolved_addrs; error = res_error; - if (is_valid(error) && !check_tag(AvmMemoryTag::FF, resolved_address)) { + if (is_ok(error) && !check_tag(AvmMemoryTag::FF, resolved_address)) { error = AvmError::TAG_ERROR; } Row row; - if (is_valid(error)) { + if (is_ok(error)) { row = create_kernel_output_opcode_with_set_metadata_output_from_hint( clk, resolved_nullifier_offset, resolved_address, resolved_dest); kernel_trace_builder.op_nullifier_exists( clk, side_effect_counter, row.main_ia, /*safe*/ static_cast(row.main_ib)); row.main_sel_op_nullifier_exists = FF(1); - if (is_valid(error) && row.main_tag_err != FF(0)) { + if (is_ok(error) && row.main_tag_err != FF(0)) { error = AvmError::TAG_ERROR; } } else { @@ -2881,13 +2881,13 @@ AvmError AvmTraceBuilder::op_l1_to_l2_msg_exists(uint8_t indirect, error = res_error; const auto leaf_index = unconstrained_read_from_memory(resolved_leaf_index); - if (is_valid(error) && !check_tag(AvmMemoryTag::FF, resolved_leaf_index)) { + if (is_ok(error) && !check_tag(AvmMemoryTag::FF, resolved_leaf_index)) { error = AvmError::TAG_ERROR; } Row row; - if (is_valid(error)) { + if (is_ok(error)) { row = create_kernel_output_opcode_for_leaf_index( clk, resolved_log, static_cast(leaf_index), resolved_dest); kernel_trace_builder.op_l1_to_l2_msg_exists(clk, @@ -2895,7 +2895,7 @@ AvmError AvmTraceBuilder::op_l1_to_l2_msg_exists(uint8_t indirect, row.main_ia, /*safe*/ static_cast(row.main_ib)); row.main_sel_op_l1_to_l2_msg_exists = FF(1); - if (is_valid(error) && row.main_tag_err != FF(0)) { + if (is_ok(error) && row.main_tag_err != FF(0)) { error = AvmError::TAG_ERROR; } } else { @@ -2954,7 +2954,7 @@ AvmError AvmTraceBuilder::op_get_contract_instance( auto read_address = constrained_read_from_memory( call_ptr, clk, resolved_address_offset, AvmMemoryTag::FF, AvmMemoryTag::FF, IntermRegister::IA); bool tag_match = read_address.tag_match; - if (is_valid(error) && !tag_match) { + if (is_ok(error) && !tag_match) { error = AvmError::TAG_ERROR; } @@ -2996,7 +2996,7 @@ AvmError AvmTraceBuilder::op_get_contract_instance( //.main_ind_addr_d = FF(write_exists.indirect_address), .main_internal_return_ptr = FF(internal_return_ptr), .main_mem_addr_a = FF(read_address.direct_address), - .main_op_err = FF(static_cast(!is_valid(error))), + .main_op_err = FF(static_cast(!is_ok(error))), //.main_mem_addr_c = FF(write_dst.direct_address), //.main_mem_addr_d = FF(write_exists.direct_address), .main_pc = FF(pc), @@ -3055,7 +3055,7 @@ AvmError AvmTraceBuilder::op_emit_unencrypted_log(uint8_t indirect, uint32_t log std::make_move_iterator(contract_address_bytes.begin()), std::make_move_iterator(contract_address_bytes.end())); - if (is_valid(error) && + if (is_ok(error) && !(check_tag(AvmMemoryTag::FF, resolved_log_offset) && check_tag(AvmMemoryTag::U32, resolved_log_size_offset))) { error = AvmError::TAG_ERROR; } @@ -3065,7 +3065,7 @@ AvmError AvmTraceBuilder::op_emit_unencrypted_log(uint8_t indirect, uint32_t log AddressWithMode direct_field_addr; uint32_t num_bytes = 0; - if (is_valid(error)) { + if (is_ok(error)) { log_size = static_cast(unconstrained_read_from_memory(resolved_log_size_offset)); // The size is in fields of 32 bytes, the length used for the hash is in terms of bytes @@ -3082,7 +3082,7 @@ AvmError AvmTraceBuilder::op_emit_unencrypted_log(uint8_t indirect, uint32_t log }; } - if (is_valid(error)) { + if (is_ok(error)) { // We need to read the rest of the log_size number of elements for (uint32_t i = 0; i < log_size; i++) { FF log_value = unconstrained_read_from_memory(direct_field_addr + i); @@ -3195,13 +3195,13 @@ AvmError AvmTraceBuilder::constrain_external_call(OpCode opcode, call_ptr, clk, resolved_args_offset, AvmMemoryTag::FF, AvmMemoryTag::FF, IntermRegister::ID); bool tag_match = read_gas_l2.tag_match && read_gas_da.tag_match && read_addr.tag_match && read_args.tag_match; - if (is_valid(error) && !(tag_match && check_tag(AvmMemoryTag::U32, resolved_args_size_offset))) { + if (is_ok(error) && !(tag_match && check_tag(AvmMemoryTag::U32, resolved_args_size_offset))) { error = AvmError::TAG_ERROR; } // TODO: constrain this auto args_size = - is_valid(error) ? static_cast(unconstrained_read_from_memory(resolved_args_size_offset)) : 0; + is_ok(error) ? static_cast(unconstrained_read_from_memory(resolved_args_size_offset)) : 0; gas_trace_builder.constrain_gas(clk, opcode, @@ -3223,7 +3223,7 @@ AvmError AvmTraceBuilder::constrain_external_call(OpCode opcode, .main_mem_addr_b = FF(read_gas_l2.direct_address + 1), .main_mem_addr_c = FF(read_addr.direct_address), .main_mem_addr_d = FF(read_args.direct_address), - .main_op_err = FF(static_cast(!is_valid(error))), + .main_op_err = FF(static_cast(!is_ok(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(AvmMemoryTag::FF)), .main_sel_mem_op_a = FF(1), @@ -3334,7 +3334,7 @@ ReturnDataError AvmTraceBuilder::op_return(uint8_t indirect, uint32_t ret_offset auto [resolved_ret_offset, resolved_ret_size_offset] = resolved_addrs; error = res_error; - if (is_valid(error) && !(tag_match && check_tag(AvmMemoryTag::U32, resolved_ret_size_offset))) { + if (is_ok(error) && !(tag_match && check_tag(AvmMemoryTag::U32, resolved_ret_size_offset))) { error = AvmError::TAG_ERROR; } @@ -3348,7 +3348,7 @@ ReturnDataError AvmTraceBuilder::op_return(uint8_t indirect, uint32_t ret_offset .main_call_ptr = call_ptr, .main_ib = ret_size, .main_internal_return_ptr = FF(internal_return_ptr), - .main_op_err = static_cast(!is_valid(error)), + .main_op_err = static_cast(!is_ok(error)), .main_pc = pc, .main_sel_op_external_return = 1, }); @@ -3376,7 +3376,7 @@ ReturnDataError AvmTraceBuilder::op_return(uint8_t indirect, uint32_t ret_offset .main_ib = ret_size, .main_internal_return_ptr = FF(internal_return_ptr), .main_mem_addr_c = resolved_ret_offset, - .main_op_err = static_cast(!is_valid(error)), + .main_op_err = static_cast(!is_ok(error)), .main_pc = pc, .main_r_in_tag = static_cast(AvmMemoryTag::FF), .main_sel_op_external_return = 1, @@ -3408,12 +3408,12 @@ ReturnDataError AvmTraceBuilder::op_revert(uint8_t indirect, uint32_t ret_offset auto [resolved_ret_offset, resolved_ret_size_offset] = resolved_addrs; error = res_error; - if (is_valid(error) && !(tag_match && check_tag(AvmMemoryTag::U32, ret_size_offset))) { + if (is_ok(error) && !(tag_match && check_tag(AvmMemoryTag::U32, ret_size_offset))) { error = AvmError::TAG_ERROR; } const auto ret_size = - is_valid(error) ? static_cast(unconstrained_read_from_memory(resolved_ret_size_offset)) : 0; + is_ok(error) ? static_cast(unconstrained_read_from_memory(resolved_ret_size_offset)) : 0; gas_trace_builder.constrain_gas(clk, OpCode::REVERT_8, ret_size); @@ -3424,7 +3424,7 @@ ReturnDataError AvmTraceBuilder::op_revert(uint8_t indirect, uint32_t ret_offset .main_call_ptr = call_ptr, .main_ib = ret_size, .main_internal_return_ptr = FF(internal_return_ptr), - .main_op_err = FF(static_cast(!is_valid(error))), + .main_op_err = FF(static_cast(!is_ok(error))), .main_pc = pc, .main_sel_op_external_return = 1, }); @@ -3451,7 +3451,7 @@ ReturnDataError AvmTraceBuilder::op_revert(uint8_t indirect, uint32_t ret_offset .main_ib = ret_size, .main_internal_return_ptr = FF(internal_return_ptr), .main_mem_addr_c = resolved_ret_offset, - .main_op_err = static_cast(!is_valid(error)), + .main_op_err = static_cast(!is_ok(error)), .main_pc = pc, .main_r_in_tag = static_cast(AvmMemoryTag::FF), .main_sel_op_external_return = 1, @@ -3489,18 +3489,18 @@ AvmError AvmTraceBuilder::op_debug_log(uint8_t indirect, auto [resolved_message_offset, resolved_fields_offset, resolved_fields_size_offset] = resolved_addrs; error = res_error; - if (is_valid(error) && !check_tag(AvmMemoryTag::U32, resolved_fields_size_offset)) { + if (is_ok(error) && !check_tag(AvmMemoryTag::U32, resolved_fields_size_offset)) { error = AvmError::TAG_ERROR; } const uint32_t fields_size = - is_valid(error) ? static_cast(unconstrained_read_from_memory(resolved_fields_size_offset)) : 0; + is_ok(error) ? static_cast(unconstrained_read_from_memory(resolved_fields_size_offset)) : 0; // Constrain gas cost gas_trace_builder.constrain_gas(clk, OpCode::DEBUGLOG, message_size + fields_size); - if (is_valid(error) && !(check_tag_range(AvmMemoryTag::U8, resolved_message_offset, message_size) && - check_tag_range(AvmMemoryTag::FF, resolved_fields_offset, fields_size))) { + 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; } @@ -3508,7 +3508,7 @@ AvmError AvmTraceBuilder::op_debug_log(uint8_t indirect, .main_clk = clk, .main_call_ptr = call_ptr, .main_internal_return_ptr = FF(internal_return_ptr), - .main_op_err = FF(static_cast(!is_valid(error))), + .main_op_err = FF(static_cast(!is_ok(error))), .main_pc = FF(pc), .main_sel_op_debug_log = FF(1), }); @@ -3584,11 +3584,11 @@ 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_valid(error) && !read_tag_valid) { + if (is_ok(error) && !read_tag_valid) { error = AvmError::TAG_ERROR; } - if (is_valid(error)) { + if (is_ok(error)) { std::array input = { read_a.val, read_b.val, read_c.val, read_d.val }; std::array result = poseidon2_trace_builder.poseidon2_permutation( input, call_ptr, clk, resolved_input_offset, resolved_output_offset); @@ -3634,7 +3634,7 @@ AvmError AvmTraceBuilder::op_poseidon2_permutation(uint8_t indirect, uint32_t in AvmMemTraceBuilder::POSEIDON2); bool write_tag_valid = write_a.tag_match && write_b.tag_match && write_c.tag_match && write_d.tag_match; - if (is_valid(error) && !write_tag_valid) { + if (is_ok(error) && !write_tag_valid) { error = AvmError::TAG_ERROR; } } @@ -3645,7 +3645,7 @@ AvmError AvmTraceBuilder::op_poseidon2_permutation(uint8_t indirect, uint32_t in .main_internal_return_ptr = FF(internal_return_ptr), .main_mem_addr_a = resolved_input_offset, .main_mem_addr_b = resolved_output_offset, - .main_op_err = FF(static_cast(!is_valid(error))), + .main_op_err = FF(static_cast(!is_ok(error))), .main_pc = FF(pc), .main_sel_op_poseidon2 = FF(1), }); @@ -3693,8 +3693,8 @@ AvmError AvmTraceBuilder::op_sha256_compression(uint8_t indirect, call_ptr, clk, resolved_inputs_offset, AvmMemoryTag::U32, AvmMemoryTag::FF, IntermRegister::IB); bool tag_match = read_a.tag_match && read_b.tag_match; - if (is_valid(error) && !(check_tag_range(AvmMemoryTag::U32, resolved_state_offset, STATE_SIZE) && - check_tag_range(AvmMemoryTag::U32, resolved_inputs_offset, INPUTS_SIZE))) { + 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; } @@ -3718,7 +3718,7 @@ AvmError AvmTraceBuilder::op_sha256_compression(uint8_t indirect, .main_internal_return_ptr = FF(internal_return_ptr), .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), - .main_op_err = FF(static_cast(!is_valid(error))), + .main_op_err = FF(static_cast(!is_ok(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(AvmMemoryTag::U32)), .main_sel_mem_op_a = FF(1), @@ -3729,7 +3729,7 @@ AvmError AvmTraceBuilder::op_sha256_compression(uint8_t indirect, .main_tag_err = FF(static_cast(!tag_match)), }); - if (!is_valid(error)) { + if (!is_ok(error)) { return error; } @@ -3792,7 +3792,7 @@ AvmError AvmTraceBuilder::op_keccakf1600(uint8_t indirect, uint32_t output_offse call_ptr, clk, resolved_input_offset, AvmMemoryTag::U64, AvmMemoryTag::FF, IntermRegister::IA); bool tag_match = input_read.tag_match; - if (is_valid(error) && + if (is_ok(error) && !(tag_match && check_tag_range(AvmMemoryTag::U64, resolved_input_offset, KECCAKF1600_INPUT_SIZE))) { error = AvmError::TAG_ERROR; } @@ -3806,7 +3806,7 @@ AvmError AvmTraceBuilder::op_keccakf1600(uint8_t indirect, uint32_t output_offse .main_ind_addr_a = FF(input_read.indirect_address), .main_internal_return_ptr = FF(internal_return_ptr), .main_mem_addr_a = FF(input_read.direct_address), - .main_op_err = FF(static_cast(!is_valid(error))), + .main_op_err = FF(static_cast(!is_ok(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(AvmMemoryTag::U64)), .main_sel_mem_op_a = FF(1), @@ -3815,7 +3815,7 @@ AvmError AvmTraceBuilder::op_keccakf1600(uint8_t indirect, uint32_t output_offse .main_tag_err = FF(static_cast(!tag_match)), }); - if (!is_valid(error)) { + if (!is_ok(error)) { return error; } @@ -3878,13 +3878,13 @@ AvmError AvmTraceBuilder::op_ec_add(uint16_t indirect, check_tag(AvmMemoryTag::U1, resolved_lhs_is_inf_offset) && check_tag(AvmMemoryTag::FF, resolved_rhs_x_offset) && check_tag(AvmMemoryTag::FF, resolved_rhs_y_offset) && check_tag(AvmMemoryTag::U1, resolved_rhs_is_inf_offset); - if (is_valid(error) && !tags_valid) { + if (is_ok(error) && !tags_valid) { error = AvmError::TAG_ERROR; } gas_trace_builder.constrain_gas(clk, OpCode::ECADD); - if (!is_valid(error)) { + if (!is_ok(error)) { main_trace.push_back(Row{ .main_clk = clk, .main_internal_return_ptr = FF(internal_return_ptr), @@ -3950,11 +3950,11 @@ AvmError AvmTraceBuilder::op_variable_msm(uint8_t indirect, resolved_addrs; error = res_error; - if (is_valid(error) && !check_tag(AvmMemoryTag::U32, resolved_point_length_offset)) { + if (is_ok(error) && !check_tag(AvmMemoryTag::U32, resolved_point_length_offset)) { error = AvmError::TAG_ERROR; } - const FF points_length = is_valid(error) ? unconstrained_read_from_memory(resolved_point_length_offset) : 0; + const FF points_length = is_ok(error) ? unconstrained_read_from_memory(resolved_point_length_offset) : 0; // Points are stored as [x1, y1, inf1, x2, y2, inf2, ...] with the types [FF, FF, U8, FF, FF, U8, ...] const uint32_t num_points = uint32_t(points_length) / 3; // 3 elements per point @@ -3974,7 +3974,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_valid(error) && !tags_valid) { + if (is_ok(error) && !tags_valid) { error = AvmError::TAG_ERROR; } @@ -3982,7 +3982,7 @@ AvmError AvmTraceBuilder::op_variable_msm(uint8_t indirect, // run out of gas. Casting/truncating here is not secure. gas_trace_builder.constrain_gas(clk, OpCode::MSM, static_cast(points_length)); - if (!is_valid(error)) { + if (!is_ok(error)) { main_trace.push_back(Row{ .main_clk = clk, .main_internal_return_ptr = FF(internal_return_ptr), @@ -4103,7 +4103,7 @@ AvmError AvmTraceBuilder::op_to_radix_be(uint8_t indirect, // auto read_radix = constrained_read_from_memory( // call_ptr, clk, resolved_radix_offset, AvmMemoryTag::U32, AvmMemoryTag::U32, IntermRegister::IB); - if (is_valid(error) && !check_tag(AvmMemoryTag::U32, resolved_radix_offset)) { + if (is_ok(error) && !check_tag(AvmMemoryTag::U32, resolved_radix_offset)) { error = AvmError::TAG_ERROR; } @@ -4111,7 +4111,7 @@ AvmError AvmTraceBuilder::op_to_radix_be(uint8_t indirect, FF input = read_src.val; - if (is_valid(error) && !read_src.tag_match) { + if (is_ok(error) && !read_src.tag_match) { error = AvmError::TAG_ERROR; } @@ -4120,13 +4120,13 @@ AvmError AvmTraceBuilder::op_to_radix_be(uint8_t indirect, uint32_t radix = static_cast(read_radix); bool radix_out_of_bounds = radix > 256; - if (is_valid(error) && radix_out_of_bounds) { + if (is_ok(error) && radix_out_of_bounds) { error = AvmError::RADIX_OUT_OF_BOUNDS; } // In case of an error, we do not perform the computation. // Therefore, we do not create any entry in gadget table and we return a vector of 0. - std::vector res = is_valid(error) + std::vector res = is_ok(error) ? conversion_trace_builder.op_to_radix_be(input, radix, num_limbs, output_bits, clk) : std::vector(num_limbs, 0); @@ -4146,7 +4146,7 @@ AvmError AvmTraceBuilder::op_to_radix_be(uint8_t indirect, .main_mem_addr_a = read_src.direct_address, // TODO(8603): uncomment //.main_mem_addr_b = read_radix.direct_address, - .main_op_err = FF(static_cast(!is_valid(error))), + .main_op_err = FF(static_cast(!is_ok(error))), .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(AvmMemoryTag::FF)), .main_sel_mem_op_a = FF(1), diff --git a/yarn-project/simulator/src/avm/avm_memory_types.ts b/yarn-project/simulator/src/avm/avm_memory_types.ts index d22e86bc9b0e..f585039d77ab 100644 --- a/yarn-project/simulator/src/avm/avm_memory_types.ts +++ b/yarn-project/simulator/src/avm/avm_memory_types.ts @@ -232,8 +232,8 @@ export class TaggedMemory implements TaggedMemoryInterface { // Whether to track and validate memory accesses for each instruction. static readonly TRACK_MEMORY_ACCESSES = process.env.NODE_ENV === 'test'; - // FIXME: memory should be 2^32, but TS doesn't allow for arrays that big. - static readonly MAX_MEMORY_SIZE = Number((1n << 32n) - 2n); + // FIXME: memory should be 2^32, but TS max array size is: 2^32 - 1 + static readonly MAX_MEMORY_SIZE = Number((1n << 32n) - 1n); private _mem: MemoryValue[]; constructor() { @@ -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; @@ -264,8 +268,7 @@ export class TaggedMemory implements TaggedMemoryInterface { } public getSlice(offset: number, size: number): MemoryValue[] { - assert(offset < TaggedMemory.MAX_MEMORY_SIZE); - assert(offset + size < TaggedMemory.MAX_MEMORY_SIZE); + assert(offset + size <= TaggedMemory.MAX_MEMORY_SIZE); const value = this._mem.slice(offset, offset + size); TaggedMemory.log.debug(`getSlice(${offset}, ${size}) = ${value}`); for (let i = 0; i < value.length; i++) { @@ -278,14 +281,12 @@ export class TaggedMemory implements TaggedMemoryInterface { } public getSliceAs(offset: number, size: number): T[] { - assert(offset < TaggedMemory.MAX_MEMORY_SIZE); - assert(offset + size < TaggedMemory.MAX_MEMORY_SIZE); + assert(offset + size <= TaggedMemory.MAX_MEMORY_SIZE); return this.getSlice(offset, size) as T[]; } public getSliceTags(offset: number, size: number): TypeTag[] { - assert(offset < TaggedMemory.MAX_MEMORY_SIZE); - assert(offset + size < TaggedMemory.MAX_MEMORY_SIZE); + assert(offset + size <= TaggedMemory.MAX_MEMORY_SIZE); return this._mem.slice(offset, offset + size).map(TaggedMemory.getTag); } @@ -296,8 +297,7 @@ export class TaggedMemory implements TaggedMemoryInterface { } public setSlice(offset: number, vs: MemoryValue[]) { - assert(offset < TaggedMemory.MAX_MEMORY_SIZE); - assert(offset + vs.length < TaggedMemory.MAX_MEMORY_SIZE); + assert(offset + vs.length <= TaggedMemory.MAX_MEMORY_SIZE); // We may need to extend the memory size, otherwise splice doesn't insert. if (offset + vs.length > this._mem.length) { this._mem.length = offset + vs.length; @@ -475,6 +475,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/opcodes/addressing_mode.ts b/yarn-project/simulator/src/avm/opcodes/addressing_mode.ts index b7dd6163fff4..0f24710fd3ae 100644 --- a/yarn-project/simulator/src/avm/opcodes/addressing_mode.ts +++ b/yarn-project/simulator/src/avm/opcodes/addressing_mode.ts @@ -1,5 +1,4 @@ import { strict as assert } from 'assert'; -import { maxUint32 } from 'viem'; import { type TaggedMemoryInterface } from '../avm_memory_types.js'; import { AddressOutOfRangeError } from '../errors.js'; @@ -67,7 +66,7 @@ export class Addressing { mem.checkIsValidMemoryOffsetTag(0); const baseAddr = Number(mem.get(0).toBigInt()); resolved[i] += baseAddr; - if (resolved[i] > maxUint32) { + if (resolved[i] >= mem.getMaxMemorySize()) { throw new AddressOutOfRangeError(baseAddr, offset); } } From 349bfea0d6c5d84da7fb220d8fb22dcbb3d392a6 Mon Sep 17 00:00:00 2001 From: jeanmon Date: Fri, 22 Nov 2024 16:15:02 +0000 Subject: [PATCH 5/5] 9131: addressing review comments --- yarn-project/simulator/src/avm/avm_memory_types.ts | 8 -------- yarn-project/simulator/src/avm/opcodes/addressing_mode.ts | 4 ++-- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/yarn-project/simulator/src/avm/avm_memory_types.ts b/yarn-project/simulator/src/avm/avm_memory_types.ts index f585039d77ab..96420655547d 100644 --- a/yarn-project/simulator/src/avm/avm_memory_types.ts +++ b/yarn-project/simulator/src/avm/avm_memory_types.ts @@ -241,10 +241,6 @@ 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; @@ -475,10 +471,6 @@ 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/opcodes/addressing_mode.ts b/yarn-project/simulator/src/avm/opcodes/addressing_mode.ts index 0f24710fd3ae..89107196cf9b 100644 --- a/yarn-project/simulator/src/avm/opcodes/addressing_mode.ts +++ b/yarn-project/simulator/src/avm/opcodes/addressing_mode.ts @@ -1,6 +1,6 @@ import { strict as assert } from 'assert'; -import { type TaggedMemoryInterface } from '../avm_memory_types.js'; +import { TaggedMemory, type TaggedMemoryInterface } from '../avm_memory_types.js'; import { AddressOutOfRangeError } from '../errors.js'; export enum AddressingMode { @@ -66,7 +66,7 @@ export class Addressing { mem.checkIsValidMemoryOffsetTag(0); const baseAddr = Number(mem.get(0).toBigInt()); resolved[i] += baseAddr; - if (resolved[i] >= mem.getMaxMemorySize()) { + if (resolved[i] >= TaggedMemory.MAX_MEMORY_SIZE) { throw new AddressOutOfRangeError(baseAddr, offset); } }