aztec-packages icon indicating copy to clipboard operation
aztec-packages copied to clipboard

chore: ACIR audit - part 5

Open federicobarbacovi opened this issue 2 months ago โ€ข 1 comments

๐Ÿงพ Audit Context

This PR is the second in the audit thread covering acir_to_constraint_buf (converting bytes into Barretenberg's constraints). It refactors the handling of memory operations.

๐Ÿ› ๏ธ Changes Made

Memory operations from Noir arrive in barretenberg in two types:

  • MemoryInit, which is the initialisation operation. In this case Noir gives barretenberg a list of witnesses indices with which to initialise the table
  • MemoryOp, which can be either a read or write operation. At the moment Noir passes us witnesses. However, in the future it might be possible that Noir passes us witnesses or constants.

Before this PR, the data contained in the memory opcodes was being treated as a poly_triple. In this PR, we restructure the relevant memory-related structure to hold:

  • witness indices for MemoryInit operations
  • WitnessOrConstant for MemoryOp operations The rationale is that this better reflects the information passed to us by Noir, and it also simplifies the code.

There is no change in circuits. However, note that after this PR Noir can send us constants to read from. This was not supported before because of an assertion in the code. The assertion clearly stated that the only reason why we had it was that Noir is supposed to optimise those reads away, so I removed it given the conversation that pre-dated this PR.

Other changes:

  • Introduction of a constructor for ROM/RAM tables that takes in a builder. This allows reading from a constant index before reading from a witness index (the table is initialised at the first read, and at the point of initialisation it needs a builder)
  • Fail if calldata/returndata appers in an UltraBuilder (disable respective acir_tests)
  • Extend test suite for block constraints to also test for constant read/writes
  • Extend test suite for block constraints to also test CallData and ReturnData

โœ… Checklist

  • [ ] Audited all methods of the relevant module/class
  • [ ] Audited the interface of the module/class with other (relevant) components
  • [ ] Documented existing functionality and any changes made (as per Doxygen requirements)
  • [ ] Resolved and/or closed all issues/TODOs pertaining to the audited files
  • [ ] Confirmed and documented any security or other issues found (if applicable)
  • [ ] Verified that tests cover all critical paths (and added tests if necessary)
  • [ ] Updated audit tracking for the files audited (check the start of each file you audited)

๐Ÿ“Œ Notes for Reviewers

federicobarbacovi avatar Nov 20 '25 16:11 federicobarbacovi

Flakey Tests

๐Ÿค– says: This CI run detected 2 tests that failed, but were tolerated due to a .test_patterns.yml entry.

\033[38;2;188;109;208mFLAKED\033[0m (\033[38;2;250;217;121m8;;http://ci.aztec-labs.com/d523aeadb6fa98acd523aeadb6fa98ac8;;\033[0m): yarn-project/end-to-end/scripts/run_test.sh simple src/e2e_epochs/epochs_multi_proof.test.ts (171s) (code: 1) group:e2e-p2p-epoch-flakes (\033[38;2;188;109;208mfedericobarbacovi\033[0m: Fixes)
\033[38;2;188;109;208mFLAKED\033[0m (\033[38;2;250;217;121m8;;http://ci.aztec-labs.com/d407054f35508fd4d407054f35508fd48;;\033[0m): yarn-project/end-to-end/scripts/run_test.sh simple src/e2e_p2p/inactivity_slash.test.ts (358s) (code: 1) group:e2e-p2p-epoch-flakes (\033[38;2;188;109;208mfedericobarbacovi\033[0m: Fixes)

AztecBot avatar Nov 21 '25 09:11 AztecBot