Skip to content

Commit

Permalink
fix(acvm): Clear ACIR call stack after successful circuit execution (#…
Browse files Browse the repository at this point in the history
…5783)

# Description

## Problem\*

Resolves <!-- Link to GitHub Issue -->

No issue as small issue found while working on bug related to brillig.

## Summary\*

Changing `fold_nested_brillig_assert_fail` to the following main (where
we expect the second call to fail):
```rust
fn main(x: Field) {
    assert(1 == fold_conditional_wrapper(!x as bool));
    assert(1 == fold_conditional_wrapper(x as bool));
}
```
would lead to this call stack:
```
error: Failed to solve program: 'Failed to solve brillig function'
   ┌─ /mnt/user-data/maxim/noir/test_programs/execution_failure/fold_nested_brillig_assert_fail/src/main.nr:27:12
   │
27 │     assert(x);
   │            -
   │
   = Call stack:
     1. /mnt/user-data/maxim/noir/test_programs/execution_failure/fold_nested_brillig_assert_fail/src/main.nr:6:17
     2. /mnt/user-data/maxim/noir/test_programs/execution_failure/fold_nested_brillig_assert_fail/src/main.nr:12:5
     3. /mnt/user-data/maxim/noir/test_programs/execution_failure/fold_nested_brillig_assert_fail/src/main.nr:7:17
     4. /mnt/user-data/maxim/noir/test_programs/execution_failure/fold_nested_brillig_assert_fail/src/main.nr:12:5
     5. /mnt/user-data/maxim/noir/test_programs/execution_failure/fold_nested_brillig_assert_fail/src/main.nr:18:9
     6. /mnt/user-data/maxim/noir/test_programs/execution_failure/fold_nested_brillig_assert_fail/src/main.nr:23:5
     7. /mnt/user-data/maxim/noir/test_programs/execution_failure/fold_nested_brillig_assert_fail/src/main.nr:27:12
```
These lines should not be included and reference the successful call
before the second one:
```
     1. /mnt/user-data/maxim/noir/test_programs/execution_failure/fold_nested_brillig_assert_fail/src/main.nr:6:17
     2. /mnt/user-data/maxim/noir/test_programs/execution_failure/fold_nested_brillig_assert_fail/src/main.nr:12:5
```
The following change now has the correct call stack:
```rust
error: Failed to solve program: 'Failed to solve brillig function'
   ┌─ /mnt/user-data/maxim/noir/test_programs/execution_failure/fold_nested_brillig_assert_fail/src/main.nr:27:12
   │
27 │     assert(x);
   │            -
   │
   = Call stack:
     1. /mnt/user-data/maxim/noir/test_programs/execution_failure/fold_nested_brillig_assert_fail/src/main.nr:7:17
     2. /mnt/user-data/maxim/noir/test_programs/execution_failure/fold_nested_brillig_assert_fail/src/main.nr:12:5
     3. /mnt/user-data/maxim/noir/test_programs/execution_failure/fold_nested_brillig_assert_fail/src/main.nr:18:9
     4. /mnt/user-data/maxim/noir/test_programs/execution_failure/fold_nested_brillig_assert_fail/src/main.nr:23:5
     5. /mnt/user-data/maxim/noir/test_programs/execution_failure/fold_nested_brillig_assert_fail/src/main.nr:27:12
```

## Additional Context



## Documentation\*

Check one:
- [ ] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** 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.
  • Loading branch information
vezenovm authored Aug 21, 2024
1 parent 50a6b90 commit 656a7d6
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// The features being tested is using assert on brillig that is triggered through nested ACIR calls.
// We want to make sure we get a call stack from the original call in main to the failed assert.
fn main(x: Field) {
assert(1 == fold_conditional_wrapper(!x as bool));
assert(1 == fold_conditional_wrapper(x as bool));
}

Expand Down
4 changes: 4 additions & 0 deletions tooling/nargo/src/ops/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,10 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver<F>, E: ForeignCallExecutor<F>>
}
}
}
// Clear the call stack if we have succeeded in executing the circuit.
// This needs to be done or else all successful ACIR call stacks will also be
// included in a failure case.
self.call_stack.clear();

Ok(acvm.finalize())
}
Expand Down

0 comments on commit 656a7d6

Please sign in to comment.