Content popularity #3660

xoriole · 2018-06-01T15:04:23Z

This pull request provides a community for sharing popular and alive content to the connected peers. Right now the content is torrent only but can be extended to channels and others.

Model for sharing the content is publish subscribe. At bootstrap, a peer subscribes to a set of its connected peers sorted in descending order based on the individual trust score of the peers.

When a peer accepts the subscription request, it sends its most popular torrents it had checked to the subscriber and then periodically sends the updated torrent checker responses.

The case of conflict when two peers send the torrent response for the same torrent, the decision to accept or reject any of the response is based on

Freshness of the response: If the database record is stale, we accept the response
Trust score of the sending peer: If the sending peer is trusted (trust>1), we accept it.

Thomasedv · 2018-06-01T15:08:34Z

Tribler/Core/APIImplementation/LaunchManyCore.py

@@ -303,6 +304,21 @@ def load_ipv8_overlays(self):

            self.ipv8.strategies.append((RandomWalk(self.market_community), 20))

+        # Popular Community
+        if self.session.config.get_popular_community_enabled() or True:


Should this line always evaluate to True?

Thanks! good catch

Thomasedv · 2018-06-01T15:24:59Z

Tribler/Core/Config/tribler_config.py

+    # Popular Community
+
+    def get_popular_community_enabled(self):
+        return self.config['popular_community']['enabled'] or True


This one is likely a mistake too.

xoriole · 2018-06-08T09:30:18Z

retest this please

xoriole · 2018-06-22T11:40:35Z

retest this please

qstokkink · 2018-06-26T10:02:36Z

Tribler/Core/APIImplementation/LaunchManyCore.py

@@ -873,6 +889,9 @@ def early_shutdown(self):
            yield self.ipv8.unload_overlay(self.tunnel_community)
            yield self.ipv8.unload_overlay(self.triblerchain_community)

+        if self.popular_community:


Why do you unload this separately? The tunnel and triblerchain are only unloaded the way they are to enforce the unload ordering.

qstokkink · 2018-06-28T14:50:36Z

Tribler/Test/Community/popular/test_community.py

+        query = "ubuntu"
+        self.nodes[0].overlay.send_torrent_search_request(query)
+
+        yield self.deliver_messages()


Where are the asserts?

qstokkink · 2018-06-28T14:54:38Z

Tribler/Test/Community/popular/test_payload.py

+        serialized_response = self.serializer.pack_multiple(in_response.to_pack_list())
+
+        # Deserialize request and test it
+        (deserialized_ressponse, _) = self.serializer.unpack_multiple(ContentInfoResponse.format_list,


Too many s's

qstokkink · 2018-06-28T16:22:06Z

The C0330 and R1707 Pylint errors should still be fixed.

devos50 · 2018-06-28T17:56:40Z

@xoriole I guess this PR is almost ready for merge. If you still have some time, please cleanup your commit history so you have a logical sequence of commits (i.e. three commits: added pubsub community, added popularity community, improved tests). @qstokkink can you also give your seal of approval soon?

I've updated the SonarQube technical debt threshold to 24 days so it should pass when you retest this PR. This is really not the way we should manage technical debt but I guess that @ichorid will significantly lower the technical debt once the ORM model becomes active (since we can remove all Tribler database crap then).

ichorid

Please, address these issues. These are easy to fix, but neglecting them will have a large impact on security.

ichorid · 2018-06-28T12:20:26Z

Tribler/community/popular/community.py

+        elif peer in self.publishers:
+            self.publishers.remove(peer)
+
+    def create_message_packet(self, message_type, payload):


Yep, we definitely have to put this into IPv8 for everyone's convenience.

ichorid · 2018-06-28T12:20:56Z

Tribler/community/popular/community.py

+        payload = payload if isinstance(payload, list) else payload.to_pack_list()
+        return self._ez_pack(self._prefix, message_type, [auth, dist, payload])
+
+    def broadcast_message(self, packet, peer=None):


I second this.

ichorid · 2018-06-28T12:36:54Z

Tribler/community/popular/community.py

+        """
+        # Add or remove the publisher peer
+        if subscribe:
+            self.publishers.add(peer)


It seems we add publisher twice: once here (on sending sub packet), and once on receiving confirmation packet:
line 178:

if payload.subscribe: self.publishers.add(peer)

Is it by design or I don't get something?

If I would design this system, I would only add peers to my publishers' list on the response packet. And to prevent abuse I would put in each sub packet some nonce, and remember it, and only accept response packets with this nonce (this would solve the packet loss/timeout problem, by the way).
How do you handle timeouts?

ichorid · 2018-06-28T15:18:22Z

Tribler/community/popular/pubsub.py

+        peer = self.get_peer_from_auth(auth, source_address)
+
+        if payload.subscribe:
+            self.publishers.add(peer)


Ноw do you prevent abuse of this packets? Do you track unanswered subscribe requests?

ichorid · 2018-06-29T07:56:20Z

Tribler/community/popularity/pubsub.py

+            self.publishers.remove(peer)
+
+        # Create subscription packet and send it
+        subscription = ContentSubscription(cache.number, subscribe)


If cache.number is incremented sequentially, and after seeing one of them you could guess the next one, you still have a security hole there. Imagine the following attack scenario:
Alice sends a subscription message to Bob. Bob's private key is compromised by Charlie. Charlie eavesdrops the message, extract from it the cache.number and generates a long sequence of cache.numbers that follow the original one. Next, Charlie creates some sybil private keys and starts to bomb Alice's machine with ContentSubscription messages. From this moment, any Alice's request for subscription will end up subscribing to Charlie.
You should never use a non-random nonce in crypto.
So, if cache.number is generated at random, you're good. If not, you're screwed.
Please, check it.

There is another way to fix this problem. You already store the whole packet in the cache, so you can check if the reply was actually from the person you send the original request to.

cache.number is generated randomly.

ichorid · 2018-06-29T08:04:17Z

Tribler/community/popularity/pubsub.py

+        cache = self.request_cache.add(ContentRequest(self.request_cache, MSG_SUBSCRIBE, None))
+        # Add or remove the publisher peer
+        if subscribe:
+            self.publishers.add(peer)


Despite you're using a set to store the publishers, you should still only add them on receiving successful ContentSubscription message. Otherwise, the confirmation message is useless, and you become vulnerable to packet loss.

Please, make publishers only be added on successful confirmation message.
(And BTW, do subscriptions time out?)

Good point! will make the change.
Subscriptions do not time out. However, there are periodically refreshed to fill up for disconnected peers.

synctext · 2018-09-05T14:55:06Z

It would be great to post some measurement graphs here.

Create crawler
Obtain anonimized dataset
conduct performance evalution
improve performance and deploy fixes as needed.

Q: Quality of popularity gossip information, freshness of information, coverage of information, usability of information, overlap, bandwidth-cost, etc.

Thomasedv reviewed Jun 1, 2018

View reviewed changes

xoriole changed the title ~~Content popularity~~ WIP: Content popularity Jun 1, 2018

xoriole force-pushed the popular_channel branch from 6511102 to e41b41a Compare June 1, 2018 15:17

Thomasedv reviewed Jun 1, 2018

View reviewed changes

xoriole force-pushed the popular_channel branch 13 times, most recently from 4e56d0a to 48bdeea Compare June 8, 2018 09:19

xoriole force-pushed the popular_channel branch 5 times, most recently from 21fb686 to 00659e4 Compare June 13, 2018 15:08

synctext mentioned this pull request Jun 13, 2018

Redesign of the Search/Channels feature #3615

Closed

xoriole force-pushed the popular_channel branch 2 times, most recently from dcc815a to 4bb446e Compare June 18, 2018 14:45

qstokkink reviewed Jun 26, 2018

View reviewed changes

xoriole force-pushed the popular_channel branch 2 times, most recently from 0c3d7f1 to bfc39e5 Compare June 26, 2018 13:47

qstokkink reviewed Jun 28, 2018

View reviewed changes

xoriole force-pushed the popular_channel branch 2 times, most recently from bc19910 to c99e53e Compare June 28, 2018 16:15

xoriole force-pushed the popular_channel branch 2 times, most recently from cbcc9d1 to 97960d5 Compare June 28, 2018 16:52

xoriole changed the title ~~WIP: Content popularity~~ READY: Content popularity Jun 28, 2018

xoriole force-pushed the popular_channel branch from 97960d5 to bfcc6a3 Compare June 28, 2018 17:03

devos50 previously approved these changes Jun 28, 2018

View reviewed changes

xoriole dismissed devos50’s stale review via 9962ed6 June 29, 2018 07:11

xoriole force-pushed the popular_channel branch from bfcc6a3 to 9962ed6 Compare June 29, 2018 07:11

xoriole added 3 commits June 29, 2018 09:15

Added content popularity community

aeecbfd

Added PubSub community

8d29fb9

Integrated popularity community search with the search manager

a0c4940

xoriole force-pushed the popular_channel branch from 9962ed6 to f6c70bd Compare June 29, 2018 07:18

qstokkink previously approved these changes Jun 29, 2018

View reviewed changes

xoriole dismissed qstokkink’s stale review via ceb0d8f June 29, 2018 07:55

qstokkink previously approved these changes Jun 29, 2018

View reviewed changes

ichorid suggested changes Jun 29, 2018

View reviewed changes

xoriole added 2 commits June 29, 2018 10:32

Updated IPv8 pointer

d84c233

Improved test coverage and resolved review comments

85c6b2a

xoriole dismissed qstokkink’s stale review via 85c6b2a June 29, 2018 08:35

xoriole force-pushed the popular_channel branch from ceb0d8f to 85c6b2a Compare June 29, 2018 08:35

ichorid approved these changes Jun 29, 2018

View reviewed changes

qstokkink changed the title ~~READY: Content popularity~~ Content popularity Jun 29, 2018

qstokkink merged commit 727b40d into Tribler:next Jun 29, 2018

xoriole deleted the popular_channel branch July 3, 2019 14:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Content popularity #3660

Content popularity #3660

xoriole commented Jun 1, 2018

Thomasedv Jun 1, 2018

xoriole Jun 1, 2018

Thomasedv Jun 1, 2018

xoriole commented Jun 8, 2018

xoriole commented Jun 22, 2018

qstokkink Jun 26, 2018

qstokkink Jun 28, 2018

qstokkink Jun 28, 2018

qstokkink commented Jun 28, 2018

devos50 commented Jun 28, 2018

ichorid left a comment

ichorid Jun 28, 2018

ichorid Jun 28, 2018

ichorid Jun 28, 2018

ichorid Jun 28, 2018

ichorid Jun 29, 2018

xoriole Jun 29, 2018

ichorid Jun 29, 2018

xoriole Jun 29, 2018

synctext commented Sep 5, 2018

Content popularity #3660

Content popularity #3660

Conversation

xoriole commented Jun 1, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xoriole commented Jun 8, 2018

xoriole commented Jun 22, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qstokkink commented Jun 28, 2018

devos50 commented Jun 28, 2018

ichorid left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

synctext commented Sep 5, 2018