diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/tests/bitwise.test.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/tests/bitwise.test.cpp index 0e4b9316ca9..9a3b1b8b369 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/tests/bitwise.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/tests/bitwise.test.cpp @@ -93,7 +93,7 @@ void common_validate_shift_op(std::vector const& trace, FF const& b, FF const& c, FF const& addr_a, - FF const& addr_b, + [[maybe_unused]] FF const& addr_b, FF const& addr_c, avm_trace::AvmMemoryTag const tag, bool shr) @@ -118,10 +118,11 @@ void common_validate_shift_op(std::vector const& trace, EXPECT_EQ(row->main_rwa, FF(0)); // Check that ib register is correctly set with memory load operations. - EXPECT_EQ(row->main_ib, b); - EXPECT_EQ(row->main_mem_addr_b, addr_b); - EXPECT_EQ(row->main_sel_mem_op_b, FF(1)); - EXPECT_EQ(row->main_rwb, FF(0)); + // TODO(8603): once instructions can have multiple different tags for reads, constrain b's read & tag + // EXPECT_EQ(row->main_ib, b); + // EXPECT_EQ(row->main_mem_addr_b, addr_b); + // EXPECT_EQ(row->main_sel_mem_op_b, FF(1)); + // EXPECT_EQ(row->main_rwb, FF(0)); // Check the instruction tags EXPECT_EQ(row->main_r_in_tag, FF(static_cast(tag))); diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/alu_trace.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/alu_trace.cpp index 0d9f7b4caf0..25d1b54ebc6 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/alu_trace.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/alu_trace.cpp @@ -450,6 +450,7 @@ FF AvmAluTraceBuilder::op_not(FF const& a, AvmMemoryTag in_tag, uint32_t const c */ FF AvmAluTraceBuilder::op_shl(FF const& a, FF const& b, AvmMemoryTag in_tag, uint32_t clk) { + // TODO(9497): this should raise error flag in main trace, not assert ASSERT(in_tag != AvmMemoryTag::FF); // Check that the shifted amount is an 8-bit integer ASSERT(uint256_t(b) < 256); @@ -512,6 +513,7 @@ FF AvmAluTraceBuilder::op_shl(FF const& a, FF const& b, AvmMemoryTag in_tag, uin */ FF AvmAluTraceBuilder::op_shr(FF const& a, FF const& b, AvmMemoryTag in_tag, uint32_t clk) { + // TODO(9497): this should raise error flag in main trace, not assert ASSERT(in_tag != AvmMemoryTag::FF); // Check that the shifted amount is an 8-bit integer ASSERT(uint256_t(b) < 256); diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp index d5400f5c8f5..7b4ee622f76 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp @@ -1064,12 +1064,16 @@ void AvmTraceBuilder::op_shl(uint8_t indirect, uint32_t a_offset, uint32_t b_off // 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 resp. ib. + // TODO(9497): if simulator fails tag check here, witgen will not. Raise error flag if in_tag is FF! auto read_a = constrained_read_from_memory(call_ptr, clk, resolved_a, in_tag, in_tag, IntermRegister::IA); - 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; + // TODO(8603): once instructions can have multiple different tags for reads, constrain b's read & tag + // 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); // should be tagged as U8 + bool tag_match = read_a.tag_match; FF a = tag_match ? read_a.val : FF(0); - FF b = tag_match ? read_b.val : FF(0); + FF b = tag_match ? read_b : FF(0); FF c = tag_match ? alu_trace_builder.op_shl(a, b, in_tag, clk) : FF(0); @@ -1083,24 +1087,24 @@ void AvmTraceBuilder::op_shl(uint8_t indirect, uint32_t a_offset, uint32_t b_off .main_alu_in_tag = FF(static_cast(in_tag)), .main_call_ptr = call_ptr, .main_ia = read_a.val, - .main_ib = read_b.val, + .main_ib = read_b, .main_ic = write_c.val, .main_ind_addr_a = FF(read_a.indirect_address), - .main_ind_addr_b = FF(read_b.indirect_address), + //.main_ind_addr_b = FF(read_b.indirect_address), .main_ind_addr_c = FF(write_c.indirect_address), .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_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), .main_pc = FF(pc++), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), .main_sel_mem_op_a = FF(1), - .main_sel_mem_op_b = FF(1), + //.main_sel_mem_op_b = FF(1), .main_sel_mem_op_c = FF(1), .main_sel_op_shl = FF(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_sel_resolve_ind_addr_b = FF(static_cast(read_b.is_indirect)), .main_sel_resolve_ind_addr_c = FF(static_cast(write_c.is_indirect)), .main_tag_err = FF(static_cast(!tag_match)), .main_w_in_tag = FF(static_cast(in_tag)), @@ -1117,12 +1121,16 @@ void AvmTraceBuilder::op_shr(uint8_t indirect, uint32_t a_offset, uint32_t b_off // 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 resp. ib. + // TODO(9497): if simulator fails tag check here, witgen will not. Raise error flag if in_tag is FF! auto read_a = constrained_read_from_memory(call_ptr, clk, resolved_a, in_tag, in_tag, IntermRegister::IA); - 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; + // TODO(8603): once instructions can have multiple different tags for reads, constrain b's read & tag + // 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); // should be tagged as U8 + bool tag_match = read_a.tag_match; FF a = tag_match ? read_a.val : FF(0); - FF b = tag_match ? read_b.val : FF(0); + FF b = tag_match ? read_b : FF(0); FF c = tag_match ? alu_trace_builder.op_shr(a, b, in_tag, clk) : FF(0); @@ -1136,24 +1144,28 @@ void AvmTraceBuilder::op_shr(uint8_t indirect, uint32_t a_offset, uint32_t b_off .main_alu_in_tag = FF(static_cast(in_tag)), .main_call_ptr = call_ptr, .main_ia = read_a.val, - .main_ib = read_b.val, + .main_ib = read_b, .main_ic = write_c.val, .main_ind_addr_a = FF(read_a.indirect_address), - .main_ind_addr_b = FF(read_b.indirect_address), + // TODO(8603): uncomment + //.main_ind_addr_b = FF(read_b.indirect_address), .main_ind_addr_c = FF(write_c.indirect_address), .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), + // TODO(8603): uncomment + //.main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), .main_pc = FF(pc++), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), .main_sel_mem_op_a = FF(1), - .main_sel_mem_op_b = FF(1), + // TODO(8603): uncomment + //.main_sel_mem_op_b = FF(1), .main_sel_mem_op_c = FF(1), .main_sel_op_shr = FF(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)), + // TODO(8603): uncomment + //.main_sel_resolve_ind_addr_b = FF(static_cast(read_b.is_indirect)), .main_sel_resolve_ind_addr_c = FF(static_cast(write_c.is_indirect)), .main_tag_err = FF(static_cast(!tag_match)), .main_w_in_tag = FF(static_cast(in_tag)), @@ -2429,7 +2441,7 @@ void AvmTraceBuilder::op_get_contract_instance( break; } - // TODO:(8603): once instructions can have multiple different tags for writes, write dst as FF and exists as U1 + // TODO(8603): once instructions can have multiple different tags for writes, write dst as FF and exists as U1 // auto write_dst = constrained_write_to_memory(call_ptr, clk, resolved_dst_offset, member_value, // AvmMemoryTag::FF, AvmMemoryTag::FF, IntermRegister::IC); auto write_exists = // constrained_write_to_memory(call_ptr, clk, resolved_exists_offset, instance.instance_found_in_address, @@ -2439,7 +2451,7 @@ void AvmTraceBuilder::op_get_contract_instance( .main_clk = clk, .main_call_ptr = call_ptr, .main_ia = read_address.val, - // TODO:(8603): uncomment this and below blocks once instructions can have multiple different tags for + // TODO(8603): uncomment this and below blocks once instructions can have multiple different tags for // writes //.main_ic = write_dst.val, //.main_id = write_exists.val, @@ -2462,7 +2474,7 @@ void AvmTraceBuilder::op_get_contract_instance( .main_tag_err = FF(static_cast(!tag_match)), }); - // TODO:(8603): once instructions can have multiple different tags for writes, remove this and do a constrained + // TODO(8603): once instructions can have multiple different tags for writes, remove this and do a constrained // writes write_to_memory(resolved_dst_offset, member_value, AvmMemoryTag::FF); write_to_memory(resolved_exists_offset, FF(static_cast(instance.exists)), AvmMemoryTag::U1); @@ -3305,13 +3317,14 @@ void AvmTraceBuilder::op_to_radix_le(uint8_t indirect, auto read_src = constrained_read_from_memory( call_ptr, clk, resolved_src_offset, AvmMemoryTag::FF, w_in_tag, IntermRegister::IA); - // TODO:(8603): once instructions can have multiple different tags for reads, constrain the radix's read + // TODO(8603): once instructions can have multiple different tags for reads, constrain the radix's read + // TODO(9497): if simulator fails tag check here, witgen will not. Raise error flag! // auto read_radix = constrained_read_from_memory( // call_ptr, clk, resolved_radix_offset, AvmMemoryTag::U32, AvmMemoryTag::U32, IntermRegister::IB); auto read_radix = unconstrained_read_from_memory(resolved_radix_offset); FF input = read_src.val; - // TODO:(8603): uncomment + // TODO(8603): uncomment // uint32_t radix = static_cast(read_radix.val); uint32_t radix = static_cast(read_radix); @@ -3337,21 +3350,21 @@ void AvmTraceBuilder::op_to_radix_le(uint8_t indirect, .main_ic = num_limbs, .main_id = output_bits, .main_ind_addr_a = read_src.indirect_address, - // TODO:(8603): uncomment + // TODO(8603): uncomment //.main_ind_addr_b = read_radix.indirect_address, .main_internal_return_ptr = FF(internal_return_ptr), .main_mem_addr_a = read_src.direct_address, - // TODO:(8603): uncomment + // TODO(8603): uncomment //.main_mem_addr_b = read_radix.direct_address, .main_op_err = error ? FF(1) : FF(0), .main_pc = FF(pc++), .main_r_in_tag = FF(static_cast(AvmMemoryTag::FF)), .main_sel_mem_op_a = FF(1), - // TODO:(8603): uncomment + // TODO(8603): uncomment //.main_sel_mem_op_b = FF(1), .main_sel_op_radix_le = FF(1), .main_sel_resolve_ind_addr_a = FF(static_cast(read_src.is_indirect)), - // TODO:(8603): uncomment + // TODO(8603): uncomment //.main_sel_resolve_ind_addr_b = FF(static_cast(read_radix.is_indirect)), .main_w_in_tag = FF(static_cast(w_in_tag)), }); diff --git a/noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr b/noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr index bc3d85ae168..d564c046037 100644 --- a/noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr @@ -569,8 +569,8 @@ contract AvmTest { let _ = read_storage_map(context.this_address()); dep::aztec::oracle::debug_log::debug_log("keccak_hash"); let _ = keccak_hash(args_u8); - // dep::aztec::oracle::debug_log::debug_log("sha256_hash"); - // let _ = sha256_hash(args_u8); + dep::aztec::oracle::debug_log::debug_log("sha256_hash"); + let _ = sha256_hash(args_u8); dep::aztec::oracle::debug_log::debug_log("poseidon2_hash"); let _ = poseidon2_hash(args_field); dep::aztec::oracle::debug_log::debug_log("pedersen_hash");