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

sqrt-via-dlog: cleanup and 20% accel for vartime Bandersnatch/Banderwagon deserialization #362

Merged
merged 2 commits into from
Feb 17, 2024

Conversation

mratsim
Copy link
Owner

@mratsim mratsim commented Feb 16, 2024

This builds on top of #354 and #356 to bring a further 20% perf improvement to Bandersnatch/Banderwagon square root and point deserialization.

The technique was hinted at in this comment: #354 (comment)

This is the first part of a larger refactoring to make the square root via discrete log constant-time (#358) and introduce a sage script for constant generation (#359)

Note: as this is only tested in verkle trees test, and those are not in nimble yet (pending #330), the local test must be:

nim c -r --outdir:build tests/t_ethereum_verkle_primitives.nim

@mratsim
Copy link
Owner Author

mratsim commented Feb 16, 2024

Benches:

image

image

@mratsim
Copy link
Owner Author

mratsim commented Feb 16, 2024

Now Tonelli-Shanks vartime is not much slower for sqrt than a simple prime p ≡ 5 (mod 8). Note that Edwards25519 curve uses Montgomery representation at the moment and does not use fast reduction techniques from it's Pseudo-Mersenne Prime properties.

image

@advaita-saha
Copy link
Collaborator

advaita-saha commented Feb 17, 2024

This builds on top of #354 and #356 to bring a further 20% perf improvement to Bandersnatch/Banderwagon square root and point deserialization.

The technique was hinted at in this comment: #354 (comment)

This is the first part of a larger refactoring to make the square root via discrete log constant-time (#358) and introduce a sage script for constant generation (#359)

Note: as this is only tested in verkle trees test, and those are not in nimble yet (pending #330), the local test must be:

nim c -r --outdir:build tests/t_ethereum_verkle_primitives.nim

Tests for SerDe and Banderwagon operations are added in constantine.nimble, which was added along with the PR #278. Only IPA tests are not yet added in the nimble mentioned here #330

Screenshot 2024-02-17 at 2 31 33 PM

Copy link
Collaborator

@advaita-saha advaita-saha left a comment

Choose a reason for hiding this comment

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

LGTM 🔥

@mratsim mratsim merged commit 008fe10 into master Feb 17, 2024
32 checks passed
@mratsim mratsim deleted the refactor-sqrt branch February 17, 2024 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants