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

Migrate mod_exp to BoringSSL, removing GMP requirement #1277

Open
Tracked by #41
bhazzard opened this issue Jun 13, 2023 · 5 comments · Fixed by #1604 or #1635
Open
Tracked by #41

Migrate mod_exp to BoringSSL, removing GMP requirement #1277

bhazzard opened this issue Jun 13, 2023 · 5 comments · Fixed by #1604 or #1635
Assignees

Comments

@bhazzard
Copy link

bhazzard commented Jun 13, 2023

As part of the effort to remove system provided "mathy" dependencies that could unexpectedly change out from under us and affect protocol consensus, remove the sole remaining usage of libgmp: the modexp host function. This likely can be refactored to use boringssl fairly easily, and that then opens up the possibility for #1255 too since we can maintain a fork with the needed hooks.

We need to be mindful to measure performance difference between the two; and pay special attention that boringssl has different optimization paths depending on the input parameters (this may mean we ought to create specialized benchmarks for such inputs).

We must add unit tests for all pertinent modexp test cases that are part of boringssl and ensure they are equivalent to the existing consensus rules. The output of boringssl/openssl's modexp has changed over time to rectify implementation defects and while the expectation is gmp's modexp is the same as boringssl at the current time, these unit tests exercise some fantastic corner cases. These unit tests should be added in a discrete PR, or at least pushed individually, for easy verification that the results are as expected on gmp.

@bhazzard
Copy link
Author

Unit tests could potentially be a separate PR.

@BenjaminGormanPMP
Copy link

Issue moved from iteration to backlog. Per @spoonincode - this issue is dependent on #644.

@greg7mdp
Copy link
Contributor

Wondering if Boost multiprecision could be used?

@spoonincode
Copy link
Member

Wondering if Boost multiprecision could be used?

https://www.boost.org/doc/libs/1_83_0/libs/multiprecision/doc/html/boost_multiprecision/perf/integer_performance.html
table 1.71 makes it look like the non-GMP impls Boost MP supports are slower than GMP too

@greg7mdp
Copy link
Contributor

Ha, that's right. Not quite as bad as your Boring results though. Not sure how critical this is.

@bhazzard bhazzard removed this from the Leap v5.0.0-rc1 Critical milestone Sep 19, 2023
@BenjaminGormanPMP BenjaminGormanPMP moved this from Blocked to Icebox in Team Backlog Sep 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment