Skip to content

More convenience methods for boxed integer resizing #809

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

Merged
merged 1 commit into from
Apr 27, 2025

Conversation

fjarri
Copy link
Contributor

@fjarri fjarri commented Apr 25, 2025

This PR attempts to simplify some scenarios that were encountered during the work on RustCrypto/RSA#506.

  • Instead of having BoxedUint::shorten()/widen() methods, introduce the Resize trait with more convenience methods. Implemented for BoxedUint, NonZero<BoxedUint>, Odd<BoxedUint>, and their references.
  • BoxedUint::shorten() and widen() are marked as deprecated.

Note that try_resize fails if self.bits() > at_least_bits_precision, unlike the more relaxed shorten() behavior.

@fjarri
Copy link
Contributor Author

fjarri commented Apr 25, 2025

@tarcieri any comments on that?

The other inconvenience in RustCrypto/RSA#506 is that the constant-time division requires both arguments to be the same size, but the variable time division has no such restriction. I'm looking into that, will be in another PR.

@tarcieri
Copy link
Member

I think "resize" is fine over "resized".

The trait is currently made to be implemented both for BoxedUint and &BoxedUint, to reduce the amount of allocations. Is it worth the increased code complexity?

Why not just have the trait borrow &self in its methods?

Do we need the shorten() and widen() shortcuts, or resize() is enough?

Maybe mark them as deprecated and we can try to migrate to resize()

try_resize() returns an Option since it is only constant-time wrt self. Is that acceptable?

Can start with that, sure

Do we make try_resize() fail if at_least_bits_precision is less than self.bits(), but the number of resulting limbs is unchanged (that is, no truncation required)?

This seems fine at first glance although I'd be curious if there might be some issues that pop up on 32-bit with this that don't on 64-bit

@fjarri
Copy link
Contributor Author

fjarri commented Apr 26, 2025

Why not just have the trait borrow &self in its methods?

To save on allocations when the new length happens to be the same as the old one, or in some cases when it's smaller (depends on the allocator).

@fjarri
Copy link
Contributor Author

fjarri commented Apr 26, 2025

This seems fine at first glance although I'd be curious if there might be some issues that pop up on 32-bit with this that don't on 64-bit

That would actually make the behavior more uniform across platforms. Imagine you have a 100-bit integer and you call shorten(90). Currently this would truncate on 32-bit platforms (to 96 bits), but keep the integer the same on 64-bit platforms.

@fjarri
Copy link
Contributor Author

fjarri commented Apr 26, 2025

An alternative resize() behavior would be to unconditionally clear all the high bits above at_least_bits_precision (in which case it can just be called bits). This way it will be marginally faster, and try_resize() is not needed.

@fjarri fjarri marked this pull request as ready for review April 26, 2025 18:03
@tarcieri tarcieri merged commit cd2ccf3 into RustCrypto:master Apr 27, 2025
18 checks passed
@fjarri fjarri deleted the widths branch April 27, 2025 21:03
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