Ignore checksum on 'version' messages.

[sipa]

Februari 15th, the protocol changes and 'version' and 'verack' messages
get a checksum. However, some NAT routers change the IP address inside
packet's payloads, causing the version message's checksum to become invalid.

This commit disables the checksum check, so clients behind such routers can
continue to connect after februari 15th.

[gmaxwell]

We know such routers exist— they've been observed to impact p2pool users (which uses a similar protocol with checksums on the version messages). But we don't know what brands or how common they are. It's a pretty bone headed thing to do.

Rather than just ignoring it, it would probably be better to still log it but leave the connection up. Then we'd at least get some helpful logs.

[laanwj]

Huh this really surprises me. The IP replacement certainly exists, but used to be only for protocols like FTP and IRC that were known by the router, not in unknown binary data.

[gavinandresen]

I'd really like to know more about the root and scope of the problem-- was this "one or two people had problems with p2pool because they misconfigured iptables/netfilter" or is it "LinkSys routers automangle the first packet if its first N bytes look like it is an FTP session" ?

[gmaxwell]

Unfortunately we don't know the scope.

We observed checksum errors on p2pool. Forrest investigated and found this to be the cause through some lucky reasoning. It appeared to be two distinct users, out of something like 150 using P2Pool at the time. So, I'd guess we could be looking at incidents rates between something like 1:10 and 1:100000 in the general bitcoin using population. There could also be port number differences that moot it.

If this were just some new feature in 0.6 I wouldn't even bother talking about it— if we found it to be an issue in practice we'd just issue new software. But because its a network wide synchronized hardcut its more troubling. I think the bluematt pull request is prudent— it's better behavior even if this is a non-issue, and should cleanly improve this behavior— Perhaps we should keep this particular patch in our pocket in case things go pear shaped?

Luke suggested we send an alert 24 hour ahead of the switch saying that an automated switchover was about to take place and advising people to visit bitcoin.org for the latest information. This sounds prudent to me, since if it does go poorly (due to the nat or other issues) we may not be able to send an alert to the impacted users.

[jgarzik]

IMO consider keeping existing (feb20 switchover) behavior as default. Add a "no-version-csum" (or "fucked-router") option. I am deeply suspect of routers that randomly rewrite binary packets transited in unknown (to the router) binary protocols.

Would rather err on the side of stronger checksum protection, and only weaken that as absolutely necessary.

[gavinandresen]

Given the lack of problems in the last 5 days (besides alternative implementations that didn't code in the change), I think this is safe to close as "unnecessary."