-
Notifications
You must be signed in to change notification settings - Fork 130
Open
Description
Background
During an external DSL audit, auditors noted that a good missing check would be a serialization roundtrip, verifying that we have not hit any issues. While Noir has roundtrip tests in test_program_serialization.rs, these only test Rust serialization - they don't exercise the auto-generated C++ deserialization code path.
The Problem
The C++ deserialization code is auto-generated from the Noir structures, and should generally be lossy without detection. Key considerations from @akosh:
- nargo can write programs with msgpack array or map format for structs
- C++ code reads both formats but writes exclusively array format
- C++ currently ignores the Brillig field, so a roundtrip would be lossy for that field
- If nargo used map format and C++ ignored a field, roundtrip would silently remove data
Proposed Solution
1. Create barretenberg/acir_tests/scripts/acir_roundtrip.sh
A new test script that:
- Takes an ACIR program bytecode as input
- Deserializes it in C++ (
bytecode → Acir::Circuit) - Re-serializes it (
Acir::Circuit → bytecode) - Since the two formats are not guaranteed to be byte-equal, we use nargo execute to have noir validate the bytecode is still valid, and we use nargo execute before and after the roundtrip and verify they are equal
2. Add new bb CLI mode
Add an undocumented bb CLI command (e.g., bb acir_roundtrip) that:
- Reads ACIR bytecode from a file
- Deserializes to
Acir::Circuit - Serializes back to bytecode
- Prints/writes the output
3. Integrate with acir_tests
- Add test entries to
acir_tests/bootstrap.sh - Run on all non-recursive test circuits
- Call our acir_roundtrip.sh on them
4. Re-enable C++ serialization code
The serialization code path (write direction) needs to be regenerated:
NOIR_CODEGEN_NO_PACK=0 NOIR_CODEGEN_OVERWRITE=1 cargo test -p acir cpp_codegenAcceptance Criteria
- C++ serialization code is regenerated and committed
- New
bb acir_roundtripCLI command exists -
acir_roundtrip.shscript created inacir_tests/scripts/ - Tests run on all applicable test programs in acir_tests
- Bootstrap.sh updated to include roundtrip test entries
- CI runs roundtrip tests as part of acir_tests suite
Notes
- Byte-for-byte equivalence may not be achievable due to format differences (array vs map). However, potentially it just works. It would be more thorough than the nargo exec check. The Brillig field being ignored is a known limitation to document. We would need to filter out brillig sections, or write a smart comparator. if we go this route maybe the bb CLI command does this all internally without nargo exec
- Otherwise consider semantic equivalence check: roundtripped program should produce same execution results. It is not prone to issues with us dropping brillig. Here we would use nargo exec to check results, though a missing constraint might produce equivalent results anyway.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels