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.

Sign up for GitHub

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

Jump to bottom

Replace partitionScalarOld by partitionScalar in batchScalarMul #284

Merged
merged 5 commits into from
Dec 16, 2022
Merged

Replace partitionScalarOld by partitionScalar in batchScalarMul #284

merged 5 commits into from
Dec 16, 2022

Conversation

gbotrel
Copy link
Collaborator

@gbotrel gbotrel commented Dec 14, 2022

  • checkpoint
  • feat: replace partitionScalarOld by partitionScalar

@gbotrel gbotrel linked an issue Dec 14, 2022 that may be closed by this pull request
BatchScalarMultiplication uses partitionScalarOld: merge with partitionScalar #268
Closed
@gbotrel gbotrel requested a review from yelhousni December 16, 2022 03:42
yelhousni
yelhousni approved these changes Dec 16, 2022
View reviewed changes
Copy link
Collaborator

@yelhousni yelhousni left a comment

Choose a reason for hiding this comment

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

LGTM 👍
We might revisit partitionScalars for secp256k1 to allow a path when a spare bit is not available.

@gbotrel
Copy link
Collaborator Author

gbotrel commented Dec 16, 2022 •
edited
Loading

ha, well, I think this new partitionScalars would work as is; the returned values is a list of []uint16 digits, so normally we would just have a last window that's larger than c. The only issue is that... if we have c = 16 and lastC = 17, the digits don't fit on uint16 🙃
(so... as a cheat, if in the template for secp we have a special case to limit the choice of c to 15, we should be good.)

@gbotrel gbotrel merged commit 0638b77 into develop Dec 16, 2022
@gbotrel gbotrel deleted the gbotrel/issue268 branch December 16, 2022 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yelhousni yelhousni yelhousni approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

BatchScalarMultiplication uses partitionScalarOld: merge with partitionScalar
2 participants
@gbotrel @yelhousni