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

noir_bignum 0.4.0 support #28

Closed
wants to merge 1 commit into from
Closed

noir_bignum 0.4.0 support #28

wants to merge 1 commit into from

Conversation

jp4g
Copy link

@jp4g jp4g commented Nov 3, 2024

Description

Problem*

Updates to noir-bignum 0.4.0

Summary*

Updates to use the new noir-bignum api in 0.4.0

Tested to work with 2048 bit keygen so it is 90% of the way done

Additional Context

Todo still:

  1. I cannot get the 1024 bit tests working for some reason - gives
error: Failed to solve program: 'Failed to solve brillig function'
  ┌─ /home/user/nargo/github.com/noir-lang/noir-bignumv0.4.0/src/utils/split_bits.nr:86:16
  │
86 │         assert(hi == 0);
  1. I have not updated the readme
  2. I have not updated the signature gen rust crate to match the new input api

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@jp4g
Copy link
Author

jp4g commented Nov 3, 2024

Also compilation of example on 0.37.0 gives

The application panicked (crashed).
Message:  assertion failed: i < b
Location: /home/runner/work/noir/noir/acvm-repo/acvm/src/compiler/optimizers/merge_expressions.rs:75

from the rsa.verify_sha256_pkcs1v15(hash, bn,65537) line which I have no clue how to address

@jp4g jp4g mentioned this pull request Nov 3, 2024
@iAmMichaelConnor
Copy link
Collaborator

Thanks @jp4g, we're working to understand why the 1024-bit test failed. It looks to be a problem with release 0.3.7 of bignum. We'll let you know what we find.

@iAmMichaelConnor
Copy link
Collaborator

We've also found the same bug relating to updating to Noir 0.37.0, thanks again! We're investigating that too :)

@TomAFrench
Copy link
Member

Thanks for this. We've updated this library to use the new bignum library in #30 so I'm going to close this PR.

@TomAFrench TomAFrench closed this Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants