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

fix: fixed reduction parameter error #31

Merged
merged 5 commits into from
Oct 10, 2024
Merged

Conversation

zac-williamson
Copy link
Collaborator

redc_param previously was only large enough to cover barrett reductions whose input was < 2^{modulus_bits * 2 + 2}

this was insufficient for elliptic curve arithmetic in bignum

redc_param is now large enough to cover barrett reduction inputs of at least 16 * modulus^2

additonally, library upgraded to not trigger compiler warnings for nargo 0.35.0

Description

Problem*

Resolves

Summary*

Additional Context

PR Checklist*

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

redc_param previously was only large enough to cover barrett reductions whose input was < 2^{modulus_bits * 2 + 2}

this was insufficient for elliptic curve arithmetic in bignum

redc_param is now large enough to cover barrett reduction inputs of at least 16 * modulus^2

additonally, library upgraded to not trigger compiler warnings for nargo 0.35.0
@zac-williamson zac-williamson changed the title fixed reduction parameter error fix: fixed reduction parameter error Oct 9, 2024
src/fields/U768.nr Outdated Show resolved Hide resolved
Copy link
Contributor

@vezenovm vezenovm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, mostly nits

src/runtime_bignum_test.nr Outdated Show resolved Hide resolved
let Y1: BigNum<$N, $typ> = unsafe{ bn.__derive_from_seed([i as u8,i as u8,6,7]) };
let Z1: BigNum<$N, $typ> = BigNum::one();

let (_, YY_mul_2): (BigNum<$N, $typ>, BigNum<$N, $typ> ) = unsafe{BigNum::__compute_quadratic_expression([[Y1]], [[false]], [[Y1, Y1]], [[false, false]], [], [])};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

file could use a nargo fmt. Although, I know the formatter struggles with macros.

src/runtime_bignum_test.nr Outdated Show resolved Hide resolved
src/runtime_bignum_test.nr Outdated Show resolved Hide resolved
};
let result = unsafe {
bn.__mul(u, v)
};
let expected: BigNum<N, Params> = BigNum::one();
assert(result.limbs == expected.limbs);
}
}

fn assert_is_not_equal<let N: u32, Params>(bn: BigNumInstance<N, Params>) where Params: BigNumParamsTrait<N> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this func used anywhere now that we have assert_is_not_equal in the macro test with

#[make_test(quote{3},quote{BNParams})]
pub struct BNTests{}

Seems we can delete some of these functions that take in a BigNumInstance as inputs.

@zac-williamson zac-williamson merged commit c312ef7 into main Oct 10, 2024
4 checks passed
@github-actions github-actions bot mentioned this pull request Oct 10, 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