v1.4.4 is a bandwidth hog - should we shun it?

[dooglus]

Due to issue #133, v1.4.4 often finds itself stuck at an old block in the chain, and keeps requesting the same block over and over from its peers, which they keep sending it.

This is quite a resource drain on any peer connected to a stuck v1.4.4 peer.

I propose we add code to drop connections from peers claiming to be v1.4.4.

Unfortunately, v1.4.5 incorrectly identifies itself as v1.4.4, so we would be dropping connections from that version too.

Anyone experiencing trouble connecting just needs to upgrade to v1.4.7 or later.

What do we think? Is this reasonable?

Something like this would do the trick:

diff --git a/src/main.cpp b/src/main.cpp
index d4c83ea..2fd8450 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -3165,6 +3165,14 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
         if (!vRecv.empty())
             vRecv >> pfrom->nStartingHeight;

+        if (pfrom->strSubVer == "/Satoshi:1.4.4/")
+        {
+            // disconnect from peers running v1.4.4 because it repeatedly requests the same block
+            LogPrintf("disconnecting %s\n", pfrom->strSubVer);
+            pfrom->fDisconnect = true;
+            return false;
+        }
+
         pfrom->addrLocal = addrMe;
         if (pfrom->fInbound && addrMe.IsRoutable())
             SeenLocal(addrMe);

[creativecuriosity]

I think that makes sense.

Might it be more appropriate and helpful to label the offending behavior misbehaving, as opposed to simply banning a specific version...?

[dooglus]

I'm not sure what the difference is.

I just copied the code that disconnects from peers with a too-old protocol version:

    if (pfrom->nVersion < MIN_PEER_PROTO_VERSION)
    {
        // disconnect from peers older than this proto version
        LogPrintf("partner %s using obsolete version %i; disconnecting\n", pfrom->addr.ToString(), pfrom->nVersion);
        pfrom->fDisconnect = true;
        return false;
    }

Edit: it seems like 'misbehaviour' is a way of adding up the offences committed by a peer so that a collection of minor offences can add up to a ban, which leads to their disconnection. If we don't want to talk to v1.4.4 clients at all, there's no need to add up their offences - we can just immediately disconnect them.

Edit2: oh, I see what you mean. You're saying we should react to the misbehaviour, rather than the version number. But we know it's just a matter of time before a v1.4.4 peer gets stuck on the wrong side of a fork, can't correct itself, and starts spamming us. So why not shun it before it does that so its owner will upgrade it to a working version?

[creativecuriosity]

Was just thinking that this is sort of a self-inflicted DDoS attack in a way. That it might be helpful in the long-perspective to identify behavior from peers that negatively affects the network; regardless of version.

[dooglus]

OK, good point. It would be useful to be able to detect when a peer is requesting blocks in an unreasonable manner. Currently we just keep sending it the blocks it asks for, repeatedly.

[creativecuriosity]

No reason it couldn't be in addition to an outright version ban as well.