Show any multiple connections from the same IP in peer list

[thalieht]

The uniqueness of peers is now determined by their IP and port
instead of just their IP.

closes #11845

[Chocobo1]

I haven't finished the review yet (you can still push commits) and I recall I looked into this issue myself too. The reason why there were no results from me is that I consider there is more to this issue and is non-trivial (at least for me at that time).
Let's start with this question: how should a connection considered to be unique? I would say it is determined by the tuple: (our IP, our port, peer IP, peer port, protocol).

• our IP: seems we don't really care in our case/PR here.
• our port: seems we don't really care in our case/PR here.
• peer IP: we already have it participate in determining connection uniqueness.
• peer port: participating in determining connection uniqueness seems to be handled by this PR.
• protocol: TCP or uTP/UDP. This field is displayed in GUI but doesn't participate in determining connection uniqueness. It is entirely possible that 2 different clients uses the same IP/port pair yet one uses only TCP and another uses only uTP.

So in short, we need to handle (peer IP, peer port, protocol) properly in order to correctly list all on-going connection.

[thalieht]

So in short, we need to handle (peer IP, peer port, protocol)

Once again i opened a can of worms... aanyway so far i've been thinking about adding another value (connection) to m_peerItems and compare it to peer.connectionType(). I don't know how yet but if you can think of another way i'm all ears.

[zero77]

@thalieht
Will this also be in the webui.

[thalieht]

@zero77 yes... inadvertently :)

[Chocobo1]

aanyway so far i've been thinking about adding another value (connection) to m_peerItems and compare it to peer.connectionType().

A possible way is to create another struct and use it only in peerlistwidget.cpp, for example:

struct PeerEndpoint
{
    BitTorrent::PeerAddress address;
    QString connectionType; // matches return type of `PeerInfo::connectionType()`

    /* Off topic discussion
    // I think `QString` as the return type of `PeerInfo::connectionType()` might 
    // be not a good choice. IMO qbt code base should have our own `enum class PeerConnectionType` defined 
    // and the values could be:
    enum class PeerConnectionType
    {
        // https://www.libtorrent.org/reference-Core.html#peer_info
        BT_TCP,
        BT_UTP,
        WebSeeds,  // either `connection_type_t::web_seed` or `connection_type_t::http_seed` in libtorrent
    };
    */
};
// add definitions of `operator==(PeerEndpoint, ...)` and `qHash(PeerEndpoint, ...)`

// Then in peerlistwidget.h
QHash<BitTorrent::PeerEndpoint, QStandardItem *> m_peerItems;

That is my basic idea, maybe @glassez could provide some suggestion too.

[thalieht]

Thanks! That's a much better idea. I'll wait for @glassez input though in case he objects.

[glassez]

IIRC, currently we show incorrect peer list since there can be multiple peers from the same IP and we show only one of them, isn't it?

That is my basic idea, maybe @glassez could provide some suggestion too.

Let's try to implement it.

[thalieht]

I didn't do the connection type enum thing because libtorrent types were mentioned which i'm not familiar with.

[glassez]

@thalieht

A possible way is to create another struct and use it only in peerlistwidget.cpp

Here is incorrect place. Seems it should be private for PeerListWidget.

[glassez]

I have one more question. How do multiple peers from the same address differ in the peer list?

[glassez]

Is it really needed to declare the following under BitTorrent namespace? One can think that it is provided by qBittorrent base...

[thalieht]

Removed.

[thalieht]

How do multiple peers from the same address differ in the peer list?

From their port and/or connection type.

[Chocobo1]

@thalieht
Thank you!