Skip to content
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

ACIR test array_dynamic_blackbox_input fails to verify in BB #4356

Closed
sirasistant opened this issue Feb 13, 2024 · 1 comment · Fixed by #4364
Closed

ACIR test array_dynamic_blackbox_input fails to verify in BB #4356

sirasistant opened this issue Feb 13, 2024 · 1 comment · Fixed by #4364
Assignees
Labels
bug Something isn't working

Comments

@sirasistant
Copy link
Contributor

sirasistant commented Feb 13, 2024

Aim

Passing checks in aztec-packages

Expected Behavior

All acir tests should verify

Bug

This test https://github.com/noir-lang/noir/blob/b2aaeab319a0c66c431a7db6852f743eccde8e98/test_programs/execution_success/array_dynamic_blackbox_input/src/main.nr fails to verify

https://app.circleci.com/pipelines/github/AztecProtocol/aztec-packages/24623/workflows/dbb07d38-8b8c-4334-89c0-db3510d9bc3f/jobs/1143720
Reproduced locally:

noirup: done
% nargo execute                                                                   1m 26s ~/aztec-packages/noir/test_programs/execution_success/array_dynamic_blackbox_input arv/pull_noir_2 mainframe
[array_dynamic_blackbox_input] Circuit witness successfully solved
% nargo prove                                                                            ~/aztec-packages/noir/test_programs/execution_success/array_dynamic_blackbox_input arv/pull_noir_2 mainframe
% nargo verify                                                                       12s ~/aztec-packages/noir/test_programs/execution_success/array_dynamic_blackbox_input arv/pull_noir_2 mainframe
Failed to verify proof /mnt/user-data/alvaro/aztec-packages/noir/test_programs/execution_success/array_dynamic_blackbox_input/proofs/array_dynamic_blackbox_input.proof

To Reproduce

  1. go to array_dynamic_blackbox_input
  2. nargo execute
  3. nargo prove
  4. nargo verify => fails

Project Impact

None

Impact Context

No response

Workaround

None

Workaround Description

No response

Additional Context

No response

Installation Method

None

Nargo Version

No response

NoirJS Version

No response

Would you like to submit a PR for this Issue?

None

Support Needs

No response

@sirasistant sirasistant added the bug Something isn't working label Feb 13, 2024
@sirasistant sirasistant changed the title array_dynamic_blackbox_input fails to verify in BB ACIR test array_dynamic_blackbox_input fails to verify in BB Feb 13, 2024
@guipublic
Copy link
Contributor

The issue is fixed with PR #4360, it seems that old sha256 was giving the wrong result.
if you add:
println(sha256(hash_input);
println(sha256([hash_input[0],..[hash_input[64]);
you will not get the same result!!!

Instead, with sha256::digest() you get the same result and this result is consistent with sha256([hash_input[0],..[hash_input[64]);

github-merge-queue bot pushed a commit that referenced this issue Feb 14, 2024
# 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 the `AcirDynamicArray`
structure. This gives us the expected output result for `sha` then.

We probably could restrict the `AcirDynamicArray` to be created only
through a constructor where we check that the `value_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:
- [ ] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[Exceptional Case]** Documentation to be submitted in a separate
PR.

# PR Checklist\*

- [ ] I have tested the changes locally.
- [ ] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

---------

Co-authored-by: TomAFrench <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
3 participants