Support for multiple Peer Discoveries

[GoogleCodeExporter]

Currently, if you add multiple discoveries, bitcoinj just adds addresses of the 
first. Any second or third discoveries are ignored.

I need multiple discoveries for connecting a trusted peer AND connection 
normally discovered peers at the same time.

Original issue reported on code.google.com by andreas....@gmail.com on 8 Feb 2013 at 5:25

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

In PeerGroup I found the code pasted below, which indeed shows that as soon as 
a peer returns an address, all other PeerDiscoverers are ignored. The naive 
solution would be to remove the check, or change the 'break' size to some 
MAX_NR_OF_ADRESSES to prevent getting too many adresses. Of course any one 
PeerDiscover implementation can still fill the memory with adresses (thinking 
of IrcDiscovery for instance) so checks shoud be in place in the PeerDiscover 
implementations as well.
Anyway, let me know if I missed something and I'll take a stab at solving this.

for (PeerDiscovery peerDiscovery : peerDiscoverers) {
            InetSocketAddress[] addresses;
            addresses = peerDiscovery.getPeers(5, TimeUnit.SECONDS);
            for (InetSocketAddress address : addresses) addressSet.add(new PeerAddress(address));
            if (addressSet.size() > 0) break;
}

Original comment by mike.ros...@gmail.com on 6 Jan 2014 at 8:32

[GoogleCodeExporter]

Nope, that sounds about right. A cap on the size sounds like the right way to 
go.

Original comment by hearn@google.com on 7 Jan 2014 at 10:02

• Changed state: Accepted
• Added labels: Type-Enhancement
• Removed labels: Type-Defect

[GoogleCodeExporter]

Ok I'll have a look

Original comment by mike.ros...@gmail.com on 8 Jan 2014 at 11:18

[GoogleCodeExporter]

Note that some peer discoveries (eg PeerDB) might *want* to fill all the 
addresses if possible, so checks around that may not be important.

Original comment by BlueMatt...@gmail.com on 12 Jan 2014 at 3:08

[GoogleCodeExporter]

You mean that some PeerDiscovery implementations won't have checks because they 
want to take all addresses? That's indeed still a possibility, but I capped the 
max_nr_peers to 100 at the moment, so they would still be limited by that. Note 
also that if a PeerDiscovery implementation which was added before PeerDB could 
fill up 100 addresses and none of the PeerDB adresses would be added.

Please have a look at my patch for this issue (and check my rusty threading 
skills), there's also some small cleanup in a separate commit:
https://github.com/mrosseel/bitcoinj

Original comment by mike.ros...@gmail.com on 12 Jan 2014 at 5:26

[GoogleCodeExporter]

Hey Mike,

Sorry for being so slow at re-reviewing the patch. I plan to revisit this after 
0.11 is out.

Original comment by mh.in.en...@gmail.com on 3 Feb 2014 at 12:07

[GoogleCodeExporter]

no worries, I just rebased it on the latest master and squashed everything in 
one commit:

https://github.com/mrosseel/bitcoinj/commits/master

the second commit you see is cleanup of a superfluous variable.

PS: might have squashed our discussion on it as well; the current solution is 
based on adding a method to PeerEventListener and notifying listeners if Peers 
are discovered. Made testing a lot easier and might be useful for client 
notifications.

Original comment by mike.ros...@gmail.com on 4 Feb 2014 at 10:16

[GoogleCodeExporter]

see the link below, rebase for current HEAD taking into account your previous 
remarks. 
Sorry it took so long, had to finish my own bitcoin-related project ;)

https://github.com/mrosseel/bitcoinj/commit/1bcdb5fa1e41fd76b3b98b8929a31c0d7352
482d

Original comment by mike.ros...@gmail.com on 4 May 2014 at 7:37

[GoogleCodeExporter]

This is now a pull request:

https://github.com/bitcoinj/bitcoinj/pull/233

Original comment by mike.ros...@gmail.com on 27 Sep 2014 at 9:40

[GoogleCodeExporter]

Accepted, thanks.

Original comment by mh.in.en...@gmail.com on 7 Oct 2014 at 1:38

• Changed state: Fixed