p521: fiat-constify update

[MasterAwesome]

Implementation for the fiat-constify update @ RustCrypto/utils#992

TODOs

• Remove dep on primeorder for other crates.

• A nicer way to do .0.0 unfortunately without const deref this is probably not easily doable.

• Benchmarks? Maybe the codegen improves ever so slightly because of the copies that LLVM can't emit.

running 24 tests
test arithmetic::field::tests::decode_invalid_field_element_returns_err ... ok
test arithmetic::field::tests::delta_constant ... ok
test arithmetic::field::tests::invert ... ok
test arithmetic::field::tests::one_is_multiplicative_identity ... ok
test arithmetic::field::tests::root_of_unity_inv_constant ... ok
test arithmetic::field::tests::root_of_unity_constant ... ok
test arithmetic::field::tests::two_inv_constant ... ok
test arithmetic::field::tests::zero_is_additive_identity ... ok
test arithmetic::hash2curve::tests::hash_to_scalar_voprf ... ok
test arithmetic::field::tests::sqrt ... ok
test arithmetic::hash2curve::tests::params ... ok
test arithmetic::scalar::tests::one_is_multiplicative_identity ... ok
test arithmetic::scalar::tests::root_of_unity_inv_constant ... ok
test arithmetic::scalar::tests::delta_constant ... ok
test arithmetic::scalar::tests::two_inv_constant ... ok
test arithmetic::scalar::tests::zero_is_additive_identity ... ok
test arithmetic::scalar::tests::root_of_unity_constant ... ok
test arithmetic::scalar::tests::invert ... ok
test arithmetic::hash2curve::tests::hash_to_curve ... ok
test arithmetic::hash2curve::tests::from_okm_fuzz ... ok
test ecdsa::tests::sign::ecdsa_signing ... ok
test ecdsa::tests::verify::ecdsa_verify_invalid_s ... ok
test ecdsa::tests::verify::ecdsa_verify_success ... ok
test ecdsa::tests::wycheproof::wycheproof ... ok

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

     Running tests/projective.rs (/mnt/workspace/elliptic-curves/target/debug/deps/projective-c2effbb63ddd4dd6)

running 11 tests
test projective_add_vs_double ... ok
test affine_to_projective ... ok
test projective_add_and_sub ... ok
test projective_double_and_sub ... ok
test test_vector_add_mixed_identity ... ok
test projective_identity_addition ... ok
test test_vector_double_generator ... ok
test projective_mixed_addition ... ok
test test_vector_repeated_add ... ok
test test_vector_repeated_add_mixed ... ok
test test_vector_scalar_mult ... ok

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

   Doc-tests p521

running 3 tests
test p521/src/arithmetic/field.rs - arithmetic::field::FieldElement::fmt (line 342) ... ignored
test p521/src/ecdsa.rs - ecdsa (line 21) ... ok
test p521/src/ecdh.rs - ecdh (line 11) ... ok

test result: ok. 2 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 0.29s

CC: @tarcieri

[tarcieri]

Awesome! Looking great.

Really weird clippy failure though. Is this fully rebased? Nevermind, I guess that's due to the primeorder changes.

[MasterAwesome]

Sweet! I'll modify the local dependents of primeorder to pull from crates.io. Is there anything else needed in this patch? what do you think about the TODOs in the PR description?

[tarcieri]

A nicer way to do .0.0 unfortunately without const deref this is probably not easily doable.

Since you now have a newtype and the code is splatted into the crate, you can define methods on the newtype, which can be const fn and at least slightly better than .0.0 maybe.

If you decide to go that route, it's probably something to stick in the macros in primeorder (sidebar: I think I want to extract out a primefield crate, since all this field stuff in primeorder seems oddly overloaded)

[MasterAwesome]

Benchmarking results

field

field element operations/mul
                        time:   [75.775 ns 75.850 ns 75.922 ns]
                        change: [+0.0648% +0.1801% +0.2878%] (p = 0.00 < 0.05)
                        Change within noise threshold.

field element operations/square
                        time:   [44.344 ns 44.347 ns 44.350 ns]
                        change: [-0.1067% -0.0814% -0.0597%] (p = 0.00 < 0.05)
                        Change within noise threshold.

field element operations/invert
                        time:   [25.669 µs 25.670 µs 25.672 µs]
                        change: [-0.2368% -0.1409% -0.0697%] (p = 0.00 < 0.05)
                        Change within noise threshold.

field element operations/sqrt
                        time:   [24.540 µs 24.541 µs 24.543 µs]
                        change: [-7.4990% -7.4626% -7.4337%] (p = 0.00 < 0.05)
                        Performance has improved.

scalar

point operations/point-scalar mul
                        time:   [860.22 µs 860.82 µs 861.67 µs]
                        change: [+0.0278% +0.0997% +0.1814%] (p = 0.02 < 0.05)
                        Change within noise threshold.

scalar operations/sub   time:   [11.260 ns 11.261 ns 11.261 ns]
                        change: [-0.0063% +0.0069% +0.0186%] (p = 0.30 > 0.05)
                        No change in performance detected.

scalar operations/add   time:   [17.158 ns 17.160 ns 17.162 ns]
                        change: [-0.0575% -0.0344% -0.0117%] (p = 0.00 < 0.05)
                        Change within noise threshold.

scalar operations/mul   time:   [145.15 ns 145.20 ns 145.25 ns]
                        change: [-0.3466% -0.2725% -0.2113%] (p = 0.00 < 0.05)
                        Change within noise threshold.

scalar operations/negate
                        time:   [12.664 ns 12.669 ns 12.674 ns]
                        change: [+0.0797% +0.1884% +0.2810%] (p = 0.00 < 0.05)
                        Change within noise threshold.

scalar operations/invert
                        time:   [119.16 µs 119.19 µs 119.23 µs]
                        change: [-0.5475% -0.4806% -0.4174%] (p = 0.00 < 0.05)
                        Change within noise threshold.

Tested on:

• AMD Ryzen 5 3600 6-Core Processor @ 3950Mhz
• Ubuntu-23.10
• rustc 1.74.1 (a28077b28 2023-12-04)

[MasterAwesome]

-7% on sqrt surprisingly 😮. Also should I remove benches since it exceeds const_eval_limit?

[tarcieri]

Interesting that the tests pass on 1.65 but the benchmarks don't. I think they got rid of const_eval_limit at some point but I'm not sure that made its way into a stable release.

[MasterAwesome]

Interesting that the tests pass on 1.65 but the benchmarks don't. I think they got rid of const_eval_limit at some point but I'm not sure that made its way into a stable release.

I could add a TODO to clean it up and add the precomputed results for the inversions instead. Alternatively just ungate the benches at CI until const_eval_limit errors goes away.

[tarcieri]

@MasterAwesome maybe try building the benches on stable rather than 1.65? const_eval_limit was removed but I'm not quite sure when it shipped: rust-lang/rust@23f93a1

[tarcieri]

We're ready to start making breaking changes, so I'm going to go ahead and merge this.