Skip to content

chore(dsl): require MemOp index and value to be witnesses#23171

Merged
TomAFrench merged 1 commit into
merge-train/barretenbergfrom
tf/memop-always-witness
May 12, 2026
Merged

chore(dsl): require MemOp index and value to be witnesses#23171
TomAFrench merged 1 commit into
merge-train/barretenbergfrom
tf/memop-always-witness

Conversation

@TomAFrench
Copy link
Copy Markdown
Member

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

Noir always emits a single unscaled witness for MemOp.index and
MemOp.value (see MemOp::read_at_mem_index / write_to_mem_index in
acvm-repo/acir/src/circuit/opcodes/memory_operation.rs). The
WitnessOrConstant<bb::fr> representation on the bb side was carrying a
dead constant path. Collapse the fields to uint32_t witness indices and
assert the witness form on deserialization. Drops the constant-ops
branch of the ROM/RAM/CallData tests.
@TomAFrench TomAFrench changed the title refactor(dsl): require MemOp index and value to be witnesses chore(dsl): require MemOp index and value to be witnesses May 12, 2026
Copy link
Copy Markdown
Contributor

@federicobarbacovi federicobarbacovi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this!

@TomAFrench TomAFrench enabled auto-merge (squash) May 12, 2026 09:02
@TomAFrench TomAFrench merged commit ef61c7c into merge-train/barretenberg May 12, 2026
17 checks passed
@TomAFrench TomAFrench deleted the tf/memop-always-witness branch May 12, 2026 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants