Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

replace old libsecp256k1 fork with upstream; boosts k1 recovery performance #9885

Merged
merged 2 commits into from
Jan 27, 2021

Conversation

spoonincode
Copy link
Contributor

Change Description

Replace usage of an old libsecp256k1 fork with latest upstream. This allows us to make use of (recently expired patented) improvements boosting key recovery performance. My measurements of the improvement:

CPU Previous New
Intel 9600k 50µs 37µs
AMD 3950X 60µs 43µs

(exact performance numbers will vary based on hardware and many other factors; these numbers we gathered with only a single core loaded)

Okay, full disclosure: the old secp256k1 fork we were using could have the patented code enabled (and the performance improvement) with a compile time option. However, I'm taking this opportunity to upgrade us to the upstream mainline code instead of an old fork. Some users have voiced concern over using an aging non-upstream fork (#8984) and while I don't think there is any reasonable chance that Cryptonomex's repos will disappear, secp256k1's main upstream repo almost certainly won't.

Needs EOSIO/fc#180, where much of the changes actually are

Change Type

Select ONE:

  • Documentation
  • Stability bug fix
  • Other
  • Other - special case

Testing Changes

Select ANY that apply:

  • New Tests
  • Existing Tests
  • Test Framework
  • CI System
  • Other

Consensus Changes

  • Consensus Changes

API Changes

  • API Changes

Documentation Additions

  • Documentation Additions

@nksanthosh nksanthosh merged commit 11bceac into develop Jan 27, 2021
@nksanthosh nksanthosh deleted the upstream_secp256k1 branch January 27, 2021 18:49
@spoonincode
Copy link
Contributor Author

Hey guys, we really need to approve and merge EOSIO/fc#180 first. Otherwise, now develop's submodule is no longer pointing to fc's eosio branch

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants