Opened 6 years ago

Last modified 5 years ago

#5883 new Enhancement

Remove 'paused' event from announce

Reported by: hexxy Owned by: jordan
Priority: Normal Milestone: None Set
Component: libtransmission Version: 2.84
Severity: Normal Keywords:
Cc:

Description

https://trac.transmissionbt.com/browser/trunk/libtransmission/announcer-http.c#L47

This code is logically broken, a hack, and should be removed. I is breaking on trackers that are adhering to the BT specification regarding events.

1) 'paused' event is not part of the BitTorrent? specification (https://wiki.theory.org/BitTorrentSpecification#Tracker_Request_Parameters) . There's no logical reason to announce a 'pause' to the tracker instead of a stop event. If you want a user to removed from the pool since they aren't seeding, announce the 'stopped' event (that's what it is there for). Also, not all trackers supported the paused event.

2) This code is obviously a hack. The logic is now split between here and tr_announce_event_get_string. This code should *NEVER* have been put here. If it was implemented it should have been in tr_announce_event_get_string

3) This code is fundamentally flawed. The logic is that if the torrent is a partial seed and is not stopped it is paused. This is causing partial seeders to send 'paused' events with every announce even when it is running.

Change History (6)

comment:1 follow-up: Changed 6 years ago by x190

http://www.bittorrent.org/beps/bep_0021.html

"Tracker Announce

In order to tell the tracker that a peer is a partial seed, it MUST send an event=paused parameter in every announce while it is a partial seed."

https://trac.transmissionbt.com/ticket/1379#comment:8

"if the tracker doesn't recognize the `event' value, it's simply ignored, but will still return a set of peers anyway.

For example, opentracker ignores everything except "stopped" and "completed"."

Last edited 6 years ago by x190 (previous) (diff)

comment:2 in reply to: ↑ 1 ; follow-up: Changed 6 years ago by hexxy

Replying to x190:

http://www.bittorrent.org/beps/bep_0021.html

Tracker Announce

In order to tell the tracker that a peer is a partial seed, it MUST send an event=paused parameter in every announce while it is a partial seed.

The referenced document is still a 'draft' status and is in no way part of the standardization. Nor should it ever be. The definition of an event is when an action occurs. Sending a 'paused' status on every announce is not an event, it is a status.

Using an existing variable for something other than it's intended and described purpose (aka as a hack) is the lazy developer's way out.

A partial seeder can be determined by the tracker using the following logic when calculating deltas. If download_delta = 0 and upload_delta > 0 and left > 0 then partial seeder.

Also keep in mind the 'paused' event is NOT supported for UDP trackers. Implementing a (badly thought through) draft proposal for only one announce protocol is also inconstant behavior.

comment:3 in reply to: ↑ 2 Changed 6 years ago by cfpp2p

Replying to hexxy:

The referenced document is still a 'draft' status and is in no way part of the standardization.

There are a lot of draft BitTorrent Enhancement Proposals currently and widely in use by many different clients. http://www.bittorrent.org/beps/bep_0000.html

Num 	Title
5 	DHT Protocol
6 	Fast Extension
7 	IPv6 Tracker Extension
10 	Extension Protocol
12 	Multitracker Metadata Extension
15 	UDP Tracker Protocol
16 	Superseeding
17 	HTTP Seeding (Hoffman-style)
18 	Search Engine Specification
19 	HTTP/FTP Seeding (GetRight-style)
21 	Extension for Partial Seeds
22 	BitTorrent Local Tracker Discovery Protocol
24 	Tracker Returns External IP
26 	Zeroconf Peer Advertising and Discovery
27 	Private Torrents
28 	Tracker exchange
29 	uTorrent transport protocol
30 	Merkle tree torrent extension
31 	Tracker Failure Retry Extension
32 	IPv6 extension for DHT
33 	DHT scrape
34 	DNS Tracker Preferences
35 	Torrent Signing
36 	Torrent RSS feeds
38 	Finding Local Data Via Torrent File Hints
39 	Updating Torrents Via Feed URL
40 	Canonical Peer Priority
41 	UDP Tracker Protocol Extensions
42 	DHT Security Extension
43 	Read-only DHT Nodes
44 	Storing arbitrary data in the DHT

Nor should it ever be. The definition of an event is when an action occurs. Sending a 'paused' status on every announce is not an event, it is a status.

What do you mean? As x190 already referenced and again exactly quoted from http://www.bittorrent.org/beps/bep_0021.html "In order to tell the tracker that a peer is a partial seed, it MUST send an event=paused parameter in every announce while it is a partial seed."

Using an existing variable for something other than it's intended and described purpose (aka as a hack) is the lazy developer's way out.

I don't understand. (and why the insult?) Please see r10149 #2888 #1379

A partial seeder can be determined by the tracker using the following logic when calculating deltas. If download_delta = 0 and upload_delta > 0 and left > 0 then partial seeder.

Also keep in mind the 'paused' event is NOT supported for UDP trackers. Implementing a (badly thought through) draft proposal for only one announce protocol is also inconstant behavior.

As far as I know all of the major bittorrents include bep21 Also: http://www.bittorrent.org/beps/bep_0015.html and http://www.bittorrent.org/beps/bep_0041.html

comment:4 follow-up: Changed 6 years ago by hexxy

Maybe I'm barking up the wrong tree and I should look at having that draft pulled. Here's my reasoning behind that.

Let's start with the field name we are using here, 'event'. 'event' is designed to tell the tracker of a particular event happening (e.g. torrent being started, or completing). Event should only be filled in when there is a status change in the torrent such as it being completed. When a torrent is completed 'event' is set to completed and sent ONCE. When a torrent is started 'event' is set to 'started' and is sent ONCE. When a torrent is stopped 'event' is set to 'stopped' and sent ONCE. See a pattern here yet? The events are only sent once and at the time of the event. 'event' was never meant to be used as a status indicator. What do I mean by status indicator? I mean when the same value is sent over and over to indicate a constant 'status' in this case we are using the value 'paused'.

On that note let's talk about using the value of 'paused'. Where did this value come from? Why was this arbitrary value chosen? Why do I say arbitrary? because it is arbitrary. 'paused' you would think would indicate the user clicked 'paused' or it was 'paused' due to scheduling. Nope. In this case 'paused' means partial seed. Wait, what? Why are we using 'paused' to indicate partial seeding? Why not use a variable called 'partial_seed' and set it to 1? Why not use a more descriptive 'event' value even, like 'partial'. I can tell you why. The developer(s) who came up with that 'draft BEP' were lazy and didn't want to implement a new status the correct way. Instead they chose to re-use an existing field 'event' for something other than it's original intended use. Not only that they got double lazy and didn't even give a forethought to the idea that maybe a more descriptive value should be used instead of 'paused'.

So yes. I have a problem with the fact that a) the field is being used for something other than it's intended purpose and b) the value is being used is a completely arbitrary and non-descript of the ACTUAL reason it's being sent.

Like i said in my last comment there is absolutely no reason for this Draft BEP due to: A partial seeder can be determined by the tracker using the following logic when calculating deltas. If download_delta = 0 and upload_delta > 0 and left > 0 then partial seeder.

comment:5 in reply to: ↑ 4 Changed 6 years ago by x190

Replying to hexxy:

Maybe I'm barking up the wrong tree and I should look at having that draft pulled.

I agree. Good Luck convincing Bittorrent Inc. :-)

comment:6 Changed 5 years ago by CHepx

  • Priority changed from Highest to Normal
  • Severity changed from Blocker to Normal
  • Type changed from Bug to Enhancement

i totally agree with hexxy. the paused event is senseless and useless since, as pointed out by hexxy already, partial seeders can easily be discovered by doing a trivial calculation. just because there's a draft somewhere doesn't mean it has to be followed blindly. drafts are drafts for a reason.

Note: See TracTickets for help on using tickets.