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

Add From<i{int}> for signed integer conversion for Fields #663

Merged
merged 2 commits into from
Jul 24, 2023

Conversation

mmaker
Copy link
Member

@mmaker mmaker commented Jul 24, 2023

Signed integer conversion is already integrated in the source code definition for Fp but it's not exposed from the trait

@mmaker mmaker requested review from a team as code owners July 24, 2023 13:30
@mmaker mmaker requested review from Pratyush, mmagician and weikengchen and removed request for a team July 24, 2023 13:30
@mmaker mmaker changed the title Add From<i{int}> for signed integer support. Add From<i{int}> for signed integer conversion for Fields Jul 24, 2023
@Pratyush
Copy link
Member

Pratyush commented Jul 24, 2023

Looks like a good idea to me; does this passes local tests?

@mmaker
Copy link
Member Author

mmaker commented Jul 24, 2023

looks like?

maker@forbici ff % cargo test
warning: some crates are on edition 2021 which defaults to `resolver = "2"`, but virtual workspaces default to `resolver = "1"`
note: to keep the current resolver, specify `workspace.resolver = "1"` in the workspace root's manifest
note: to use the edition 2021 resolver, specify `workspace.resolver = "2"` in the workspace root's manifest
    Finished test [optimized + debuginfo] target(s) in 0.14s
     Running unittests src/lib.rs (/Users/maker/Code/arkworks/algebra/target/debug/deps/ark_ff-829d73734feaaeeb)

running 21 tests
test biginteger::arithmetic::test_find_relaxed_naf_usefulness ... ok
test biginteger::tests::test_biginteger64 ... ok
test biginteger::tests::test_biginteger128 ... ok
test biginteger::tests::test_biginteger256 ... ok
test biginteger::tests::test_biginteger448 ... ok
test biginteger::tests::test_biginteger384 ... ok
test biginteger::tests::test_biginteger768 ... ok
test biginteger::tests::test_biginteger832 ... ok
test const_helpers::tests::test_mul_buffer_correctness ... ok
test biginteger::arithmetic::test_find_relaxed_naf_correctness ... ok
test fields::models::cubic_extension::cube_ext_tests::test_from_base_prime_field_element ... ok
test fields::models::cubic_extension::cube_ext_tests::test_from_base_prime_field_elements ... ok
test fields::models::cubic_extension::cube_ext_tests::test_norm_for_towers ... ok
test fields::models::fp12_2over3over2::test::test_characteristic_square_mod_6_is_one ... ok
test fields::models::fp::montgomery_backend::test::test_mont_macro_correctness ... ok
test fields::models::quadratic_extension::quad_ext_tests::test_from_base_prime_field_element ... ok
test fields::models::quadratic_extension::quad_ext_tests::test_from_base_prime_field_elements ... ok
test fields::no_std_tests::test_from_into_biguint ... ok
test const_helpers::tests::test_mul_buffer_soundness - should panic ... ok
test fields::no_std_tests::test_batch_inversion ... ok
test fields::no_std_tests::test_from_be_bytes_mod_order ... ok

test result: ok. 21 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s

   Doc-tests ark-ff

running 39 tests
test ff/src/biginteger/mod.rs - biginteger::BigInteger::is_zero (line 905) ... ok
test ff/src/biginteger/mod.rs - biginteger::BigInteger::div2 (line 841) ... ok
test ff/src/biginteger/mod.rs - biginteger::BigInteger::from_bits_le (line 959) ... ok
test ff/src/biginteger/mod.rs - biginteger::BigInteger::get_bit (line 932) ... ok
test ff/src/biginteger/mod.rs - biginteger::BigInteger::add_with_carry (line 749) ... ok
test ff/src/biginteger/mod.rs - biginteger::BigInteger::divn (line 863) ... ok
test ff/src/biginteger/mod.rs - biginteger::BigInt (line 77) ... ok
test ff/src/biginteger/mod.rs - biginteger::BigInteger::is_odd (line 883) ... ok
test ff/src/biginteger/mod.rs - biginteger::BigInteger::from_bits_be (line 945) ... ok
test ff/src/biginteger/mod.rs - biginteger::BigInteger::is_even (line 894) ... ok
test ff/src/biginteger/mod.rs - biginteger::BigInteger::mul2 (line 791) ... ok
test ff/src/biginteger/mod.rs - biginteger::BigInteger::muln (line 816) ... ok
test ff/src/biginteger/mod.rs - biginteger::BigInteger::num_bits (line 915) ... ok
test ff/src/biginteger/mod.rs - biginteger::BigInteger::to_bits_le (line 990) ... ok
test ff/src/biginteger/mod.rs - biginteger::BigInteger::sub_with_borrow (line 771) ... ok
test ff/src/biginteger/mod.rs - biginteger::BigInteger::to_bits_be (line 973) ... ok
test ff/src/biginteger/mod.rs - biginteger::BigInteger::to_bytes_be (line 1007) ... ok
test ff/src/biginteger/mod.rs - biginteger::BigInteger::to_bytes_le (line 1022) ... ok
test ff/src/const_helpers.rs - const_helpers::const_for (line 8) ... ok
test ff/src/biginteger/mod.rs - biginteger::signed_mod_reduction (line 687) ... ok
test ff/src/fields/field_hashers/mod.rs - fields::field_hashers::DefaultFieldHasher (line 30) ... ok
test ff/src/fields/mod.rs - fields::Field (line 117) ... ok
test ff/src/fields/models/cubic_extension.rs - fields::models::cubic_extension::CubicExtField<P>::new (line 103) ... ok
test ff/src/fields/models/fp/montgomery_backend.rs - fields::models::fp::montgomery_backend::MontFp (line 576) ... ok
test ff/src/fields/mod.rs - fields::Field::from_base_prime_field (line 230) ... ok
test ff/src/fields/mod.rs - fields::Field (line 142) ... ok
test ff/src/fields/models/quadratic_extension.rs - fields::models::quadratic_extension::QuadExtField<P>::new (line 119) ... ok
test ff/src/fields/models/fp2.rs - fields::models::fp2::Fp2<P>::mul_assign_by_fp (line 105) ... ok
test ff/src/fields/sqrt.rs - fields::sqrt::LegendreSymbol (line 4) ... ok
test ff/src/fields/models/fp3.rs - fields::models::fp3::Fp3<P>::mul_assign_by_fp (line 78) ... ok
test ff/src/fields/models/quadratic_extension.rs - fields::models::quadratic_extension::QuadExtField<P>::norm (line 144) ... ok
test ff/src/fields/prime.rs - fields::prime::PrimeField (line 8) ... ok
test ff/src/fields/sqrt.rs - fields::sqrt::LegendreSymbol::is_qnr (line 38) ... ok
test ff/src/fields/sqrt.rs - fields::sqrt::LegendreSymbol::is_qr (line 49) ... ok
test ff/src/fields/sqrt.rs - fields::sqrt::LegendreSymbol::is_zero (line 23) ... ok
test ff/src/lib.rs - (line 60) ... ok
test ff/src/lib.rs - (line 135) ... ok
test ff/src/lib.rs - (line 162) ... ok
test ff/src/lib.rs - (line 96) ... ok

test result: ok. 39 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 10.18s
maker@forbici ff % git status
On branch master
Your branch is ahead of 'origin/master' by 1 commit.
  (use "git push" to publish your local commits)

nothing to commit, working tree clean
maker@forbici ff % git push mmaker
Everything up-to-date

@Pratyush
Copy link
Member

Sounds good! I think we might not have tests for this functionality, so if you could add those then we can merge ASAP. Thanks!

@mmaker
Copy link
Member Author

mmaker commented Jul 24, 2023

we don't have them for unsigned integers either, do we?

@mmaker
Copy link
Member Author

mmaker commented Jul 24, 2023

(just so that I can add them too)

@Pratyush
Copy link
Member

We don't I think. If you could add those in this PR that would be great, otherwise I can do it in a follow-up PR.

@mmaker
Copy link
Member Author

mmaker commented Jul 24, 2023

added

@mmaker mmaker merged commit 13fd33e into arkworks-rs:master Jul 24, 2023
6 of 15 checks passed
aleasims pushed a commit to NilFoundation/arkworks-algebra that referenced this pull request Oct 18, 2023
aleasims added a commit to NilFoundation/arkworks-algebra that referenced this pull request Oct 18, 2023
mmagician pushed a commit to mmagician/algebra that referenced this pull request Nov 3, 2023
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.

2 participants