Make anti-leech choker never increase score for dishonesty #7658
Conversation
The current anti-leech choker takes the maximum of client-reported have and our own session uploaded size as the progress of the client. This may paradoxically inflate the score of dishonest clients if we've been uploading to it for long enough, that our uploaded size exceeds 50% of the total size. This commit caps "given_size" to 50% of total_size, so that the above- mentioned situation can never happen.
|
this will penalise peers that are close to completing the torrent, but that we see for the first time, right? Also, with this change, the comment above, with the ascii-art plot, would need to be updated. But I'm not sold on this being an improvement. The scenario you're talking about is that a "dishonest" peer is the only peer in the swarm, and you're uploading almost all of the torrent to it. In this scenario, there is no distinction between an honest and a dishonest peer. If another peer joins, what is it that makes the old peer dishonest? If the new peer has not downloaded anything, that peer would have a higher anti-leech score than the old one (which seems right). |
|
If you want to detect dishonesty, you could compare the number of bytes uploaded to the number of pieces the peer claims to have. But that's also risky; earlier versions of libtorrent would not send HAVE messages to peers that already have those pieces (as they are redundant from the download-protocol point of view). |
It will not. Let's say the total is 100 blocks worth of things and the client reports having 99. Our known uploaded amount is 0, so we take min(0, 50) which is still 0. We then take max(0, 99) for the amount that counts toward scoring, and that would be still 99.
This is no more than a fix on top of the old dishonesty check. That one required no plot update, neither would this one.
The definition of dishonesty is already dealt with in the old anti-dishonesty PR #3833. Let's model it as a client that never sends HAVE. We start with the dishonest peer as the only one in the swarm. We as a result only upload to it. The 3833 calculation successfully reduces the score while the amount we uploaded is less than 50% of the total file size; beyond 50% the score will appear to rebound, because its have reckoning is simply max(HAVE,uploaded). Given enough time uploading, and a dishonest client with either choppy checksums or just plain maliciousness, we could reasonably have our upload amount exceed the total file size. At this point the anti-leech score will start to exceed the designed maximum of 1000 (3833 rescales the range from 0-2000 to 0-1000). Now suppose you get a new peer with 0% done in. The anti-leech choke is supposed to prioritize it since it knows the client has 0% done, giving it a score of 1000. But our dishonest peer, let's say it has received 101% of file size via upload, wins out with a score of 1020. That is unacceptable. This behavior would not happen without 3833. Or consider a less ridiculous case. Dishonest peer reports 0% but has received 90% file size. Gets a score of 800, because max(0, 90) is 90. New honest peer that reports having 20% joins. Gets score of 600, because max(20,0) is 20. Honest peer chokes. The same happens before 3833, but we can clearly do better. |
|
I see how this works now. it makes sense |
The current anti-leech choker takes the maximum of client-reported have and our own session uploaded size as the progress of the client. This may paradoxically inflate the score of dishonest clients if we've been uploading to it for long enough, that our uploaded size exceeds 50% of the total size. (That could in turn happen if we had nobody else to upload to for a while. That in turn screws over some honest client that then appears in our peer list, until the dishonest peer disconnects.)
This commit caps "given_size" to 50% of total_size, so that we never cause an increase of the score by taking into account the "given".