Skip to content

Use tuple for MultiscalarMul #792

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

daxpedda
Copy link
Contributor

MultiscalarMul currently takes two iterators for scalars and points respectively. This has the downside that if they are differently sized we currently simply panic.

This PR changes that by putting them both into one iterator with Item = (Point, Scalar). This way we get rid of the panic and don't loose any of the flexibility.
Migration for users should be quite straightforward, just add a zip() between the two parameters, as shown in the example.

I also switched the order of scalars and points around to align more with elliptic_curve::LinearCombination and applied the same tuple-treatment to the fixed-sized array method as well. This does affect its flexibility and maybe we should change elliptic_curve::LinearCombination accordingly.

As requested in #790.

@daxpedda daxpedda force-pushed the multiscalar-tuple branch from 1c898bf to 8b9538e Compare July 17, 2025 16:40
@tarcieri tarcieri requested review from rozbb and tarcieri and removed request for rozbb July 17, 2025 16:58
@tarcieri
Copy link
Contributor

It seems like if we do this, it should be done for VartimeMultiscalarMul and VartimePrecomputedMultiscalarMul as well

@daxpedda
Copy link
Contributor Author

It seems like if we do this, it should be done for VartimeMultiscalarMul and VartimePrecomputedMultiscalarMul as well

Sounds good! Will do it when we get #790 in.

I can also turn this around if you want, rebase #790 on top of this, then we can do this one first.
Let me know!

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