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

assert_max_bit_size() causes panic #6839

Closed
calummoore opened this issue Dec 17, 2024 · 4 comments · Fixed by #6840
Closed

assert_max_bit_size() causes panic #6839

calummoore opened this issue Dec 17, 2024 · 4 comments · Fixed by #6840
Labels
bug Something isn't working

Comments

@calummoore
Copy link

Aim

Verify that the the sum of two fields is less than the bit size, to prevent overflow attack.

Expected Behavior

Constraint is applied.

Bug

The application panicked (crashed).
Message: Non-numeric type variable used in expression expecting a value
Location: compiler/noirc_frontend/src/monomorphization/mod.rs:934

I also note in the docs that this field takes a parameter (field.assert_max_bit_size(32);), but it doesn't seem to in the Noir validation. I get an error if I try to use a parameter.

error: Function expects 1 parameter but 2 were given
   ┌─ src/main.nr:15:5
   │
15 │     output_value.assert_max_bit_size(32);
   │     ------------------------------------
   │

To Reproduce

#[recursive]
fn main(field1: Field, field2: Field) {
    let output_value = field1 + field2;

    // Contrain the output value
    output_value.assert_max_bit_size();
}

#[test]
fn test_main() {
    main(1, 2)
}

Workaround

None

Workaround Description

No response

Additional Context

No response

Project Impact

Blocker

Blocker Context

No response

Nargo Version

nargo version = 0.36.0 noirc version = 0.36.0+801c71880ecf8386a26737a5d8bb5b4cb164b2ab (git version hash: 801c718, is dirty: false)

NoirJS Version

n/a

Proving Backend Tooling & Version

No response

Would you like to submit a PR for this Issue?

None

Support Needs

No response

@calummoore calummoore added the bug Something isn't working label Dec 17, 2024
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Noir Dec 17, 2024
TomAFrench added a commit that referenced this issue Dec 17, 2024
@TomAFrench
Copy link
Member

TomAFrench commented Dec 17, 2024

Hey, thanks for opening this. Looks like the example in the docs is stale so I've opened #6840 to address this.

Due to *reasons*, we've moved the argument of this function into a numeric generic so to constrain output_value to fit in 32 bits you would need to modify your example as below.

#[recursive]
fn main(field1: Field, field2: Field) {
    let output_value = field1 + field2;

    // Contrain the output value
    output_value.assert_max_bit_size::<32>(); // See turbofish usage.
}

#[test]
fn test_main() {
    main(1, 2)
}

@calummoore
Copy link
Author

Ok, sounds good, it worked with specifying a value using the turbofish syntax.

Should it also work with output_value.assert_max_bit_size(); - i.e. no parameter? Currently that triggers an error.

@TomAFrench
Copy link
Member

There's no way for the compiler to know how many bits you want to constrain the field to if you don't use the turbofish syntax.

@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Noir Dec 17, 2024
@calummoore
Copy link
Author

Ah okay, I thought it would be the max Field size looking at the code.

github-merge-queue bot pushed a commit that referenced this issue Dec 20, 2024
TomAFrench added a commit to winderica/noir that referenced this issue Jan 3, 2025
* master:
  chore: add if/loop tip (separate from no-predicate noir-lang#5657) (noir-lang#6806)
  chore: move implementation of print foreign call into `nargo` (noir-lang#6865)
  chore: document format strings (noir-lang#6920)
  chore: add `rollup_root` and `rollup_block_merge` to tracked protocol circuits (noir-lang#6903)
  fix: consistent file_id across installation paths (noir-lang#6912)
  fix: bigint builtins are foreigns (noir-lang#6892)
  fix: remove unnecessary cast in bit-shift (noir-lang#6890)
  chore: Release Noir(1.0.0-beta.1) (noir-lang#6622)
  chore: Add `Instruction::Noop` (noir-lang#6899)
  chore: remove malformed functions from brillig reports (noir-lang#6898)
  chore: clean up gates reports script (noir-lang#6896)
  chore: move empty programs to `compile_success_empty` (noir-lang#6891)
  feat: add a warning when using unsafe blocks without safety comments (noir-lang#6860)
  chore: quick docs fix for noir-lang#6839 (noir-lang#6840)
  chore: Avoid duplicate Not instructions during flattening (noir-lang#6886)
  chore: Use smallvec for instruction results (noir-lang#6877)
  chore(ci): Display times in compilation and execution reports only with seconds (noir-lang#6880)
  feat: flatten nested if-else statements with equivalent conditions (noir-lang#6875)
  chore(ci): Take averages for compilation and execution report of small programs (noir-lang#6874)
  fix: don't deduplicate binary math of unsigned types (noir-lang#6848)
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.

2 participants