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

Check for infinity in eip-196 ecmul #7509

Merged
merged 3 commits into from
Aug 28, 2024

Conversation

garyschulte
Copy link
Contributor

@garyschulte garyschulte commented Aug 23, 2024

PR description

Fixed Issue(s)

Highlighted by arewefastyet report, this PR addresses a recent regression in performance for eip-196 scalar multiplication when the point is at infinity

local testing shows significant improvement for these benchmarks:

  24.6.0 24.7.1 24.8.0 24.8-develop
EcMulInfinities2Scalar 1455.8 381.58 390.36 2526.56
EcMulInfinities32ByteScalar 1246.28 242.1 245.67 2478.99

related to #7542

Thanks for sending a pull request! Have you done the following?

  • Checked out our contribution guidelines?
  • Considered documentation and added the doc-change-required label to this PR if updates are required.
  • Considered the changelog and included an update if required.
  • For database changes (e.g. KeyValueSegmentIdentifier) considered compatibility and performed forwards and backwards compatibility tests

Locally, you can run these tests to catch failures early:

  • unit tests: ./gradlew build
  • acceptance tests: ./gradlew acceptanceTest
  • integration tests: ./gradlew integrationTest
  • reference tests: ./gradlew ethereum:referenceTests:referenceTests

@garyschulte garyschulte force-pushed the feature/ecmul-infinity branch from a3aa2a9 to 1262ec5 Compare August 23, 2024 00:26
@garyschulte garyschulte changed the title Check for infinity before calling out to jni for ecmul Check for infinity for ecmul Aug 23, 2024
@garyschulte garyschulte changed the title Check for infinity for ecmul Check for infinity in eip-196 ecmul Aug 23, 2024
@garyschulte garyschulte force-pushed the feature/ecmul-infinity branch from 1262ec5 to fed6ce9 Compare August 23, 2024 02:30
@garyschulte garyschulte marked this pull request as ready for review August 23, 2024 02:37
Copy link
Contributor

@pinges pinges left a comment

Choose a reason for hiding this comment

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

LGTM

@garyschulte
Copy link
Contributor Author

waiting on fuzz testing results before merging

Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte garyschulte force-pushed the feature/ecmul-infinity branch from 59b7895 to 7d2a3aa Compare August 28, 2024 19:19
@garyschulte garyschulte enabled auto-merge (squash) August 28, 2024 19:19
@garyschulte garyschulte merged commit 8ae0db4 into hyperledger:main Aug 28, 2024
40 checks passed
@garyschulte garyschulte deleted the feature/ecmul-infinity branch August 28, 2024 19:48
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.

2 participants