Ignore discriminants in Hash for AV + ArrayValue + nix len-prefix for PV

[Centril]

Description of Changes

Fixes #1048.

• Hash for AV does not include Discriminant<AV> anymore.
• Hash for PV is no longer length-prefixed.
• Hash for ArrayValue does not include Discriminant<ArrayValue> anymore.

In spacetimedb, we only compare hashes between homogeneously-typed values. Therefore, this PR does less work and does in fact not weaken the hash function. Rather, by avoiding to feed in known-equal data into the hasher, we can reduce the rate of conflicts, as we're now packing the same number of "interesting" bits alongside fewer "boring" bits into a hash.

API and ABI breaking changes

Semantics in the APIs above are broken by this PR.

Expected complexity level and risk

2

[gefjon]

Does this make BFLATN hashing consistent with ProductValue::hash? I suspect not until we merge #1063 . If that's the case, I'd like to hold off on merging this until after that PR, and then add a (prop)test to this PR that the hash functions are indeed consistent.

I'd also like to see a comment on SumValue about what constraints need to be upheld by its Hash impl, and the fact that derive(Hash) does indeed uphold those constraints.

Otherwise, code looks good. I'll mention that, for our case of comparing hashes only between homogeneously-typed values, this does not actually weaken the hash function at all - in fact, avoiding feeding known-equal data into the hasher should improve the hashes, as we're now packing the same number of "interesting" bits alongside fewer "boring" bits into a hash.

[Centril]

Does this make BFLATN hashing consistent with ProductValue::hash? I suspect not until we merge #1063 . If that's the case, I'd like to hold off on merging this until after that PR, and then add a (prop)test to this PR that the hash functions are indeed consistent.

This does not, but is a stepping stone towards that goal. The full implementation is provided in #1107 with proptests and Hash + Eq for BSATN + RelValue. However, in the interests of smaller PRs for review, I'd like to merge these piece-meal.

Note however that #1063, now merged, has no bearing on this or #1107.

I'd also like to see a comment on SumValue about what constraints need to be upheld by its Hash impl, and the fact that derive(Hash) does indeed uphold those constraints.

Added a larger comment in algebraic_value_hash.rs instead as I felt that provided better context.

I'll mention that, for our case of comparing hashes only between homogeneously-typed values, this does not actually weaken the hash function at all - in fact, avoiding feeding known-equal data into the hasher should improve the hashes, as we're now packing the same number of "interesting" bits alongside fewer "boring" bits into a hash.

Yep, that's definitely true. I'll adjust the PR title and description to capture this.

Addressed