libplanet icon indicating copy to clipboard operation
libplanet copied to clipboard

Fix poorly implemented `GetHashCode()`

Open greymistcube opened this issue 2 years ago • 2 comments

I haven't checked thoroughly, but BaseEqualityComparer.GetHashCode(byte[]), AppProtocolVersion.GetHashCode(), and KeyBytes.GetHashCode() seem to be poorly implemented.

The gist of it is that all of these use something akin to the following:

hash = 17;
foreach (byte b in bytes)
{
    unchecked
    {
        hash *= 31 + b;
    }
}

return hash;

Note that

hash *= 31 + b;

is equivalent to

hash = hash * (31 + b);

which means every result is a multiple of 17 (before wrapping around). As * is commutative, this also becomes order agnostic. For example, if we have bytes = [5, 7], we get the same result as having bytes = [7, 5] since

$$ 17 * (31 + 5) * (31 + 7) = 17 * (31 + 7) * (31 + 5). $$

The proper formula to apply is to use

hash = hash * 31 + b;

greymistcube avatar Mar 23 '23 08:03 greymistcube

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

stale[bot] avatar Jun 18 '23 11:06 stale[bot]

How about code below

public int GetHashCode(byte[] obj)
{
    HashCode hashCode = default;
    hashCode.AddBytes(obj);
    return hashCode.ToHashCode();
}

HashCode struct is available in .netstandard 2.1

s2quake avatar Apr 23 '24 06:04 s2quake