Skip to content

implement PartialOrd<Uint> for BoxedUint #803

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 1 commit into
base: master
Choose a base branch
from

Conversation

baloo
Copy link
Member

@baloo baloo commented Apr 21, 2025

TODO:

  • Provide an implementation for BoxedInt

@baloo baloo marked this pull request as draft April 21, 2025 15:44
@baloo
Copy link
Member Author

baloo commented Apr 21, 2025

This came up in RustCrypto/signatures#906 but I'm not sure that's the right approach.

The alternative would be to duplicate the implementation of the subtle::ConstantTimeGreater and ConstantTimeLess, but I'm not sure I like this approach better than creating a short-lived BoxedUint.

Opinion?

@tarcieri
Copy link
Member

@baloo I think the real goal of this is to make it possible to compare a Uint constant to a BoxedUint while avoiding boxing the former, which this doesn't accomplish.

It should be possible to have a generic core implementation of this functionality that operates over [Limb] (or a higher level type, i.e. #756) which can be reused for comparing Uint, BoxedUint, or a mixture thereof.

@tarcieri
Copy link
Member

That said, if this came with TODOs and an open issue to optimize it, adding it now does open the door to future optimizations and enable code to be written with Uint constants, so I think it's OK for a start.

@baloo baloo closed this Apr 21, 2025
@tarcieri
Copy link
Member

tarcieri commented Apr 21, 2025

Err, sorry, my last comment meant this is fine as a start and we can optimize later.

It would be good to add TODOs for that though, and open a tracking issue

@tarcieri tarcieri reopened this Apr 21, 2025
@baloo
Copy link
Member Author

baloo commented Apr 21, 2025

Oh that's wasn't about your comment, but I'd rather start over.
After looking at the code for a little while, I have actually no idea how to achieve an abstraction over [Limb]. That would not carry whether the outer type was actually a Uint or an Int, so I'm a bit lost, this is why I closed that PR.

@tarcieri
Copy link
Member

@baloo we can leave that out for now. If the impls are in place they can be optimized later.

To answer your question, originally I used the name UintRef before changing it to LimbSlice, though that's a good point and it should still carry Uint in the name somehow. But in the interim it can just be function names.

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