constant time Ord and PartialOrd implementations

[imuli]

Fixes #202.

This does not follow the recommended path of utilizing subtle as it does not provide any helpers to do constant time ordering on containers.

The overall method is to compare each u32 in the hashes, turn each of those into -1, 0, or 1, accumulate them into a single signed word, and then finally compare that word to 0.

The optimized object code on x86-64 is somewhat large but appears to be constant time - no jumps, early returns, or anything like that. This is, of course, not a guarantee that it will never face optimization in hardware or software in the future.

[oconnor663]

Thank you for putting in the work to write this. I'm leaning against landing it, for two reasons:

• Like we discussed in Hash struct lacks an Ord implementation #202, it's not clear that we want a constant time Ord implementation, because many (most?) practical use cases for Ord will silently break that constant-time guarantee. For experienced callers who know exactly what they're doing, the conversion to &[u8; 32] is no problem.
• Constant-time equality and comparison code is notoriously tricky to get right. I'm sure your implementation here is correct as-written, but optimizing compilers have a tendency to betray us in unexpected ways, inevitably on some obscure platform that we aren't going to audit. I much prefer to rely on crates like constant_time_eq or subtle for this stuff.

(Side note: the CI failures are because we test this crate in no_std mode, where std::cmp doesn't exist. We already use core::cmp at the top of lib.rs, so the fix would be to just remove the std:: prefix. You can test this yourself with something like cargo check --no-default-features.)

[imuli]

• Like we discussed in Hash struct lacks an Ord implementation #202, it's not clear that we want a constant time Ord implementation, because many (most?) practical use cases for Ord will silently break that constant-time guarantee. For experienced callers who know exactly what they're doing, the conversion to &[u8; 32] is no problem.

Yeah, given that discussion I'd be inclined toward the original PR.

(Side note: the CI failures are because we test this crate in no_std mode, where std::cmp doesn't exist. We already use core::cmp at the top of lib.rs, so the fix would be to just remove the std:: prefix. You can test this yourself with something like cargo check --no-default-features.)

That makes a lot of sense!

[oconnor663]

it's not clear that we want a constant time Ord implementation

Actually I should've said that it's not clear (to me) that we want any Ord implementation at all. In my mind, forcing the caller to convert hashes to bytes to sort them has some value, because it makes it clear that the comparison semantics are the regular [u8] comparison semantics. Given that we do encourage users to rely on our constant-time Eq impl, I worry it would be confusing to expose an Ord impl without making a similar guarantee.