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

Failed to solve brillig function, reason: explicit trap hit in brillig 'self._is_some' #3689

Closed
jp4g opened this issue Dec 13, 2023 · 5 comments
Assignees
Labels
T-feedback Type: recording user feedback

Comments

@jp4g
Copy link

jp4g commented Dec 13, 2023

Issue with calling private function from private function returns Failed to solve brillig function, reason: explicit trap hit in brillig 'self._is_some'. This is likely from notes not being emitted properly and happens when one function calls another function. However, calling the functions sequentially in different transactions does not encounter this issue.

Reproduce:

  1. clone https://github.com/Mach-34/aztec-state-channels/
  2. switch to branch brillig_issue and install
  3. ensure sandbox is running through docker
  4. run yarn test
  • This code runs increment_multiple which recursively calls increment_multiple conditionally (increment value until value == end)
  • The same issue occurs even when increment_multiple calls increment which results in a deterministic callstack depth of 2 (not conditional according to number of calls needed to make value == end)
  • This is being experienced on 0.16.7 with the docker sandbox/ aztec-cli packages
@github-project-automation github-project-automation bot moved this to Todo in A3 Dec 13, 2023
@critesjosh critesjosh added the T-feedback Type: recording user feedback label Dec 14, 2023
@jp4g
Copy link
Author

jp4g commented Dec 15, 2023

After updating to 0.16.9, I tried making a super basic private -> private function call in the originally linked repository:

#[aztec(private)]
fn function_one() {
    context.call_private_function_no_args(
        context.this_address(),
        compute_selector("function_two()")
    );
}

#[aztec(private)]
fn function_two() { }

Which encountered the same error.

So I spun up an example repo that explicitly demonstrated how it didn't work. However, the contract called another function perfectly normally (as I've been able to do in any other project as well). I also tried the counter contract and trying to reproduce the issue in the new repository (albeit with different notes) and this as well worked fine.

Edit: added in the minimal logic for the original CounterStateChannel contract and it still encounters the brillig.is_some issue. Currently exploring whether note implementation or contract-level nullifier hash function is incorrectly implemented. More details in the example repo

@LHerskind LHerskind self-assigned this Dec 17, 2023
@LHerskind
Copy link
Contributor

@jp4g, I will take a look at this the coming week.

@LHerskind
Copy link
Contributor

@jp4g think I found an issue with the contract. So when you are calling get_note(false) to read the value, that is emitting a nullifier, then later when you are calling replace() you try to emit the same nullifier. So it is trying to "double-spend". The error message is a bit funky, I just tried with the master branch instead of the release, and then I'm getting Assertion Failed: Hinted commitment does not match so might be something additional wrong.

I tried just creating a get_unsafe_note which does not emit the nullifier when reading and then the code works, but it is not secure anymore.

I think this is a flaw in the singleton, as its current construction don't allow you to have a value at t' that is dependent on the value at t + function inputs.

@jp4g I havent followed to closely what you are working on, but does it need the singletons? I haven't been using them much myself as things mostly can be modelled with the sets, but that might be a bit more nasty. (Just as a temp fix for you while singleton is to be looked at). Often you can make a wrapper around a set that code-wise makes it behave very similar.

People that might find interest in this: @sirasistant, @critesjosh, @signorecello

Created #3753

@LHerskind
Copy link
Contributor

Referring to #3123 for completeness

@LHerskind
Copy link
Contributor

Closing this issue as the problem was found, and is to be addressed through #3753

@github-project-automation github-project-automation bot moved this from Todo to Done in A3 Jan 2, 2024
@LHerskind LHerskind removed this from A3 Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-feedback Type: recording user feedback
Projects
None yet
Development

No branches or pull requests

3 participants