Allow peers to determine how much they are seen to be misbehaving. #4389

This can allow peers in future to modify their behaviour to avoid netsplits.

On one hand, this is very useful for diagnostics.

On the other hand this moves our internal anti-DDoS mechanism into the P2P interface. This means that a) we can't change it to some other mechanism anymore at a later time b) other node implementations need to use the same or a similar system

Not sure how much of a drawback this is. Accumulating a ban score is a straightforward mechanism. All in all I'm mildly positive about this. But I'd suggest proposing this on the mailing list.

Why close it?

@laanwj only because I noticed it didn't compile, re-opened now.

Providing more information to identify node misbehavior seems like a good move, but:

• I'm having a hard time picturing how a running node could modify its behavior in response to misbehavior messages; if a node knows "I could do X so that my peer won't increase my banscore," wouldn't it be doing X anyway?
• It seems difficult to impossible to identify what misbehavior warning is in response to what message.

Given these difficulties with interpreting and acting on the information, why not treat this more like the "reject" message -- as an informational message for developers debugging clients, sent with a short string identifying the type of misbehavior encountered?

I think there's a general intent to be conservative about offences causing DoS scoring; in particular, anything that can be relayed by older nodes can generally not trigger DoS, as it could lead to network partitioning.

They are not random frequently occurring things - they are highly exceptional, intended to never trigger for non-attacking nodes, and the only reason they don't always cause a disconnect/ban immediately is to limit the impact of a software bug.

I consider them to be highly implementation specific, and not useful information that peers should care about or act upon. If you're developing a P2P client it's useful of course, but you'll need more information anyway for that case, and likely need to develop by running against a local node with -debug anyway.

Good point @sipa.

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4389_d7b5f1fbab7d9c8c9c37f7edd43bc00577eba9f3/ for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

Seems there is no consensus on this, closing.

I have an alternative idea to this which doesn't expose as much of the internal state, but is still useful for troubleshooting and network diagnostics: a bye message.

An optional message that can be sent to the peer you're about to disconnect with a message why you're disconnecting them (ping timeout, ban, etc). The receiving peer would only log this - it's not possible to act on it - but it could be useful when troubleshooting network problems.

@laanwj that is a very good idea.

@laanwj I did think of that (a message when disconnecting), but that wouldn't allow nodes to adapt to their misbehavior. A good example of when this might be needed is with the recent pull that has been merged that allows nodes to double-spend. It was only noticed after the merge about a potential attack vector where nodes could be manipulated into being banned. If this patch (and a following patch to modify behaviour) were applied, then this would protect against all future examples of this happening. It would be quite easy to link the misbehave and reject messages. Simply remember the last "reject" message and the last message , and then when a "misbehave" message arrives, you'll have a good idea what it's referring to. @kazcw Above paragraph addresses your question also.

I have my doubts on to how much a node can intelligently adapt its behaviour to such messages. Also as to whether it should at all. It could open it up to 'bad feedback' sybil attacks. What if your node is the one that gets it right?

In my eyes a message like this would only be useful for troubleshooting by actual human developers. And in that case a message at disconnection works better.

@laanwj How is the "bye" message better? It's effectively the same as this, but less granular.

OK - done arguing this, I'm pretty sure neither of both will be implemented anyway.

@laanwj Just re-read your message #4389 (comment) above, and I realise the assumption you were making, i.e. that behaviour learned about misbehaviour would apply to more nodes than the node giving the feedback.

I don't think a node should assume all nodes classify the same behaviour as misbehaviour, this would certainly open up to the "bad feedback" sybil atack you mention. Instead, nodes should regard any feedback as being applicable only to the node giving it.

I think we should just get rid of misbehaviour scores entirely - all they do is making it harder to debug problems. Things that result in a ban score should just immediately trigger a ban, and others should have no effect at all. The only thing they help with is reduce the chance of accidentally have a relayed network message not immediately partition the network, but if this can happen with a 100-score misbehaviour, it can probably happen ten times with a 10-score misbehaviour.

that would be more efficient, wrt connection management, and eliminate the need for node behavior accounting/tracking.

it seems that notions of "misbehavior" belong in consensus code. Blatant breaches of protocol should just result in a ban, straight-away

if a ban disconnect is accompanied by a bye message as suggested by @laanwj then that could be useful to other clients and their developers, It will also mean that protocol breach is explicitly stated as the reason for disconnect both for specifically challenging unacceptable protocol behavior and to assist client software developers with correcting such incorrect behavior.

Yes, client development should still be done via connection to a Satoshi client running with -debug but the cases for punishing peers are quite specific and it would be useful to address such behavior directly and specifically via bye with a varchar string payload (or perhaps a specific set of error codes)

This includes @sipa 's suggestion to doaway with banscore accumulation. Just ban and disconnect at the initial offense and inform the client of the reason for disconnection with a bye message.

@sipa Is there some sort of roadmap document for Bitcoin Core? If so, I think your idea to abolish the current misbehaviour model should be added and I am in agreement. I like the "bye" message idea also, although technically, it's not needed as all unknown commands are currently logged so any messages could be sent that way :)

I'd be happy to write a commit now that sends bye messages prior to disconnects and ban 100s. For the ban <100s, what could be used? A reject message? e.g. a reject message is currently sent in response to multiple version messages.

This was just a personal opinion, certainly not something that has been addressed project-wide, and people may very well disagree.

Also, sorry for furthering this. We should hijact a old issue for this discussion.

@sipa Given the varying timezones we occupy, this is a better place to discuss this (for me at least) than the IRC channel.. We really need a better medium to discuss these things. Both IRC and the mailing list seem less than ideal compared to here. Perhaps wiki.bitcoin.com ?