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

primeorder: use PrimeField::NUM_BITS to bound the scalar size during point-scalar multiplication #1119

Merged
merged 1 commit into from
Feb 8, 2025

Conversation

fjarri
Copy link
Contributor

@fjarri fjarri commented Feb 7, 2025

This probably won't affect actual curves much, but it is useful for test curves which have a small order, but the representation size still has to be large. See also RustCrypto/formats#1649

@fjarri
Copy link
Contributor Author

fjarri commented Feb 8, 2025

Actually, this should speed up some regular curves too, e.g. p521 which uses U576 for its scalars.

@tarcieri tarcieri merged commit 6033a21 into RustCrypto:master Feb 8, 2025
9 checks passed
@tarcieri
Copy link
Member

tarcieri commented Feb 8, 2025

Sidebar: seems like we could also add a mul_vartime which bounds by the bit length of the input scalar

@@ -122,7 +122,7 @@ where
}

let mut q = Self::IDENTITY;
let mut pos = C::Uint::BITS as usize - 4;
let mut pos = (<Scalar<C> as PrimeField>::NUM_BITS.div_ceil(8) * 8) as usize - 4;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, now that I think about it, .div_ceil(4) * 4 would be more accurate, since we need an integer number of window sizes, not bytes.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe extract a function for calculating this? It could also be used by a hypothetical mul_vartime

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