impl_array_newtype in FFI should not autoderive std::Hash

[apoelstra]

In our FFI module we implement the std::Hash trait by hashing the contents of the inner array ... but the contents of these arrays for FFI types is not guaranteed to be consistent between different copies of the library. We shoud instead be serializing the data and comparings serializations.

The same applies to Ord and Eq.

related #462

[apoelstra]

Fixed by #506 ?

[tcharding]

Fixed by #506 ?

506 only does PublicKey, we need to do all the other types that wrap an ffi type, right? (KeyPair, XOnlyPublicKey, and ecdsa::Signature)

Do we need to do the fuzzing thing that we do on PublicKey and have different impls when fuzzing or not?

[apoelstra]

Only if those types already have a different representation while fuzzing.

IOW if it works without thinking about fuzzing, that's the way to go.

[tcharding]

I'm not currently able to find all the discussion we had on this ord/cmp stuff. I've done #515 that fixes ord/cmp at the level of secp256k1 but I'm not sure now what the behaviour should be in secp256k1-sys - should we be providing ord/cmp implementations that use the underlying bytes or not? Is the fix in #515 at the right layer or should it have been in secp256k1-sys? Should we be providing cmp_fast_unstable or some such thing?

[apoelstra]

I like the idea of providing cmp_fast_unstable and documenting that it may not be consistent between versions of the library, even minor ones. I don't think that's MSRV breaking, or rather, I think it's an acceptable MSRV break. This would just compare the underlying bytes.

As for "where is the correct layer to do this", I'm fine either way: we could add these impls in secp256k1-sys and then call into those from secp256k1, or just not have Ord/Eq at all on the secp-sys types and only implement them in secp256k1. I'll ACK whatever you've already implemented :P.