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

Unexpected ABIError from benign code change #4778

Closed
alexghr opened this issue Apr 11, 2024 · 4 comments · Fixed by #4798
Closed

Unexpected ABIError from benign code change #4778

alexghr opened this issue Apr 11, 2024 · 4 comments · Fixed by #4798
Assignees
Labels
bug Something isn't working

Comments

@alexghr
Copy link
Contributor

alexghr commented Apr 11, 2024

Aim

I've encountered this issue doing two different things:

  1. I've added an if statement in aztec-packages in order to update a field on a struct
  2. I ran an "empty" transaction in aztec (a new BatchCall([])). Internally this uses if statements to only execute non-empty function calls

Expected Behavior

It should work (field should be updated in the first case/the tx should be valid in the second case)

Bug

In both instances aztec's private_kernel_init fails with an ABIError.

The first code change is here and leads to this error in CI

In the second case here's a minimal example and here's the CI run for it

In the first case we were able to find a workaround where we replaced the if statement with a "max" function and that has fixed the problem. I haven't looked for a workaround for the empty BatchCall example.

To Reproduce

Run the test in AztecProtocol/aztec-packages#5666

Project Impact

Blocker

Impact Context

A workaround was found for the first case. The second case, with the BatchCall, is contrived but realistically it should work since there's nothing wrong with it.

It's unclear what triggers the bug and what other (valid) use cases could lead to it.

Workaround

None

Workaround Description

We found this workaround for the first use case.

Additional Context

No response

Installation Method

Compiled from source

Nargo Version

No response

NoirJS Version

No response

Would you like to submit a PR for this Issue?

Maybe

Support Needs

No response

@alexghr alexghr added the bug Something isn't working label Apr 11, 2024
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Noir Apr 11, 2024
@TomAFrench
Copy link
Member

My understanding was that Aztec was using their own abi encoder though? Is that not correct?

@TomAFrench
Copy link
Member

@alexghr I've pushed a change which will highlight exactly which input is breaking type checks during abi encoding. Looks like your counter for note hash read requests is not a valid u32.

2024-04-12 16:42:37 end-to-end_1  |     Received promise rejected instead of resolved
2024-04-12 16:42:37 end-to-end_1  |     Rejected to value: [Error: (JSON-RPC PROPAGATED) The value passed for parameter `input.private_call.call_stack_item.public_inputs.note_hash_read_requests[0].counter` does not match the specified type:
2024-04-12 16:42:37 end-to-end_1  |     Value Field(11425886363827509554056097664977951424140858054424059518964806857959181738141) does not fall within range of allowable values for a Integer { sign: Unsigned, width: 32 }]

github-merge-queue bot pushed a commit that referenced this issue Apr 12, 2024
#4798)

# Description

## Problem\*

Resolves #3560 
Resolves #4778 

## Summary\*

This PR updates `InputValue.matches_abi` to
`InputValue.find_type_mismatch` which returns an error related to the
type which is failing to be abi encoded.

## Additional Context

This should help @alexghr with debugging #4778 

## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Noir Apr 12, 2024
@alexghr
Copy link
Contributor Author

alexghr commented Apr 13, 2024

Thanks @TomAFrench, I think this could this be related to #4600 where we were seeing corruption of the note_hash_read_requests. I'll retest the flow now that #4600 has been fixed (and there's extra debug info)

@alexghr
Copy link
Contributor Author

alexghr commented Apr 16, 2024

Can confirm the latest changes in Noir have fixed this issue https://github.com/AztecProtocol/aztec-packages/actions/runs/8701866086/job/23865456943

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
Development

Successfully merging a pull request may close this issue.

2 participants