mixing: Validate RS hashes during blame assignment

[jholdstock]

Ensuring that the RS hashes from rs.SeenSecrets actually exist in the current session closes a loophole where a peer could escape blame by simply filling rs.SeenSecrets with junk in order to bypass the len!=0 check.

[jrick]

This looks necessary but we should be stricter about it. If a RS message references both junk and a valid RS message, we should similarly treat the peer as misbehaving, rather than permitting them because at least one of the hashes was valid.

[jholdstock]

Done

[jrick]

This is correct but the loop could read simpler. If there are no hashes in the slices, blame the peer (and continue to the next RS). Otherwise, iterate all hashes, and if any hash is unknown, blame the peer. And we can remove the check for a self-referencing RS, because that is only happening if our hash function is horribly broken.

[jholdstock]

I don't think we can remove the self-reference check because the message hash does not commit to the SeenSecrets field - i.e. it's possible to hash the message then insert that hash into the SeenSecrets field without invalidating the hash.

With the assumption that the self check needs to stay, I don't think the loop can be simplified much further. I have wrapped the decision into a func which removes one of the bools, removes the duplicated logging, and just overall helps things to read more smoothly (IMO).

[jrick]

you're right, thanks.