Implement the peer discovery protocol

[GoogleCodeExporter]

Right now, you have to know the IP address of another network peer to connect 
to. See for example the PingService class, which connects to localhost (where 
presumably the native client is running).

We should implement a class that implements peer discovery, using the same 
methods as the native client (i.e. a built-in list of IPs that are usually 
online, and querying the IRC channel).

Original issue reported on code.google.com by thilopl...@googlemail.com on 8 Apr 2011 at 1:11

[GoogleCodeExporter]

I'm thinking DNS based discovery is likely to work better than the current IRC 
based mechanism. Jeff and Vladimir already did some work on that. IRC 
bootstrapping is very slow and quite a few networks run intrusion detection 
systems that flag obviously bot controlled IRC connections as a sign of malware.

Original comment by hearn@google.com on 10 Apr 2011 at 7:47

[GoogleCodeExporter]

Okay, let's skip IRC and wait for DNS discovery.

But how about the hard-coded fallback host list?
That would have helped William over here: 
http://groups.google.com/group/bitcoinj/browse_thread/thread/6f5726616970c47

Original comment by thilopl...@googlemail.com on 25 Apr 2011 at 1:13

[GoogleCodeExporter]

I've been working on adding some bitcoinj features localally and would like to 
try submitting back to the project. Are you guys ruling out IRC bootstrapping 
or would you include if someone wrote it (me)?

Original comment by jwsam...@gmail.com on 26 Apr 2011 at 12:19

[GoogleCodeExporter]

I'd be happy to include it as long as the design was right. The problem is I'm 
not sure what the right design is yet :-)

There definitely needs to be some abstraction over discovery protocols, so 
there'd be a PeerDiscovery interface with implementations (probably in a new 
package) like IrcDiscovery, DnsDiscovery, ConfigFileDiscovery, 
SeedNodesDiscovery, AmazingDiscovery etc ...

Original comment by hearn@google.com on 26 Apr 2011 at 8:22

• Added labels: Type-Enhancement
• Removed labels: Type-Defect

[GoogleCodeExporter]

I'd be happy to include it as long as the design was right. The problem is I'm 
not sure what the right design is yet :-)

There definitely needs to be some abstraction over discovery protocols, so 
there'd be a PeerDiscovery interface with implementations (probably in a new 
package) like IrcDiscovery, DnsDiscovery, ConfigFileDiscovery, 
SeedNodesDiscovery, AmazingDiscovery etc ...

Original comment by hearn@google.com on 26 Apr 2011 at 8:22

[GoogleCodeExporter]

> A question: can bitcoinj as is work with multiple peers or can it only
> handle one?

It currently only handles one. There's no infrastructure for managing multiple 
connections simultaneously. That said, I'll happily take your code even if the 
user has to plug things together themselves for now. It'll be a useful base on 
which to build out more infrastructure.

Please note that for me to accept your code, you'll have to (electronically) 
sign the CLA:

http://code.google.com/legal/individual-cla-v1.0.html

This isn't a copyright assignment, it's just a formal declaration that you own 
the code the submitting (ie it's not owned by an employer or university).

Original comment by hearn@google.com on 27 Apr 2011 at 2:54

[GoogleCodeExporter]

I plan on submitting a patch tonight with IRC discovery tonight but I have a 
question about contributions.
Do I put copyright google or copyright me in the files?
I'm not sure what the protocol is.

Original comment by jwsam...@gmail.com on 28 Apr 2011 at 2:54

[GoogleCodeExporter]

Your own name. You own the copyright on your code. But to check it into 
Subversion I'll need you to fill out that form (it's electronic, just a regular 
web form).

Original comment by hearn@google.com on 28 Apr 2011 at 2:59

[GoogleCodeExporter]

Here is the patch.
Some notes on the design decisions:
I am a c# dev by trade so I tried my best to keep to Java conventions but there 
could be idiomatic stuff that needs to change.
I left the classes in the root. They can be reorganized to whatever location 
everyone sees fit. I imagine the PeerDiscovery class would stay root but the 
individual implementations would possibly move to new packages.

New/Changed Classes:

Utils : added readUint16BE for reading port

PeerDiscovery : interface that just returns InetSocketAddresses. This should 
support naming by ip or hostname and doesn't assume the default port. More can 
be added later but I couldn't think of anything else at the moment.

PeerDiscoveryException : for wrapping anything that goes wrong in the interface

IrcDiscovery : the IRC implementation. Initially I tried using available 
libraries but they were way overblown for this purpose and I didn't want to be 
the first to add an external dependency. For now it only supports ipv4 peers 
but ipv6 can be added easily. 

example/IRCPeers : prints the current list of potential peers in #bitcoin or 
#bitcoinTEST

tests/IrcDiscoveryTest : tests address parsing and ignoring invalid addresses

I didn't include a full example of peering and connecting but I might post one 
soon. Keep in mind when getting the list you will have to try many addresses 
before you find one that will let you connect. I left the IRC call synchronous 
but it can be called async easily enough.

There is no advertisement of the client in the discovery system but that can be 
added when bitcoinj is up to it.

As a quick test you can launch the c++ client, run the example, and check for 
your ip in the potential peers list.

Original comment by jwsam...@gmail.com on 28 Apr 2011 at 7:57

Attachments:

• bitcoinj-irc.patch

[GoogleCodeExporter]

I've committed a modified version of this patch in r64. I made quite a few 
changes so I'm afraid you'll have some merge conflicts ... if you didn't make 
any changes yourself, best to just delete your copies and pull the ones from 
SVN directly. 

Changes:
- Reformatted the code using the IDEA "reformat" command: spaces not tabs, 100 
column width.
- Changed some comments.
- Moved the address checksum functionality into the Base58 class, added some 
unit tests. This relied on a Java 6ish that I removed along the way.
- Added JavaDocs to the PeerDiscovery interface.
- Changed some code that was using object casts to use String.format instead

Original comment by hearn@google.com on 2 May 2011 at 11:56

• Changed state: Fixed

[GoogleCodeExporter]

Cool. I'll make sure future patches are more grammatically correct.

Original comment by jwsam...@gmail.com on 2 May 2011 at 12:19

[GoogleCodeExporter]

https://github.com/thiloplanz/bitcoinj/pull/1

This patch makes it possible to connect to peers defined by PNSeed found in the 
c++ version of bitcoin.

Hope it is ok. I have to sign some a contributors agreement?

Original comment by bobby.si...@gmail.com on 31 May 2011 at 6:10

[GoogleCodeExporter]

Thanks. I'll look at this soon. The CLA is here:

http://code.google.com/legal/individual-cla-v1.0.html

By the way, it's easier for me to work with .patch files. github requires some 
magic URL I am always forgetting to extract useful diffs.

Original comment by hearn@google.com on 31 May 2011 at 7:57

[GoogleCodeExporter]

The magic URL seem to be adding ".diff" or ".patch" at the end:

https://github.com/thiloplanz/bitcoinj/pull/1.diff
https://github.com/thiloplanz/bitcoinj/pull/1.patch

Those are based against rev 86 of the trunk.

Not sure why they do not expose this in the GUI...

Original comment by thilopl...@googlemail.com on 31 May 2011 at 2:01

[GoogleCodeExporter]

Could you start a thread on the mailing list about this patch? I'll reply with 
some comments there.

Original comment by hearn@google.com on 31 May 2011 at 4:11