Skip to content

Conversation

@tarcieri
Copy link
Member

@tarcieri tarcieri commented Jul 23, 2020

Converts ECDSA tests to use the new_signing_test! and new_verification_test! macros added to the new dev feature of the ecdsa crate in RustCrypto/signatures#103.

The macros use a few new APIs which were also added in this PR, namely AffinePoint::{from_compressed_point, from_uncompressed_point} which provide a more direct conversion path from SEC-1 encodings to AffinePoint which skip the PublicKey interface (which is more of a high-level, user-facing API).

@tarcieri tarcieri force-pushed the k256/use-ecdsa-test-macros branch 2 times, most recently from 7603899 to 7ac05ac Compare July 23, 2020 18:31
@codecov-commenter
Copy link

codecov-commenter commented Jul 23, 2020

Codecov Report

Merging #85 into master will increase coverage by 0.02%.
The diff coverage is 96.15%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #85      +/-   ##
==========================================
+ Coverage   53.70%   53.73%   +0.02%     
==========================================
  Files          17       17              
  Lines        3374     3376       +2     
==========================================
+ Hits         1812     1814       +2     
  Misses       1562     1562              
Impacted Files Coverage Δ
k256/src/ecdsa.rs 0.00% <ø> (ø)
k256/src/arithmetic.rs 85.28% <96.15%> (+0.09%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 061a1a2...4593ecd. Read the comment docs.

@tarcieri tarcieri force-pushed the k256/use-ecdsa-test-macros branch 2 times, most recently from 722b1f0 to 9147b66 Compare July 23, 2020 18:49
Converts ECDSA tests to use the `new_signing_test` and
`new_verification_test` macros added to the new `dev` feature of the
`ecdsa` crate in RustCrypto/signatures#103.

The macros use a few new APIs which were also added in this PR, namely
`AffinePoint::{from_compressed_point, from_uncompressed_point}` which
provide a more direct conversion path from SEC-1 encodings to
`AffinePoint` which skip the `PublicKey` interface (which is more of a
high-level, user-facing API)
@tarcieri tarcieri force-pushed the k256/use-ecdsa-test-macros branch from 9147b66 to 4593ecd Compare July 23, 2020 19:30
Comment on lines +43 to +88
# codecov:
# runs-on: ubuntu-latest
# steps:
# - name: Checkout repository
# uses: actions/checkout@v2
#
# - name: Cache cargo registry
# uses: actions/cache@v1
# with:
# path: ~/.cargo/registry
# key: ${{ runner.os }}-cargo-registry-${{ hashFiles('Cargo.lock') }}
#
# - name: Cache cargo index
# uses: actions/cache@v1
# with:
# path: ~/.cargo/git
# key: ${{ runner.os }}-cargo-index-${{ hashFiles('Cargo.lock') }}
#
# - name: Cache cargo build
# uses: actions/cache@v1
# with:
# path: target
# key: ${{ runner.os }}-coverage-cargo-build-target-${{ hashFiles('Cargo.lock') }}
#
# - name: Install stable toolchain
# uses: actions-rs/toolchain@v1
# with:
# toolchain: stable
# override: true
#
# - name: Run cargo-tarpaulin
# uses: actions-rs/[email protected]
# env:
# CARGO_INCREMENTAL: 0
# with:
# version: 0.14.2
# args: --all -- --test-threads 1
#
# - name: Upload to codecov.io
# uses: codecov/[email protected]
#
# - name: Archive code coverage results
# uses: actions/upload-artifact@v1
# with:
# name: code-coverage-report
# path: cobertura.xml
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure why but cargo-tarpaulin is failing:

https://github.com/RustCrypto/elliptic-curves/pull/85/checks?check_run_id=904067088

I attempted to reproduce it locally but cannot. It doesn't appear to be cache-related as this commit is a cache breaker due to the Cargo.lock changes.

I'd like to do a pass over p256 to get it in to a similar state as this PR (as part of #84) after which I can try re-enabling this.

@tarcieri tarcieri merged commit 5acab4d into master Jul 23, 2020
@tarcieri tarcieri deleted the k256/use-ecdsa-test-macros branch July 23, 2020 20:57
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