Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Bump k256 from 0.11.6 to 0.13.0 #13624

Merged
merged 2 commits into from
Mar 21, 2023
Merged

Bump k256 from 0.11.6 to 0.13.0 #13624

merged 2 commits into from
Mar 21, 2023

Conversation

dependabot[bot]
Copy link
Contributor

@dependabot dependabot bot commented on behalf of github Mar 17, 2023 •
edited by davxy
Loading

polkadot companion: paritytech/polkadot#6924
cumulus companion: paritytech/cumulus#2354

Bumps k256 from 0.11.6 to 0.13.0.

Commits

Dependabot compatibility score

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.

Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

@dependabot dependabot bot added A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. E2-dependencies Pull requests that update a dependency file. labels Mar 17, 2023
@bkchr
Copy link
Member

bkchr commented Mar 20, 2023

@dependabot rebase

@bkchr bkchr added the D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit label Mar 20, 2023
@bkchr bkchr requested a review from davxy March 20, 2023 12:01
@dependabot
Bump k256 from 0.11.6 to 0.13.0
dc1e128 
Bumps [k256](https://github.com/RustCrypto/elliptic-curves) from 0.11.6 to 0.13.0.
- [Release notes](https://github.com/RustCrypto/elliptic-curves/releases)
- [Commits](RustCrypto/elliptic-curves@k256/v0.11.6...k256/v0.13.0)

---
updated-dependencies:
- dependency-name: k256
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
@dependabot dependabot bot force-pushed the dependabot/cargo/k256-0.13.0 branch from f26874a to dc1e128 Compare March 20, 2023 12:11
@davxy
Copy link
Member

davxy commented Mar 21, 2023 •
edited
Loading

@bkchr @tarcieri compared to 0.11.6, the 0.13.0 noticeably improved the perfs, especially for signing operation.

Just to recap.

  • Currently we are using secp256k1 a rust wrapper around a C lib.
  • Not so long ago we were using libsecp256k1 a pure rust implementation.
  • We would like to switch to k256 pure rust implementation as soon as the perfs are fairly close to secp256k1.

Some numbers from criterion benches (https://github.com/davxy/crypto-benches/tree/main/ecdsa)

Signing

* rust-crypto (0.11.6)    [74.929 µs 74.966 µs 75.011 µs]
* rust-crypto (0.13.0)    [41.314 µs 41.320 µs 41.329 µs]
* secp256k1               [27.813 µs 27.829 µs 27.846 µs] 
* libsecp256k1            [95.844 µs 95.853 µs 95.864 µs]

Verification

* rust-crypto (0.11.6)   [99.389 µs 99.421 µs 99.459 µs]
* rust-crypto (0.13.0)   [88.997 µs 89.014 µs 89.032 µs]    
* secp256k1              [31.669 µs 31.697 µs 31.747 µs]
* libsecp256k1           [137.86 µs 137.93 µs 137.99 µs]

Conclusions

As can be seen secp256k1 is still the faster but the gap is not so huge, especially with respect to signing operation.

It would be awesome if we could squeeze the perfs a bit more wrt verification the IMO we can start using k256 all over the place

@bkchr
Copy link
Member

bkchr commented Mar 21, 2023

@davxy could you create the companions? 🙈

This was referenced Mar 21, 2023
Merged
Merged
@davxy
Copy link
Member

davxy commented Mar 21, 2023

bot merge

@paritytech-processbot
Copy link

Error: "Check reviews" status is not passing for paritytech/cumulus#2354

@davxy
Copy link
Member

davxy commented Mar 21, 2023

bot merge

@paritytech-processbot
Copy link

Waiting for commit status.

@paritytech-processbot
Copy link

Merge cancelled due to error. Error: Github API says paritytech/polkadot#6924 is not mergeable

@tarcieri
Copy link

It would be awesome if we could squeeze the perfs a bit more wrt verification the IMO we can start using k256 all over the place

@davxy we do have plans to improve verification in the next release. Several aspects of verification still operate in constant-time (which is a reasonable place to start when reusing code for both signing and verification).

In v0.13 we started using variable-time inversions for verification which slightly improved performance. In the next release we'd like to move to the generic implementation of wNAF provided by the group crate (already a dependency). There's some initial work here:

https://github.com/RustCrypto/elliptic-curves/pull/708/files

Unfortunately we need some upstream changes to the group crate first:

zkcrypto/group#46

@davxy
Copy link
Member

davxy commented Mar 21, 2023

bot merge

@paritytech-processbot paritytech-processbot bot merged commit a142360 into master Mar 21, 2023
@paritytech-processbot paritytech-processbot bot deleted the dependabot/cargo/k256-0.13.0 branch March 21, 2023 13:53
breathx pushed a commit to gear-tech/substrate that referenced this pull request Apr 22, 2023
@dependabot @davxy @breathx
Bump k256 from 0.11.6 to 0.13.0 (paritytech#13624)
5fcd2c9 
Bumps [k256](https://github.com/RustCrypto/elliptic-curves) from 0.11.6 to 0.13.0.
- [Release notes](https://github.com/RustCrypto/elliptic-curves/releases)
- [Commits](RustCrypto/elliptic-curves@k256/v0.11.6...k256/v0.13.0)

---
updated-dependencies:
- dependency-name: k256
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Davide Galassi <davxy@datawok.net>
@github-actions github-actions bot mentioned this pull request Jun 15, 2023
Closed
@kacperzuk-neti kacperzuk-neti mentioned this pull request Jun 23, 2023
Merged
15 tasks
@gilescope
Copy link
Contributor

The pure rust versions have the added advantage that they compile.

@gilescope gilescope mentioned this pull request Jun 30, 2023
Merged
5 tasks
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
@dependabot @davxy @nathanwhit
Bump k256 from 0.11.6 to 0.13.0 (paritytech#13624)
b3ef461 
Bumps [k256](https://github.com/RustCrypto/elliptic-curves) from 0.11.6 to 0.13.0.
- [Release notes](https://github.com/RustCrypto/elliptic-curves/releases)
- [Commits](RustCrypto/elliptic-curves@k256/v0.11.6...k256/v0.13.0)

---
updated-dependencies:
- dependency-name: k256
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Davide Galassi <davxy@datawok.net>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@acatangiu acatangiu acatangiu approved these changes

@davxy davxy davxy approved these changes

@bkchr bkchr bkchr approved these changes

Assignees
No one assigned
Labels
A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit E2-dependencies Pull requests that update a dependency file.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@bkchr @davxy @tarcieri @gilescope @acatangiu