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

mul_bits_be endianess footgun #678

Open
elichai opened this issue Jul 30, 2024 · 1 comment
Open

mul_bits_be endianess footgun #678

elichai opened this issue Jul 30, 2024 · 1 comment

Comments

@elichai
Copy link
Contributor

elichai commented Jul 30, 2024

Until recently all the Scalar APIs are little endian based (Scalar::from_bytes_mod_order_{wide}/from_scanonical_bytes/from_hash/hash_from_bytes/{to/as}_bytes)
The same is true for raw scalars in the MontgomeryPoint API: MontgomeryPoint::mul_{base}_clamped.
But the new MontgomeryPoint::mul_bits_be uses Big Endian.
On one hand, this is pretty clear in the name of the function, on the other it is not clear on the rest of the functions,
So someone might naively think they can replace MontgomeryPoint::mul_clamped(bytes) with something like MontgomeryPoint::mul_bits_be(bytes.into_iter().flat_map(|byte| (0..8).map(|i| (byte >> i) & 1 == 1))), which is obviously incorrect.

Should this be renamed to mul_bits and use LE order? should we also add mul_bits_le? Or are we OK to leave it like that because it's a "hazmat" function anyways?

@tarcieri
Copy link
Contributor

I'd note mul_bits_be already has a rather large warning on it. I'm not sure there's much that can be done other than adding a replacement and deprecating it?

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

No branches or pull requests

2 participants