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

reduce boilerplate implementing comparisons for user-defined types #99

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cosmicexplorer
Copy link

@cosmicexplorer cosmicexplorer commented Dec 31, 2022

Problem

In signalapp/libsignal#469, we discussed having to hand-roll a constant-time comparison function for a public key with a slice of bytes and an enum tag. After seeing #78 where we implement ConstantTimeEq for slices, I realized we could extend this method of iterated constant-time computation to make it more fluent to implement comparison operations for structs with multiple fields.

Proposed Solution

  1. Introduce IteratedOperation and IteratedEq to modularize the approach used in the existing ConstantTimeEq impl for slices.
    • Add a doctest demonstrating how to apply this to user structs with multiple fields.
  2. Develop a novel method to calculate ConstantTimeGreater over a collection of elements as LexicographicIteratedGreater.
    • Implement ConstantTimeGreater for slices using LexicographicIteratedGreater.

Result

- fix compiler warnings about unused #[inline]
- spelling fix
- rustfmt change
- remove trait bounds on the ConstantTimeLess trait declaration itself
- implement ConstantTime{Less,Greater} with shortlex too
- improve comments on ct_gt impl
- remove extraneous mentions of shortlex
- iterated operations
- revert ConstantTimeLess back to using the default trait method
- remove `.unwrap_u8()`: does this break `ConstantTimeEq` slice impl?
- remove unused code
- implement ConstantTimeLess for slices
@cosmicexplorer cosmicexplorer force-pushed the expand-constant-time-cmp branch from ececdca to 8bc4d32 Compare January 4, 2023 15:33
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.

1 participant