Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AnnounceShare rework #2068

Open
wants to merge 14 commits into
base: py3
Choose a base branch
from
Open

Conversation

wandrien
Copy link
Contributor

@wandrien wandrien commented Jul 3, 2019

  • Allow sharing trackers of any supported protocols, not just zero://. (Right now it includes BitTorrent over HTTP(S) and BitTorrent over UDP.)
    • The supported protocols are autodetected, not hardcoded, so the modification of the plugin is not required in case of implementation of new protocols.
  • Working trackers are now counted on per protocol basis. The plugin tries to maintain the up-to-date list of functional trackers for each available protocol.
  • Be more conservative on removal trackers from the list. Don't delete trackers on errors if no successful connections have been established recently, in the case of the network connectivity issues.
  • Other small code refactorings, such as replacing magic constants with meaningful names.

The updated plugin is meant to be used as a general-purpose tracker exchange facility, not limited to sharing locally running autodetected Boostrappers.

… basis.

AnnounceShare tries to autodetect supported tracker protocols and keep the actual list of working trackers for each protocol.

The udp:// and http(s):// versions of the BitTorrent protocol are considered as 2 distinct protocols.
http:// and https:// are considered as the same protocol.

Added option --working_shared_trackers_limit_per_protocol to adjust the minimum number of working trackers for each protocol (3, by default).
…en any successful announces recently, to protect from the false positives during network connectivity issues.
…r of zero:// trackers still were 5, as before the redesign
…ore uniform filling of per-protocol buckets.
@wandrien
Copy link
Contributor Author

wandrien commented Jul 3, 2019

Further work, after merging the PR:

  • TrackerList is a plugin, that fetches a list of trackers from any external source (URL or local file path) and adds them into the AnnounceShare list. If the trackers are functional, AnnounceShare spreads them throughout the network to other peers. (The plugin is functional, but some features are missing and additional testing is required.)

  • Rewrite the interaction of AnnounceShare and Bootstrapper:

    • Now: AnnounceShare checks if Bootstrapper is running and if so, adds the IP addresses to the shared list.
    • Should be: Bootstrapper reports the listened addresses to AnnounceShare, according to its own configuration.
  • Add optional support of .onion addresses to Bootstrapper, including addresses persistent between program restarts.

@styromaniac
Copy link
Contributor

styromaniac commented Jul 3, 2019

Don't share own IP if own device is running Bootstrapper but is unsuccessful.

I've attempted this many times on my cable broadband. That will need to be continued behavior after the pull request is merged in.

@HelloZeroNet
Copy link
Owner

HelloZeroNet commented Jul 4, 2019

Sharing torrent trackers are disabled, because it requires more resources, than zero:// ones and does not support tor (onion) addresses. So I would avoid using them if possible.

@wandrien
Copy link
Contributor Author

wandrien commented Jul 4, 2019

Sharing torrent trackers are disabled, because it requires more resources, than zero:// ones and does not support tor (onion) addresses.

The default config contains 6 torrent trackers and just 3 zero ones.

So I would avoid using them if possible.

Shouldn't it be up to a user to choose, which transport and tracker types s/he prefers to use?

@HelloZeroNet
Copy link
Owner

Ideally we should replace the torrent trackers with zero:// ones, but right now the torrent trackers are more decentralized by location and ownership.

@wandrien
Copy link
Contributor Author

wandrien commented Jul 4, 2019

I run a ZeroNet instance on a cheap VPS ($5/month), and it sends announces to 50+ trackers, most of them are torrent ones. (Actually, to 80+ trackers, but 30 are mostly not responding.) I cannot notice any slowdown in practice.

We can utilize the existing large torrent infrastructure for the benefit of ZeroNet network in this way. In my opinion, Zeronet should be able to transfer the data over any possible transports and networks, and it's up to users to choose ones that are more appropriate to their conditions.

In fact, the BitTorrent protocol cannot be disabled in the current implementation, since it's not moved to a plugin, but implemented in the core. The issue you talk about can be solved automatically if the code is moved to a plugin. Just disable the plugin and ZeroNet doesn't try to connect to any torrent trackers.

…e the defaults: zero => 5 trackers, any other protocol => 2 trackers
@wandrien
Copy link
Contributor Author

wandrien commented Jul 4, 2019

I modified the code, now it allows to set a separate limit value for each protocol.
By default, the limit is 5 for zero:// and 2 for any other protocol, so it now avoids using too many torrent trackers.

@styromaniac
Copy link
Contributor

I'm thinking: For Arm devices, maybe we should use zero:// exclusively by default if torrent trackers impact performance.

@HelloZeroNet
Copy link
Owner

It's not just about cpu, but bw: torrent trackers requires separate request for every site, for zero trackers you can announce multiple sites in one request.
So if you have 100 site you need to send 100 request every 20 minute for every torrent tracker.

@wandrien
Copy link
Contributor Author

wandrien commented Jul 4, 2019

You update torrent trackers in the hardcoded list every time they get down, but you don't agree for users to be able to add torrent trackers by themselves, and the network to be able to switch those trackers automatically. What's wrong, really?

I'm now experimenting with DHT, and I can say, it requires much more network queries per an entry, than a torrent tracker. Are you going to reject a DHT PR for the same network performance reason? Please tell me now, so I didn't waste my time in that case.

@ghost
Copy link

ghost commented Jul 5, 2019

I support the user zeronet, torrent trackers should not be inside zeronet, but we could try to add this code for the sake of tests and then either modify or completely cut it.

@HelloZeroNet
Copy link
Owner

I think DHT would be an improvement in some situations, so it would be a great addition, but I'm not sure if we should enable it by default. Depends on the resource usage or as a fallback if low amount of trackers are available.

@slrslr
Copy link

slrslr commented Jul 5, 2019

I understand this the way that this PR is a step in good direction (automaticaly on background it will exchange various kind of trackers among users). As said by @HelloZeroNet zero:// trackers should be better and preffered among zeronet peers. But if these gets blocked, zeronet should use less optimal (HTTP etc) trackers (obtained by this plugin), to immediately took place even these are claimed to use more connections, bandwidth. Note that if peers are spread among too many trackers that would not be good, so some mechanism that for example sort trackers by address and use first x trackers which will make sure these will track as much zeronet peers as possible. Or in case primary zero:// trackers are down, zeronet plugin will query all available trackers (obtained from exchange) and discover zeronet peer counts and use those with top peer count. (we hope that highest peer count tracker is not a evil tracker made to break peer connectability).

@wandrien
Copy link
Contributor Author

wandrien commented Jul 5, 2019

Note that if peers are spread among too many trackers that would not be good

Splitting peers over many trackers is a vulnerability, that is mitigated neither in the core, nor in the original plugin, nor in this pull request. The redesigned plugin adds nothing to the possibility of that kind of attack for splitting the network into unbound subnets. Actually, since the code in the PR has higher limits for trackers than the original code, and also adds additional shuffling of the received data, it is harder, nor easier to sucessfully attack the plugin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants