Skip to content

Commit ef61c7c

Browse files
authored
chore(dsl): require MemOp index and value to be witnesses (#23171)
## Summary - `acir_format::MemOp.index` and `MemOp.value` become plain `uint32_t` witness indices instead of `WitnessOrConstant<bb::fr>`. - The deserializer in `acir_to_constraint_buf.cpp` now asserts the Noir-side `Acir::Expression` is a single unscaled witness, matching what Noir actually emits (see `MemOp::read_at_mem_index` / `write_to_mem_index` in `acvm-repo/acir/src/circuit/opcodes/memory_operation.rs` — both take `Witness`, not `Expression`). - Drops the `add_constant_ops` helper and `perform_constant_ops` parameterization from ROM/RAM/CallData tests, since the path being exercised never came up in production. ## Test plan - [ ] `ninja dsl_tests` passes (note: my local build hits a pre-existing gtest cxx11-ABI link error; the affected `.cpp.o` objects compile cleanly) - [ ] CI green
1 parent f5e77a9 commit ef61c7c

6 files changed

Lines changed: 89 additions & 178 deletions

File tree

barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_to_constraint_buf.cpp

Lines changed: 13 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -817,27 +817,19 @@ BlockConstraint memory_init_to_block_constraint(Acir::Opcode::MemoryInit const&
817817

818818
void add_memory_op_to_block_constraint(Acir::Opcode::MemoryOp const& mem_op, BlockConstraint& block)
819819
{
820-
// Lambda to convert an Acir::Expression to a witness index
821-
auto acir_expression_to_witness_or_constant = [](const Acir::Expression& expr) {
822-
// Noir gives us witnesses or constants for read/write operations. We use the following assertions to ensure
823-
// that the data coming from Noir is in the correct form.
820+
// Lambda to convert an Acir::Expression to a witness index. Noir always emits a single unscaled witness term for
821+
// memory op indices and values, so anything else is a malformed input.
822+
auto acir_expression_to_witness = [](const Acir::Expression& expr) -> uint32_t {
824823
BB_ASSERT(expr.mul_terms.empty(), "MemoryOp should not have multiplication terms");
825-
BB_ASSERT_LTE(expr.linear_combinations.size(), 1U, "MemoryOp should have at most one linear term");
824+
BB_ASSERT_EQ(expr.linear_combinations.size(), 1U, "MemoryOp expression must be a single witness");
826825

827-
const fr a_scaling = expr.linear_combinations.size() == 1
828-
? from_buffer_with_bound_checks(std::get<0>(expr.linear_combinations[0]))
829-
: fr::zero();
826+
const fr a_scaling = from_buffer_with_bound_checks(std::get<0>(expr.linear_combinations[0]));
830827
const fr constant_term = from_buffer_with_bound_checks(expr.q_c);
831828

832-
bool is_witness = a_scaling == fr::one() && constant_term == fr::zero();
833-
bool is_constant = a_scaling == fr::zero();
834-
BB_ASSERT(is_witness || is_constant, "MemoryOp expression must be a witness or a constant");
829+
BB_ASSERT(a_scaling == fr::one() && constant_term == fr::zero(),
830+
"MemoryOp expression must be a single unscaled witness with no constant term");
835831

836-
return WitnessOrConstant<bb::fr>{
837-
.index = is_witness ? std::get<1>(expr.linear_combinations[0]).value : bb::stdlib::IS_CONSTANT,
838-
.value = is_constant ? constant_term : fr::zero(),
839-
.is_constant = is_constant,
840-
};
832+
return std::get<1>(expr.linear_combinations[0]).value;
841833
};
842834

843835
// Lambda to determine whether a memory operation is a read or write operation
@@ -862,11 +854,11 @@ void add_memory_op_to_block_constraint(Acir::Opcode::MemoryOp const& mem_op, Blo
862854
block.type = BlockType::RAM;
863855
}
864856

865-
// Update the ranges of the index using the array length
866-
WitnessOrConstant<bb::fr> index = acir_expression_to_witness_or_constant(mem_op.op.index);
867-
WitnessOrConstant<bb::fr> value = acir_expression_to_witness_or_constant(mem_op.op.value);
868-
869-
MemOp acir_mem_op = MemOp{ .access_type = access_type, .index = index, .value = value };
857+
MemOp acir_mem_op = MemOp{
858+
.access_type = access_type,
859+
.index = acir_expression_to_witness(mem_op.op.index),
860+
.value = acir_expression_to_witness(mem_op.op.value),
861+
};
870862
block.trace.push_back(acir_mem_op);
871863
}
872864

barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,8 @@ void process_ROM_operations(Builder& builder,
9494

9595
rom_table_ct table(&builder, init);
9696
for (const auto& op : constraint.trace) {
97-
field_ct value = to_field_ct(op.value, builder);
98-
field_ct index = to_field_ct(op.index, builder);
97+
field_ct value = field_ct::from_witness_index(&builder, op.value);
98+
field_ct index = field_ct::from_witness_index(&builder, op.index);
9999

100100
switch (op.access_type) {
101101
case AccessType::Read:
@@ -118,8 +118,8 @@ void process_RAM_operations(Builder& builder,
118118

119119
ram_table_ct table(&builder, init);
120120
for (const auto& op : constraint.trace) {
121-
field_ct value = to_field_ct(op.value, builder);
122-
field_ct index = to_field_ct(op.index, builder);
121+
field_ct value = field_ct::from_witness_index(&builder, op.value);
122+
field_ct index = field_ct::from_witness_index(&builder, op.index);
123123

124124
switch (op.access_type) {
125125
case AccessType::Read:
@@ -151,8 +151,8 @@ void process_call_data_operations(Builder& builder,
151151
calldata_array.set_values(init); // Initialize the data in the bus array
152152

153153
for (const auto& op : constraint.trace) {
154-
field_ct value = to_field_ct(op.value, builder);
155-
field_ct index = to_field_ct(op.index, builder);
154+
field_ct value = field_ct::from_witness_index(&builder, op.value);
155+
field_ct index = field_ct::from_witness_index(&builder, op.index);
156156

157157
switch (op.access_type) {
158158
case AccessType::Read:

barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.hpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66

77
#pragma once
88
#include "barretenberg/constants.hpp"
9-
#include "barretenberg/dsl/acir_format/witness_constant.hpp"
109
#include "barretenberg/stdlib/primitives/field/field.hpp"
1110
#include <cstdint>
1211
#include <vector>
@@ -27,13 +26,13 @@ enum CallDataType : std::uint32_t {
2726
};
2827

2928
/**
30-
* @brief Memory operation. Index and value store the index of the memory location, and value is the value to be read or
31-
* written.
29+
* @brief Memory operation. `index` is the witness index of the memory location, and `value` is the witness index of the
30+
* value to be read or written.
3231
*/
3332
struct MemOp {
3433
AccessType access_type;
35-
WitnessOrConstant<bb::fr> index;
36-
WitnessOrConstant<bb::fr> value;
34+
uint32_t index;
35+
uint32_t value;
3736
};
3837

3938
enum BlockType : std::uint8_t {

0 commit comments

Comments
 (0)