Opened 9 years ago

Closed 8 years ago

#4888 closed Bug (fixed)

Incorrect "seed status" sent to trackers when adding a magnet link

Reported by: x190 Owned by: jordan
Priority: Normal Milestone: 2.60
Component: libtransmission Version: 2.51
Severity: Normal Keywords:
Cc:

Description

When adding a magnet link, trackers are incorrectly sent and list me as having "seed status". If there are no other leeches then the transfer will never demagnetize as other seeds will not connect. The same transfer added as a .torrent correctly shows me as having "leech status" and thus seeds will connect. This can easily be tested by creating a torrent with local files using any public tracker or by finding a torrent with 0 seeds.

Attachments (1)

magnet-left-until-complete.patch (2.8 KB) - added by jordan 9 years ago.

Download all attachments as: .zip

Change History (17)

comment:1 Changed 9 years ago by x190

https://forum.transmissionbt.com/download/file.php

I'm just another seeder??? Not!

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

comment:2 Changed 9 years ago by x190

https://forum.transmissionbt.com/viewtopic.php?f=4&t=13356

See #4089 and #4567 for what happens if you actually do have all or some of the data.

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

comment:3 Changed 9 years ago by x190

r13261: completion.h Line 92:

static inline uint64_t
tr_cpLeftUntilComplete( const tr_completion * cp )
{
+Line 95 if( !tr_torrentHasMetadata( cp->tor ) )
+Line 96     return 1;

    return tr_torrentInfo(cp->tor)->totalSize - cp->sizeNow;
}

r13261: announcer.c Line 898:

static tr_announce_request *
announce_request_new( const tr_announcer  * announcer,
                      const tr_torrent    * tor,
                      const tr_tier       * tier,
                      tr_announce_event     event )
{
...
Line 913 req->left = tr_cpLeftUntilComplete( &tor->completion );

Will now be non-zero for magnet links and trackers and Transmission
will now show you correctly as a leecher which should help you initiate
handshakes with seeders to obtain the metadata.

comment:4 Changed 9 years ago by jordan

x190, thanks for reporting this and for tracking down the broken code.

Regarding the proposed change to tr_cpLeftUntilComplete()... it feels a little wrong to add that logic. IMO the problem is that tr_cpLeftUntilComplete() is based on assumptions that aren't always right. This isn't necessarily a showstopper, but happily for us it looks like we can do better this time because nobody else uses this function. So we can move it from completion.h to be private inside announcer.c and make the assumptions more explicit.

What do you think of magnet-left-until-complete.patch ?

Last edited 9 years ago by jordan (previous) (diff)

Changed 9 years ago by jordan

comment:5 Changed 9 years ago by jordan

  • Status changed from new to assigned

comment:6 Changed 9 years ago by jordan

  • Milestone changed from None Set to 2.53

comment:7 Changed 9 years ago by x190

I changed my code to return ~0 instead of 1 and trackers did get the correct message, so yes, your patch should be okay.

comment:8 Changed 9 years ago by jordan

  • Resolution set to fixed
  • Status changed from assigned to closed

Patch committed in r13310

comment:9 Changed 9 years ago by livings124

  • Milestone changed from 2.53 to 2.60

comment:10 Changed 8 years ago by x190

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:12 Changed 8 years ago by x190

r13901 breaks the fix committed in r13310.

comment:13 follow-up: Changed 8 years ago by jordan

unbroken in r14068.

r13310 came about because I was sniffing packets between Transmission and other torrent engines and noticed this disparity, and was looking for consistency with them. I guess this is just one of those holes in the spec... specifically, numwant was developed years & years before magnet links were common in BitTorrent?.

comment:14 in reply to: ↑ 13 Changed 8 years ago by x190

Replying to jordan:

unbroken in r14068.

r13310 came about because I was sniffing packets between Transmission and other torrent engines and noticed this disparity, and was looking for consistency with them. I guess this is just one of those holes in the spec... specifically, numwant was developed years & years before magnet links were common in BitTorrent?.

There should be a better way to indicate seed/leach status for magnet links. ~(uint64_t)0 is a huge number---maybe 1 would be better?

~ (uint64_t)0 Is that whitespace okay?

comment:15 Changed 8 years ago by jordan

It's arguably better to have the huge number, since it's clearly outside the range of [0...tr_info.totalSize]. Also, in lieu of a Right Answer, at least this is consistent with the 2.7x series.

Yes the whitespace is okay.

comment:16 Changed 8 years ago by jordan

  • Resolution set to fixed
  • Status changed from reopened to closed
Note: See TracTickets for help on using tickets.