This looks good, but consider the case where we want different scores sent for different reasons. This is why I mentioned it might be beneficial for the reactor to send the numeric value.

Otherwise, in the future, if we want to do this, we have send unique Status types to the router and router has to switch on them. Which I guess is fine? But it seems like a code smell to me for the router to know about things like e.g. PeerScoreUpdateBadVote.

I definitely see where you're coming from, and I think we have to balance the risk of putting reactor-specific code in the router (e.g. constants and score values) with the cost of putting routing impacting code (e.g. number values,) in the code. I'm more worried by the possibility of one reactor dominating the scoring for peers at the expense of other reactors, and having that distributed across the code base.

I'm more worried by the possibility of one reactor dominating the scoring for peers at the expense of other reactors, and having that distributed across the code base.

Wdym by dominating exactly? If a reactor deems that a peer is behaving very poorly or maliciously and consistently, it should be punished as so.

Thoughts @cmwaters?

while it seems important that a bad peer from the perspective of the consensus reactor is very-bad indeed, if a peer is fine from consensus, but bad from the perspective of blockchain or mempool, is that as bad as being bad at consensus?

if any badness is equal to any other badness, then we don't really need scores at all and the current "peerbad" status are fine. if some badness is worse than other badness, I guess I'm arguing that the router should be the one to judge relative levels of badness rather than letting one reactor decide it's badness (or goodness!) relative to other reactors.

Perhaps in other words: "pushing determination of actual scores into the router, means the router has to know something (very small) about all reactors, but pushing scoring into the reactors, means that every reactor needs to know about every other reactor. The former seems like the better option.

Perhaps, though, the best option is that we won't need weights at all.

My two cents on this is that although I agree that the router perhaps shouldn't know reactor specific details, I do think from the perspective of code legibility and understanding the system it would be nice to list all the possible behaviours and the impact they would have on peer scoring in a single file, so someone reading that file can see exactly how peers are ranked. If we feel like there may be too many different behaviours to keep track of then I would also feel that it is valid to just have 5 or 6 different values that the individual reactors can decide on.

Do you have enough to go on to wrap up this PR @tychoish? We can keep it simple for now I guess and just do +/- 1. But I do think ultimately, the reactors should be providing means to adjust scores directly.

Makes sense. I think the PR is pretty wrapped up at this point, aside from the (likely orthogonal) test failures.