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

Point Addition in Unentered else Block Causes Circuit Failure #5045

Closed
LHerskind opened this issue May 17, 2024 · 3 comments · Fixed by #5076
Closed

Point Addition in Unentered else Block Causes Circuit Failure #5045

LHerskind opened this issue May 17, 2024 · 3 comments · Fixed by #5076
Assignees
Labels
bug Something isn't working

Comments

@LHerskind
Copy link
Contributor

Aim

A point addition inside an else that is NOT "entered" will cause the circuit to fail entirely. The simulator seems to apply the checks when generating the witness and if using 0 as the base value just fails.

I encountered this when having a computation that is only to happen when we have non-zero values, but anyway fails even if unused. A minimal test to reproduce is here.

#[test]
fn weird_add() {
    let a = EmbeddedCurvePoint {
        x: 0x1d8eb4378a3bde41e0b6a9a8dcbd21b7ff9c51bdd6ca13ce989abbbf90df3666,
        y: 0x06075b63354f2504f9cddba0b94ed0cef35fc88615e69ec1f853b51eb79a24a0
    };

    let aa = a.double();

    if aa.x == a.add(a).x {
        println(f"We are not trying to do a double of zero");
    } else {
        println("We are trying to do a double of zero");
        let d = EmbeddedCurvePoint { x: 0, y: 0 }.double();
        println(f"d*2 {d}");
    }
}

Similar failure is possible with the multi_scalar_mul and fixed_base_scalar_mul if providing fields with values > 128 bits inside the if.

Expected Behavior

I expected to see

We are not trying to do a double of zero

And have the test pass.

Bug

Instead the test fails with the following log.

We are not trying to do a double of zero
[noir_aztec] Testing encrypted_logs::payload::weird_add... FAIL
Failed to solve program: 'Failed to solve blackbox function: embedded_curve_add, reason: Point (0000000000000000000000000000000000000000000000000000000000000000, 0000000000000000000000000000000000000000000000000000000000000000) is not on curve'

To Reproduce

  1. Run the test
  2. Profit???

Project Impact

Blocker

Impact Context

We need to use the computation to constrain log production in Aztec.

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

@LHerskind LHerskind added the bug Something isn't working label May 17, 2024
@TomAFrench
Copy link
Member

This is addressed by AztecProtocol/aztec-packages#6384

Dupe of #4978

@TomAFrench
Copy link
Member

TomAFrench commented May 17, 2024

Reopening as this is actually different and won't be addressed in that PR.

@TomAFrench
Copy link
Member

We'll add some behaviour in the flattening pass to apply predicates to these inputs in if statements. This will be blocked pending being able to represent the point at infinity for now though.

github-merge-queue bot pushed a commit that referenced this issue May 30, 2024
# Description

## Problem\*

Resolves #5045

## Summary\*
use predicate to set is_infinite to true when side_effects are disabled,
to ensure that the curve operation will not fail.


## Additional Context



## 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.
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
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants