Consensus: Peer Stats on Faulty Behavior

[alexanderbez]

Summary

ref: #5969 (review)

I guess the only qualm I have is that we have two stats that we use to keep track of peer behavior (votes and blocks) but the peer manager is never informed when anything bad happens i.e. invalid proposal or invalid vote.

While the peer manager and scoring system is still, at this moment, being fleshed out, we should consider if it is worthwhile to introduce stats, and as a result, peer updates to adjust peers scores when a given peer sends an invalid proposal or vote.

Currently, we only track "good" behavior and not any "bad" behavior.

/cc @cmwaters @ebuchman @brapse @melekes

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[cmwaters]

I would just like to append a few observations concerning this:

1. Errors are currently just logged and not sent back to the reactor to update the peer manager
2. What is sent back, if successful, is the entire original message through the statsMsgQueue whereas the only information the reactor cares about is "was it either a valid and helpful message or invalid and malicious".
3. This comment here:

// if err == ErrAddingVote {
	// TODO: punish peer
	// We probably don't want to stop the peer here. The vote does not
	// necessarily comes from a malicious peer but can be just broadcasted by
	// a typical peer.
	// https://github.com/tendermint/tendermint/issues/1281
// }

seems to indicate that nodes will continue to broadcast invalid votes. Why does this happen?

[alexanderbez]

Right, it seems like we can and just should augment the statsMsgQueue to reflect good and bad behavior so Reactor#peerStatsRoutine can handle these events accordingly.