removed bloom filter code

[jdonszelmann]

While working on the rust-ipv8 implementation (with @ichorid) we noticed that the bloom filter (originally from dispersy) is absolutely never used in py-ipv8. Therefore this pull request removes it from py-ipv8.

However this isn't the end of the story. tribler/Tribler does use the bloom filter. We will make a pull request ASAP to add it to tribler/Tribler itself.

[tribler-ci]

Can one of the admins verify this patch?

[jdonszelmann]

We have verified that no other code uses the bloom filter Except for tribler/Tribler which we will as promised patch. https://github.com/search?q=org%3ATribler+messaging.bloomfilter&type=Code

[ichorid]

ok to test

[devos50]

@qstokkink could you please confirm that moving the Bloom filter to Tribler is not breaking anything?

[jdonszelmann]

this should be it Tribler/tribler#4503

[synctext]

Bloom filter seems to be exclusively used by the market community in a few places.
https://github.com/Tribler/tribler/blob/6f3ad63384c27eeaebe218c58ee697704af950bf/Tribler/community/market/payload.py#L12

Due for re-factor? Do we need it?

[devos50]

Due for re-factor? Do we need it?

This code has been directly copied from Dispersy and therefore not up to our current code standards. The market uses it for order synchronisation between matchmakers to save bandwidth (in particular to save bandwidth and to prevent sending orders to another matchmaker that already has this order).

[qstokkink]

@qstokkink could you please confirm that moving the Bloom filter to Tribler is not breaking anything?

Nope, everything will still work. Actually my first intention -when moving from Dispersy to IPv8- was to remove the Bloomfilter class.

However, IPv8's purpose is to be a swiss army knife for easy network overlay creation. We see uses for Bloom filters pop up every so often. Continuously adding and removing the same code also doesn't really appeal to me (versioning hell etc.).

Also, for the future, I forsee us moving in a direction where we will be using knowledge (set membership) tests. Stuff like secure proof-of-storage constructions and, for cases not involving malicious actors, Bloom filters.

Open question: Should IPv8 cater (1) for the future and a wider audience or (2) just Tribler?

We could argue for both answers in my opinion. I'd like to avoid shooting myself in the foot long term, give me some time to think about this.

[qstokkink]

Alternative formulation: Is the Bloomfilter class (1) an IPv8 feature or (2) internal dead code within the Tribler project?

[NULLx76]

Maybe the Bloomfilter class can be made its own repo/package, so that it can be imported when and where needed. This is in my opinion more logical as to keeping it in py-ipv8. With how I look at it, it is a bit weird to include it without it actually being used by it (right now at least). This also potentially allows other people to make use of it without including all of py-ipv8 or tribler.

[synctext]

Interesting...
Short term: Bloom.py class not up to our coding standards, Martijn market community is only user. He uses this as private class and might find time to re-factor. IPv8 has no dead code: first prove itself with 1/4 million users, before dreaming the big dream.

Long-term: no dead code and no overengineering for future please. In 2020 we can regroup and accept PR for broader IPv8 use cases.

Good policy proposal?

[devos50]

However, IPv8's purpose is to be a swiss army knife for easy network overlay creation. We see uses for Bloom filters pop up every so often. Continuously adding and removing the same code also doesn't really appeal to me (versioning hell etc.).

My policy for this would be: if some code is not used, remove it. However, what makes it difficult is that another package that depends on IPv8 is using it. The Bloom filter itself, however, does not have any interaction with core IPv8 classes so it feels detached from the rest of the code. I would say this makes it at least a candidate for removal or migration to another package.

Also, for the future, I forsee us moving in a direction where we will be using knowledge (set membership) tests. Stuff like secure proof-of-storage constructions and, for cases not involving malicious actors, Bloom filters.

We should also ask ourselves how feature-complete IPv8 currently is. Judging from the activity on this repository and my own experience, IPv8 is very stable and does not get major new functionality anytime soon (at least, the coming few months?). On the other hand, I agree that we have seen use-cases of Bloom filters popping up now and then. Leaving code in a repository just because we might need it in the future feels like a bad decision for me and by structurally doing so, we will end up with Dispersy 2.0.

I would say that it is better to move it and as @synctext pointed out, at one point refactor the code so it is up to our standards. Future developments with TrustChain and other overlays will tell us whether we need Bloom filters as fundamental data structure, in which case we can put them back in IPv8.

[ichorid]

As Bloom filter really has nothing to do with IPv8, we should move it to keep the codebase tight and clean.

[qstokkink]

After some continued discussion in the office, I think we can indeed remove the Bloom filter.

[jdonszelmann]

nice :D