-
Notifications
You must be signed in to change notification settings - Fork 219
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(acir): Use types on dynamic arrays #4364
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Edit: gonna hold off for the regression test.
While adding the extra test I realized each of our blackbox functions does in fact only accept an array of a singular type. I have included the test anyway as it is a good starting example for testing a dynamic array that has many possible numeric types as per this comment (#4364 (comment)). I have left the dynamic array types as a vector for correctness sake. Using a singular type on the dynamic array passes right now as we only have blackbox functions that accept arrays of a singular type. Keeping a vector of the internal types allows us to future-proof this new |
🤖 I have created a release *beep* *boop* --- <details><summary>0.25.0</summary> ## [0.25.0](v0.24.0...v0.25.0) (2024-03-11) ### ⚠ BREAKING CHANGES * Internal as a macro (AztecProtocol/aztec-packages#4898) * reserve `unchecked` keyword ([#4432](#4432)) * Remove empty value from bounded vec ([#4431](#4431)) * Ban Fields in for loop indices and bitwise ops ([#4376](#4376)) * bump msrv to 1.73.0 ([#4406](#4406)) * **ci:** Bump MSRV to 1.72.1 and enforce that ACVM can be published using updated lockfile ([#4385](#4385)) * Restrict bit sizes ([#4235](#4235)) * move noir out of yarn-project (AztecProtocol/aztec-packages#4479) * note type ids (AztecProtocol/aztec-packages#4500) ### Features * Add eddsa_poseidon_to_pub function to stdlib with test + docs ([#4473](#4473)) ([00d2c32](00d2c32)) * Add HashMap to the stdlib ([#4242](#4242)) ([650ffc5](650ffc5)) * Add option to set max memory for bb.js ([#4227](#4227)) ([8a6b131](8a6b131)) * Add overflow and underflow checks for unsigned integers in brillig ([#4445](#4445)) ([21fc4b8](21fc4b8)) * Add poseidon2 opcode implementation for acvm/brillig, and Noir ([#4398](#4398)) ([10e8292](10e8292)) * Added cast opcode and cast calldata (AztecProtocol/aztec-packages#4423) ([78ef013](78ef013)) * Allow type aliases to reference other aliases ([#4353](#4353)) ([c44ef14](c44ef14)) * Backpropagate constants in ACIR during optimization ([#3926](#3926)) ([aad0da0](aad0da0)) * **ci:** Use wasm-opt when compiling wasm packages ([#4334](#4334)) ([e382921](e382921)) * DAP Preflight and debugger compilation options ([#4185](#4185)) ([e0ad0b2](e0ad0b2)) * Expose separate functions to compile programs vs contracts in `noir_wasm` ([#4413](#4413)) ([7cd5fdb](7cd5fdb)) * Internal as a macro (AztecProtocol/aztec-packages#4898) ([5f57ebb](5f57ebb)) * Note type ids (AztecProtocol/aztec-packages#4500) ([78ef013](78ef013)) * Restrict bit sizes ([#4235](#4235)) ([1048f81](1048f81)) * Run tests in parallel in `nargo test` ([#4484](#4484)) ([761734e](761734e)) * Skip redundant range checks in brillig ([#4460](#4460)) ([cb4c1c5](cb4c1c5)) * Sync from aztec-packages ([#4483](#4483)) ([fe8f277](fe8f277)) * Track stack frames and their variables in the debugger ([#4188](#4188)) ([ae1a9d9](ae1a9d9)) * TypeVariableKind for just Integers ([#4118](#4118)) ([c956be8](c956be8)) * Update error message when trying to load workspace as dependency ([#4393](#4393)) ([d2585e7](d2585e7)) ### Bug Fixes * **acir:** Array dynamic flatten ([#4351](#4351)) ([b2aaeab](b2aaeab)) * **acir:** Use types on dynamic arrays ([#4364](#4364)) ([ba2c541](ba2c541)) * Add `follow_bindings` to follow `Type::Alias` links ([#4521](#4521)) ([b94adb9](b94adb9)) * Add handling to `noir_wasm` for projects without dependencies ([#4344](#4344)) ([4982251](4982251)) * Allow type aliases in main ([#4505](#4505)) ([8a5359c](8a5359c)) * Ban Fields in for loop indices and bitwise ops ([#4376](#4376)) ([601fd9a](601fd9a)) * Brillig range check with consistent bit size ([#4357](#4357)) ([ea47d4a](ea47d4a)) * Build noir_codegen when publishing ([#4448](#4448)) ([cb1ceee](cb1ceee)) * Consistent bit size for truncate ([#4370](#4370)) ([dcd7a1e](dcd7a1e)) * Correct formatting for databus visibility types ([#4423](#4423)) ([cd796de](cd796de)) * Correct invalid brillig codegen for `EmbeddedCurvePoint.add` ([#4382](#4382)) ([5051ec4](5051ec4)) * **docs:** Update install versions ([#4396](#4396)) ([b283637](b283637)) * **docs:** Update noirjs_app for 0.23 ([#4378](#4378)) ([f77f702](f77f702)) * Enforce matching types of binary ops in SSA ([#4391](#4391)) ([70866ae](70866ae)) * Fix brillig slowdown when assigning arrays in loops ([#4472](#4472)) ([2a53545](2a53545)) * **flake:** Stop flake.nix removing ignored-tests.txt ([#4455](#4455)) ([ebaf05a](ebaf05a)) * Force src impl for == on slices ([#4507](#4507)) ([1691274](1691274)) * Handling of gh deps in noir_wasm ([#4499](#4499)) ([1d65370](1d65370)) * Iterative flattening pass ([#4492](#4492)) ([33c1ef7](33c1ef7)) * Noir test incorrect reporting (AztecProtocol/aztec-packages#4925) ([5f57ebb](5f57ebb)) * Only add `.nr` files to file manager ([#4380](#4380)) ([8536c7c](8536c7c)) * Remove panic when generic array length is not resolvable ([#4408](#4408)) ([00ab3db](00ab3db)) * Remove print from monomorphization pass ([#4417](#4417)) ([27c66b3](27c66b3)) * **ssa:** Handle mergers of slices returned from calls ([#4496](#4496)) ([f988d02](f988d02)) * Use correct type for numeric generics ([#4386](#4386)) ([0a1d109](0a1d109)) * Variables from trait constraints being permanently bound over when used within a trait impl ([#4450](#4450)) ([ac60ef5](ac60ef5)) ### Miscellaneous Chores * Bump msrv to 1.73.0 ([#4406](#4406)) ([b5e5c30](b5e5c30)) * **ci:** Bump MSRV to 1.72.1 and enforce that ACVM can be published using updated lockfile ([#4385](#4385)) ([2fc95d2](2fc95d2)) * Move noir out of yarn-project (AztecProtocol/aztec-packages#4479) ([78ef013](78ef013)) * Remove empty value from bounded vec ([#4431](#4431)) ([b9384fb](b9384fb)) * Reserve `unchecked` keyword ([#4432](#4432)) ([9544813](9544813)) </details> <details><summary>0.41.0</summary> ## [0.41.0](v0.40.0...v0.41.0) (2024-03-11) ### ⚠ BREAKING CHANGES * Internal as a macro (AztecProtocol/aztec-packages#4898) * move noir out of yarn-project (AztecProtocol/aztec-packages#4479) * note type ids (AztecProtocol/aztec-packages#4500) * rename bigint_neg into bigint_sub (AztecProtocol/aztec-packages#4420) * Add expression width into acir (AztecProtocol/aztec-packages#4014) * init storage macro (AztecProtocol/aztec-packages#4200) * **acir:** Move `is_recursive` flag to be part of the circuit definition (AztecProtocol/aztec-packages#4221) * Sync commits from `aztec-packages` ([#4144](#4144)) * Breaking changes from aztec-packages ([#3955](#3955)) * Rename Arithmetic opcode to AssertZero ([#3840](#3840)) * Remove unused methods on ACIR opcodes ([#3841](#3841)) * Remove partial backend feature ([#3805](#3805)) ### Features * Add bit size to const opcode (AztecProtocol/aztec-packages#4385) ([158c8ce](158c8ce)) * Add expression width into acir (AztecProtocol/aztec-packages#4014) ([158c8ce](158c8ce)) * Add instrumentation for tracking variables in debugging ([#4122](#4122)) ([c58d691](c58d691)) * Add poseidon2 opcode implementation for acvm/brillig, and Noir ([#4398](#4398)) ([10e8292](10e8292)) * Add support for overriding expression width ([#4117](#4117)) ([c8026d5](c8026d5)) * Added cast opcode and cast calldata (AztecProtocol/aztec-packages#4423) ([78ef013](78ef013)) * Allow brillig to read arrays directly from memory (AztecProtocol/aztec-packages#4460) ([158c8ce](158c8ce)) * Allow nested arrays and vectors in Brillig foreign calls (AztecProtocol/aztec-packages#4478) ([158c8ce](158c8ce)) * Allow variables and stack trace inspection in the debugger ([#4184](#4184)) ([bf263fc](bf263fc)) * **avm:** Back in avm context with macro - refactor context (AztecProtocol/aztec-packages#4438) ([158c8ce](158c8ce)) * **aztec-nr:** Initial work for aztec public vm macro (AztecProtocol/aztec-packages#4400) ([158c8ce](158c8ce)) * Aztec-packages ([#3754](#3754)) ([c043265](c043265)) * Backpropagate constants in ACIR during optimization ([#3926](#3926)) ([aad0da0](aad0da0)) * Breaking changes from aztec-packages ([#3955](#3955)) ([5be049e](5be049e)) * Evaluation of dynamic assert messages ([#4101](#4101)) ([c284e01](c284e01)) * Init storage macro (AztecProtocol/aztec-packages#4200) ([158c8ce](158c8ce)) * Internal as a macro (AztecProtocol/aztec-packages#4898) ([5f57ebb](5f57ebb)) * Note type ids (AztecProtocol/aztec-packages#4500) ([78ef013](78ef013)) * Remove range constraints from witnesses which are constrained to be constants ([#3928](#3928)) ([afe9c7a](afe9c7a)) * Remove replacement of boolean range opcodes with `AssertZero` opcodes ([#4107](#4107)) ([dac0e87](dac0e87)) * Speed up transformation of debug messages ([#3815](#3815)) ([2a8af1e](2a8af1e)) * Sync `aztec-packages` ([#4011](#4011)) ([fee2452](fee2452)) * Sync commits from `aztec-packages` ([#4068](#4068)) ([7a8f3a3](7a8f3a3)) * Sync commits from `aztec-packages` ([#4144](#4144)) ([0205d3b](0205d3b)) * Sync from aztec-packages ([#4483](#4483)) ([fe8f277](fe8f277)) ### Bug Fixes * Deserialize odd length hex literals ([#3747](#3747)) ([4000fb2](4000fb2)) * Noir test incorrect reporting (AztecProtocol/aztec-packages#4925) ([5f57ebb](5f57ebb)) * Remove panic from `init_log_level` in `acvm_js` ([#4195](#4195)) ([2e26530](2e26530)) * Return error rather instead of panicking on invalid circuit ([#3976](#3976)) ([67201bf](67201bf)) ### Miscellaneous Chores * **acir:** Move `is_recursive` flag to be part of the circuit definition (AztecProtocol/aztec-packages#4221) ([158c8ce](158c8ce)) * Move noir out of yarn-project (AztecProtocol/aztec-packages#4479) ([78ef013](78ef013)) * Remove partial backend feature ([#3805](#3805)) ([0383100](0383100)) * Remove unused methods on ACIR opcodes ([#3841](#3841)) ([9e5d0e8](9e5d0e8)) * Rename Arithmetic opcode to AssertZero ([#3840](#3840)) ([836f171](836f171)) * Rename bigint_neg into bigint_sub (AztecProtocol/aztec-packages#4420) ([158c8ce](158c8ce)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: Savio <72797635+Savio-Sou@users.noreply.github.com>
Description
Problem*
Resolves #4356
Supercedes #4360
Summary*
An ACIR dynamic array is a pointer to flat memory. We have been treating this flat memory as a list of fields, however, this breaks if we do in fact need accurate numeric type information such as when working black box function inputs. For example for hash inputs we set up the byte array based upon the bit size. This needs to be the correct bit size or else we will get a lot of extra garbage when calling
fetch_nearest_bytes
on a FieldElement.This PR attaches a list of
Vec<NumericType>
to theAcirDynamicArray
structure. This gives us the expected output result forsha
then.We probably could restrict the
AcirDynamicArray
to be created only through a constructor where we check that thevalue_types
match the supplied len in size. I left it for a follow-up as this is a quick fix but I can do it as part of this PR.Additional Context
Documentation*
Check one:
PR Checklist*
cargo fmt
on default settings.